WIP: Implement APNG

third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp

Index: third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp
diff --git a/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp b/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp
index 2ef4182cbe85b28b4158c7acf551d76c72a85a91..6aa63c8858c34406a43e1ab1bdf36e5c1a34cdbe 100644
--- a/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp
@@ -38,8 +38,8 @@
 
 #include "platform/image-decoders/png/PNGImageDecoder.h"
 
+#include "platform/image-decoders/png/PNGImageReader.h"
 #include "png.h"
-#include "wtf/PtrUtil.h"
 #include <memory>
 
 #if !defined(PNG_LIBPNG_VER_MAJOR) || !defined(PNG_LIBPNG_VER_MINOR)
@@ -53,115 +53,257 @@
 #define JMPBUF(png_ptr) png_ptr->jmpbuf
 #endif
 
-namespace {
+namespace blink {
+
+PNGImageDecoder::PNGImageDecoder(AlphaOption alphaOption,
+                                 ColorSpaceOption colorOptions,
+                                 size_t maxDecodedBytes,
+                                 size_t offset)
+    : ImageDecoder(alphaOption, colorOptions, maxDecodedBytes),
+      m_offset(offset),
+      m_frameCount(0),
+      m_currentFrame(0),
+      m_repetitionCount(cAnimationLoopOnce) {}
 
-inline blink::PNGImageDecoder* imageDecoder(png_structp png) {
-  return static_cast<blink::PNGImageDecoder*>(png_get_progressive_ptr(png));
+PNGImageDecoder::~PNGImageDecoder() {}
+
+size_t PNGImageDecoder::decodeFrameCount() {
+  if (!m_reader || !m_reader->parseCompleted())

[scroggo_chromium, 2016/11/07 13:02:16]

It's somewhat noticeable that this method checks parseCompleted before calling
parse, but the next method (decode) does not. I'm guessing that parse will check
parseCompleted first anyway?

[joostouwerling, 2016/11/08 21:58:17]

I put the check for parseCompleted at the beginning of PNGImageReader::parse()
(per your suggestion) so the calls to PNGImageDecoder::parse(), and
PNGImageDecoder::parse() itself, don't need to check for this anymore.

+    parse(PNGParseQuery::PNGMetaDataQuery);
+  return m_frameCount;
 }
 
-void PNGAPI pngHeaderAvailable(png_structp png, png_infop) {
-  imageDecoder(png)->headerAvailable();
+void PNGImageDecoder::decode(size_t index) {
+  parse(PNGParseQuery::PNGMetaDataQuery);
+
+  // @TODO(joostouwerling): show complete frames even if a later frame fails.
+  if (failed())
+    return;
+
+  updateAggressivePurging(index);
+
+  Vector<size_t> framesToDecode;
+  size_t frameToDecode = index;
+
+  // This method is only called by ImageDecoder::frameBufferAtIndex if the frame
+  // status of frame |index| is not ImageFrane::FrameComplete. Therefore, it is

[scroggo_chromium, 2016/11/07 13:02:16]

ImageFrame*

[joostouwerling, 2016/11/08 21:58:17]

Done.

+  // OK that the do-while loop always appends |index| to |m_framesToDecode|,
+  // without checking for it's status.

[scroggo_chromium, 2016/11/07 13:02:16]

its*

[joostouwerling, 2016/11/08 21:58:17]

Done.

+  //
+  // The requiredPreviousFrameIndex for each frame is set in
+  // PNGImageDecoder::initializeNewFrame().
+  do {
+    framesToDecode.append(frameToDecode);
+    frameToDecode =
+        m_frameBufferCache[frameToDecode].requiredPreviousFrameIndex();
+  } while (frameToDecode != kNotFound &&
+           m_frameBufferCache[frameToDecode].getStatus() !=
+               ImageFrame::FrameComplete);
+
+  for (auto i = framesToDecode.rbegin(); i != framesToDecode.rend(); i++) {
+    m_currentFrame = *i;
+    m_reader->decode(*m_data, *i);
+    if (failed())
+      return;
+
+    // If the frame is not yet complete, we need more data to continue.
+    if (m_frameBufferCache[*i].getStatus() != ImageFrame::FrameComplete)
+      break;
+
+    if (m_purgeAggressively)
+      clearCacheExceptFrame(*i);
+  }
 }
 
-void PNGAPI pngRowAvailable(png_structp png,
-                            png_bytep row,
-                            png_uint_32 rowIndex,
-                            int state) {
-  imageDecoder(png)->rowAvailable(row, rowIndex, state);
+// @TODO(joostouwerling) Consolidate this with a proposed change in
+//                       ImageDecoder::clearCacheExceptFrame. See
+//                       crrev.com/2468233002
+size_t PNGImageDecoder::clearCacheExceptFrame(size_t clearExceptFrame) {
+  // As per the comments at ImageDecoder::clearCacheExceptFrame
+  if (m_frameBufferCache.size() <= 1)
+    return 0;
+
+  // We expect that after this call, we'll be asked to decode frames after
+  // this one.  So we want to avoid clearing frames such that those requests
+  // would force re-decoding from the beginning of the image.
+  //
+  // When |clearExceptFrame| is e.g. DisposeKeep, simply not clearing that
+  // frame is sufficient, as the next frame will be based on it, and in
+  // general future frames can't be based on anything previous.
+  //
+  // However, if this frame is DisposeOverwritePrevious, then subsequent
+  // frames will depend on this frame's required previous frame.  In this
+  // case, we need to preserve both this frame and that one.
+  size_t clearExceptFrame2 = kNotFound;
+  if (clearExceptFrame < m_frameBufferCache.size()) {
+    const ImageFrame& frame = m_frameBufferCache[clearExceptFrame];
+    if (frame.getStatus() != ImageFrame::FrameEmpty &&
+        frame.getDisposalMethod() == ImageFrame::DisposeOverwritePrevious) {
+      clearExceptFrame2 = clearExceptFrame;
+      clearExceptFrame = frame.requiredPreviousFrameIndex();
+    }
+  }
+
+  // Now |clearExceptFrame| indicates the frame that future frames will
+  // depend on.  But if decoding is skipping forward past intermediate frames,
+  // this frame may be FrameEmpty.  So we need to keep traversing back through
+  // the required previous frames until we find the nearest non-empty
+  // ancestor.  Preserving that will minimize the amount of future decoding
+  // needed.
+  while (clearExceptFrame < m_frameBufferCache.size() &&
+         m_frameBufferCache[clearExceptFrame].getStatus() ==
+             ImageFrame::FrameEmpty)
+    clearExceptFrame =
+        m_frameBufferCache[clearExceptFrame].requiredPreviousFrameIndex();
+
+  return clearCacheExceptTwoFrames(clearExceptFrame, clearExceptFrame2);
 }
 
-void PNGAPI pngComplete(png_structp png, png_infop) {
-  imageDecoder(png)->complete();
+size_t PNGImageDecoder::clearCacheExceptTwoFrames(size_t clearExceptFrame1,
+                                                  size_t clearExceptFrame2) {
+  size_t frameBytesCleared = 0;
+  for (size_t i = 0; i < m_frameBufferCache.size(); ++i) {
+    if (m_frameBufferCache[i].getStatus() != ImageFrame::FrameEmpty &&
+        i != clearExceptFrame1 && i != clearExceptFrame2) {
+      frameBytesCleared += frameBytesAtIndex(i);
+      clearFrameBuffer(i);
+    }
+  }
+  return frameBytesCleared;
 }
 
-void PNGAPI pngFailed(png_structp png, png_const_charp) {
-  longjmp(JMPBUF(png), 1);
+void PNGImageDecoder::clearFrameBuffer(size_t frameIndex) {
+  if (m_frameBufferCache[frameIndex].getStatus() == ImageFrame::FramePartial)
+    m_reader->clearDecodeState(frameIndex);
+
+  m_frameBufferCache[frameIndex].clearPixelData();
 }
 
-}  // namespace
+void PNGImageDecoder::parse(PNGParseQuery query) {
+  if (failed())
+    return;
 
-namespace blink {
+  if (!m_reader)
+    m_reader = wrapUnique(new PNGImageReader(this, m_offset));
 
-class PNGImageReader final {
-  USING_FAST_MALLOC(PNGImageReader);
-  WTF_MAKE_NONCOPYABLE(PNGImageReader);
-
- public:
-  PNGImageReader(PNGImageDecoder* decoder, size_t readOffset)
-      : m_decoder(decoder),
-        m_readOffset(readOffset),
-        m_currentBufferSize(0),
-        m_decodingSizeOnly(false),
-        m_hasAlpha(false) {
-    m_png = png_create_read_struct(PNG_LIBPNG_VER_STRING, 0, pngFailed, 0);
-    m_info = png_create_info_struct(m_png);
-    png_set_progressive_read_fn(m_png, m_decoder, pngHeaderAvailable,
-                                pngRowAvailable, pngComplete);
-  }
+  if (!m_reader->parse(*m_data, query) && isAllDataReceived())
+    setFailed();
 
-  ~PNGImageReader() {
-    png_destroy_read_struct(m_png ? &m_png : 0, m_info ? &m_info : 0, 0);
-    ASSERT(!m_png && !m_info);
+  if (query == PNGParseQuery::PNGMetaDataQuery)
+    m_frameCount = m_reader->frameCount();
+}
 
-    m_readOffset = 0;
-  }
+void PNGImageDecoder::setRepetitionCount(size_t repetitionCount) {
+  m_repetitionCount =
+      (repetitionCount == 0) ? cAnimationLoopInfinite : repetitionCount;
+}
 
-  bool decode(const SegmentReader& data, bool sizeOnly) {
-    m_decodingSizeOnly = sizeOnly;
-
-    // We need to do the setjmp here. Otherwise bad things will happen.
-    if (setjmp(JMPBUF(m_png)))
-      return m_decoder->setFailed();
-
-    const char* segment;
-    while (size_t segmentLength = data.getSomeData(segment, m_readOffset)) {
-      m_readOffset += segmentLength;
-      m_currentBufferSize = m_readOffset;
-      png_process_data(m_png, m_info,
-                       reinterpret_cast<png_bytep>(const_cast<char*>(segment)),
-                       segmentLength);
-      if (sizeOnly ? m_decoder->isDecodedSizeAvailable()
-                   : m_decoder->frameIsCompleteAtIndex(0))
-        return true;
-    }
+// This matches the existing behavior to loop once if decoding fails, but this
+// should be changed to stick with m_repetitionCount to match other browsers.

[scroggo_chromium, 2016/11/07 13:02:16]

I don't think you want m_repetitionCount - that could include frames that had
errors, right?

[joostouwerling, 2016/11/08 21:58:17]

I think you are confusing the repetition count with the frame count? I would say
that we want to loop the frames we were able to decode |m_repetitionCount|
times.

[scroggo_chromium, 2016/11/09 13:42:50]

Haha, yes, I was confused.

+// See crbug.com/267883
+int PNGImageDecoder::repetitionCount() const {
+  if (m_reader->parseCompleted() && isAllDataReceived() &&
+      m_reader->frameCount() == 1)
+    return cAnimationNone;
+  return failed() ? cAnimationLoopOnce : m_repetitionCount;
+}
 
-    return false;
+// These are mapped according to:
+// https://wiki.mozilla.org/APNG_Specification#.60fcTL.60:_The_Frame_Control_Chunk
+static inline ImageFrame::DisposalMethod getDisposalMethod(
+    uint8_t disposalMethod) {
+  switch (disposalMethod) {
+    case 0:
+      return ImageFrame::DisposalMethod::DisposeKeep;
+    case 1:
+      return ImageFrame::DisposalMethod::DisposeOverwriteBgcolor;
+    case 2:
+      return ImageFrame::DisposalMethod::DisposeOverwritePrevious;
+    default:
+      return ImageFrame::DisposalMethod::DisposeNotSpecified;
   }
+}
 
-  png_structp pngPtr() const { return m_png; }
-  png_infop infoPtr() const { return m_info; }
+// These are mapped according to:
+// https://wiki.mozilla.org/APNG_Specification#.60fcTL.60:_The_Frame_Control_Chunk
+static inline ImageFrame::AlphaBlendSource getAlphaBlend(uint8_t alphaBlend) {
+  if (alphaBlend == 1)
+    return ImageFrame::AlphaBlendSource::BlendAtopPreviousFrame;
+  return ImageFrame::AlphaBlendSource::BlendAtopBgcolor;
+}
 
-  size_t getReadOffset() const { return m_readOffset; }
-  void setReadOffset(size_t offset) { m_readOffset = offset; }
-  size_t currentBufferSize() const { return m_currentBufferSize; }
-  bool decodingSizeOnly() const { return m_decodingSizeOnly; }
-  void setHasAlpha(bool hasAlpha) { m_hasAlpha = hasAlpha; }
-  bool hasAlpha() const { return m_hasAlpha; }
+void PNGImageDecoder::initializeNewFrame(size_t index) {
+  const PNGImageReader::FrameInfo& frameInfo = m_reader->frameInfo(index);
+  ImageFrame* buffer = &m_frameBufferCache[index];
+
+  IntRect frameRectWithinSize =
+      intersection(frameInfo.frameRect, {IntPoint(), size()});
+  buffer->setOriginalFrameRect(frameRectWithinSize);
+  buffer->setDuration(frameInfo.duration);
+  buffer->setDisposalMethod(getDisposalMethod(frameInfo.disposalMethod));
+  buffer->setAlphaBlendSource(getAlphaBlend(frameInfo.alphaBlend));
+  buffer->setRequiredPreviousFrameIndex(
+      findRequiredPreviousFrame(index, false));
+}
+
+// Initialize the frame buffer before decoding. The returned boolean indicates
+// whether initialisation succeeded when it is true, false otherwise.
+bool PNGImageDecoder::initFrameBuffer(size_t index) {
+  ImageFrame* const buffer = &m_frameBufferCache[index];
+
+  if (!buffer->setSizeAndColorSpace(size().width(), size().height(),
+                                    colorSpace()))
+    return false;
 
-  png_bytep interlaceBuffer() const { return m_interlaceBuffer.get(); }
-  void createInterlaceBuffer(int size) {
-    m_interlaceBuffer = wrapArrayUnique(new png_byte[size]);
+  // Create the interlace buffer if:
+  // A) The image is encoded with interlacing, or
+  // B) |index| > 0. In this case, the interlace buffer is used to store the
+  //    row data, so it can be written to the frame buffer at once in
+  //    PNGImageDecoder::complete(). This prevents overwriting previous frames

[scroggo_chromium, 2016/11/07 13:02:16]

I don't see why this is necessary. Let's take frame 1. If it depends on frame 0,
we'll start by copying frame 0 into frame 1's frame buffer. Are you saying you
want to leave frame 1 looking like frame 0 until it's complete? I would think
the correct way to not show partial frames is somewhere else - i.e. just don't
show a frame that is FramePartial.

(But if this *was* the right place, we should make a couple of changes anyway
- this is unnecessary if frame 1 is independent
- the interlaceBuffer should be renamed, since that name no
  longer reflects its use)

Edit: I think I understand - I forgot about takeBitmapDataIfWritable, which is a
recent addition. Maybe instead we should skip that, if we're afraid of stealing
frame 0's data while frame 1 may still be incomplete?

(On another note, GIFImageDecoder doesn't do this trick of writing to an
interlaceBuffer first. Does it have a bug? Or does it accomplish this
differently?)

[joostouwerling, 2016/11/08 21:58:17]

I don't think that the FramePartial check is enforced higher up. At [1], it will
make an SkImage from the frame bitmap that exists for FramePartial, instead of
finalizing the complete frame. Following the call hierarchy up to [2], I don't
see a check for the frame status. In BitmapImage::startAnimation() there is a
check for frameIsCompleteAtIndex() but that reports true when the frame is fully
received, not when it is fully decoded [3].  And sometimes, at least for the
first frames, it makes sense to show partial frames.

The partial overlap of frames can only be caused by slow decoding, since we only
report on the frames when they're fully received. I'm not sure if that delay is
noticeable. If possible, this extra buffer should be avoided since it'll use 4 *
width * height bytes in extra memory for decoding the image.

[1]
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/graph...
[2]
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/graph...
[3]
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/graph...
Done.
 
I don't think that makes any difference, since it'll still write row per row to
the frame buffer.
I can't find any other mechanism that would do this, so that'd mean its a bug
(or a non-issue). But in all fairness, I am not entirely sure of my reasoning at
this point. 

[scroggo_chromium, 2016/11/09 13:42:50]

Agreed, but those two will typically be the same - if the frame is fully
received, it will also be Complete, unless there is an error. Below you talk
about slow decoding, but in the current architecture we will fully decode all
the data that has been received, so if you ask for frame i, and it is fully
received, it will be completely decoded before drawing, even if it's slow. (It
could be FramePartial if there is an error, but that shouldn't be the typical
case. In fact, I think you mark it FrameComplete when you run out of data, so it
could only be Partial if you called setFailed(), in which case I think we'll
avoid showing that frame in another way?)
I agree that it makes sense to show the first frame even if it's partial. But I
don't see how that's relevant here. The question is, for frame i, where i > 0
and depends on a prior frame (say i-1), should we draw directly into
m_frameBufferCache[i], or should we leave m_frameBufferCache[i] looking like i-1
and draw into a temporary buffer and then copy it when it's complete?

But I don't think we'll ever hit the case where i is incomplete. Since you only
start decoding i when all of its data has been received, it is going to receive
the IEND chunk unless there's an error, in which case I think you'll call
setFailed(), so I don't think there's an issue of i being Partial. In the
typical case, you'll draw into your temporary buffer and then copy immediately
into m_frameBufferCache[i]. So there's no sense in preserving the contents of
i-1 in i.
Agreed. This is precisely what I'm pushing for here. I'm not convinced you need
to use this extra memory.
Here's the difference I was talking about: Let's say you're decoding frame i,
which depends on frame i-1. Your use of the extra buffer
(interlaceBuffer/imageBuffer) preserves the contents of frame i-1 in the
ImageFrame for i. The only reason I think you'd want to do that is because you
want to keep showing frame i-1. Assuming we took the old case of copying frame
i-1 into i when we initialize i, if the client really wanted to keep showing
i-1, they could just keep showing ImageFrame i-1. But if
takeBitmapDataIfWritable succeeded frame i-1 is now empty, so the client could
not show i-1 if they wanted to. (IIUC, takeBitmapDataIfWritable will only
succeed if we skipped displaying frame i-1, since I'm assuming that if we're
decoding i, we must have all the data for i-1, and if we didn't skip i-1, we
would have called the code in your link at [1], which calls
finalizePixelsAndGetImage(), which makes future calls to
takeBitmapDataIfWritable fail.) But ultimately I think you're correct that it
doesn't make a difference - as I point out above, we'll only decode i if we have
all the data, so we shouldn't end up with a partial frame anyway.
This is definitely a complicated system that is hard to reason about, but I
think your link at [3] takes care of this.

[joostouwerling, 2016/11/11 20:22:18]

Yes, I agree with you now. Since the decode call will only return once it has
decoded all data, we shouldn't worry about partial frames due to slow decoding.

In the case of a failure during decoding, I was planning on clearing the frame
and reducing the frame count to |i| if decoding of frame |i| fails. (that is,
reducing the frame count to 2 if decoding of frame 3, with index 2, fails).

+  //    partially.
+  png_structp png = m_reader->pngPtr();
+  if (PNG_INTERLACE_ADAM7 == png_get_interlace_type(png, m_reader->infoPtr()) ||
+      index > 0) {
+    unsigned colorChannels = m_reader->hasAlpha() ? 4 : 3;
+    m_reader->createInterlaceBuffer(colorChannels * size().width() *
+                                    size().height());
+    if (!m_reader->interlaceBuffer())
+      return false;
   }
 
- private:
-  png_structp m_png;
-  png_infop m_info;
-  PNGImageDecoder* m_decoder;
-  size_t m_readOffset;
-  size_t m_currentBufferSize;
-  bool m_decodingSizeOnly;
-  bool m_hasAlpha;
-  std::unique_ptr<png_byte[]> m_interlaceBuffer;
-};
+  buffer->setHasAlpha(false);
+  size_t requiredPreviousFrameIndex = buffer->requiredPreviousFrameIndex();
 
-PNGImageDecoder::PNGImageDecoder(AlphaOption alphaOption,
-                                 ColorSpaceOption colorOptions,
-                                 size_t maxDecodedBytes,
-                                 size_t offset)
-    : ImageDecoder(alphaOption, colorOptions, maxDecodedBytes),
-      m_offset(offset) {}
+  // If frame |index| does not depend on any other frame, ensure the frame is
+  // fully transparant black after initialisation.

[scroggo_chromium, 2016/11/07 13:02:16]

transparent*

[joostouwerling, 2016/11/08 21:58:17]

Done.

+  if (requiredPreviousFrameIndex == kNotFound) {
+    buffer->zeroFillPixelData();
+  } else {
+    ImageFrame* prevBuffer = &m_frameBufferCache[requiredPreviousFrameIndex];
+    ASSERT(prevBuffer->getStatus() == ImageFrame::FrameComplete);
+
+    // We try to reuse |prevBuffer| as starting state to avoid copying.
+    // For DisposeOverwritePrevious, the next frame will also use
+    // |prevBuffer| as its starting state, so we can't take over its image
+    // data using takeBitmapDataIfWritable.  Copy the data instead.
+    if ((buffer->getDisposalMethod() == ImageFrame::DisposeOverwritePrevious ||
+         !buffer->takeBitmapDataIfWritable(prevBuffer)) &&
+        !buffer->copyBitmapData(*prevBuffer))
+      return false;
+
+    // We want to clear the previous frame to transparant, without affecting

[scroggo_chromium, 2016/11/07 13:02:16]

transparent*

[joostouwerling, 2016/11/08 21:58:17]

Done.

+    // pixels in the image outside of the frame.
+    if (prevBuffer->getDisposalMethod() ==
+        ImageFrame::DisposeOverwriteBgcolor) {
+      const IntRect& prevRect = prevBuffer->originalFrameRect();
+      ASSERT(!prevRect.contains(IntRect(IntPoint(), size())));
+      buffer->zeroFillFrameRect(prevRect);
+    }
+  }
 
-PNGImageDecoder::~PNGImageDecoder() {}
+  buffer->setStatus(ImageFrame::FramePartial);
+  return true;
+}
 
 inline float pngFixedToFloat(png_fixed_point x) {
   return ((float)x) * 0.00001f;
@@ -223,17 +365,21 @@ void PNGImageDecoder::headerAvailable() {
   png_uint_32 width = png_get_image_width(png, info);
   png_uint_32 height = png_get_image_height(png, info);
 
-  // Protect against large PNGs. See http://bugzil.la/251381 for more details.
-  const unsigned long maxPNGSize = 1000000UL;
-  if (width > maxPNGSize || height > maxPNGSize) {
-    longjmp(JMPBUF(png), 1);
-    return;
-  }
+  // Only set the size of the image once. Since single frames also use this
+  // method, we don't want them to override the size to their frame rect.
+  if (!isDecodedSizeAvailable()) {
+    // Protect against large PNGs. See http://bugzil.la/251381 for more details.
+    const unsigned long maxPNGSize = 1000000UL;
+    if (width > maxPNGSize || height > maxPNGSize) {
+      longjmp(JMPBUF(png), 1);
+      return;
+    }
 
-  // Set the image size now that the image header is available.
-  if (!setSize(width, height)) {
-    longjmp(JMPBUF(png), 1);
-    return;
+    // Set the image size now that the image header is available.
+    if (!setSize(width, height)) {
+      longjmp(JMPBUF(png), 1);
+      return;
+    }
   }
 
   int bitDepth, colorType, interlaceType, compressionType, filterType, channels;
@@ -309,19 +455,6 @@ void PNGImageDecoder::headerAvailable() {
   ASSERT(channels == 3 || channels == 4);
 
   m_reader->setHasAlpha(channels == 4);
-
-  if (m_reader->decodingSizeOnly()) {
-// If we only needed the size, halt the reader.
-#if PNG_LIBPNG_VER_MAJOR > 1 || \
-    (PNG_LIBPNG_VER_MAJOR == 1 && PNG_LIBPNG_VER_MINOR >= 5)
-    // Passing '0' tells png_process_data_pause() not to cache unprocessed data.
-    m_reader->setReadOffset(m_reader->currentBufferSize() -
-                            png_process_data_pause(png, 0));
-#else
-    m_reader->setReadOffset(m_reader->currentBufferSize() - png->buffer_size);
-    png->buffer_size = 0;
-#endif
-  }
 }
 
 void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
@@ -330,73 +463,57 @@ void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
   if (m_frameBufferCache.isEmpty())
     return;
 
-  // Initialize the framebuffer if needed.
-  ImageFrame& buffer = m_frameBufferCache[0];
-  if (buffer.getStatus() == ImageFrame::FrameEmpty) {
-    png_structp png = m_reader->pngPtr();
-    if (!buffer.setSizeAndColorSpace(size().width(), size().height(),
-                                     colorSpace())) {
-      longjmp(JMPBUF(png), 1);
-      return;
-    }
-
-    unsigned colorChannels = m_reader->hasAlpha() ? 4 : 3;
-    if (PNG_INTERLACE_ADAM7 ==
-        png_get_interlace_type(png, m_reader->infoPtr())) {
-      m_reader->createInterlaceBuffer(colorChannels * size().width() *
-                                      size().height());
-      if (!m_reader->interlaceBuffer()) {
-        longjmp(JMPBUF(png), 1);
-        return;
-      }
-    }
-
-    buffer.setStatus(ImageFrame::FramePartial);
-    buffer.setHasAlpha(false);
-
-    // For PNGs, the frame always fills the entire image.
-    buffer.setOriginalFrameRect(IntRect(IntPoint(), size()));
+  ImageFrame& buffer = m_frameBufferCache[m_currentFrame];
+  if (buffer.getStatus() == ImageFrame::FrameEmpty &&
+      !initFrameBuffer(m_currentFrame)) {
+    setFailed();
+    return;
   }
 
+  // This frameRect is already clipped, so that it fits within the size of the
+  // image. This is done in initializeNewFrame() after a frameCount() call.
+  const IntRect& frameRect = buffer.originalFrameRect();
+
   /* libpng comments (here to explain what follows).
-     *
-     * this function is called for every row in the image. If the
-     * image is interlacing, and you turned on the interlace handler,
-     * this function will be called for every row in every pass.
-     * Some of these rows will not be changed from the previous pass.
-     * When the row is not changed, the new_row variable will be NULL.
-     * The rows and passes are called in order, so you don't really
-     * need the row_num and pass, but I'm supplying them because it
-     * may make your life easier.
-     */
+   *
+   * this function is called for every row in the image. If the
+   * image is interlacing, and you turned on the interlace handler,
+   * this function will be called for every row in every pass.
+   * Some of these rows will not be changed from the previous pass.
+   * When the row is not changed, the new_row variable will be NULL.
+   * The rows and passes are called in order, so you don't really
+   * need the row_num and pass, but I'm supplying them because it
+   * may make your life easier.
+   */
 
   // Nothing to do if the row is unchanged, or the row is outside
   // the image bounds: libpng may send extra rows, ignore them to
   // make our lives easier.
   if (!rowBuffer)
     return;
-  int y = rowIndex;
-  if (y < 0 || y >= size().height())
+  int y = rowIndex + frameRect.y();
+  ASSERT(y >= 0);
+  if (y >= size().height())
     return;
 
   /* libpng comments (continued).
-     *
-     * For the non-NULL rows of interlaced images, you must call
-     * png_progressive_combine_row() passing in the row and the
-     * old row.  You can call this function for NULL rows (it will
-     * just return) and for non-interlaced images (it just does the
-     * memcpy for you) if it will make the code easier. Thus, you
-     * can just do this for all cases:
-     *
-     *    png_progressive_combine_row(png_ptr, old_row, new_row);
-     *
-     * where old_row is what was displayed for previous rows. Note
-     * that the first pass (pass == 0 really) will completely cover
-     * the old row, so the rows do not have to be initialized. After
-     * the first pass (and only for interlaced images), you will have
-     * to pass the current row, and the function will combine the
-     * old row and the new row.
-     */
+   *
+   * For the non-NULL rows of interlaced images, you must call
+   * png_progressive_combine_row() passing in the row and the
+   * old row.  You can call this function for NULL rows (it will
+   * just return) and for non-interlaced images (it just does the
+   * memcpy for you) if it will make the code easier. Thus, you
+   * can just do this for all cases:
+   *
+   *    png_progressive_combine_row(png_ptr, old_row, new_row);
+   *
+   * where old_row is what was displayed for previous rows. Note
+   * that the first pass (pass == 0 really) will completely cover
+   * the old row, so the rows do not have to be initialized. After
+   * the first pass (and only for interlaced images), you will have
+   * to pass the current row, and the function will combine the
+   * old row and the new row.
+   */
 
   bool hasAlpha = m_reader->hasAlpha();
   png_bytep row = rowBuffer;
@@ -407,11 +524,17 @@ void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
     png_progressive_combine_row(m_reader->pngPtr(), row, rowBuffer);
   }
 
+  // For non-first frames, don't write rows incrementally to the buffer, since
+  // this may result in partial frames being displayed. Instead, write the
+  // rows in the complete() callback.
+  if (m_currentFrame > 0)
+    return;
+
   // Write the decoded row pixels to the frame buffer. The repetitive
   // form of the row write loops is for speed.
-  ImageFrame::PixelData* const dstRow = buffer.getAddr(0, y);
+  ImageFrame::PixelData* const dstRow = buffer.getAddr(frameRect.x(), y);
   unsigned alphaMask = 255;
-  int width = size().width();
+  int width = frameRect.width();
 
   png_bytep srcPtr = row;
   if (hasAlpha) {
@@ -468,32 +591,116 @@ void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
   buffer.setPixelsChanged(true);
 }
 
+bool PNGImageDecoder::frameIsCompleteAtIndex(size_t index) const {
+  // @TODO(joostouwerling): show complete frames even if a later frame fails.
+  if (failed())
+    return false;
+  if (index >= m_frameBufferCache.size())
+    return false;
+  if (index == 0)
+    return ImageDecoder::frameIsCompleteAtIndex(index);
+  return true;
+}
+
+float PNGImageDecoder::frameDurationAtIndex(size_t index) const {
+  return (index < m_frameBufferCache.size()
+              ? m_frameBufferCache[index].duration()
+              : 0);
+}
+
 void PNGImageDecoder::complete() {
   if (m_frameBufferCache.isEmpty())
     return;
 
-  m_frameBufferCache[0].setStatus(ImageFrame::FrameComplete);
-}
-
-inline bool isComplete(const PNGImageDecoder* decoder) {
-  return decoder->frameIsCompleteAtIndex(0);
-}
+  // @TODO(joostouwerling) if necessary, do a check if all expected data has
+  //                       been received. This is because the IEND chunk is sent
+  //                       artificially. The necessity of this check depends on
+  //                       how libpng handles in- and overcomplete frame data.
+  ImageFrame* buffer = &m_frameBufferCache[m_currentFrame];
 
-void PNGImageDecoder::decode(bool onlySize) {
-  if (failed())
+  // For the first frame, all data is written to the buffer in rowAvailable.
+  if (m_currentFrame == 0) {
+    buffer->setStatus(ImageFrame::FrameComplete);
     return;
+  }
 
-  if (!m_reader)
-    m_reader = wrapUnique(new PNGImageReader(this, m_offset));
+  // For non-first frames, the frame data was written to the interlace buffer
+  // in rowAvailable. Write the decoded pixels from the the interlace buffer to
+  // the frame buffer.
+  png_bytep interlaceBuffer = m_reader->interlaceBuffer();
 
-  // If we couldn't decode the image but have received all the data, decoding
-  // has failed.
-  if (!m_reader->decode(*m_data, onlySize) && isAllDataReceived())
-    setFailed();
+  IntRect frameRect = buffer->originalFrameRect();
+  bool hasAlpha = m_reader->hasAlpha();
+  unsigned colorChannels = hasAlpha ? 4 : 3;
+  unsigned alphaMask = 255;
+
+  png_bytep row = interlaceBuffer;
+
+  // TODO(joostouwerling): for now, this is a straight copy of the code in
+  //                       rowAvailable, except for writing multiple rows. Alpha
+  //                       blending and possible code sharing need to be added.
+  for (int y = frameRect.y(); y < frameRect.maxY();
+       ++y, row += colorChannels * size().width()) {
+    ImageFrame::PixelData* const dstRow = buffer->getAddr(frameRect.x(), y);
+    unsigned alphaMask = 255;
+    int width = frameRect.width();
+
+    png_bytep srcPtr = row;
+    if (hasAlpha) {
+      // Here we apply the color space transformation to the dst space.
+      // It does not really make sense to transform to a gamma-encoded
+      // space and then immediately after, perform a linear premultiply.
+      // Ideally we would pass kPremul_SkAlphaType to xform->apply(),
+      // instructing SkColorSpaceXform to perform the linear premultiply
+      // while the pixels are a linear space.
+      // We cannot do this because when we apply the gamma encoding after
+      // the premultiply, we will very likely end up with valid pixels
+      // where R, G, and/or B are greater than A.  The legacy drawing
+      // pipeline does not know how to handle this.
+      if (SkColorSpaceXform* xform = colorTransform()) {
+        SkColorSpaceXform::ColorFormat colorFormat =
+            SkColorSpaceXform::kRGBA_8888_ColorFormat;
+        xform->apply(colorFormat, dstRow, colorFormat, srcPtr, size().width(),
+                     kUnpremul_SkAlphaType);
+        srcPtr = (png_bytep)dstRow;
+      }
+
+      if (buffer->premultiplyAlpha()) {
+        for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
+             dstPixel++, srcPtr += 4) {
+          buffer->setRGBAPremultiply(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2],
+                                     srcPtr[3]);
+          alphaMask &= srcPtr[3];
+        }
+      } else {
+        for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
+             dstPixel++, srcPtr += 4) {
+          buffer->setRGBARaw(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2],
+                             srcPtr[3]);
+          alphaMask &= srcPtr[3];
+        }
+      }
+    } else {
+      for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
+           dstPixel++, srcPtr += 3) {
+        buffer->setRGBARaw(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2], 255);
+      }
+
+      // We'll apply the color space xform to opaque pixels after they have been
+      // written to the ImageFrame, purely because SkColorSpaceXform supports
+      // RGBA (and not RGB).
+      if (SkColorSpaceXform* xform = colorTransform()) {
+        xform->apply(xformColorFormat(), dstRow, xformColorFormat(), dstRow,
+                     size().width(), kOpaque_SkAlphaType);
+      }
+    }
+  }
+
+  if (alphaMask != 255 && !buffer->hasAlpha())
+    buffer->setHasAlpha(true);
 
-  // If decoding is done or failed, we don't need the PNGImageReader anymore.
-  if (isComplete(this) || failed())
-    m_reader.reset();
+  buffer->setPixelsChanged(true);
+  buffer->setStatus(ImageFrame::FrameComplete);
 }
 
 }  // namespace blink