commit c0e80c139e15496a8a96eec7848689b6f0e7bcc1
author msarett <msarett@google.com> Wed Jul 01 06:50:35 2015 -0700
committer Commit bot <commit-bot@chromium.org> Wed Jul 01 06:50:35 2015 -0700
tree 6c3fbc2df1ec557f196c4a0dde6a28989f95a43e
parent f06c389f0fe07ce06acd8568d508d6413b8d3728

In the case of subset decodes, we will often not decode to
the bottom of the image.  In our code before this patch,
this would result in cleanup code in onFinish() never being
called.

We can allow subclasses to take ownership of the
SkScanlineDecoder in order to make sure that it is
finished/deleted before deleting the decode manager.

BUG=skia:

Review URL: https://codereview.chromium.org/1212593003

src/codec/SkCodec_libpng.cpp

diff --git a/src/codec/SkCodec_libpng.cpp b/src/codec/SkCodec_libpng.cpp
index 444e2ad..3200436 100644
--- a/src/codec/SkCodec_libpng.cpp
+++ b/src/codec/SkCodec_libpng.cpp
@@ -372,6 +372,12 @@
 {}
 
 SkPngCodec::~SkPngCodec() {
+    // First, ensure that the scanline decoder is left in a finished state.
+    SkAutoTDelete<SkScanlineDecoder> decoder(this->detachScanlineDecoder());
+    if (NULL != decoder) {
+        this->finish();
+    }
+
     this->destroyReadStruct();
 }
 
@@ -514,6 +520,12 @@
 SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& requestedInfo, void* dst,
                                         size_t rowBytes, const Options& options,
                                         SkPMColor ctable[], int* ctableCount) {
+    // Do not allow a regular decode if the caller has asked for a scanline decoder
+    if (NULL != this->scanlineDecoder()) {
+        SkCodecPrintf("cannot getPixels() if a scanline decoder has been created\n");
+        return kInvalidParameters;
+    }
+
     if (!this->handleRewind()) {
         return kCouldNotRewind;
     }
@@ -633,10 +645,6 @@
         return SkImageGenerator::kSuccess;
     }
 
-    void onFinish() override {
-        fCodec->finish();
-    }
-
     bool onReallyHasAlpha() const override { return fHasAlpha; }
 
 private:
@@ -716,10 +724,6 @@
         return SkImageGenerator::kSuccess;
     }
 
-    void onFinish() override {
-        fCodec->finish();
-    }
-
     bool onReallyHasAlpha() const override { return fHasAlpha; }
 
 private:
@@ -734,7 +738,7 @@
     
     
     
-    
+
 
     typedef SkScanlineDecoder INHERITED;
 };

src/codec/SkJpegCodec.cpp

diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 2eaff1a..9c9bd77 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -297,9 +297,15 @@
                                          void* dst, size_t dstRowBytes,
                                          const Options& options, SkPMColor*, int*) {
 
+    // Do not allow a regular decode if the caller has asked for a scanline decoder
+    if (NULL != this->scanlineDecoder()) {
+        return fDecoderMgr->returnFailure("cannot getPixels() if a scanline decoder has been"
+                "created", kInvalidParameters);
+    }
+
     // Rewind the stream if needed
     if (!this->handleRewind()) {
-        fDecoderMgr->returnFailure("could not rewind stream", kCouldNotRewind);
+        return fDecoderMgr->returnFailure("could not rewind stream", kCouldNotRewind);
     }
 
     // Get a pointer to the decompress info since we will use it quite frequently
@@ -388,6 +394,25 @@
 }
 
 /*
+ * We override the destructor to ensure that the scanline decoder is left in a
+ * finished state before destroying the decode manager.
+ */
+SkJpegCodec::~SkJpegCodec() {
+    SkAutoTDelete<SkScanlineDecoder> decoder(this->detachScanlineDecoder());
+    if (NULL != decoder) {
+        if (setjmp(fDecoderMgr->getJmpBuf())) {
+            SkCodecPrintf("setjmp: Error in libjpeg finish_decompress\n");
+            return;
+        }
+
+        // We may not have decoded the entire image.  Prevent libjpeg-turbo from failing on a
+        // partial decode.
+        fDecoderMgr->dinfo()->output_scanline = this->getInfo().height();
+        jpeg_finish_decompress(fDecoderMgr->dinfo());
+    }
+}
+
+/*
  * Enable scanline decoding for jpegs
  */
 class SkJpegScanlineDecoder : public SkScanlineDecoder {
@@ -443,15 +468,6 @@
         return SkImageGenerator::kSuccess;
     }
 
-    void onFinish() override {
-        if (setjmp(fCodec->fDecoderMgr->getJmpBuf())) {
-            SkCodecPrintf("setjmp: Error in libjpeg finish_decompress\n");
-            return;
-        }
-
-        jpeg_finish_decompress(fCodec->fDecoderMgr->dinfo());
-    }
-
 private:
     SkJpegCodec*        fCodec;     // unowned
     SkAutoMalloc        fStorage;

src/codec/SkJpegCodec.h

diff --git a/src/codec/SkJpegCodec.h b/src/codec/SkJpegCodec.h
index 87c925d..f39bc49 100644
--- a/src/codec/SkJpegCodec.h
+++ b/src/codec/SkJpegCodec.h
@@ -95,6 +95,12 @@
     SkJpegCodec(const SkImageInfo& srcInfo, SkStream* stream, JpegDecoderMgr* decoderMgr);
 
     /*
+     * Explicit destructor is used to ensure that the scanline decoder is deleted
+     * before the decode manager.
+     */
+    ~SkJpegCodec() override;
+
+    /*
      * Handles rewinding the input stream if it is necessary
      */
     bool handleRewind();