Destroy SkScanlineDecoder in the SkCodec subclass destructors

include/codec/SkScanlineDecoder.h

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #ifndef SkScanlineDecoder_DEFINED
 #define SkScanlineDecoder_DEFINED
 
 #include "SkTypes.h"
 #include "SkTemplates.h"
 #include "SkImageGenerator.h"
 #include "SkImageInfo.h"
 
 class SkScanlineDecoder : public SkNoncopyable {
 public:
     // Note for implementations: An SkScanlineDecoder will be deleted by (and
     // therefore *before*) its associated SkCodec, in case the order matters.
-    virtual ~SkScanlineDecoder() {}
+    virtual ~SkScanlineDecoder() {
+        this->onFinish();

[scroggo, 2015/06/30 13:47:38]

I almost missed this: you shouldn't call a virtual function from a destructor.
(See http://www.artima.com/cppsource/nevercall.html)

Go ahead and remove onFinish, and put its implementation in destructor
overrides.

[msarett, 2015/06/30 20:43:05]

Acknowledged.

+    }
 
     /**
      *  Write the next countLines scanlines into dst.
      *
      *  @param dst Must be non-null, and large enough to hold countLines
      *      scanlines of size rowBytes.
      *  @param countLines Number of lines to write.
      *  @param rowBytes Number of bytes per row. Must be large enough to hold
      *      a scanline based on the SkImageInfo used to create this object.
      */
     SkImageGenerator::Result getScanlines(void* dst, int countLines, size_t rowBytes) {
         if ((rowBytes < fDstInfo.minRowBytes() && countLines > 1 ) || countLines <= 0
                 || fCurrScanline + countLines > fDstInfo.height()) {
             return SkImageGenerator::kInvalidParameters;
         }
         const SkImageGenerator::Result result = this->onGetScanlines(dst, countLines, rowBytes);
-        this->checkForFinish(countLines);
+        fCurrScanline += countLines;
         return result;
     }
 
     /**
      *  Skip count scanlines.
      *
      *  The default version just calls onGetScanlines and discards the dst.
      *  NOTE: If skipped lines are the only lines with alpha, this default
      *  will make reallyHasAlpha return true, when it could have returned
      *  false.
      */
     SkImageGenerator::Result skipScanlines(int countLines) {
         if (fCurrScanline + countLines > fDstInfo.height()) {
             // Arguably, we could just skip the scanlines which are remaining,
             // and return kSuccess. We choose to return invalid so the client
             // can catch their bug.
             return SkImageGenerator::kInvalidParameters;
         }
         const SkImageGenerator::Result result = this->onSkipScanlines(countLines);
-        this->checkForFinish(countLines);
+        fCurrScanline += countLines;
         return result;
     }
 
     /**
      *  Some images may initially report that they have alpha due to the format
      *  of the encoded data, but then never use any colors which have alpha
      *  less than 100%. This function can be called *after* decoding to
      *  determine if such an image truly had alpha. Calling it before decoding
      *  is undefined.
      *  FIXME: see skbug.com/3582.
@@ skipping 23 matching lines @@
         // onGetScanlines bypasses that check.
         // Calling the virtual method also means we do not double count
         // countLines.
         return this->onGetScanlines(storage.get(), countLines, 0);
     }
 
     virtual SkImageGenerator::Result onGetScanlines(void* dst, int countLines,
                                                     size_t rowBytes) = 0;
 
     /**
-     *  Called after any set of scanlines read/skipped. Updates fCurrScanline,
-     *  and, if we are at the end, calls onFinish().
-     */
-    void checkForFinish(int countLines) {
-        fCurrScanline += countLines;
-        if (fCurrScanline >= fDstInfo.height()) {
-            this->onFinish();
-        }
-    }
-
-    /**
-     *  This function will be called after reading/skipping all scanlines to do
+     *  This function will be called after reading/skipping scanlines to do
      *  any necessary cleanups.
+     *  It is possible that not all scanlines will have been read/skipped when this
+     *  function is called.  In fact, in the case of subset decodes, it is likely
+     *  that there will be scanlines at the bottom of the image that have been ignored.
      */
     virtual void onFinish() {} // Default does nothing.
 };
 #endif // SkScanlineDecoder_DEFINED