Consolidate SkCodec functions for handling rewind

Previously, many of our codec implementations followed the same
pattern (often in a function named handleRewind):

switch (this->rewindIfNeeded()) {
  case CouldNotRewind:
    return CouldNotRewind;
  case NoRewindNecessary:
    // keep going
    break;
  case Rewound:
     <re-read header etc>
    break;
}

In this CL, remove the enum, and put the piece that happens in the
Rewound case into a virtual function, onRewind. rewindIfNeeded now
contains the common pieces from various functions named handleRewind.

In SkBmpCodec, add a function that returns whether the BMP is in ICO,
so it can have a common implementation for onRewind.

BUG=skia:3257

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

include/codec/SkCodec.h

@@ -233,26 +233,27 @@ protected:
 
     virtual bool onReallyHasAlpha() const { return false; }
 
-    enum RewindState {
-        kRewound_RewindState,
-        kNoRewindNecessary_RewindState,
-        kCouldNotRewind_RewindState
-    };
     /**
      *  If the stream was previously read, attempt to rewind.
-     *  @returns:
-     *      kRewound if the stream needed to be rewound, and the
-     *               rewind succeeded.
-     *      kNoRewindNecessary if the stream did not need to be
-     *                         rewound.
-     *      kCouldNotRewind if the stream needed to be rewound, and
-     *                      rewind failed.
+     *
+     *  If the stream needed to be rewound, call onRewind.
+     *  @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.
      */
