Make SkCodec more flexible about its required frame

SkCodec sets fRequiredFrame to be the earliest possible frame that a
given frame can depend on. e.g.

- Frame A fills the screen, Keep
- Frame B does not cover A, Keep
- Frame C covers B but not A, and is opaque

Frame C can depend on either A or B. SkCodec already reports that C
depends on A. This CL allows a client of SkCodec to use either A or
B to create C.

Also expose the DisposalMethod. Since any frame between A and C can
be used to create C except for DisposePrevious frames, the client
needs to be able to know the disposal method so they do not try to
use such a frame to create C.

Further, the disposal method can be used to give the client a better
idea whether they will continue to need a frame. (e.g. if frame i is
DisposePrevious and depends on i-1, the client may not want to steal
i-1 to create i, since i+1 may also depend on i-1.)

TODO: Share code for decoding prior frames between GIF and WEBP

Change-Id: I91a5ae22ba3d8dfbe0bde833fa67ae3da0d81ed6
Reviewed-on: https://skia-review.googlesource.com/13722
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Chris Blume <cblume@chromium.org>
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>

13 files changed, 256 insertions, 148 deletions

diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 983a0c1b15..4ac7e80626 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -504,9 +504,9 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
                         && priorFramePixels.get()) {
                     // Copy into pixels
                     memcpy(pixels.get(), priorFramePixels.get(), safeSize);
-                    options.fHasPriorFrame = true;
+                    options.fPriorFrame = reqFrame;
                 } else {
-                    options.fHasPriorFrame = false;
+                    options.fPriorFrame = SkCodec::kNone;
                 }
                 SkCodec::Result result = codec->getPixels(decodeInfo, pixels.get(),
                                                           rowBytes, &options,
diff --git a/gm/animatedGif.cpp b/gm/animatedGif.cpp
index 5f4fe71cb0..80bcdd9ee2 100644
--- a/gm/animatedGif.cpp
+++ b/gm/animatedGif.cpp
@@ -53,7 +53,6 @@ private:
 
             SkCodec::Options opts;
             opts.fFrameIndex = frameIndex;
-            opts.fHasPriorFrame = false;
             const int requiredFrame = fFrameInfos[frameIndex].fRequiredFrame;
             if (requiredFrame != SkCodec::kNone) {
                 SkASSERT(requiredFrame >= 0
@@ -62,7 +61,7 @@ private:
                 // For simplicity, do not try to cache old frames
                 if (requiredBitmap.getPixels() &&
                         sk_tool_utils::copy_to(&bm, requiredBitmap.colorType(), requiredBitmap)) {
-                    opts.fHasPriorFrame = true;
+                    opts.fPriorFrame = requiredFrame;
                 }
             }
 
diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h
index d498007486..640ff468c5 100644
--- a/include/codec/SkCodec.h
+++ b/include/codec/SkCodec.h
@@ -9,6 +9,7 @@
 #define SkCodec_DEFINED
 
 #include "../private/SkTemplates.h"
+#include "SkCodecAnimation.h"
 #include "SkColor.h"
 #include "SkColorSpaceXform.h"
 #include "SkEncodedImageFormat.h"
@@ -249,7 +250,7 @@ public:
             : fZeroInitialized(kNo_ZeroInitialized)
             , fSubset(nullptr)
             , fFrameIndex(0)
-            , fHasPriorFrame(false)
+            , fPriorFrame(kNone)
             , fPremulBehavior(SkTransferFunctionBehavior::kRespect)
         {}
 
@@ -281,23 +282,19 @@ public:
         int                        fFrameIndex;
 
         /**
-         *  If true, the dst already contains the prior frame.
+         *  If not kNone, the dst already contains the prior frame at this index.
          *
          *  Only meaningful for multi-frame images.
          *
          *  If fFrameIndex needs to be blended with a prior frame (as reported by
          *  getFrameInfo[fFrameIndex].fRequiredFrame), the client can set this to
-         *  either true or false:
+         *  any non-kRestorePrevious frame in [fRequiredFrame, fFrameIndex) to
+         *  indicate that that frame is already in the dst. Options.fZeroInitialized
+         *  is ignored in this case.
          *
-         *  true means that the prior frame is already in the dst, and this
-         *  codec only needs to decode fFrameIndex and blend it with the dst.
-         *  Options.fZeroInitialized is ignored in this case.
-         *
-         *  false means that the dst does not contain the prior frame, so this
-         *  codec needs to first decode the prior frame (which in turn may need
-         *  to decode its prior frame).
+         *  If set to kNone, the codec will decode any necessary required frame(s) first.
          */
-        bool                       fHasPriorFrame;
+        int                        fPriorFrame;
 
         /**
          *  Indicates whether we should do a linear premultiply or a legacy premultiply.
@@ -613,7 +610,11 @@ public:
     struct FrameInfo {
         /**
          *  The frame that this frame needs to be blended with, or
-         *  kNone.
+         *  kNone if this frame is independent.
+         *
+         *  Note that this is the *earliest* frame that can be used
+         *  for blending. Any frame from [fRequiredFrame, i) can be
+         *  used, unless its fDisposalMethod is kRestorePrevious.
          */
         int fRequiredFrame;
 
@@ -635,6 +636,11 @@ public:
          *  color index-based frame has a color with alpha but does not use it.
          */
         SkAlphaType fAlphaType;
+
+        /**
+         *  How this frame should be modified before decoding the next one.
+         */
+        SkCodecAnimation::DisposalMethod fDisposalMethod;
     };
 
     /**
diff --git a/src/codec/SkCodecAnimation.h b/include/codec/SkCodecAnimation.h
index 46a878a801..9a4daff8ae 100644
--- a/src/codec/SkCodecAnimation.h
+++ b/include/codec/SkCodecAnimation.h
@@ -8,11 +8,7 @@
 #ifndef SkCodecAnimation_DEFINED
 #define SkCodecAnimation_DEFINED
 
-#include "SkCodec.h"
-#include "SkRect.h"
-
-class SkCodecAnimation {
-public:
+namespace SkCodecAnimation {
     /**
      *  This specifies how the next frame is based on this frame.
      *
@@ -20,20 +16,20 @@ public:
      *
      *  The numbers correspond to values in a GIF.
      */
-    enum DisposalMethod {
+    enum class DisposalMethod {
         /**
          *  The next frame should be drawn on top of this one.
          *
          *  In a GIF, a value of 0 (not specified) is also treated as Keep.
          */
-        Keep_DisposalMethod             = 1,
+        kKeep               = 1,
 
         /**
          *  Similar to Keep, except the area inside this frame's rectangle
          *  should be cleared to the BackGround color (transparent) before
          *  drawing the next frame.
          */
-        RestoreBGColor_DisposalMethod   = 2,
+        kRestoreBGColor     = 2,
 
         /**
          *  The next frame should be drawn on top of the previous frame - i.e.
@@ -41,29 +37,7 @@ public:
          *
          *  In a GIF, a value of 4 is also treated as RestorePrevious.
          */
-        RestorePrevious_DisposalMethod  = 3,
-    };
-
-    /**
-     * How to blend the current frame.
-     */
-    enum class Blend {
-        /**
-         *  Blend with the prior frame. This is the typical case, supported
-         *  by all animated image types.
-         */
-        kPriorFrame,
-
-        /**
-         *  Do not blend.
-         *
-         *  This frames pixels overwrite previous pixels "blending" with
-         *  the background color of transparent.
-         */
-        kBG,
+        kRestorePrevious    = 3,
     };
-
-private:
-    SkCodecAnimation();
 };
 #endif // SkCodecAnimation_DEFINED
diff --git a/resources/required.gif b/resources/required.gif
new file mode 100644
index 0000000000..91a9fd17e8
--- /dev/null
+++ b/resources/required.gif
diff --git a/resources/required.webp b/resources/required.webp
new file mode 100644
index 0000000000..9f9a8f8b8d
--- /dev/null
+++ b/resources/required.webp
diff --git a/src/codec/SkCodecAnimationPriv.h b/src/codec/SkCodecAnimationPriv.h
new file mode 100644
index 0000000000..233a79b211
--- /dev/null
+++ b/src/codec/SkCodecAnimationPriv.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkCodecAnimationPriv_DEFINED
+#define SkCodecAnimationPriv_DEFINED
+
+namespace SkCodecAnimation {
+    /**
+     * How to blend the current frame.
+     */
+    enum class Blend {
+        /**
+         *  Blend with the prior frame. This is the typical case, supported
+         *  by all animated image types.
+         */
+        kPriorFrame,
+
+        /**
+         *  Do not blend.
+         *
+         *  This frames pixels overwrite previous pixels "blending" with
+         *  the background color of transparent.
+         */
+        kBG,
+    };
+
+}
+#endif // SkCodecAnimationPriv_DEFINED
diff --git a/src/codec/SkFrameHolder.h b/src/codec/SkFrameHolder.h
index e92b40f9d8..0f30b65e5a 100644
--- a/src/codec/SkFrameHolder.h
+++ b/src/codec/SkFrameHolder.h
@@ -10,6 +10,7 @@
 
 #include "SkTypes.h"
 #include "SkCodecAnimation.h"
+#include "SkCodecAnimationPriv.h"
 
 /**
  *  Base class for a single frame of an animated image.
@@ -23,7 +24,7 @@ public:
         : fId(id)
         , fHasAlpha(false)
         , fRequiredFrame(kUninitialized)
-        , fDisposalMethod(SkCodecAnimation::Keep_DisposalMethod)
+        , fDisposalMethod(SkCodecAnimation::DisposalMethod::kKeep)
         , fDuration(0)
         , fBlend(SkCodecAnimation::Blend::kPriorFrame)
     {
diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index f0495bc4b3..6c4273444c 100644
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -153,6 +153,7 @@ bool SkGifCodec::onGetFrameInfo(int i, SkCodec::FrameInfo* frameInfo) const {
         frameInfo->fFullyReceived = frameContext->isComplete();
         frameInfo->fAlphaType = frameContext->hasAlpha() ? kUnpremul_SkAlphaType
                                                          : kOpaque_SkAlphaType;
+        frameInfo->fDisposalMethod = frameContext->getDisposalMethod();
     }
     return true;
 }
@@ -398,11 +399,24 @@ SkCodec::Result SkGifCodec::decodeFrame(bool firstAttempt, const Options& opts,
             }
         } else {
             // Not independent
-            if (!opts.fHasPriorFrame) {
+            // 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.fHasPriorFrame = false;
+                prevFrameOpts.fPriorFrame = kNone;
                 // The prior frame may have a different color table, so update it and the
                 // swizzler.
                 this->initializeColorTable(dstInfo, prevFrameOpts.fFrameIndex);
@@ -424,24 +438,33 @@ SkCodec::Result SkGifCodec::decodeFrame(bool firstAttempt, const Options& opts,
                 this->initializeColorTable(dstInfo, frameIndex);
                 this->initializeSwizzler(dstInfo, frameIndex);
             }
-            const auto* prevFrame = fReader->frameContext(frameContext->getRequiredFrame());
-            if (prevFrame->getDisposalMethod() == SkCodecAnimation::RestoreBGColor_DisposalMethod) {
-                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.)
-                    auto left = prevRect.fLeft / fSwizzler->sampleX();
-                    auto top = prevRect.fTop / fSwizzler->sampleY();
-                    void* const eraseDst = SkTAddOffset<void>(fDst, top * fDstRowBytes
-                            + left * SkColorTypeBytesPerPixel(dstInfo.colorType()));
-                    auto width = get_scaled_dimension(prevRect.width(), fSwizzler->sampleX());
-                    auto height = get_scaled_dimension(prevRect.height(), fSwizzler->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);
+
+            // 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);
+                    }
                 }
             }
             filledBackground = true;
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index 21d45da233..f702a3ae98 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -7,6 +7,8 @@
 
 #include "SkBitmap.h"
 #include "SkCanvas.h"
+#include "SkCodecAnimation.h"
+#include "SkCodecAnimationPriv.h"
 #include "SkCodecPriv.h"
 #include "SkColorSpaceXform.h"
 #include "SkRasterPipeline.h"
@@ -253,8 +255,8 @@ int SkWebpCodec::onGetFrameCount() {
         Frame* frame = fFrameHolder.appendNewFrame(iter.has_alpha);
         frame->setXYWH(iter.x_offset, iter.y_offset, iter.width, iter.height);
         frame->setDisposalMethod(iter.dispose_method == WEBP_MUX_DISPOSE_BACKGROUND ?
-                SkCodecAnimation::RestoreBGColor_DisposalMethod :
-                SkCodecAnimation::Keep_DisposalMethod);
+                SkCodecAnimation::DisposalMethod::kRestoreBGColor :
+                SkCodecAnimation::DisposalMethod::kKeep);
         frame->setDuration(iter.duration);
         if (WEBP_MUX_BLEND != iter.blend_method) {
             frame->setBlend(SkCodecAnimation::Blend::kBG);
@@ -292,6 +294,7 @@ bool SkWebpCodec::onGetFrameInfo(int i, FrameInfo* frameInfo) const {
         // animated image.
         frameInfo->fFullyReceived = true;
         frameInfo->fAlphaType = alpha_type(frame->hasAlpha());
+        frameInfo->fDisposalMethod = frame->getDisposalMethod();
     }
 
     return true;
@@ -437,9 +440,15 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
             SkSampler::Fill(dstInfo, dst, rowBytes, 0, options.fZeroInitialized);
         }
     } else {
-        if (!options.fHasPriorFrame) {
+        // 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) {
@@ -452,16 +461,20 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
             }
         }
 
-        // Dispose bg color
-        const Frame* priorFrame = fFrameHolder.frame(requiredFrame);
-        if (priorFrame->getDisposalMethod() == SkCodecAnimation::RestoreBGColor_DisposalMethod) {
-            // 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 (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);
+            }
         }
     }
 
diff --git a/tests/CodecAnimTest.cpp b/tests/CodecAnimTest.cpp
index 74ee3c63ed..27fe0038b5 100644
--- a/tests/CodecAnimTest.cpp
+++ b/tests/CodecAnimTest.cpp
@@ -57,37 +57,53 @@ DEF_TEST(Codec_565, r) {
 
     SkCodec::Options options;
     options.fFrameIndex = 1;
-    options.fHasPriorFrame = false;
+    options.fPriorFrame = SkCodec::kNone;
 
     const auto result = codec->getPixels(info, bm.getPixels(), bm.rowBytes(),
                                          &options, nullptr, nullptr);
     REPORTER_ASSERT(r, result == SkCodec::kSuccess);
 }
 
+static bool restore_previous(const SkCodec::FrameInfo& info) {
+    return info.fDisposalMethod == SkCodecAnimation::DisposalMethod::kRestorePrevious;
+}
 
 DEF_TEST(Codec_frames, r) {
-    #define kOpaque     kOpaque_SkAlphaType
-    #define kUnpremul   kUnpremul_SkAlphaType
+    #define kOpaque         kOpaque_SkAlphaType
+    #define kUnpremul       kUnpremul_SkAlphaType
+    #define kKeep           SkCodecAnimation::DisposalMethod::kKeep
+    #define kRestoreBG      SkCodecAnimation::DisposalMethod::kRestoreBGColor
+    #define kRestorePrev    SkCodecAnimation::DisposalMethod::kRestorePrevious
     static const struct {
-        const char*              fName;
-        int                      fFrameCount;
+        const char*                                   fName;
+        int                                           fFrameCount;
         // One less than fFramecount, since the first frame is always
         // independent.
-        std::vector<int>         fRequiredFrames;
+        std::vector<int>                              fRequiredFrames;
         // Same, since the first frame should match getInfo.
-        std::vector<SkAlphaType> fAlphaTypes;
+        std::vector<SkAlphaType>                      fAlphaTypes;
         // The size of this one should match fFrameCount for animated, empty
         // otherwise.
-        std::vector<int>         fDurations;
-        int                      fRepetitionCount;
+        std::vector<int>                              fDurations;
+        int                                           fRepetitionCount;
+        std::vector<SkCodecAnimation::DisposalMethod> fDisposalMethods;
     } gRecs[] = {
+        { "required.gif", 7,
+            { 0, 1, 1, SkCodec::kNone, 4, 4 },
+            { kOpaque, kUnpremul, kUnpremul, kOpaque, kOpaque, kOpaque },
+            { 100, 100, 100, 100, 100, 100, 100 },
+            0,
+            { kKeep, kRestoreBG, kKeep, kKeep, kKeep, kRestoreBG, kKeep } },
         { "alphabetAnim.gif", 13,
             { SkCodec::kNone, 0, 0, 0, 0, 5, 6, SkCodec::kNone,
               SkCodec::kNone, SkCodec::kNone, 10, 11 },
             { kUnpremul, kUnpremul, kUnpremul, kUnpremul, kUnpremul, kUnpremul,
               kUnpremul, kUnpremul, kUnpremul, kOpaque, kOpaque, kUnpremul },
             { 100, 100, 100, 100, 100, 100, 100, 100, 100, 100, 100, 100, 100 },
-            0 },
+            0,
+            { kKeep, kRestorePrev, kRestorePrev, kRestorePrev, kRestorePrev,
+              kRestoreBG, kKeep, kRestoreBG, kRestoreBG, kKeep, kKeep,
+              kRestoreBG, kKeep } },
         { "randPixelsAnim2.gif", 4,
             // required frames
             { 0, 0, 1 },
@@ -96,7 +112,8 @@ DEF_TEST(Codec_frames, r) {
             // durations
             { 0, 1000, 170, 40 },
             // repetition count
-            0 },
+            0,
+            { kKeep, kKeep, kRestorePrev, kKeep } },
         { "randPixelsAnim.gif", 13,
             // required frames
             { SkCodec::kNone, 1, 2, 3, 4, 3, 6, 7, 7, 7, 9, 9 },
@@ -105,33 +122,49 @@ DEF_TEST(Codec_frames, r) {
             // durations
             { 0, 1000, 170, 40, 220, 7770, 90, 90, 90, 90, 90, 90, 90 },
             // repetition count
-            0 },
-        { "box.gif", 1, {}, {}, {}, 0 },
-        { "color_wheel.gif", 1, {}, {}, {}, 0 },
+            0,
+            { kKeep, kKeep, kKeep, kKeep, kRestoreBG, kRestoreBG, kRestoreBG,
+              kRestoreBG, kRestorePrev, kRestoreBG, kRestorePrev, kRestorePrev,
+              kRestorePrev,  } },
+        { "box.gif", 1, {}, {}, {}, 0, { kKeep } },
+        { "color_wheel.gif", 1, {}, {}, {}, 0, { kKeep } },
         { "test640x479.gif", 4, { 0, 1, 2 },
                 { kOpaque, kOpaque, kOpaque },
                 { 200, 200, 200, 200 },
-                SkCodec::kRepetitionCountInfinite },
-        { "colorTables.gif", 2, { 0 }, { kOpaque }, { 1000, 1000 }, 5 },
-
-        { "arrow.png",  1, {}, {}, {}, 0 },
-        { "google_chrome.ico", 1, {}, {}, {}, 0 },
-        { "brickwork-texture.jpg", 1, {}, {}, {}, 0 },
+                SkCodec::kRepetitionCountInfinite,
+                { kKeep, kKeep, kKeep, kKeep } },
+        { "colorTables.gif", 2, { 0 }, { kOpaque }, { 1000, 1000 }, 5,
+                { kKeep, kKeep } },
+
+        { "arrow.png",  1, {}, {}, {}, 0, {} },
+        { "google_chrome.ico", 1, {}, {}, {}, 0, {} },
+        { "brickwork-texture.jpg", 1, {}, {}, {}, 0, {} },
 #if defined(SK_CODEC_DECODES_RAW) && (!defined(_WIN32))
-        { "dng_with_preview.dng", 1, {}, {}, {}, 0 },
+        { "dng_with_preview.dng", 1, {}, {}, {}, 0, {} },
 #endif
-        { "mandrill.wbmp", 1, {}, {}, {}, 0 },
-        { "randPixels.bmp", 1, {}, {}, {}, 0 },
-        { "yellow_rose.webp", 1, {}, {}, {}, 0 },
+        { "mandrill.wbmp", 1, {}, {}, {}, 0, {} },
+        { "randPixels.bmp", 1, {}, {}, {}, 0, {} },
+        { "yellow_rose.webp", 1, {}, {}, {}, 0, {} },
         { "webp-animated.webp", 3, { 0, 1 }, { kOpaque, kOpaque },
-            { 1000, 500, 1000 }, SkCodec::kRepetitionCountInfinite },
+            { 1000, 500, 1000 }, SkCodec::kRepetitionCountInfinite,
+            { kKeep, kKeep, kKeep } },
         { "blendBG.webp", 7, { 0, SkCodec::kNone, SkCodec::kNone, SkCodec::kNone,
                                4, 4 },
             { kOpaque, kOpaque, kUnpremul, kOpaque, kUnpremul, kUnpremul },
-            { 525, 500, 525, 437, 609, 729, 444 }, 7 },
+            { 525, 500, 525, 437, 609, 729, 444 }, 7,
+            { kKeep, kKeep, kKeep, kKeep, kKeep, kKeep, kKeep } },
+        { "required.webp", 7,
+            { 0, 1, 1, SkCodec::kNone, 4, 4 },
+            { kOpaque, kUnpremul, kUnpremul, kOpaque, kOpaque, kOpaque },
+            { 100, 100, 100, 100, 100, 100, 100 },
+            1,
+            { kKeep, kRestoreBG, kKeep, kKeep, kKeep, kRestoreBG, kKeep } },
     };
     #undef kOpaque
     #undef kUnpremul
+    #undef kKeep
+    #undef kRestorePrev
+    #undef kRestoreBG
 
     for (const auto& rec : gRecs) {
         sk_sp<SkData> data(GetResourceAsData(rec.fName));
@@ -178,6 +211,13 @@ DEF_TEST(Codec_frames, r) {
                        rec.fName, expected - 1, rec.fAlphaTypes.size());
                 continue;
             }
+
+            if (rec.fDisposalMethods.size() != static_cast<size_t>(expected)) {
+                ERRORF(r, "'%s' has wrong number entries in fDisposalMethods; "
+                       "expected %i\tactual: %i",
+                       rec.fName, expected, rec.fDisposalMethods.size());
+                continue;
+            }
         }
 
         enum class TestMode {
@@ -254,6 +294,8 @@ DEF_TEST(Codec_frames, r) {
                     ERRORF(r, "%s's frame %i has wrong dependency! expected: %i\tactual: %i",
                            rec.fName, i, rec.fRequiredFrames[i-1], frameInfo.fRequiredFrame);
                 }
+
+                REPORTER_ASSERT(r, frameInfo.fDisposalMethod == rec.fDisposalMethods[i]);
             }
 
             if (TestMode::kIndividual == mode) {
@@ -261,67 +303,84 @@ DEF_TEST(Codec_frames, r) {
                 continue;
             }
 
-            // Compare decoding in two ways:
-            // 1. Provide the frame that a frame depends on, so the codec just has to blend.
-            //    (in the array cachedFrames)
-            // 2. Do not provide the frame that a frame depends on, so the codec has to decode
-            //    all the way back to a key-frame. (in a local variable uncachedFrame)
-            // The two should look the same.
+            // Compare decoding in multiple ways:
+            // - Start from scratch for each frame. |codec| will have to decode the required frame
+            //   (and any it depends on) to decode. This is stored in |cachedFrames|.
+            // - Provide the frame that a frame depends on, so |codec| just has to blend.
+            // - Provide a frame after the required frame, which will be covered up by the newest
+            //   frame.
+            // All should look the same.
             std::vector<SkBitmap> cachedFrames(frameCount);
             const auto info = codec->getInfo().makeColorType(kN32_SkColorType);
 
-            auto decode = [&](SkBitmap* bm, bool cached, int index) {
+            auto decode = [&](SkBitmap* bm, int index, int cachedIndex) {
                 auto decodeInfo = info;
                 if (index > 0) {
                     decodeInfo = info.makeAlphaType(frameInfos[index].fAlphaType);
                 }
                 bm->allocPixels(decodeInfo);
-                if (cached) {
+                if (cachedIndex != SkCodec::kNone) {
                     // First copy the pixels from the cached frame
-                    const int requiredFrame = frameInfos[index].fRequiredFrame;
-                    if (requiredFrame != SkCodec::kNone) {
-                        const bool success = sk_tool_utils::copy_to(bm, kN32_SkColorType,
-                                cachedFrames[requiredFrame]);
-                        REPORTER_ASSERT(r, success);
-                    }
+                    const bool success = sk_tool_utils::copy_to(bm, kN32_SkColorType,
+                            cachedFrames[cachedIndex]);
+                    REPORTER_ASSERT(r, success);
                 }
                 SkCodec::Options opts;
                 opts.fFrameIndex = index;
-                opts.fHasPriorFrame = cached;
+                opts.fPriorFrame = cachedIndex;
                 const auto result = codec->getPixels(decodeInfo, bm->getPixels(), bm->rowBytes(),
                                                      &opts, nullptr, nullptr);
+                if (cachedIndex != SkCodec::kNone && restore_previous(frameInfos[cachedIndex])) {
+                    if (result == SkCodec::kInvalidParameters) {
+                        return true;
+                    }
+                    ERRORF(r, "Using a kRestorePrevious frame as fPriorFrame should fail");
+                    return false;
+                }
                 if (result != SkCodec::kSuccess) {
-                    ERRORF(r, "Failed to decode frame %i from %s when %scached, error %i",
-                           index, rec.fName, (cached ? "" : "not "), result);
+                    ERRORF(r, "Failed to decode frame %i from %s when providing prior frame %i, "
+                              "error %i", index, rec.fName, cachedIndex, result);
                 }
                 return result == SkCodec::kSuccess;
             };
 
             for (int i = 0; i < frameCount; i++) {
                 SkBitmap& cachedFrame = cachedFrames[i];
-                if (!decode(&cachedFrame, true, i)) {
+                if (!decode(&cachedFrame, i, SkCodec::kNone)) {
                     continue;
                 }
-                SkBitmap uncachedFrame;
-                if (!decode(&uncachedFrame, false, i)) {
+                const auto reqFrame = frameInfos[i].fRequiredFrame;
+                if (reqFrame == SkCodec::kNone) {
+                    // Nothing to compare against.
                     continue;
                 }
+                for (int j = reqFrame; j < i; j++) {
+                    SkBitmap frame;
+                    if (restore_previous(frameInfos[j])) {
+                        (void) decode(&frame, i, j);
+                        continue;
+                    }
+                    if (!decode(&frame, i, j)) {
+                        continue;
+                    }
 
-                // Now verify they're equal.
-                const size_t rowLen = info.bytesPerPixel() * info.width();
-                for (int y = 0; y < info.height(); y++) {
-                    const void* cachedAddr = cachedFrame.getAddr(0, y);
-                    SkASSERT(cachedAddr != nullptr);
-                    const void* uncachedAddr = uncachedFrame.getAddr(0, y);
-                    SkASSERT(uncachedAddr != nullptr);
-                    const bool lineMatches = memcmp(cachedAddr, uncachedAddr, rowLen) == 0;
-                    if (!lineMatches) {
-                        SkString name = SkStringPrintf("cached_%i", i);
-                        write_bm(name.c_str(), cachedFrame);
-                        name = SkStringPrintf("uncached_%i", i);
-                        write_bm(name.c_str(), uncachedFrame);
-                        ERRORF(r, "%s's frame %i is different depending on caching!", rec.fName, i);
-                        break;
+                    // Now verify they're equal.
+                    const size_t rowLen = info.bytesPerPixel() * info.width();
+                    for (int y = 0; y < info.height(); y++) {
+                        const void* cachedAddr = cachedFrame.getAddr(0, y);
+                        SkASSERT(cachedAddr != nullptr);
+                        const void* addr = frame.getAddr(0, y);
+                        SkASSERT(addr != nullptr);
+                        const bool lineMatches = memcmp(cachedAddr, addr, rowLen) == 0;
+                        if (!lineMatches) {
+                            SkString name = SkStringPrintf("cached_%i", i);
+                            write_bm(name.c_str(), cachedFrame);
+                            name = SkStringPrintf("frame_%i", i);
+                            write_bm(name.c_str(), frame);
+                            ERRORF(r, "%s's frame %i is different (starting from line %i) when "
+                                      "providing prior frame %i!", rec.fName, i, y, j);
+                            break;
+                        }
                     }
                 }
             }
diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index ffaeb2b61c..f839aaad80 100644
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -1514,7 +1514,8 @@ DEF_TEST(Codec_InvalidAnimated, r) {
     SkCodec::Options opts;
     for (int i = 0; static_cast<size_t>(i) < frameInfos.size(); i++) {
         opts.fFrameIndex = i;
-        opts.fHasPriorFrame = frameInfos[i].fRequiredFrame == i - 1;
+        const auto reqFrame = frameInfos[i].fRequiredFrame;
+        opts.fPriorFrame = reqFrame == i - 1 ? reqFrame : SkCodec::kNone;
         auto result = codec->startIncrementalDecode(info, bm.getPixels(), bm.rowBytes(), &opts);
         if (result != SkCodec::kSuccess) {
             ERRORF(r, "Failed to start decoding frame %i (out of %i) with error %i\n", i,
diff --git a/third_party/gif/SkGifImageReader.cpp b/third_party/gif/SkGifImageReader.cpp
index 0666fedbc8..76f3edcef0 100644
--- a/third_party/gif/SkGifImageReader.cpp
+++ b/third_party/gif/SkGifImageReader.cpp
@@ -616,11 +616,11 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query)
             case 4:
                 // Some specs say that disposal method 3 is "overwrite previous", others that setting
                 // the third bit of the field (i.e. method 4) is. We map both to the same value.
-                currentFrame->setDisposalMethod(SkCodecAnimation::RestorePrevious_DisposalMethod);
+                currentFrame->setDisposalMethod(SkCodecAnimation::DisposalMethod::kRestorePrevious);
                 break;
             default:
                 // Other values use the default.
-                currentFrame->setDisposalMethod(SkCodecAnimation::Keep_DisposalMethod);
+                currentFrame->setDisposalMethod(SkCodecAnimation::DisposalMethod::kKeep);
                 break;
             }
             currentFrame->setDuration(GETINT16(currentComponent + 1) * 10);
@@ -903,7 +903,7 @@ static bool independent(const SkFrame& frame) {
 }
 
 static bool restore_bg(const SkFrame& frame) {
-    return frame.getDisposalMethod() == SkCodecAnimation::RestoreBGColor_DisposalMethod;
+    return frame.getDisposalMethod() == SkCodecAnimation::DisposalMethod::kRestoreBGColor;
 }
 
 bool SkGIFFrameContext::onReportsAlpha() const {
@@ -935,7 +935,7 @@ void SkFrameHolder::setAlphaAndRequiredFrame(SkFrame* frame) {
     }
 
     const SkFrame* prevFrame = this->getFrame(i-1);
-    while (prevFrame->getDisposalMethod() == SkCodecAnimation::RestorePrevious_DisposalMethod) {
+    while (prevFrame->getDisposalMethod() == SkCodecAnimation::DisposalMethod::kRestorePrevious) {
         const int prevId = prevFrame->frameId();
         if (0 == prevId) {
             frame->setHasAlpha(true);
@@ -992,7 +992,7 @@ void SkFrameHolder::setAlphaAndRequiredFrame(SkFrame* frame) {
         return;
     }
 
-    SkASSERT(prevFrame->getDisposalMethod() == SkCodecAnimation::Keep_DisposalMethod);
+    SkASSERT(prevFrame->getDisposalMethod() == SkCodecAnimation::DisposalMethod::kKeep);
     frame->setRequiredFrame(prevFrame->frameId());
     frame->setHasAlpha(prevFrame->hasAlpha() || (reportsAlpha && !blendWithPrevFrame));
 }