Fix required frame bug in APNGs

third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp

 /*
  * Copyright (C) Research In Motion Limited 2009-2010. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Library General Public
  *  License as published by the Free Software Foundation; either
  *  version 2 of the License, or (at your option) any later version.
  *
  *  This library is distributed in the hope that it will be useful,
  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 424 matching lines @@
   const ImageFrame* curr_buffer = &frame_buffer_cache_[frame_index];
   if ((frame_rect_is_opaque ||
        curr_buffer->GetAlphaBlendSource() == ImageFrame::kBlendAtopBgcolor) &&
       curr_buffer->OriginalFrameRect().Contains(IntRect(IntPoint(), Size())))
     return kNotFound;
 
   // The starting state for this frame depends on the previous frame's
   // disposal method.
   size_t prev_frame = frame_index - 1;
   const ImageFrame* prev_buffer = &frame_buffer_cache_[prev_frame];
+  while (prev_buffer->GetDisposalMethod() ==
+         ImageFrame::kDisposeOverwritePrevious) {
+    // Frames that use the DisposeOverwritePrevious method are effectively

[Peter Kasting, 2017/05/17 23:30:53]

Nit: I'd put this comment above the loop rather than inside it, since it
describes the function of the whole loop.

[scroggo_chromium, 2017/05/18 12:24:05]

Done.

+    // no-ops in terms of changing the starting state of a frame compared to
+    // the starting state of the previous frame, so skip over them.
+    if (prev_frame == 0) {
+      return kNotFound;
+    }
+    prev_frame--;
+    prev_buffer = &frame_buffer_cache_[prev_frame];
+  }
 
   switch (prev_buffer->GetDisposalMethod()) {
     case ImageFrame::kDisposeNotSpecified:
     case ImageFrame::kDisposeKeep:
       // prevFrame will be used as the starting state for this frame.
       // FIXME: Be even smarter by checking the frame sizes and/or
       // alpha-containing regions.
       return prev_frame;
-    case ImageFrame::kDisposeOverwritePrevious:
-      // Frames that use the DisposeOverwritePrevious method are effectively
-      // no-ops in terms of changing the starting state of a frame compared to
-      // the starting state of the previous frame, so skip over them and
-      // return the required previous frame of it.
-      return prev_buffer->RequiredPreviousFrameIndex();
     case ImageFrame::kDisposeOverwriteBgcolor:
       // If the previous frame fills the whole image, then the current frame
       // can be decoded alone. Likewise, if the previous frame could be
       // decoded without reference to any prior frame, the starting state for
       // this frame is a blank frame, so it can again be decoded alone.
       // Otherwise, the previous frame contributes to this frame.
       return (prev_buffer->OriginalFrameRect().Contains(
                   IntRect(IntPoint(), Size())) ||
               (prev_buffer->RequiredPreviousFrameIndex() == kNotFound))
                  ? kNotFound
                  : prev_frame;
+    case ImageFrame::kDisposeOverwritePrevious:
     default:
       NOTREACHED();
       return kNotFound;
   }
 }
 
 ImagePlanes::ImagePlanes() {
   for (int i = 0; i < 3; ++i) {
     planes_[i] = 0;
     row_bytes_[i] = 0;
@@ skipping 96 matching lines @@
     }
 
     // For color spaces without an identifiable gamut, just fall through to
     // sRGB.
   }
 
   return SkColorSpace::MakeSRGB();
 }
 
 }  // namespace blink