Call rewindIfNeeded in SkCodec

Rather than calling it in each subclass, call it once in the base
class. Call it first, since other steps may modify internal structures
which would be replaced by a call to onRewind.

BUG=skia:4284

Review URL: https://codereview.chromium.org/1381483002

12 files changed, 17 insertions, 52 deletions

diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h
index 01fd4fa62a..c0cce853a3 100644
--- a/include/codec/SkCodec.h
+++ b/include/codec/SkCodec.h
@@ -402,9 +402,8 @@ protected:
      *  @returns true if the codec is at the right position and can be used.
      *      false if there was a failure to rewind.
      *
-     *  Subclasses MUST call this function before reading the stream (e.g. in
-     *  onGetPixels). If it returns false, onGetPixels should return
-     *  kCouldNotRewind.
+     *  This is called by getPixels() and start(). Subclasses may call if they
+     *  need to rewind at another time.
      */
     bool SK_WARN_UNUSED_RESULT rewindIfNeeded();
 
diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index 4e801f0be1..4ae57c057a 100644
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -576,9 +576,6 @@ uint32_t SkBmpCodec::computeNumColors(uint32_t numColors) {
 
 SkCodec::Result SkBmpCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
         const SkCodec::Options& options, SkPMColor inputColorPtr[], int* inputColorCount) {
-    if (!this->rewindIfNeeded()) {
-        return kCouldNotRewind;
-    }
     if (options.fSubset) {
         // Subsets are not supported.
         return kUnimplemented;
diff --git a/src/codec/SkBmpMaskCodec.cpp b/src/codec/SkBmpMaskCodec.cpp
index 2e88d54f02..81067c7711 100644
--- a/src/codec/SkBmpMaskCodec.cpp
+++ b/src/codec/SkBmpMaskCodec.cpp
@@ -30,9 +30,6 @@ SkCodec::Result SkBmpMaskCodec::onGetPixels(const SkImageInfo& dstInfo,
                                             const Options& opts,
                                             SkPMColor* inputColorPtr,
                                             int* inputColorCount) {
-    if (!this->rewindIfNeeded()) {
-        return kCouldNotRewind;
-    }
     if (opts.fSubset) {
         // Subsets are not supported.
         return kUnimplemented;
diff --git a/src/codec/SkBmpRLECodec.cpp b/src/codec/SkBmpRLECodec.cpp
index 79dd63307e..e76a23ed14 100644
--- a/src/codec/SkBmpRLECodec.cpp
+++ b/src/codec/SkBmpRLECodec.cpp
@@ -39,9 +39,6 @@ SkCodec::Result SkBmpRLECodec::onGetPixels(const SkImageInfo& dstInfo,
                                            const Options& opts,
                                            SkPMColor* inputColorPtr,
                                            int* inputColorCount) {
-    if (!this->rewindIfNeeded()) {
-        return kCouldNotRewind;
-    }
     if (opts.fSubset) {
         // Subsets are not supported.
         return kUnimplemented;
diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp
index 02f2d4215a..5e65ebbe53 100644
--- a/src/codec/SkBmpStandardCodec.cpp
+++ b/src/codec/SkBmpStandardCodec.cpp
@@ -37,9 +37,6 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo,
                                         const Options& opts,
                                         SkPMColor* inputColorPtr,
                                         int* inputColorCount) {
-    if (!this->rewindIfNeeded()) {
-        return kCouldNotRewind;
-    }
     if (opts.fSubset) {
         // Subsets are not supported.
         return kUnimplemented;
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index 97bcf3e1d8..cabcadcccd 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -87,6 +87,12 @@ SkCodec::SkCodec(const SkImageInfo& info, SkStream* stream)
 SkCodec::~SkCodec() {}
 
 bool SkCodec::rewindIfNeeded() {
+    if (!fStream) {
+        // Some codecs do not have a stream, but they hold others that do. They
+        // must handle rewinding themselves.
+        return true;
+    }
+
     // Store the value of fNeedsRewind so we can update it. Next read will
     // require a rewind.
     const bool needsRewind = fNeedsRewind;
@@ -138,6 +144,10 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t
         }
     }
 
+    if (!this->rewindIfNeeded()) {
+        return kCouldNotRewind;
+    }
+
     // Default options.
     Options optsStorage;
     if (nullptr == options) {
@@ -172,6 +182,10 @@ SkCodec::Result SkCodec::startScanlineDecode(const SkImageInfo& dstInfo,
         ctable = nullptr;
     }
 
+    if (!this->rewindIfNeeded()) {
+        return kCouldNotRewind;
+    }
+
     // Set options.
     Options optsStorage;
     if (nullptr == options) {
diff --git a/src/codec/SkCodec_libgif.cpp b/src/codec/SkCodec_libgif.cpp
index ad7fb5b578..e6d1141669 100644
--- a/src/codec/SkCodec_libgif.cpp
+++ b/src/codec/SkCodec_libgif.cpp
@@ -456,11 +456,6 @@ void SkGifCodec::initializeColorTable(const SkImageInfo& dstInfo, SkPMColor* inp
 
 SkCodec::Result SkGifCodec::prepareToDecode(const SkImageInfo& dstInfo, SkPMColor* inputColorPtr,
         int* inputColorCount, const Options& opts) {
-    // Rewind if necessary
-    if (!this->rewindIfNeeded()) {
-        return kCouldNotRewind;
-    }
-
     // Check for valid input parameters
     if (opts.fSubset) {
         // Subsets are not supported.
diff --git a/src/codec/SkCodec_libgif.h b/src/codec/SkCodec_libgif.h
index 3999c41ede..d7dc2b9e1d 100644
--- a/src/codec/SkCodec_libgif.h
+++ b/src/codec/SkCodec_libgif.h
@@ -108,7 +108,7 @@ private:
             int* inputColorCount);
 
    /*
-    * Checks for invalid inputs and calls rewindIfNeeded(), setFramDimensions(), and
+    * Checks for invalid inputs and calls setFrameDimensions(), and
     * initializeColorTable() in the proper sequence.
     */
     Result prepareToDecode(const SkImageInfo& dstInfo, SkPMColor* inputColorPtr,
diff --git a/src/codec/SkCodec_libpng.cpp b/src/codec/SkCodec_libpng.cpp
index faba79f6a2..1af8658e1e 100644
--- a/src/codec/SkCodec_libpng.cpp
+++ b/src/codec/SkCodec_libpng.cpp
@@ -477,9 +477,6 @@ SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& requestedInfo, void*
     if (requestedInfo.dimensions() != this->getInfo().dimensions()) {
         return kInvalidScale;
     }
-    if (!this->rewindIfNeeded()) {
-        return kCouldNotRewind;
-    }
 
     // Note that ctable and ctableCount may be modified if there is a color table
     const Result result = this->initializeSwizzler(requestedInfo, options,
@@ -592,10 +589,6 @@ public:
 
     Result onStartScanlineDecode(const SkImageInfo& dstInfo, const Options& options,
             SkPMColor ctable[], int* ctableCount) override {
-        if (!this->rewindIfNeeded()) {
-            return kCouldNotRewind;
-        }
-
         if (!conversion_possible(dstInfo, this->getInfo())) {
             return kInvalidConversion;
         }
@@ -690,10 +683,6 @@ public:
     Result onStartScanlineDecode(const SkImageInfo& dstInfo, const Options& options,
             SkPMColor ctable[], int* ctableCount) override
     {
-        if (!this->rewindIfNeeded()) {
-            return kCouldNotRewind;
-        }
-
         if (!conversion_possible(dstInfo, this->getInfo())) {
             return kInvalidConversion;    
         }
diff --git a/src/codec/SkCodec_wbmp.cpp b/src/codec/SkCodec_wbmp.cpp
index 0c85eada7c..1696dfb585 100644
--- a/src/codec/SkCodec_wbmp.cpp
+++ b/src/codec/SkCodec_wbmp.cpp
@@ -110,9 +110,6 @@ SkCodec::Result SkWbmpCodec::onGetPixels(const SkImageInfo& info,
                                          const Options& options,
                                          SkPMColor ctable[],
                                          int* ctableCount) {
-    if (!this->rewindIfNeeded()) {
-        return kCouldNotRewind;
-    }
     if (options.fSubset) {
         // Subsets are not supported.
         return kUnimplemented;
@@ -180,9 +177,6 @@ SkCodec::Result SkWbmpCodec::onGetScanlines(void* dst, int count, size_t dstRowB
 
 SkCodec::Result SkWbmpCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
         const Options& options, SkPMColor inputColorTable[], int* inputColorCount) {
-    if (!this->rewindIfNeeded()) {
-        return kCouldNotRewind;
-    }
     if (options.fSubset) {
         // Subsets are not supported.
         return kUnimplemented;
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index dac0c17b77..b4c794c906 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -303,11 +303,6 @@ bool SkJpegCodec::nativelyScaleToDimensions(uint32_t dstWidth, uint32_t dstHeigh
 SkCodec::Result SkJpegCodec::onGetPixels(const SkImageInfo& dstInfo,
                                          void* dst, size_t dstRowBytes,
                                          const Options& options, SkPMColor*, int*) {
-    // Rewind the stream if needed
-    if (!this->rewindIfNeeded()) {
-        return fDecoderMgr->returnFailure("could not rewind stream", kCouldNotRewind);
-    }
-
     if (options.fSubset) {
         // Subsets are not supported.
         return kUnimplemented;
@@ -412,11 +407,6 @@ SkCodec::Result SkJpegCodec::initializeSwizzler(const SkImageInfo& info, const O
 
 SkCodec::Result SkJpegCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
         const Options& options, SkPMColor ctable[], int* ctableCount) {
-    // Rewind the stream if needed
-    if (!this->rewindIfNeeded()) {
-        return kCouldNotRewind;
-    }
-
     // Set the jump location for libjpeg errors
     if (setjmp(fDecoderMgr->getJmpBuf())) {
         SkCodecPrintf("setjmp: Error from libjpeg\n");
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index ccf49649ed..5970baeef6 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -152,10 +152,6 @@ bool SkWebpCodec::onGetValidSubset(SkIRect* desiredSubset) const {
 
 SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, size_t rowBytes,
                                          const Options& options, SkPMColor*, int*) {
-    if (!this->rewindIfNeeded()) {
-        return kCouldNotRewind;
-    }
-
     if (!webp_conversion_possible(dstInfo, this->getInfo())) {
         return kInvalidConversion;
     }