Destroy SkScanlineDecoder in the SkCodec subclass destructors

include/codec/SkScanlineDecoder.h

Index: include/codec/SkScanlineDecoder.h
diff --git a/include/codec/SkScanlineDecoder.h b/include/codec/SkScanlineDecoder.h
index d7f73dda56f92ef4bccc7bc431282b43d05a05a0..eac31ef40f05ca28b838c4031b037f8c3e979c6e 100644
--- a/include/codec/SkScanlineDecoder.h
+++ b/include/codec/SkScanlineDecoder.h
@@ -15,8 +15,20 @@
 
 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.

 
     /**
@@ -34,7 +46,7 @@ public:
             return SkImageGenerator::kInvalidParameters;
         }
         const SkImageGenerator::Result result = this->onGetScanlines(dst, countLines, rowBytes);
-        this->checkForFinish(countLines);
+        fCurrScanline += countLines;
         return result;
     }
 
@@ -54,7 +66,7 @@ public:
             return SkImageGenerator::kInvalidParameters;
         }
         const SkImageGenerator::Result result = this->onSkipScanlines(countLines);
-        this->checkForFinish(countLines);
+        fCurrScanline += countLines;
         return result;
     }
 
@@ -97,21 +109,5 @@ private:
     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