-    RewindState SK_WARN_UNUSED_RESULT rewindIfNeeded();
+    bool SK_WARN_UNUSED_RESULT rewindIfNeeded();
+
+    /**
+     *  Called by rewindIfNeeded, if the stream needed to be rewound.
+     *
+     *  Subclasses should do any set up needed after a rewind.
+     */
+    virtual bool onRewind() {
+        return true;
+    }
 
     /**
      * Get method for the input stream

src/codec/SkBmpCodec.cpp

@@ -533,19 +533,8 @@ SkBmpCodec::SkBmpCodec(const SkImageInfo& info, SkStream* stream,
     , fRowOrder(rowOrder)
 {}
 
-/*
- * Rewinds the image stream if necessary
- */
-bool SkBmpCodec::handleRewind(bool inIco) {
-    SkCodec::RewindState rewindState = this->rewindIfNeeded();
-    if (rewindState == kCouldNotRewind_RewindState) {
-        return false;
-    } else if (rewindState == kRewound_RewindState) {
-        if (!SkBmpCodec::ReadHeader(this->stream(), inIco, NULL)) {
-            return false;
-        }
-    }
-    return true;
+bool SkBmpCodec::onRewind() {
+    return SkBmpCodec::ReadHeader(this->stream(), this->inIco(), NULL);
 }
 
 /*

src/codec/SkBmpCodec.h

@@ -63,10 +63,18 @@ protected:
      */
     static bool ReadHeader(SkStream*, bool inIco, SkCodec** codecOut);
 
+    bool onRewind() override;
+
     /*
-     * Rewinds the image stream if necessary
+     * Returns whether this BMP is part of an ICO image.
      */
-    bool handleRewind(bool inIco);
+    bool inIco() const {
+        return this->onInIco();
+    }
+
+    virtual bool onInIco() const {
+        return false;
+    }
 
     /*
      * Get the destination row to start filling from

src/codec/SkBmpMaskCodec.cpp

@@ -56,7 +56,7 @@ SkCodec::Result SkBmpMaskCodec::onGetPixels(const SkImageInfo& dstInfo,
                                             const Options& opts,
                                             SkPMColor* inputColorPtr,
                                             int* inputColorCount) {
-    if (!this->handleRewind(false)) {
+    if (!this->rewindIfNeeded()) {
         return kCouldNotRewind;
     }
     if (opts.fSubset) {

src/codec/SkBmpRLECodec.cpp

@@ -66,7 +66,7 @@ SkCodec::Result SkBmpRLECodec::onGetPixels(const SkImageInfo& dstInfo,
                                         const Options& opts,
                                         SkPMColor* inputColorPtr,
                                         int* inputColorCount) {
-    if (!this->handleRewind(false)) {
+    if (!this->rewindIfNeeded()) {
         return kCouldNotRewind;
     }
     if (opts.fSubset) {

src/codec/SkBmpStandardCodec.cpp

@@ -66,7 +66,7 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo,
                                         const Options& opts,
                                         SkPMColor* inputColorPtr,
                                         int* inputColorCount) {
-    if (!this->handleRewind(fInIco)) {
+    if (!this->rewindIfNeeded()) {
         return kCouldNotRewind;
     }
     if (opts.fSubset) {

src/codec/SkBmpStandardCodec.h

@@ -45,6 +45,9 @@ protected:
                        size_t dstRowBytes, const Options&, SkPMColor*,
                        int*) override;
 
+    bool onInIco() const override {
+        return fInIco;
+    }
 private:
 
     /*

src/codec/SkCodec.cpp

@@ -83,16 +83,20 @@ SkCodec::SkCodec(const SkImageInfo& info, SkStream* stream)
 
 SkCodec::~SkCodec() {}
 
-SkCodec::RewindState SkCodec::rewindIfNeeded() {
+bool SkCodec::rewindIfNeeded() {
     // Store the value of fNeedsRewind so we can update it. Next read will
     // require a rewind.
     const bool needsRewind = fNeedsRewind;
     fNeedsRewind = true;
     if (!needsRewind) {
-        return kNoRewindNecessary_RewindState;
+        return true;
     }
-    return fStream->rewind() ? kRewound_RewindState
-                             : kCouldNotRewind_RewindState;
+
+    if (!fStream->rewind()) {
+        return false;
+    }
+
+    return this->onRewind();
 }
 
 SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,

src/codec/SkCodec_libgif.cpp

@@ -234,6 +234,17 @@ static bool conversion_possible(const SkImageInfo& dst,
     }
 }
 
+bool SkGifCodec::onRewind() {
+    GifFileType* gifOut = NULL;
+    if (!ReadHeader(this->stream(), NULL, &gifOut)) {
+        return false;
+    }
+
+    SkASSERT(NULL != gifOut);
+    fGif.reset(gifOut);
+    return true;
+}
+
 /*
  * Initiates the gif decode
  */
@@ -243,17 +254,8 @@ SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo,
                                         SkPMColor* inputColorPtr,
                                         int* inputColorCount) {
     // Rewind if necessary
-    SkCodec::RewindState rewindState = this->rewindIfNeeded();
-    if (rewindState == kCouldNotRewind_RewindState) {
+    if (!this->rewindIfNeeded()) {
         return kCouldNotRewind;
-    } else if (rewindState == kRewound_RewindState) {
-        GifFileType* gifOut = NULL;
-        if (!ReadHeader(this->stream(), NULL, &gifOut)) {
-            return kCouldNotRewind;
-        } else {
-            SkASSERT(NULL != gifOut);
-            fGif.reset(gifOut);
-        }
     }
 
     // Check for valid input parameters

src/codec/SkCodec_libgif.h

@@ -64,6 +64,8 @@ protected:
         return kGIF_SkEncodedFormat;
     }
 
+    bool onRewind() override;
+
 private:
 
     /*

src/codec/SkCodec_libpng.cpp

@@ -471,32 +471,23 @@ SkCodec::Result SkPngCodec::initializeSwizzler(const SkImageInfo& requestedInfo,
 }
 
 
-bool SkPngCodec::handleRewind() {
-    switch (this->rewindIfNeeded()) {
-        case kNoRewindNecessary_RewindState:
-            return true;
-        case kCouldNotRewind_RewindState:
-            return false;
-        case kRewound_RewindState: {
-            // This sets fPng_ptr and fInfo_ptr to NULL. If read_header
-            // succeeds, they will be repopulated, and if it fails, they will
-            // remain NULL. Any future accesses to fPng_ptr and fInfo_ptr will
-            // come through this function which will rewind and again attempt
-            // to reinitialize them.
-            this->destroyReadStruct();
-            png_structp png_ptr;
-            png_infop info_ptr;
-            if (read_header(this->stream(), &png_ptr, &info_ptr, NULL, NULL)) {
-                fPng_ptr = png_ptr;
-                fInfo_ptr = info_ptr;
-                return true;
-            }
-            return false;
-        }
-        default:
-            SkASSERT(false);
-            return false;
+bool SkPngCodec::onRewind() {
+    // This sets fPng_ptr and fInfo_ptr to NULL. If read_header
+    // succeeds, they will be repopulated, and if it fails, they will
+    // remain NULL. Any future accesses to fPng_ptr and fInfo_ptr will
+    // come through this function which will rewind and again attempt
+    // to reinitialize them.
+    this->destroyReadStruct();
+
+    png_structp png_ptr;
+    png_infop info_ptr;
+    if (!read_header(this->stream(), &png_ptr, &info_ptr, NULL, NULL)) {
+        return false;
     }
+
+    fPng_ptr = png_ptr;
+    fInfo_ptr = info_ptr;
+    return true;
 }
 
 SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& requestedInfo, void* dst,
@@ -512,7 +503,7 @@ SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& requestedInfo, void*
     if (requestedInfo.dimensions() != this->getInfo().dimensions()) {
         return kInvalidScale;
     }
-    if (!this->handleRewind()) {
+    if (!this->rewindIfNeeded()) {
         return kCouldNotRewind;
     }
 
@@ -593,7 +584,7 @@ public:
                             const SkCodec::Options& options,
                             SkPMColor ctable[], int* ctableCount) override
     {
-        if (!fCodec->handleRewind()) {
+        if (!fCodec->rewindIfNeeded()) {
             return SkCodec::kCouldNotRewind;
         }
 
@@ -677,7 +668,7 @@ public:
                             const SkCodec::Options& options,
                             SkPMColor ctable[], int* ctableCount) override
     {
-        if (!fCodec->handleRewind()) {
+        if (!fCodec->rewindIfNeeded()) {
             return SkCodec::kCouldNotRewind;
         }
 
@@ -714,7 +705,7 @@ public:
             // We already rewound in onStart, so there is no reason to rewind.
             // Next time onGetScanlines is called, we will need to rewind.
             fCanSkipRewind = false;
-        } else if (!fCodec->handleRewind()) {
+        } else if (!fCodec->rewindIfNeeded()) {
             return SkCodec::kCouldNotRewind;
         }
 

src/codec/SkCodec_libpng.h

@@ -35,6 +35,7 @@ protected:
     Result onGetPixels(const SkImageInfo&, void*, size_t, const Options&, SkPMColor*, int*)
             override;
     SkEncodedFormat onGetEncodedFormat() const override { return kPNG_SkEncodedFormat; }
+    bool onRewind() override;
     bool onReallyHasAlpha() const override { return fReallyHasAlpha; }
 private:
     png_structp                 fPng_ptr;
@@ -56,8 +57,6 @@ private:
     Result initializeSwizzler(const SkImageInfo& requestedInfo, const Options&,
                               SkPMColor*, int* ctableCount);
 
-    // Calls rewindIfNeeded and returns true if the decoder can continue.
-    bool handleRewind();
     bool decodePalette(bool premultiply, int* ctableCount);
     void destroyReadStruct();
 

src/codec/SkCodec_wbmp.cpp

@@ -66,16 +66,8 @@ static bool read_header(SkStream* stream, SkISize* size) {
     return true;
 }
 
-bool SkWbmpCodec::handleRewind() {
-    SkCodec::RewindState rewindState = this->rewindIfNeeded();
-    if (rewindState == kCouldNotRewind_RewindState) {
-        return false;
-    } else if (rewindState == kRewound_RewindState) {
-        if (!read_header(this->stream(), NULL)) {
-            return false;
-        }
-    }
-    return true;
+bool SkWbmpCodec::onRewind() {
+    return read_header(this->stream(), NULL);
 }
 
 SkSwizzler* SkWbmpCodec::initializeSwizzler(const SkImageInfo& info,
@@ -117,7 +109,7 @@ SkCodec::Result SkWbmpCodec::onGetPixels(const SkImageInfo& info,
                                          const Options& options,
                                          SkPMColor ctable[],
                                          int* ctableCount) {
-    if (!this->handleRewind()) {
+    if (!this->rewindIfNeeded()) {
         return kCouldNotRewind;
     }
     if (options.fSubset) {
@@ -197,7 +189,7 @@ public:
     SkCodec::Result onStart(const SkImageInfo& dstInfo,
             const SkCodec::Options& options, SkPMColor inputColorTable[],
             int* inputColorCount) {
-        if (!fCodec->handleRewind()) {
+        if (!fCodec->rewindIfNeeded()) {
             return SkCodec::kCouldNotRewind;
         }
         if (options.fSubset) {

src/codec/SkCodec_wbmp.h

@@ -33,13 +33,8 @@ protected:
     SkEncodedFormat onGetEncodedFormat() const override;
     Result onGetPixels(const SkImageInfo&, void*, size_t,
                        const Options&, SkPMColor[], int*) override;
+    bool onRewind() override;
 private:
-
-    /*
-     * Calls rewindIfNeeded and returns true if the decoder can continue
-     */
-    bool handleRewind();
-
     /*
      * Returns a swizzler on success, NULL on failure
      */

src/codec/SkJpegCodec.cpp

@@ -189,28 +189,14 @@ SkISize SkJpegCodec::onGetScaledDimensions(float desiredScale) const {
     return SkISize::Make(dinfo.output_width, dinfo.output_height);
 }
 
-/*
- * Handles rewinding the input stream if it is necessary
- */
-bool SkJpegCodec::handleRewind() {
-    switch(this->rewindIfNeeded()) {
-        case kCouldNotRewind_RewindState:
-            return fDecoderMgr->returnFalse("could not rewind");
-        case kRewound_RewindState: {
-            JpegDecoderMgr* decoderMgr = NULL;
-            if (!ReadHeader(this->stream(), NULL, &decoderMgr)) {
-                return fDecoderMgr->returnFalse("could not rewind");
-            }
-            SkASSERT(NULL != decoderMgr);
-            fDecoderMgr.reset(decoderMgr);
-            return true;
-        }
-        case kNoRewindNecessary_RewindState:
-            return true;
-        default:
-            SkASSERT(false);
-            return false;
+bool SkJpegCodec::onRewind() {
+    JpegDecoderMgr* decoderMgr = NULL;
+    if (!ReadHeader(this->stream(), NULL, &decoderMgr)) {
+        return fDecoderMgr->returnFalse("could not rewind");
     }
+    SkASSERT(NULL != decoderMgr);
+    fDecoderMgr.reset(decoderMgr);
+    return true;
 }
 
 /*
@@ -304,7 +290,7 @@ SkCodec::Result SkJpegCodec::onGetPixels(const SkImageInfo& dstInfo,
                                          void* dst, size_t dstRowBytes,
                                          const Options& options, SkPMColor*, int*) {
     // Rewind the stream if needed
-    if (!this->handleRewind()) {
+    if (!this->rewindIfNeeded()) {
         return fDecoderMgr->returnFailure("could not rewind stream", kCouldNotRewind);
     }
 
@@ -399,7 +385,7 @@ public:
                             SkPMColor ctable[], int* ctableCount) override {
 
         // Rewind the stream if needed
-        if (!fCodec->handleRewind()) {
+        if (!fCodec->rewindIfNeeded()) {
             return SkCodec::kCouldNotRewind;
         }
 

src/codec/SkJpegCodec.h

@@ -65,6 +65,8 @@ protected:
         return kJPEG_SkEncodedFormat;
     }
 
+    bool onRewind() override;
+
 private:
 
     /*
@@ -100,11 +102,6 @@ private:
      */
     SkJpegCodec(const SkImageInfo& srcInfo, SkStream* stream, JpegDecoderMgr* decoderMgr);
 
-    /*
-     * Handles rewinding the input stream if it is necessary
-     */
-    bool handleRewind();
-
     /*
      * Checks if the conversion between the input image and the requested output
      * image has been implemented

src/codec/SkWebpCodec.cpp

@@ -153,16 +153,8 @@ bool SkWebpCodec::onGetValidSubset(SkIRect* desiredSubset) const {
 
 SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, size_t rowBytes,
                                          const Options& options, SkPMColor*, int*) {
-    switch (this->rewindIfNeeded()) {
-        case kCouldNotRewind_RewindState:
-            return kCouldNotRewind;
-        case kRewound_RewindState:
-            // Rewound to the beginning. Since creation only does a peek, the stream is at the
-            // correct position.
-            break;
-        case kNoRewindNecessary_RewindState:
-            // Already at the right spot for decoding.
-            break;
+    if (!this->rewindIfNeeded()) {
+        return kCouldNotRewind;
     }
 
     if (!conversion_possible(dstInfo, this->getInfo())) {