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.
+    /**
+     *  Clean up after reading/skipping scanlines.
+     *
+     *  It is possible that not all scanlines will have been read/skipped.  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.
+     *
+     *  Note for implementations: An SkScanlineDecoder will be deleted by (and
+     *  therefore *before*) its associated SkCodec, in case the order matters.
+     *  However, while the SkCodec base class maintains ownership of the
+     *  SkScanlineDecoder, the subclass will be deleted before the scanline
+     *  decoder.  If this is an issue, detachScanlineDecoder() provides
+     *  a means for the subclass to take ownership of the SkScanlineDecoder.
+     */
     virtual ~SkScanlineDecoder() {}

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

Do any clients override this? Maybe we can leave it out (and leave the implicit,
non-virtual destructor).

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

No clients override this.  My original response was to agree to remove this. 
But with this removed we get:

error: ‘class SkScanlineDecoder’ has virtual functions and accessible
non-virtual destructor [-Werror=non-virtual-dtor]

The internet tells me this is bad because if we wrote:
BaseClass obj = new SubClass();
delete obj;
The call to delete would not be able to call the destructor of the SubClass. 
This is not an issue for us since none of our subclasses have destructors.

I still think it's better to avoid the warning and keep it virtual.  We do in
fact need a public destructor in order to use SkAutoTDeletes.  I guess maybe we
will actually want to override it in a subclass sometime.

 
     /**
      *  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 22 matching lines @@
         // Also note that while getScanlines checks that rowBytes is big enough,
         // 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
-     *  any necessary cleanups.
-     */
-    virtual void onFinish() {} // Default does nothing.
 };
 #endif // SkScanlineDecoder_DEFINED