Consolidate decoding frames into SkCodec

Add a new private method to SkCodec that handles Options.fFrameIndex:
- Check to ensure the index is valid
- Call onGetFrameCount to force parsing the stream
- Recursively call getPixels (it should be complete, so no need for
  incremental decoding) to decode the prior frame if necessary
- Zero fill a RestoreBGColor frame

Call the method in getPixels and startIncrementalDecode, and remove
duplicate code from GIF and WEBP.

Remove support for scaling frames beyond the first, which is currently
unused.

Preserve the feature of SkGifCodec that it will only parse to the end
of the first frame if the first frame is asked for. (Also note that
when we continue a partial frame, we won't force parsing the full
stream.) If the client only wants the first frame, parsing the rest
would be unnecessary. But if the client wants the second, we assume
they will want any remaining frames, so we parse the remainder of the
stream. This simplifies the code (so SkCodec does not have to ask its
subclass to parse up to a particular frame).

Update tests that relied on the old behavior:
- Codec_partialAnim now hardcodes the bytes needed. Previously it
  relied on the old behavior that GIF only parsed up to the frame being
  decoded.
- Codec_skipFullParse now only tests the first frame, since that is the
  case where it is important to skip a full parse.

TBR=reed@google.com
No changes to the public API.

Change-Id: Ic2f075452dfeedb4e3e60e6cf4df33ee7bd38495
Reviewed-on: https://skia-review.googlesource.com/19276
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>

8 files changed, 177 insertions, 207 deletions

diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h
index 4773d20a0c..6b3aa5e020 100644
--- a/include/codec/SkCodec.h
+++ b/include/codec/SkCodec.h
@@ -23,8 +23,8 @@
 #include <vector>
 
 class SkColorSpace;
-class SkColorSpaceXform;
 class SkData;
+class SkFrameHolder;
 class SkPngChunkReader;
 class SkSampler;
 
@@ -880,6 +880,18 @@ private:
         return dim == fSrcInfo.dimensions() || this->onDimensionsSupported(dim);
     }
 
