Move gif image decoder to SkCodec

third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp

Index: third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp
diff --git a/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp b/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp
index dc4823d5cd945fd16c187b6e55cd19c99f508994..ab620a8a80bedeece3ef278babf4f5a211acfd79 100644
--- a/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp
@@ -26,9 +26,9 @@
 #include "platform/image-decoders/gif/GIFImageDecoder.h"
 
 #include <limits>
-#include "platform/image-decoders/gif/GIFImageReader.h"
 #include "platform/wtf/NotFound.h"
 #include "platform/wtf/PtrUtil.h"
+#include "third_party/skia/include/core/SkImageInfo.h"
 
 namespace blink {
 
@@ -36,247 +36,317 @@ GIFImageDecoder::GIFImageDecoder(AlphaOption alpha_option,
                                  const ColorBehavior& color_behavior,
                                  size_t max_decoded_bytes)
     : ImageDecoder(alpha_option, color_behavior, max_decoded_bytes),
-      repetition_count_(kAnimationLoopOnce) {}
-
-GIFImageDecoder::~GIFImageDecoder() {}
+      codec_(),
+      segment_stream_(nullptr) {}
 
 void GIFImageDecoder::OnSetData(SegmentReader* data) {
-  if (reader_)
-    reader_->SetData(data);
-}
+  if (!data) {
+    if (segment_stream_)
+      segment_stream_->SetReader(nullptr);
+    return;
+  }
 
-int GIFImageDecoder::RepetitionCount() const {
-  // This value can arrive at any point in the image data stream.  Most GIFs
-  // in the wild declare it near the beginning of the file, so it usually is
-  // set by the time we've decoded the size, but (depending on the GIF and the
-  // packets sent back by the webserver) not always.  If the reader hasn't
-  // seen a loop count yet, it will return kCLoopCountNotSeen, in which case we
-  // should default to looping once (the initial value for
-  // |repetition_count_|).
-  //
-  // There are some additional wrinkles here. First, ImageSource::Clear()
-  // may destroy the reader, making the result from the reader _less_
-  // authoritative on future calls if the recreated reader hasn't seen the
-  // loop count.  We don't need to special-case this because in this case the
-  // new reader will once again return kCLoopCountNotSeen, and we won't
-  // overwrite the cached correct value.
-  //
-  // Second, a GIF might never set a loop count at all, in which case we
-  // should continue to treat it as a "loop once" animation.  We don't need
-  // special code here either, because in this case we'll never change
-  // |repetition_count_| from its default value.
-  //
-  // Third, we use the same GIFImageReader for counting frames and we might
-  // see the loop count and then encounter a decoding error which happens
-  // later in the stream. It is also possible that no frames are in the
-  // stream. In these cases we should just loop once.
-  if (IsAllDataReceived() && ParseCompleted() && reader_->ImagesCount() == 1)
-    repetition_count_ = kAnimationNone;
-  else if (Failed() || (reader_ && (!reader_->ImagesCount())))
-    repetition_count_ = kAnimationLoopOnce;
-  else if (reader_ && reader_->LoopCount() != kCLoopCountNotSeen)
-    repetition_count_ = reader_->LoopCount();
-  return repetition_count_;
-}
+  std::unique_ptr<SegmentStream> segment_stream;
+  if (!segment_stream_) {
+    segment_stream = base::MakeUnique<SegmentStream>();
+    segment_stream_ = segment_stream.get();
+  }
 
-bool GIFImageDecoder::FrameIsReceivedAtIndex(size_t index) const {
-  return reader_ && (index < reader_->ImagesCount()) &&
-         reader_->FrameContext(index)->IsComplete();
+  segment_stream_->SetReader(PassRefPtr<SegmentReader>(data));
+
+  if (!codec_) {
+    SkCodec::Result codec_creation_result;
+    codec_ = SkCodec::MakeFromStream(std::move(segment_stream),
+                                     &codec_creation_result, nullptr);
+    switch (codec_creation_result) {
+      case SkCodec::kSuccess: {
+        // SkCodec::MakeFromStream will read enough of the image to get the
+        // image size.
+        SkImageInfo image_info = codec_->getInfo();
+        SetSize(image_info.width(), image_info.height());
+        return;
+      }
+      case SkCodec::kIncompleteInput:
+        // |segment_stream_|'s ownership is passed into MakeFromStream.
+        // It is deleted if MakeFromStream fails.
+        // If MakeFromStream fails, we set |segment_stream_| to null so
+        // we aren't pointing to reclaimed memory.
+        segment_stream_ = nullptr;
+        return;
+      default:
+        SetFailed();
+        return;
+    }
+  }
 }
 
-float GIFImageDecoder::FrameDurationAtIndex(size_t index) const {
-  return (reader_ && (index < reader_->ImagesCount()) &&
-          reader_->FrameContext(index)->IsHeaderDefined())
-             ? reader_->FrameContext(index)->DelayTime()
-             : 0;
-}
+int GIFImageDecoder::RepetitionCount() const {
+  DCHECK(!Failed());

[scroggo_chromium, 2017/08/08 19:39:25]

In the old code, Failed() could be true here, or perhaps it couldn't, but we
still tried to handle it? Why did this change?

[cblume, 2017/08/09 17:29:29]

I think you are right, that this is incorrect.
This came from a series of separate changes.
I walked through the history to figure out how that happened. It was a mistake I
made recently while trying to make the code more simple.

The first relevant change was a code review comment you requested in Patch Set
81, Line 83
https://codereview.chromium.org/2565323003/diff2/1580001:1600001/third_party/...

The code at the time was:
if (!codec_ || Failed())
    return kCAnimationLoopOnce;

The thinking was SetFailed() would delete codec_ and thus the second part of
this if was useless. That became if (!codec_) with a DCHECK(!Failed()) as
suggested. This was fine because it would return early if the codec_ was deleted
and not hit the DCHECK.


Later, in Patch Set 146, Line 81 I added a check to see if the segment stream
had been cleared to make sure we don't attempt to read from a cleared buffer.
https://codereview.chromium.org/2565323003/diff2/2880001:2900001/third_party/...

We had talked about caching a value instead of returning the default. In the
very next Patch Set is when I introduced this problem.
https://codereview.chromium.org/2565323003/diff2/2900001:2920001/third_party/...
The thinking (ignoring the DCHECK) was that we update the cached value if we
can. Then we return the cached value (updated or not).

I think if we want to leave the DCHECK in, we want to return early if we cannot
update the cached value. I'll do that now.

+
+  if (codec_ && !segment_stream_->IsCleared()) {
+    // This value can arrive at any point in the image data stream.  Most GIFs
+    // in the wild declare it near the beginning of the file, so it usually is
+    // set by the time we've decoded the size, but (depending on the GIF and the
+    // packets sent back by the webserver) not always.
+    //
+    // SkCodec will parse forward in the file if the repetition count has not
+    // been seen yet.
+    int repetition_count = codec_->getRepetitionCount();
+
+    switch (repetition_count) {
+      case 0: {
+        // SkCodec returns 0 for both still images and animated images which
+        // only play once.
+        if (IsAllDataReceived() && codec_->getFrameCount() == 1) {
+          cached_repetition_count_ = kAnimationNone;
+          break;
+        }
+
+        cached_repetition_count_ = kAnimationLoopOnce;
+        break;
+      }
+      case SkCodec::kRepetitionCountInfinite:
+        cached_repetition_count_ = kAnimationLoopInfinite;
+        break;
+      default:
+        cached_repetition_count_ = repetition_count;
+        break;
+    }
+  }
 
-bool GIFImageDecoder::SetFailed() {
-  reader_.reset();
-  return ImageDecoder::SetFailed();
+  return cached_repetition_count_;
 }
 
-bool GIFImageDecoder::HaveDecodedRow(size_t frame_index,
-                                     GIFRow::const_iterator row_begin,
-                                     size_t width,
-                                     size_t row_number,
-                                     unsigned repeat_count,
-                                     bool write_transparent_pixels) {
-  const GIFFrameContext* frame_context = reader_->FrameContext(frame_index);
-  // The pixel data and coordinates supplied to us are relative to the frame's
-  // origin within the entire image size, i.e.
-  // (frameC_context->xOffset, frame_context->yOffset). There is no guarantee
-  // that width == (size().width() - frame_context->xOffset), so
-  // we must ensure we don't run off the end of either the source data or the
-  // row's X-coordinates.
-  const int x_begin = frame_context->XOffset();
-  const int y_begin = frame_context->YOffset() + row_number;
-  const int x_end = std::min(static_cast<int>(frame_context->XOffset() + width),
-                             Size().Width());
-  const int y_end = std::min(
-      static_cast<int>(frame_context->YOffset() + row_number + repeat_count),
-      Size().Height());
-  if (!width || (x_begin < 0) || (y_begin < 0) || (x_end <= x_begin) ||
-      (y_end <= y_begin))
-    return true;
-
-  const GIFColorMap::Table& color_table =
-      frame_context->LocalColorMap().IsDefined()
-          ? frame_context->LocalColorMap().GetTable()
-          : reader_->GlobalColorMap().GetTable();
-
-  if (color_table.IsEmpty())
-    return true;
-
-  GIFColorMap::Table::const_iterator color_table_iter = color_table.begin();
-
-  // Initialize the frame if necessary.
-  ImageFrame& buffer = frame_buffer_cache_[frame_index];
-  if (!InitFrameBuffer(frame_index))
+bool GIFImageDecoder::FrameIsReceivedAtIndex(size_t index) const {
+  if (!codec_)
     return false;
 
-  const size_t transparent_pixel = frame_context->TransparentPixel();
-  GIFRow::const_iterator row_end = row_begin + (x_end - x_begin);
-  ImageFrame::PixelData* current_address = buffer.GetAddr(x_begin, y_begin);
-
-  // We may or may not need to write transparent pixels to the buffer.
-  // If we're compositing against a previous image, it's wrong, and if
-  // we're writing atop a cleared, fully transparent buffer, it's
-  // unnecessary; but if we're decoding an interlaced gif and
-  // displaying it "Haeberli"-style, we must write these for passes
-  // beyond the first, or the initial passes will "show through" the
-  // later ones.
-  //
-  // The loops below are almost identical. One writes a transparent pixel
-  // and one doesn't based on the value of |write_transparent_pixels|.
-  // The condition check is taken out of the loop to enhance performance.
-  // This optimization reduces decoding time by about 15% for a 3MB image.
-  if (write_transparent_pixels) {
-    for (; row_begin != row_end; ++row_begin, ++current_address) {
-      const size_t source_value = *row_begin;
-      if ((source_value != transparent_pixel) &&
-          (source_value < color_table.size())) {
-        *current_address = color_table_iter[source_value];
-      } else {
-        *current_address = 0;
-        current_buffer_saw_alpha_ = true;
-      }
-    }
-  } else {
-    for (; row_begin != row_end; ++row_begin, ++current_address) {
-      const size_t source_value = *row_begin;
-      if ((source_value != transparent_pixel) &&
-          (source_value < color_table.size()))
-        *current_address = color_table_iter[source_value];
-      else
-        current_buffer_saw_alpha_ = true;
-    }
+  if (segment_stream_->IsCleared()) {
+    if (max_received_frame_index_ == kNotFound)
+      return false;
+
+    return index <= max_received_frame_index_;
   }
 
-  // Tell the frame to copy the row data if need be.
-  if (repeat_count > 1)
-    buffer.CopyRowNTimes(x_begin, x_end, y_begin, y_end);
+  int frame_count = codec_->getFrameCount();
+  for (int i = 0; i < frame_count; i++) {
+    SkCodec::FrameInfo frame_info;
+    codec_->getFrameInfo(index, &frame_info);

[scroggo_chromium, 2017/08/08 19:39:25]

Here's the bug referenced in [1]. You probably meant to pass |i| here, rather
than index. As written, if frame 0 is complete and frame 1 is partially
available, and you call this method with 0, we'll go through this loop twice.
Both times we'll set max_received_frame_index_ to i, and the second time it will
set it (incorrectly) to 1.

I don't think this method is what you want anyway. For one thing, you only need
to check the last two frames (there won't be a frame that is incomplete followed
by one that is incomplete). getFrameCount is also non-const and will parse more
data if possible.

What if you update max_received_frame_index_ in DecodeFrameCount? It would need
to call getFrameInfo on the final frame, and know that the frame prior to the
latest one is complete if the latest one is not.

[1]
https://codereview.chromium.org/2565323003/diff/2960001/third_party/WebKit/So...

[cblume, 2017/08/09 17:29:29]

Thank you for finding this. :)
Changing index back to i allowed me to put the unit test back to where it was.
You are right -- this wasn't reporting only complete frames like I had thought.

I understand what you are saying that the gif format provides one frame after
another and so the only way we know frame i+1 exists is if frame i is completely
downloaded. Ideally, I want to be able to reuse and share this code with other
formats. And those other formats could specify in advance their frame count. So
I was hoping not to rely on details of the gif file format if at all possible.

I'll do what you suggest in DecodeFrameCount. It's definitely small and simple.

[scroggo_chromium, 2017/08/09 17:50:48]

You're imagining a format where the frames might be interspersed (presumably to
aid compression)? The three formats we support for animation don't support that,
but I suppose that could be possible.

In that case, how about the following implementation:
{
  if (!codec_) {
    return false;
  }

  FrameInfo frame_info;
  return codec->getFrameInfo(index, &frame_info) && frame_info.fFullyReceived;
}
getFrameInfo will not read the stream any, and will just return its cached
information. (And it will return false if index is out of range.)

This is even simpler than caching the max received index.

[cblume, 2017/08/09 22:31:54]

I was thinking a format even more simple than that.
file_header:
--frame_count: 10
begin_frame...

Although, maybe my thought process there is misguided. GIF doesn't work that
way. APNG doesn't work that way. I've only looked at WebP a little bit but it
seems like it also doesn't work that way. The only one which might is ICO? But
it would be silly to write code around ICO's behavior rather than the common
GIF/APNG/WebP behavior.

I am happy with what we currently having, including the caching.

[scroggo_chromium, 2017/08/10 19:04:41]

I don't follow what you mean here. APNG tells you the frame count before any
frames appear. They still appear in the order you'd expect though. (You'll never
see a complete frame 1 with an incomplete frame 0.)
ICO is a more simple way of having complete frames out of order - the frames
themselves are not necessarily in order. ICO is a weird case, but it does seem
like the non-caching version covers it *and* is simpler.

[cblume, 2017/08/10 20:32:17]

Ah okay. If APNG reports frame count early than that is exactly the scenario I
was coding for. I'm not worried about the order of the frames as they arrive.
I'm worried about when we report the frame count.

Right now SkPNGCodec does not handle APNG but suppose it did. If we get the
frame count from the header, I cannot assume frame 9 is complete just because
the frame count is 10 -- I cannot only care about the last two frames. If we
only received the header then no frames are complete yet.  (Alternatively, we
could not report what the header says and only report frames as we parse them.
This has its own pros and cons.)

Since these calls can happen in any order, I also cannot assume that the last
known complete frame (say 2) will be followed by a request about frame 3. The
best I could do is loop from frame 2 to the requested frame if I am going to
maintain a cache. This loop which these comments are on starts at 0 instead of
the last known complete frame. This is because the last known complete frame
could be kNotFound. The added complexity of checking for this didn't seem worth
it.

But anyway, we can cross that bridge when we get to SkPNGCodec. For now, this
code works for me for this patch.
I previously incorrectly thought the old Blink decoders didn't cache these
values (repetition & frame count). But they do -- there is a mutable int
repetition_count_ in GIFImageDecoder. So keeping the cache might be keeping
existing behavior.

It is probably more correct for now, since this is supposed to be a shim. Major
behavioral differences should happen elsewhere. You had wisely pointed out over
a month ago that I had lost this caching behavior:
[1]
https://codereview.chromium.org/2565323003/diff/2900001/third_party/WebKit/So...
[2]
https://codereview.chromium.org/2565323003/diff/2900001/third_party/WebKit/So...

Given that the IsCleared() case may not even happen any more (I have encountered
it in our unit tests but not in normal Chrome code), we could potentially get
rid of the caching and go back to the old code I had which was more simple. We
could even further simplify things if we get rid of the IsCleared() case.

But I feel like we should add that as a TODO/new bug and get to it in the
refactor phase. I would like to land this patch. Getting rid of the caching fits
well with getting rid of the IsCleared() state. Those probably go hand-in-hand.
And it is probably best to keep that major behavior change out of this patch.

[scroggo_chromium, 2017/08/10 23:31:26]

Today, Chromium's APNG logic only reports frames that have been received.
(Actually, the first frame it reports immediately, but later frames are only
reported once they're *fully* received, since we don't want to draw them if
they're partial.) So I was assuming that behavior. (What are the cons?)
sgtm

[cblume, 2017/08/11 16:56:23]

During the first loop of the animation, we won't know information about the
animation as a whole. So we cannot plan for it.

For example, if I know in advance that this animation has 3 frames and takes 1.5
MB of memory. That might be small enough for Blink to say "You know, might as
well just keep the whole thing cached rather than constantly decoding the the
oldest frame again." But since I don't know that until the animation has looped
once, we'll end up decoding one extra time.

+    if (frame_info.fFullyReceived)
+      max_received_frame_index_ = i;
+  }
 
-  buffer.SetPixelsChanged(true);
-  return true;
-}
+  if (max_received_frame_index_ == kNotFound)
+    return false;
 
-bool GIFImageDecoder::ParseCompleted() const {
-  return reader_ && reader_->ParseCompleted();
+  return index <= max_received_frame_index_;
 }
 
-bool GIFImageDecoder::FrameComplete(size_t frame_index) {
-  // Initialize the frame if necessary.  Some GIFs insert do-nothing frames,
-  // in which case we never reach HaveDecodedRow() before getting here.
-  if (!InitFrameBuffer(frame_index))
-    return SetFailed();
-
-  if (!current_buffer_saw_alpha_)
-    CorrectAlphaWhenFrameBufferSawNoAlpha(frame_index);
-
-  frame_buffer_cache_[frame_index].SetStatus(ImageFrame::kFrameComplete);
+float GIFImageDecoder::FrameDurationAtIndex(size_t index) const {
+  if (index < frame_buffer_cache_.size())
+    return frame_buffer_cache_[index].Duration();
+  return 0;
+}
 
-  return true;
+bool GIFImageDecoder::SetFailed() {
+  segment_stream_ = nullptr;
+  codec_.reset();
+  return ImageDecoder::SetFailed();
 }
 
-void GIFImageDecoder::ClearFrameBuffer(size_t frame_index) {
-  if (reader_ && frame_buffer_cache_[frame_index].GetStatus() ==
-                     ImageFrame::kFramePartial) {
-    // Reset the state of the partial frame in the reader so that the frame
-    // can be decoded again when requested.
-    reader_->ClearDecodeState(frame_index);
+size_t GIFImageDecoder::ClearCacheExceptFrame(size_t index) {
+  // SkCodec attempts to report the earliest possible required frame, but it is
+  // possible that frame has been evicted, while a later frame (which could also
+  // be used as the required frame) is still cached. Try to preserve a frame
+  // that is still cached.
+  if (frame_buffer_cache_.size() <= 1)
+    return 0;
+
+  size_t index2 = kNotFound;
+  if (index < frame_buffer_cache_.size()) {
+    const ImageFrame& frame = frame_buffer_cache_[index];
+    if (!FrameStatusSufficientForSuccessors(index) ||
+        frame.GetDisposalMethod() == ImageFrame::kDisposeOverwritePrevious) {
+      index2 = GetViableReferenceFrameIndex(index);
+    }
   }
-  ImageDecoder::ClearFrameBuffer(frame_index);
+
+  return ClearCacheExceptTwoFrames(index, index2);
 }
 
 size_t GIFImageDecoder::DecodeFrameCount() {
-  Parse(kGIFFrameCountQuery);
-  // If decoding fails, |reader_| will have been destroyed.  Instead of
-  // returning 0 in this case, return the existing number of frames.  This way
-  // if we get halfway through the image before decoding fails, we won't
-  // suddenly start reporting that the image has zero frames.
-  return Failed() ? frame_buffer_cache_.size() : reader_->ImagesCount();
+  if (!codec_ || segment_stream_->IsCleared())
+    return frame_buffer_cache_.size();
+
+  return codec_->getFrameCount();
 }
 
 void GIFImageDecoder::InitializeNewFrame(size_t index) {
-  ImageFrame* buffer = &frame_buffer_cache_[index];
-  const GIFFrameContext* frame_context = reader_->FrameContext(index);
-  buffer->SetOriginalFrameRect(
-      Intersection(frame_context->FrameRect(), IntRect(IntPoint(), Size())));
-  buffer->SetDuration(frame_context->DelayTime());
-  buffer->SetDisposalMethod(frame_context->GetDisposalMethod());
-  buffer->SetRequiredPreviousFrameIndex(
-      FindRequiredPreviousFrame(index, false));
+  DCHECK(codec_);
+
+  ImageFrame& frame = frame_buffer_cache_[index];
+  // SkCodec does not inform us if only a portion of the image was updated
+  // in the current frame. Because of this, rather than correctly filling in
+  // the frame rect, we set the frame rect to be the image's full size.
+  // The original frame rect is not used, anyway.
+  IntSize full_image_size = Size();
+  frame.SetOriginalFrameRect(IntRect(IntPoint(), full_image_size));
+
+  SkCodec::FrameInfo frame_info;
+  bool frame_info_received = codec_->getFrameInfo(index, &frame_info);
+  DCHECK(frame_info_received);
+  frame.SetDuration(frame_info.fDuration);
+  size_t required_previous_frame_index;
+  if (frame_info.fRequiredFrame == SkCodec::kNone) {
+    required_previous_frame_index = kNotFound;
+  } else {
+    required_previous_frame_index =
+        static_cast<size_t>(frame_info.fRequiredFrame);
+  }
+  frame.SetRequiredPreviousFrameIndex(required_previous_frame_index);
+
+  ImageFrame::DisposalMethod disposal_method = ImageFrame::kDisposeNotSpecified;
+  switch (frame_info.fDisposalMethod) {
+    case SkCodecAnimation::DisposalMethod::kKeep:
+      disposal_method = ImageFrame::kDisposeKeep;
+      break;
+    case SkCodecAnimation::DisposalMethod::kRestoreBGColor:
+      disposal_method = ImageFrame::kDisposeOverwriteBgcolor;
+      break;
+    case SkCodecAnimation::DisposalMethod::kRestorePrevious:
+      disposal_method = ImageFrame::kDisposeOverwritePrevious;
+      break;
+  }
+  frame.SetDisposalMethod(disposal_method);
 }
 
 void GIFImageDecoder::Decode(size_t index) {
-  Parse(kGIFFrameCountQuery);
-
-  if (Failed())
+  if (!codec_ || segment_stream_->IsCleared())
     return;
 
+  DCHECK(!Failed());
+
+  DCHECK_LT(index, frame_buffer_cache_.size());
+
   UpdateAggressivePurging(index);
+  SkImageInfo image_info = codec_->getInfo()
+                               .makeColorType(kN32_SkColorType)
+                               .makeColorSpace(ColorSpaceForSkImages());
+
+  SkCodec::Options options;
+  options.fFrameIndex = index;
+  options.fPriorFrame = SkCodec::kNone;
+  options.fZeroInitialized = SkCodec::kNo_ZeroInitialized;
+
+  ImageFrame& frame = frame_buffer_cache_[index];
+  if (frame.GetStatus() == ImageFrame::kFrameEmpty) {
+    size_t required_previous_frame_index = frame.RequiredPreviousFrameIndex();
+    if (required_previous_frame_index == kNotFound) {
+      frame.AllocatePixelData(Size().Width(), Size().Height(),
+                              ColorSpaceForSkImages());
+      frame.ZeroFillPixelData();
+    } else {
+      size_t previous_frame_index = GetViableReferenceFrameIndex(index);
+      if (previous_frame_index == kNotFound) {
+        previous_frame_index = required_previous_frame_index;
+        Decode(previous_frame_index);
+      }
 
-  Vector<size_t> frames_to_decode = FindFramesToDecode(index);
-  for (auto i = frames_to_decode.rbegin(); i != frames_to_decode.rend(); ++i) {
-    if (!reader_->Decode(*i)) {
-      SetFailed();
-      return;
+      // We try to reuse |previous_frame| as starting state to avoid copying.
+      // If CanReusePreviousFrameBuffer returns false, we must copy the data
+      // since |previous_frame| is necessary to decode this or later frames.
+      // In that case copy the data instead.
+      ImageFrame& previous_frame = frame_buffer_cache_[previous_frame_index];
+      if ((!CanReusePreviousFrameBuffer(index) ||
+           !frame.TakeBitmapDataIfWritable(&previous_frame)) &&
+          !frame.CopyBitmapData(previous_frame)) {
+        SetFailed();
+        return;
+      }
+      options.fPriorFrame = previous_frame_index;
     }
+  }
 
-    // If this returns false, we need more data to continue decoding.
-    if (!PostDecodeProcessing(*i))
-      break;
+  if (frame.GetStatus() == ImageFrame::kFrameAllocated) {
+    SkCodec::Result start_incremental_decode_result =
+        codec_->startIncrementalDecode(image_info, frame.Bitmap().getPixels(),
+                                       frame.Bitmap().rowBytes(), &options);
+    switch (start_incremental_decode_result) {
+      case SkCodec::kSuccess:
+        break;
+      case SkCodec::kIncompleteInput:
+        return;
+      default:
+        SetFailed();
+        return;
+    }
+    frame.SetStatus(ImageFrame::kFramePartial);
   }
 
-  // It is also a fatal error if all data is received and we have decoded all
-  // frames available but the file is truncated.
-  if (index >= frame_buffer_cache_.size() - 1 && IsAllDataReceived() &&
-      reader_ && !reader_->ParseCompleted())
-    SetFailed();
+  SkCodec::Result incremental_decode_result = codec_->incrementalDecode();
+  switch (incremental_decode_result) {
+    case SkCodec::kSuccess: {
+      SkCodec::FrameInfo frame_info;
+      bool frame_info_received = codec_->getFrameInfo(index, &frame_info);
+      DCHECK(frame_info_received);
+      frame.SetHasAlpha(!SkAlphaTypeIsOpaque(frame_info.fAlphaType));
+      frame.SetPixelsChanged(true);
+      frame.SetStatus(ImageFrame::kFrameComplete);
+      PostDecodeProcessing(index);
+      break;
+    }
+    case SkCodec::kIncompleteInput:
+      frame.SetPixelsChanged(true);
+      if (FrameIsReceivedAtIndex(index) || IsAllDataReceived()) {
+        SetFailed();
+      }
+      break;
+    default:
+      SetFailed();
+      break;
+  }
 }
 
-void GIFImageDecoder::Parse(GIFParseQuery query) {
-  if (Failed())
-    return;
+bool GIFImageDecoder::CanReusePreviousFrameBuffer(size_t index) const {
+  DCHECK_LT(index, frame_buffer_cache_.size());
+  return frame_buffer_cache_[index].GetDisposalMethod() !=
+         ImageFrame::kDisposeOverwritePrevious;
+}
 
-  if (!reader_) {
-    reader_ = WTF::MakeUnique<GIFImageReader>(this);
-    reader_->SetData(data_);
-  }
+size_t GIFImageDecoder::GetViableReferenceFrameIndex(
+    size_t dependent_index) const {
+  DCHECK_LT(dependent_index, frame_buffer_cache_.size());
 
-  if (!reader_->Parse(query))
-    SetFailed();
-}
+  size_t required_previous_frame_index =
+      frame_buffer_cache_[dependent_index].RequiredPreviousFrameIndex();
 
-void GIFImageDecoder::OnInitFrameBuffer(size_t frame_index) {
-  current_buffer_saw_alpha_ = false;
-}
+  // Any frame in the range [|required_previous_frame_index|, |dependent_index|)
+  // which has a disposal method other than kRestorePrevious can be provided as
+  // the prior frame to SkCodec.
+  //
+  // SkCodec sets SkCodec::FrameInfo::fRequiredFrame to the earliest frame which
+  // can be used. This might come up when several frames update the same
+  // subregion. If that same subregion is about to be overwritten, it doesn't
+  // matter which frame in that chain is provided.
+  size_t previous_reference_frame_index = kNotFound;
+  if (required_previous_frame_index != kNotFound) {
+    // Loop backwards because the frames most likely to be in cache are the most
+    // recent.
+    for (size_t i = dependent_index - 1; i != required_previous_frame_index;
+         i--) {
+      const ImageFrame& frame = frame_buffer_cache_[i];
+
+      if (frame.GetDisposalMethod() == ImageFrame::kDisposeOverwritePrevious)
+        continue;
+
+      if (frame.GetStatus() == ImageFrame::kFrameComplete) {
+        previous_reference_frame_index = i;
+        break;
+      }
+    }
+  }
 
-bool GIFImageDecoder::CanReusePreviousFrameBuffer(size_t frame_index) const {
-  DCHECK(frame_index < frame_buffer_cache_.size());
-  return frame_buffer_cache_[frame_index].GetDisposalMethod() !=
-         ImageFrame::kDisposeOverwritePrevious;
+  return previous_reference_frame_index;
 }
 
 }  // namespace blink