Destroy SkScanlineDecoder in the SkCodec subclass destructors

src/codec/SkJpegCodec.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkCodec.h"
 #include "SkJpegCodec.h"
 #include "SkJpegDecoderMgr.h"
@@ skipping 279 matching lines @@
     fSrcRowBytes = SkSwizzler::BytesPerPixel(srcConfig) * dstInfo.width();
 }
 
 /*
  * Performs the jpeg decode
  */
 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
     jpeg_decompress_struct* dinfo = fDecoderMgr->dinfo();
 
     // Set the jump location for libjpeg errors
     if (setjmp(fDecoderMgr->getJmpBuf())) {
         return fDecoderMgr->returnFailure("setjmp", kInvalidInput);
     }
 
@@ skipping 68 matching lines @@
             jpeg_finish_decompress(dinfo);
             return fDecoderMgr->returnFailure("Incomplete image data", kIncompleteInput);
         }
     }
     jpeg_finish_decompress(dinfo);
 
     return kSuccess;
 }
 
 /*
+ * 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");

[scroggo, 2015/06/30 20:59:58]

Do we need this error message? I guess it could be a sign of a deeper problem,
but it seems like we have already decoded the image?

[msarett, 2015/06/30 21:33:00]

I would lean towards keeping it.  It doesn't print under default settings, and
I'd like to be warned if somehow we aren't using the jpeg library properly.  We
may not care, but I'd like to know something is "wrong" and make sure it's not
really wrong.

That said this is not a huge issue and if you feel more strongly than me, I'll
drop it :).

+            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 {
 public:
     SkJpegScanlineDecoder(const SkImageInfo& dstInfo, SkJpegCodec* codec)
         : INHERITED(dstInfo)
         , fCodec(codec)
     {
         fStorage.reset(fCodec->fSrcRowBytes);
         fSrcRow = static_cast<uint8_t*>(fStorage.get());
@@ skipping 35 matching lines @@
         }
 
         // Read rows but ignore the output
         for (int y = 0; y < count; y++) {
             jpeg_read_scanlines(fCodec->fDecoderMgr->dinfo(), &fSrcRow, 1);
         }
 
         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;
     uint8_t*            fSrcRow;    // ptr into fStorage
 
     typedef SkScanlineDecoder INHERITED;
 };
 
 SkScanlineDecoder* SkJpegCodec::onGetScanlineDecoder(const SkImageInfo& dstInfo,
         const Options& options, SkPMColor ctable[], int* ctableCount) {
@@ skipping 31 matching lines @@
     // Create the swizzler
     this->initializeSwizzler(dstInfo, NULL, dstInfo.minRowBytes(), options);
     if (NULL == fSwizzler) {
         SkCodecPrintf("Could not create swizzler\n");
         return NULL;
     }
 
     // Return the new scanline decoder
     return SkNEW_ARGS(SkJpegScanlineDecoder, (dstInfo, this));
 }