+    /**
+     *  For multi-framed images, return the object with information about the frames.
+     */
+    virtual const SkFrameHolder* getFrameHolder() const {
+        return nullptr;
+    }
+
+    /**
+     *  Check for a valid Options.fFrameIndex, and decode prior frames if necessary.
+     */
+    Result handleFrameIndex(const SkImageInfo&, void* pixels, size_t rowBytes, const Options&);
+
     // Methods for scanline decoding.
     virtual Result onStartScanlineDecode(const SkImageInfo& /*dstInfo*/,
             const Options& /*options*/, SkPMColor* /*ctable*/, int* /*ctableCount*/) {
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index 2cae6db1df..60d25210c8 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -11,6 +11,7 @@
 #include "SkColorSpace.h"
 #include "SkColorSpaceXform_Base.h"
 #include "SkData.h"
+#include "SkFrameHolder.h"
 #include "SkGifCodec.h"
 #include "SkHalf.h"
 #include "SkIcoCodec.h"
@@ -179,6 +180,90 @@ bool SkCodec::rewindIfNeeded() {
         ctable = nullptr;                                   \
     }
 
+static void zero_rect(const SkImageInfo& dstInfo, void* pixels, size_t rowBytes,
+                      SkIRect frameRect) {
+    if (!frameRect.intersect(dstInfo.bounds())) {
+        return;
+    }
+    const auto info = dstInfo.makeWH(frameRect.width(), frameRect.height());
+    const size_t bpp = SkColorTypeBytesPerPixel(dstInfo.colorType());
+    const size_t offset = frameRect.x() * bpp + frameRect.y() * rowBytes;
+    auto* eraseDst = SkTAddOffset<void>(pixels, offset);
+    SkSampler::Fill(info, eraseDst, rowBytes, 0, SkCodec::kNo_ZeroInitialized);
+}
+
+SkCodec::Result SkCodec::handleFrameIndex(const SkImageInfo& info, void* pixels, size_t rowBytes,
+                                          const Options& options) {
+    const int index = options.fFrameIndex;
+    if (0 == index) {
+        return kSuccess;
+    }
+
+    if (options.fSubset || info.dimensions() != fSrcInfo.dimensions()) {
+        // If we add support for these, we need to update the code that zeroes
+        // a kRestoreBGColor frame.
+        return kInvalidParameters;
+    }
+
+    // index 8 is not supported beyond the first frame.
+    if (index < 0 || info.colorType() == kIndex_8_SkColorType) {
+        return kInvalidParameters;
+    }
+
+    if (index >= this->onGetFrameCount()) {
+        return kIncompleteInput;
+    }
+
+    const auto* frameHolder = this->getFrameHolder();
+    SkASSERT(frameHolder);
+
+    const auto* frame = frameHolder->getFrame(index);
+    SkASSERT(frame);
+
+    const int requiredFrame = frame->getRequiredFrame();
+    if (requiredFrame == kNone) {
+        return kSuccess;
+    }
+
+    if (options.fPriorFrame != kNone) {
+        // Check for a valid frame as a starting point. Alternatively, we could
+        // treat an invalid frame as not providing one, but rejecting it will
+        // make it easier to catch the mistake.
+        if (options.fPriorFrame < requiredFrame || options.fPriorFrame >= index) {
+            return kInvalidParameters;
+        }
+        const auto* prevFrame = frameHolder->getFrame(options.fPriorFrame);
+        switch (prevFrame->getDisposalMethod()) {
+            case SkCodecAnimation::DisposalMethod::kRestorePrevious:
+                return kInvalidParameters;
+            case SkCodecAnimation::DisposalMethod::kRestoreBGColor:
+                // If a frame after the required frame is provided, there is no
+                // need to clear, since it must be covered by the desired frame.
+                if (options.fPriorFrame == requiredFrame) {
+                    zero_rect(info, pixels, rowBytes, prevFrame->frameRect());
+                }
+                break;
+            default:
+                break;
+        }
+        return kSuccess;
+    }
+
+    Options prevFrameOptions(options);
+    prevFrameOptions.fFrameIndex = requiredFrame;
+    prevFrameOptions.fZeroInitialized = kNo_ZeroInitialized;
+    const Result result = this->getPixels(info, pixels, rowBytes, &prevFrameOptions,
+                                          nullptr, nullptr);
+    if (result == kSuccess) {
+        const auto* prevFrame = frameHolder->getFrame(requiredFrame);
+        const auto disposalMethod = prevFrame->getDisposalMethod();
+        if (disposalMethod == SkCodecAnimation::DisposalMethod::kRestoreBGColor) {
+            zero_rect(info, pixels, rowBytes, prevFrame->frameRect());
+        }
+    }
+
+    return result;
+}
 
 SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
                                    const Options* options, SkPMColor ctable[], int* ctableCount) {
@@ -202,12 +287,18 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t
     Options optsStorage;
     if (nullptr == options) {
         options = &optsStorage;
-    } else if (options->fSubset) {
-        SkIRect subset(*options->fSubset);
-        if (!this->onGetValidSubset(&subset) || subset != *options->fSubset) {
-            // FIXME: How to differentiate between not supporting subset at all
-            // and not supporting this particular subset?
-            return kUnimplemented;
+    } else {
+        const Result frameIndexResult = this->handleFrameIndex(info, pixels, rowBytes, *options);
+        if (frameIndexResult != kSuccess) {
+            return frameIndexResult;
+        }
+        if (options->fSubset) {
+            SkIRect subset(*options->fSubset);
+            if (!this->onGetValidSubset(&subset) || subset != *options->fSubset) {
+                // FIXME: How to differentiate between not supporting subset at all
+                // and not supporting this particular subset?
+                return kUnimplemented;
+            }
         }
     }
 
@@ -280,16 +371,22 @@ SkCodec::Result SkCodec::startIncrementalDecode(const SkImageInfo& info, void* p
     Options optsStorage;
     if (nullptr == options) {
         options = &optsStorage;
-    } else if (options->fSubset) {
-        SkIRect size = SkIRect::MakeSize(info.dimensions());
-        if (!size.contains(*options->fSubset)) {
-            return kInvalidParameters;
+    } else {
+        const Result frameIndexResult = this->handleFrameIndex(info, pixels, rowBytes, *options);
+        if (frameIndexResult != kSuccess) {
+            return frameIndexResult;
         }
+        if (options->fSubset) {
+            SkIRect size = SkIRect::MakeSize(info.dimensions());
+            if (!size.contains(*options->fSubset)) {
+                return kInvalidParameters;
+            }
 
-        const int top = options->fSubset->top();
-        const int bottom = options->fSubset->bottom();
-        if (top < 0 || top >= info.height() || top >= bottom || bottom > info.height()) {
-            return kInvalidParameters;
+            const int top = options->fSubset->top();
+            const int bottom = options->fSubset->bottom();
+            if (top < 0 || top >= info.height() || top >= bottom || bottom > info.height()) {
+                return kInvalidParameters;
+            }
         }
     }
 
diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index 6c4273444c..eefe8986b7 100644
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -196,51 +196,40 @@ SkCodec::Result SkGifCodec::prepareToDecode(const SkImageInfo& dstInfo, SkPMColo
     }
 
     const int frameIndex = opts.fFrameIndex;
-    if (frameIndex > 0) {
-        switch (dstInfo.colorType()) {
-            case kIndex_8_SkColorType:
-                // FIXME: It is possible that a later frame can be decoded to index8, if it does one
-                // of the following:
-                // - Covers the entire previous frame
-                // - Shares a color table (and transparent index) with any prior frames that are
-                //   showing.
-                // We must support index8 for the first frame to be backwards compatible on Android,
-                // but we do not (currently) need to support later frames as index8.
-                return gif_error("Cannot decode multiframe gif (except frame 0) as index 8.\n",
-                                 kInvalidConversion);
-            case kRGB_565_SkColorType:
-                // FIXME: In theory, we might be able to support this, but it's not clear that it
-                // is necessary (Chromium does not decode to 565, and Android does not decode
-                // frames beyond the first). Disabling it because it is somewhat difficult:
-                // - If there is a transparent pixel, and this frame draws on top of another frame
-                //   (if the frame is independent with a transparent pixel, we should not decode to
-                //   565 anyway, since it is not opaque), we need to skip drawing the transparent
-                //   pixels (see writeTransparentPixels in haveDecodedRow). We currently do this by
-                //   first swizzling into temporary memory, then copying into the destination. (We
-                //   let the swizzler handle it first because it may need to sample.) After
-                //   swizzling to 565, we do not know which pixels in our temporary memory
-                //   correspond to the transparent pixel, so we do not know what to skip. We could
-                //   special case the non-sampled case (no need to swizzle), but as this is
-                //   currently unused we can just not support it.
-                return gif_error("Cannot decode multiframe gif (except frame 0) as 565.\n",
-                                 kInvalidConversion);
-            default:
-                break;
-        }
-    }
-
-    fReader->parse((SkGifImageReader::SkGIFParseQuery) frameIndex);
-
-    if (frameIndex >= fReader->imagesCount()) {
-        return gif_error("frame index out of range!\n", kIncompleteInput);
+    if (frameIndex > 0 && kRGB_565_SkColorType == dstInfo.colorType()) {
+        // FIXME: In theory, we might be able to support this, but it's not clear that it
+        // is necessary (Chromium does not decode to 565, and Android does not decode
+        // frames beyond the first). Disabling it because it is somewhat difficult:
+        // - If there is a transparent pixel, and this frame draws on top of another frame
+        //   (if the frame is independent with a transparent pixel, we should not decode to
+        //   565 anyway, since it is not opaque), we need to skip drawing the transparent
+        //   pixels (see writeTransparentPixels in haveDecodedRow). We currently do this by
+        //   first swizzling into temporary memory, then copying into the destination. (We
+        //   let the swizzler handle it first because it may need to sample.) After
+        //   swizzling to 565, we do not know which pixels in our temporary memory
+        //   correspond to the transparent pixel, so we do not know what to skip. We could
+        //   special case the non-sampled case (no need to swizzle), but as this is
+        //   currently unused we can just not support it.
+        return gif_error("Cannot decode multiframe gif (except frame 0) as 565.\n",
+                         kInvalidConversion);
     }
 
     const auto* frame = fReader->frameContext(frameIndex);
-    if (!frame->reachedStartOfData()) {
-        // We have parsed enough to know that there is a color map, but cannot
-        // parse the map itself yet. Exit now, so we do not build an incorrect
-        // table.
-        return gif_error("color map not available yet\n", kIncompleteInput);
+    SkASSERT(frame);
+    if (0 == frameIndex) {
+        // SkCodec does not have a way to just parse through frame 0, so we
+        // have to do so manually, here.
+        fReader->parse((SkGifImageReader::SkGIFParseQuery) 0);
+        if (!frame->reachedStartOfData()) {
+            // We have parsed enough to know that there is a color map, but cannot
+            // parse the map itself yet. Exit now, so we do not build an incorrect
+            // table.
+            return gif_error("color map not available yet\n", kIncompleteInput);
+        }
+    } else {
+        // Parsing happened in SkCodec::getPixels.
+        SkASSERT(frameIndex < fReader->imagesCount());
+        SkASSERT(frame->reachedStartOfData());
     }
 
     const auto at = frame->hasAlpha() ? kUnpremul_SkAlphaType : kOpaque_SkAlphaType;
@@ -398,75 +387,8 @@ SkCodec::Result SkGifCodec::decodeFrame(bool firstAttempt, const Options& opts,
                 filledBackground = true;
             }
         } else {
-            // Not independent
-            // FIXME: Share this code with WEBP
-            const int reqFrame = frameContext->getRequiredFrame();
-            if (opts.fPriorFrame != kNone) {
-                if (opts.fPriorFrame < reqFrame || opts.fPriorFrame >= frameIndex) {
-                    // Alternatively, we could correct this to kNone.
-                    return kInvalidParameters;
-                }
-                const auto* prevFrame = fReader->frameContext(opts.fPriorFrame);
-                if (prevFrame->getDisposalMethod()
-                        == SkCodecAnimation::DisposalMethod::kRestorePrevious) {
-                    // Similarly, this could be corrected to kNone.
-                    return kInvalidParameters;
-                }
-            } else {
-                // Decode that frame into pixels.
-                Options prevFrameOpts(opts);
-                prevFrameOpts.fFrameIndex = frameContext->getRequiredFrame();
-                prevFrameOpts.fPriorFrame = kNone;
-                // The prior frame may have a different color table, so update it and the
-                // swizzler.
-                this->initializeColorTable(dstInfo, prevFrameOpts.fFrameIndex);
-                this->initializeSwizzler(dstInfo, prevFrameOpts.fFrameIndex);
-
-                const Result prevResult = this->decodeFrame(true, prevFrameOpts, nullptr);
-                switch (prevResult) {
-                    case kSuccess:
-                        // Prior frame succeeded. Carry on.
-                        break;
-                    case kIncompleteInput:
-                        // Prior frame was incomplete. So this frame cannot be decoded.
-                        return kInvalidInput;
-                    default:
-                        return prevResult;
-                }
-
-                // Go back to using the correct color table for this frame.
-                this->initializeColorTable(dstInfo, frameIndex);
-                this->initializeSwizzler(dstInfo, frameIndex);
-            }
-
-            // If the required frame is RestoreBG, we need to erase it. If a frame after the
-            // required frame is provided, there is no need to erase, since it will be covered
-            // anyway.
-            if (opts.fPriorFrame == reqFrame || opts.fPriorFrame == kNone) {
-                const auto* prevFrame = fReader->frameContext(reqFrame);
-                if (prevFrame->getDisposalMethod()
-                        == SkCodecAnimation::DisposalMethod::kRestoreBGColor) {
-                    SkIRect prevRect = prevFrame->frameRect();
-                    if (prevRect.intersect(this->getInfo().bounds())) {
-                        // Do the divide ourselves for left and top, since we do not want
-                        // get_scaled_dimension to upgrade 0 to 1. (This is similar to
-                        // SkSampledCodec's sampling of the subset.)
-                        const auto sampleX = fSwizzler->sampleX();
-                        const auto sampleY = fSwizzler->sampleY();
-                        auto left = prevRect.fLeft / sampleX;
-                        auto top = prevRect.fTop / sampleY;
-                        void* const eraseDst = SkTAddOffset<void>(fDst, top * fDstRowBytes
-                                + left * SkColorTypeBytesPerPixel(dstInfo.colorType()));
-                        auto width = get_scaled_dimension(prevRect.width(), sampleX);
-                        auto height = get_scaled_dimension(prevRect.height(), sampleY);
-                        // fSwizzler->fill() would fill to the scaled width of the frame, but we
-                        // want to fill to the scaled with of the width of the PRIOR frame, so we
-                        // do all the scaling ourselves and call the static version.
-                        SkSampler::Fill(dstInfo.makeWH(width, height), eraseDst, fDstRowBytes,
-                                        this->getFillValue(dstInfo), kNo_ZeroInitialized);
-                    }
-                }
-            }
+            // Not independent.
+            // SkCodec ensured that the prior frame has been decoded.
             filledBackground = true;
         }
 
diff --git a/src/codec/SkGifCodec.h b/src/codec/SkGifCodec.h
index 314b50b798..33472d8ac1 100644
--- a/src/codec/SkGifCodec.h
+++ b/src/codec/SkGifCodec.h
@@ -59,6 +59,10 @@ protected:
 
     Result onIncrementalDecode(int*) override;
 
+    const SkFrameHolder* getFrameHolder() const override {
+        return fReader.get();
+    }
+
 private:
 
     /*
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index 65250a41c1..68301b93f8 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -381,11 +381,8 @@ static void blend_line(SkColorType dstCT, void* dst,
 SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, size_t rowBytes,
                                          const Options& options, SkPMColor*, int*,
                                          int* rowsDecodedPtr) {
-    // Ensure that we have parsed this far.
     const int index = options.fFrameIndex;
-    if (index >= this->onGetFrameCount()) {
-        return kIncompleteInput;
-    }
+    SkASSERT(0 == index || index < fFrameHolder.size());
 
     const auto& srcInfo = this->getInfo();
     {
@@ -401,13 +398,7 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
         }
     }
 
-    if (index > 0 && (options.fSubset || dstInfo.dimensions() != srcInfo.dimensions())) {
-        // Subsetting and scaling are tricky when asking for frames beyond frame 0. In order to
-        // support it, we'll need to determine the proper rectangle for a
-        // WEBP_MUX_DISPOSE_BACKGROUND required frame before erasing it. (Currently the order
-        // is backwards.) Disable until it becomes clear that supporting it is important.
-        return kUnimplemented;
-    }
+    SkASSERT(0 == index || (!options.fSubset && dstInfo.dimensions() == srcInfo.dimensions()));
 
     WebPDecoderConfig config;
     if (0 == WebPInitDecoderConfig(&config)) {
@@ -424,53 +415,15 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
     // If this succeeded in onGetFrameCount(), it should succeed again here.
     SkAssertResult(WebPDemuxGetFrame(fDemux, index + 1, &frame));
 
-    const int requiredFrame = index == 0 ? kNone : fFrameHolder.frame(index)->getRequiredFrame();
+    const bool independent = index == 0 ? true :
+            (fFrameHolder.frame(index)->getRequiredFrame() == kNone);
     // Get the frameRect.  libwebp will have already signaled an error if this is not fully
     // contained by the canvas.
     auto frameRect = SkIRect::MakeXYWH(frame.x_offset, frame.y_offset, frame.width, frame.height);
     SkASSERT(srcInfo.bounds().contains(frameRect));
     const bool frameIsSubset = frameRect != srcInfo.bounds();
-    if (kNone == requiredFrame) {
-        if (frameIsSubset) {
-            SkSampler::Fill(dstInfo, dst, rowBytes, 0, options.fZeroInitialized);
-        }
-    } else {
-        // FIXME: Share with GIF
-        if (options.fPriorFrame != kNone) {
-            if (options.fPriorFrame < requiredFrame || options.fPriorFrame >= index) {
-                return kInvalidParameters;
-            }
-        } else {
-            Options prevFrameOpts(options);
-            prevFrameOpts.fFrameIndex = requiredFrame;
-            prevFrameOpts.fPriorFrame = kNone;
-            const auto result = this->getPixels(dstInfo, dst, rowBytes, &prevFrameOpts,
-                                                nullptr, nullptr);
-            switch (result) {
-                case kSuccess:
-                    break;
-                case kIncompleteInput:
-                    return kInvalidInput;
-                default:
-                    return result;
-            }
-        }
-
-        if (options.fPriorFrame == requiredFrame || options.fPriorFrame == kNone) {
-            // Dispose bg color
-            const Frame* priorFrame = fFrameHolder.frame(requiredFrame);
-            if (priorFrame->getDisposalMethod()
-                    == SkCodecAnimation::DisposalMethod::kRestoreBGColor) {
-                // FIXME: If we add support for scaling/subsets, this rectangle needs to be
-                // adjusted.
-                const auto priorRect = priorFrame->frameRect();
-                const auto info = dstInfo.makeWH(priorRect.width(), priorRect.height());
-                const size_t bpp = SkColorTypeBytesPerPixel(dstInfo.colorType());
-                const size_t offset = priorRect.x() * bpp + priorRect.y() * rowBytes;
-                auto* eraseDst = SkTAddOffset<void>(dst, offset);
-                SkSampler::Fill(info, eraseDst, rowBytes, 0, kNo_ZeroInitialized);
-            }
-        }
+    if (independent && frameIsSubset) {
+        SkSampler::Fill(dstInfo, dst, rowBytes, 0, options.fZeroInitialized);
     }
 
     int dstX = frameRect.x();
@@ -540,7 +493,7 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
         config.options.scaled_height = scaledHeight;
     }
 
-    const bool blendWithPrevFrame = requiredFrame != kNone && frame.blend_method == WEBP_MUX_BLEND
+    const bool blendWithPrevFrame = !independent && frame.blend_method == WEBP_MUX_BLEND
         && frame.has_alpha;
     if (blendWithPrevFrame && options.fPremulBehavior == SkTransferFunctionBehavior::kRespect) {
         // Blending is done with SkRasterPipeline, which requires a color space that is valid for
diff --git a/src/codec/SkWebpCodec.h b/src/codec/SkWebpCodec.h
index f4c7910ee6..fc9e6838a6 100644
--- a/src/codec/SkWebpCodec.h
+++ b/src/codec/SkWebpCodec.h
@@ -45,6 +45,10 @@ protected:
     bool onGetFrameInfo(int, FrameInfo*) const override;
     int onGetRepetitionCount() override;
 
+    const SkFrameHolder* getFrameHolder() const override {
+        return &fFrameHolder;
+    }
+
 private:
     SkWebpCodec(int width, int height, const SkEncodedInfo&, sk_sp<SkColorSpace>, SkStream*,
                 WebPDemuxer*, sk_sp<SkData>);
diff --git a/tests/CodecPartialTest.cpp b/tests/CodecPartialTest.cpp
index a232290d0d..975ca63ee9 100644
--- a/tests/CodecPartialTest.cpp
+++ b/tests/CodecPartialTest.cpp
@@ -200,13 +200,9 @@ DEF_TEST(Codec_partialAnim, r) {
     // frameByteCounts stores the number of bytes to decode a particular frame.
     // - [0] is the number of bytes for the header
     // - frames[i] requires frameByteCounts[i+1] bytes to decode
-    std::vector<size_t> frameByteCounts;
+    const std::vector<size_t> frameByteCounts = { 455, 69350, 1344, 1346, 1327 };
     std::vector<SkBitmap> frames;
-    size_t lastOffset = 0;
     for (size_t i = 0; true; i++) {
-        frameByteCounts.push_back(stream->getPosition() - lastOffset);
-        lastOffset = stream->getPosition();
-
         SkBitmap frame;
         frame.allocPixels(info);
 
@@ -216,8 +212,6 @@ DEF_TEST(Codec_partialAnim, r) {
                 frame.rowBytes(), &opts, nullptr, nullptr);
 
         if (result == SkCodec::kIncompleteInput || result == SkCodec::kInvalidInput) {
-            frameByteCounts.push_back(stream->getPosition() - lastOffset);
-
             // We need to distinguish between a partial frame and no more frames.
             // getFrameInfo lets us do this, since it tells the number of frames
             // not considering whether they are complete.
diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index f839aaad80..04c5a088bd 100644
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -1254,8 +1254,8 @@ static void decode_frame(skiatest::Reporter* r, SkCodec* codec, size_t frame) {
             bm.getPixels(), bm.rowBytes(), &opts, nullptr, nullptr));
 }
 
-// For an animated image, we should only read enough to decode the requested
-// frame if the client never calls getFrameInfo.
+// For an animated GIF, we should only read enough to decode frame 0 if the
+// client never calls getFrameInfo and only decodes frame 0.
 DEF_TEST(Codec_skipFullParse, r) {
     auto path = "test640x479.gif";
     SkStream* stream(GetResourceAsStream(path));
@@ -1282,26 +1282,10 @@ DEF_TEST(Codec_skipFullParse, r) {
     REPORTER_ASSERT(r, positionAfterFirstFrame > sizePosition
                        && positionAfterFirstFrame < stream->getLength());
 
-    // Again, this should read more of the stream.
-    decode_frame(r, codec.get(), 2);
-    const size_t positionAfterThirdFrame = stream->getPosition();
-    REPORTER_ASSERT(r, positionAfterThirdFrame > positionAfterFirstFrame
-                       && positionAfterThirdFrame < stream->getLength());
-
-    // This does not need to read any more of the stream, since it has already
-    // parsed the second frame.
-    decode_frame(r, codec.get(), 1);
-    REPORTER_ASSERT(r, stream->getPosition() == positionAfterThirdFrame);
-
-    // This should read the rest of the frames.
-    decode_frame(r, codec.get(), 3);
-    const size_t finalPosition = stream->getPosition();
-    REPORTER_ASSERT(r, finalPosition > positionAfterThirdFrame);
-
-    // There may be more data in the stream.
+    // There is more data in the stream.
     auto frameInfo = codec->getFrameInfo();
     REPORTER_ASSERT(r, frameInfo.size() == 4);
-    REPORTER_ASSERT(r, stream->getPosition() >= finalPosition);
+    REPORTER_ASSERT(r, stream->getPosition() > positionAfterFirstFrame);
 }
 
 // Only rewinds up to a limit.