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

3 files changed, 45 insertions, 19 deletions

diff --git a/src/codec/SkCodec_libpng.cpp b/src/codec/SkCodec_libpng.cpp
index 444e2ad87e..32004366f8 100644
--- a/src/codec/SkCodec_libpng.cpp
+++ b/src/codec/SkCodec_libpng.cpp
@@ -372,6 +372,12 @@ SkPngCodec::SkPngCodec(const SkImageInfo& info, SkStream* stream,
 {}
 
 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 @@ bool SkPngCodec::handleRewind() {
 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 @@ public:
         return SkImageGenerator::kSuccess;
     }
 
-    void onFinish() override {
-        fCodec->finish();
-    }
-
     bool onReallyHasAlpha() const override { return fHasAlpha; }
 
 private:
@@ -716,10 +724,6 @@ public:
         return SkImageGenerator::kSuccess;
     }
 
-    void onFinish() override {
-        fCodec->finish();
-    }
-
     bool onReallyHasAlpha() const override { return fHasAlpha; }
 
 private:
@@ -734,7 +738,7 @@ private:
     
     
     
-    
+
 
     typedef SkScanlineDecoder INHERITED;
 };
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 2eaff1a9a6..9c9bd77c13 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -297,9 +297,15 @@ SkCodec::Result SkJpegCodec::onGetPixels(const SkImageInfo& dstInfo,
                                          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 @@ SkCodec::Result SkJpegCodec::onGetPixels(const SkImageInfo& dstInfo,
 }
 
 /*
+ * 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 @@ public:
         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;
diff --git a/src/codec/SkJpegCodec.h b/src/codec/SkJpegCodec.h
index 87c925d9d1..f39bc49910 100644
--- a/src/codec/SkJpegCodec.h
+++ b/src/codec/SkJpegCodec.h
@@ -95,6 +95,12 @@ private:
     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();