Add support for Animated PNG

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 6deba9ce22adfd250319cac12dada224f706df3d..a9b9e054001da68fa9991f3d3217d738053a6c39 100644
--- a/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp
@@ -38,11 +38,6 @@
 
 #include "platform/image-decoders/png/PNGImageDecoder.h"
 
-#include "platform/image-decoders/png/PNGImageReader.h"
-#include "png.h"
-#include "wtf/PtrUtil.h"
-#include <memory>
-
 namespace blink {
 
 PNGImageDecoder::PNGImageDecoder(AlphaOption alphaOption,
@@ -50,10 +45,105 @@ PNGImageDecoder::PNGImageDecoder(AlphaOption alphaOption,
                                  size_t maxDecodedBytes,
                                  size_t offset)
     : ImageDecoder(alphaOption, colorBehavior, maxDecodedBytes),
-      m_offset(offset) {}
+      m_offset(offset),
+      m_currentFrame(0),
+      // It would be logical to default to cAnimationNone, but BitmapImage uses
+      // that as a signal to never check again, meaning the actual count will
+      // never be respected.
+      m_repetitionCount(cAnimationLoopOnce),
+      m_hasAlphaChannel(false),
+      m_currentBufferSawAlpha(false) {}
 
 PNGImageDecoder::~PNGImageDecoder() {}
 
+bool PNGImageDecoder::setFailed() {
+  m_reader.reset();
+  return ImageDecoder::setFailed();
+}
+
+size_t PNGImageDecoder::decodeFrameCount() {
+  parse(ParseQuery::MetaData);
+  return failed() ? m_frameBufferCache.size() : m_reader->frameCount();
+}
+
+void PNGImageDecoder::decode(size_t index) {
+  parse(ParseQuery::MetaData);
+
+  if (failed())
+    return;
+
+  updateAggressivePurging(index);
+
+  Vector<size_t> framesToDecode = findFramesToDecode(index);
+  for (auto i = framesToDecode.rbegin(); i != framesToDecode.rend(); i++) {
+    m_currentFrame = *i;
+    if (!m_reader->decode(*m_data, *i)) {
+      setFailed();
+      return;
+    }
+
+    // If this returns false, we need more data to continue decoding.
+    if (!postDecodeProcessing(*i))
+      break;
+  }
+
+  // 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 >= m_frameBufferCache.size() - 1 && isAllDataReceived() &&
+      m_reader && !m_reader->parseCompleted())
+    setFailed();
+}
+
+void PNGImageDecoder::parse(ParseQuery query) {
+  if (failed() || (m_reader && m_reader->parseCompleted()))
+    return;
+
+  if (!m_reader)
+    m_reader = WTF::makeUnique<PNGImageReader>(this, m_offset);
+
+  if (!m_reader->parse(*m_data, query))
+    setFailed();
+}
+
+void PNGImageDecoder::clearFrameBuffer(size_t index) {
+  if (m_reader)
+    m_reader->clearDecodeState(index);
+  ImageDecoder::clearFrameBuffer(index);
+}
+
+bool PNGImageDecoder::canReusePreviousFrameBuffer(size_t index) const {
+  DCHECK(index < m_frameBufferCache.size());
+  return m_frameBufferCache[index].getDisposalMethod() !=
+         ImageFrame::DisposeOverwritePrevious;
+}
+
+void PNGImageDecoder::setRepetitionCount(int repetitionCount) {
+  m_repetitionCount = repetitionCount;
+}
+
+int PNGImageDecoder::repetitionCount() const {
+  return failed() ? cAnimationLoopOnce : m_repetitionCount;
+}
+
+void PNGImageDecoder::initializeNewFrame(size_t index) {
+  const PNGImageReader::FrameInfo& frameInfo = m_reader->frameInfo(index);
+  ImageFrame& buffer = m_frameBufferCache[index];
+
+  DCHECK(IntRect(IntPoint(), size()).contains(frameInfo.frameRect));
+  buffer.setOriginalFrameRect(frameInfo.frameRect);
+
+  buffer.setDuration(frameInfo.duration);
+  buffer.setDisposalMethod(frameInfo.disposalMethod);
+  buffer.setAlphaBlendSource(frameInfo.alphaBlend);
+
+  size_t previousFrameIndex = findRequiredPreviousFrame(index, false);
+  buffer.setRequiredPreviousFrameIndex(previousFrameIndex);
+}
+
+inline float pngFixedToFloat(png_fixed_point x) {

[Noel Gordon, 2017/03/06 05:21:47]

Merge funk: remove this function ...

[scroggo_chromium, 2017/03/07 20:25:36]

Done.

+  return ((float)x) * 0.00001f;
+}
+
 inline sk_sp<SkColorSpace> readColorSpace(png_structp png, png_infop info) {
   if (png_get_valid(png, info, PNG_INFO_sRGB))
     return SkColorSpace::MakeSRGB();
@@ -111,22 +201,8 @@ inline sk_sp<SkColorSpace> readColorSpace(png_structp png, png_infop info) {
 void PNGImageDecoder::headerAvailable() {
   png_structp png = m_reader->pngPtr();
   png_infop info = m_reader->infoPtr();
-  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;
-  }
-
-  // Set the image size now that the image header is available.
-  if (!setSize(width, height)) {
-    longjmp(JMPBUF(png), 1);
-    return;
-  }
 
+  png_uint_32 width, height;
   int bitDepth, colorType, interlaceType, compressionType;
   png_get_IHDR(png, info, &width, &height, &bitDepth, &colorType,
                &interlaceType, &compressionType, nullptr);
@@ -148,11 +224,29 @@ void PNGImageDecoder::headerAvailable() {
       colorType == PNG_COLOR_TYPE_GRAY_ALPHA)
     png_set_gray_to_rgb(png);
 
-  if ((colorType & PNG_COLOR_MASK_COLOR) && !ignoresColorSpace()) {
-    // We only support color profiles for color PALETTE and RGB[A] PNG.
-    // TODO(msarret): Add GRAY profile support, block CYMK?
-    if (sk_sp<SkColorSpace> colorSpace = readColorSpace(png, info))
-      setEmbeddedColorSpace(std::move(colorSpace));
+  // Only set the size and the color space of the image once since non-first
+  // frames also use this method: there is no per-frame color space, and the
+  // image size is determined from the header width and height.
+  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;
+    }
+
+    if ((colorType & PNG_COLOR_MASK_COLOR) && !ignoresColorSpace()) {
+      // We only support color profiles for color PALETTE and RGB[A] PNG.
+      // TODO(msarret): Add GRAY profile support, block CYMK?
+      if (sk_sp<SkColorSpace> colorSpace = readColorSpace(png, info))
+        setEmbeddedColorSpace(colorSpace);
+    }
   }
 
   if (!hasEmbeddedColorSpace()) {
@@ -171,6 +265,8 @@ void PNGImageDecoder::headerAvailable() {
     }
   }
 
+  DCHECK(isDecodedSizeAvailable());
+
   // Tell libpng to send us rows for interlaced pngs.
   if (interlaceType == PNG_INTERLACE_ADAM7)
     png_set_interlace_handling(png);
@@ -180,56 +276,41 @@ void PNGImageDecoder::headerAvailable() {
 
   int channels = png_get_channels(png, info);
   DCHECK(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
-  }
+  m_hasAlphaChannel = (channels == 4);
 }
 
 void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
                                    unsigned rowIndex,
                                    int) {
-  if (m_frameBufferCache.isEmpty())
+  if (m_currentFrame >= m_frameBufferCache.size())
     return;
 
-  // Initialize the framebuffer if needed.
-  ImageFrame& buffer = m_frameBufferCache[0];
+  ImageFrame& buffer = m_frameBufferCache[m_currentFrame];
   if (buffer.getStatus() == ImageFrame::FrameEmpty) {
     png_structp png = m_reader->pngPtr();
-    if (!buffer.setSizeAndColorSpace(size().width(), size().height(),
-                                     colorSpaceForSkImages())) {
+    if (!initFrameBuffer(m_currentFrame)) {
       longjmp(JMPBUF(png), 1);
       return;
     }
 
-    unsigned colorChannels = m_reader->hasAlpha() ? 4 : 3;
+    DCHECK_EQ(ImageFrame::FramePartial, buffer.getStatus());
+
     if (PNG_INTERLACE_ADAM7 ==
         png_get_interlace_type(png, m_reader->infoPtr())) {
-      m_reader->createInterlaceBuffer(colorChannels * size().width() *
-                                      size().height());
+      unsigned colorChannels = m_hasAlphaChannel ? 4 : 3;
+      m_reader->createInterlaceBuffer(colorChannels * size().area());
       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()));
+    m_currentBufferSawAlpha = false;
   }
 
+  const IntRect& frameRect = buffer.originalFrameRect();
+  DCHECK(IntRect(IntPoint(), size()).contains(frameRect));
+
   /* libpng comments (here to explain what follows).
    *
    * this function is called for every row in the image. If the
@@ -242,13 +323,16 @@ void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
    * 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.
+  // Nothing to do if the row is unchanged, or the row is outside the image
+  // bounds. In the case that a frame presents more data than the indicated
+  // frame size, ignore the extra rows and use the frame size as the source
+  // of truth. libpng can send extra rows: ignore them too, this to prevent
+  // memory writes outside of the image bounds (security).
   if (!rowBuffer)
     return;
-  int y = rowIndex;
-  if (y < 0 || y >= size().height())
+  int y = rowIndex + frameRect.y();

[Noel Gordon, 2017/03/06 05:21:47]

The comment says the frameRect is used as the source of truth.  But this code
does not match the comment, viz., where is rowIndex compared to the
frameRect.height()?

Most the code currently does is compare y to size().height(), but
frameRect.height() can be less than that height.  So either the code or the
comment is wrong.  Can we fix?

[scroggo_chromium, 2017/03/07 20:25:36]

Code is wrong. Fixed.

Do you recall cases where libpng sends extra rows? I would expect to potentially
see them in cases like 60 in crbug.com/698808, but it does not in that case.

[Noel Gordon, 2017/03/08 15:36:01]

Good.
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2010-1205
https://bugs.webkit.org/attachment.cgi?id=140137&action=prettypatch

[scroggo_chromium, 2017/03/08 20:53:21]

Thanks! According to http://libpng.org/pub/png/libpng.html, this has been fixed
in versions 1.4.3 and 1.2.44, which I would expect to include our version (1.6).

[Noel Gordon, 2017/03/13 12:16:17]

Yes, 1.6 has the fix.  But other packagers of Chromium code (linux distos, etc)
often configure their Chromium build to use an external (system) libpng and we
do not prevent that e.g., by checking PNG version with PNG_LIBPNG_VER_XXXXX in
the PNGImageDecoder code.  So we retain this code so we do not expose Chromium
packagers to the CVE.

If we wanted to ditch this CVE checking code, we could, but we'd maybe need to
also enforce it with PNG_LIBPNG_VER_MINOR/MAJOR etc checks to ensure the PNG
version being used has the CVE fix.
^^^ notes fast/images/png-extra-row-crash.html, now called
images/png-extra-row-crash.html, presumably uses a PNG image that caused the
problem (if you wanted to see an example of such an image).

+  DCHECK_GE(y, 0);

[Noel Gordon, 2017/03/06 05:21:47]

If this y calculation ever wrapped around and produced a negative, a DCHECK
won't save you from the security flaw :)

You could try using this:

if (UNLIKELY(y < 0))
  return;

or otherwise, leave the case as it was:

if (y < 0 || y >= size().height())
  return;

[scroggo_chromium, 2017/03/07 20:25:36]

Done.

+  if (y >= size().height())
     return;
 
   /* libpng comments (continued).
@@ -270,7 +354,7 @@ void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
    * old row and the new row.
    */
 
-  bool hasAlpha = m_reader->hasAlpha();
+  bool hasAlpha = m_hasAlphaChannel;
   png_bytep row = rowBuffer;
 
   if (png_bytep interlaceBuffer = m_reader->interlaceBuffer()) {
@@ -281,8 +365,8 @@ void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
 
   // 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);
-  int width = size().width();
+  ImageFrame::PixelData* const dstRow = buffer.getAddr(frameRect.x(), y);
+  int width = frameRect.width();
 
   png_bytep srcPtr = row;
   if (hasAlpha) {
@@ -305,23 +389,47 @@ void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
     }
 
     unsigned alphaMask = 255;
-    if (buffer.premultiplyAlpha()) {
-      for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
-           srcPtr += 4, ++dstPixel) {
-        buffer.setRGBAPremultiply(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2],
-                                  srcPtr[3]);
-        alphaMask &= srcPtr[3];
+    if (m_frameBufferCache[m_currentFrame].getAlphaBlendSource() ==
+        ImageFrame::BlendAtopBgcolor) {
+      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;
-           srcPtr += 4, ++dstPixel) {
-        buffer.setRGBARaw(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2], srcPtr[3]);
-        alphaMask &= srcPtr[3];
+      // Now, the blend method is ImageFrame::BlendAtopPreviousFrame. Since the
+      // frame data of the previous frame is copied at initFrameBuffer, we can
+      // blend the pixel of this frame, stored in |srcPtr|, over the previous
+      // pixel stored in |dstPixel|.
+      if (buffer.premultiplyAlpha()) {
+        for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
+             dstPixel++, srcPtr += 4) {
+          buffer.blendRGBAPremultiplied(dstPixel, srcPtr[0], srcPtr[1],
+                                        srcPtr[2], srcPtr[3]);
+          alphaMask &= srcPtr[3];
+        }
+      } else {
+        for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
+             dstPixel++, srcPtr += 4) {
+          buffer.blendRGBARaw(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2],
+                              srcPtr[3]);
+          alphaMask &= srcPtr[3];
+        }
       }
     }
 
-    if (alphaMask != 255 && !buffer.hasAlpha())
-      buffer.setHasAlpha(true);
+    if (alphaMask != 255 && !m_currentBufferSawAlpha)

[Noel Gordon, 2017/03/06 05:21:46]

Is the && !m_currentBufferSawAlpha part worth it anymore?  Anything wrong with
writing as follows?

if (alphaMask != 255)
  m_currentBufferSawAlpha = true;

[scroggo_chromium, 2017/03/07 20:25:36]

I have not timed the difference. It just saves a write to memory.
That looks fine to me. Done.

+      m_currentBufferSawAlpha = true;
 
   } else {
     for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
@@ -341,32 +449,50 @@ void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
   buffer.setPixelsChanged(true);
 }
 
-void PNGImageDecoder::complete() {
-  if (m_frameBufferCache.isEmpty())
-    return;
+bool PNGImageDecoder::frameIsCompleteAtIndex(size_t index) const {
+  if (!isDecodedSizeAvailable())
+    return false;
 
-  m_frameBufferCache[0].setStatus(ImageFrame::FrameComplete);
+  DCHECK(!failed() && m_reader);
+

[Noel Gordon, 2017/03/06 05:21:46]

I don't see failed() being used in other image decoder impls of this function,
not sure its needed.  But then again, with the failure strategy you are using
for APNG, it might have a use.  Hmmm, start this routine with  

if (failed() || !isDecodedSizeAvailable())
  return false;

if that gels with your APNG failure strategy?

[scroggo_chromium, 2017/03/07 20:25:36]

Not directly, but GIFImageDecoder checks for m_reader, which it deletes on
failure.
That would be redundant - isDecodedSizeAvailable returns false if failed().

+  // For non-animated images, return whether the status of the frame is
+  // ImageFrame::FrameComplete with ImageDecoder::frameIsCompleteAtIndex.
+  // This matches the behavior of WEBPImageDecoder.
+  if (m_reader->parseCompleted() && m_reader->frameCount() == 1)

[Noel Gordon, 2017/03/06 05:21:46]

frameIsCompleteAtIndex, frameHasAlphaAtIndex: you made some points about them
earlier in reply #20, providing references to where Peter or myself complained
about these functions, and the patch set that changed their meaning
https://codereview.chromium.org/14317005 (which as an aside, lead to the
chicken+egg bug I believe).

Anyho, we could maybe simplify the code here for the first frame and other
frames as follows:

  if (!index)
    return ImageDecoder::frameIsCompleteAtIndex(0);
  DCHECK(m_reader);
  bool frameIsReceivedAtIndex = index < m_reader->frameCount();
  return frameIsReceivedAtIndex;

and with a layout test for the <canvas> use-case, I'd expect we'd be fine.

[scroggo_chromium, 2017/03/07 20:25:36]

Do you mean crbug.com/593430?
That's definitely simpler, but it's different from both WEBP and GIF, which will
both return whether the frame has been received for index 0 of an animated
image.

One of the few places this makes a difference is here [1], which would be hard
to test for.

[1]
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/graph...
As in crrev.com/2728193004? That passes for me whether I use
firstFrameFullyReceived or ImageDecoder::frameIsCompleteAtIndex, but it does not
deal with [1].

[Noel Gordon, 2017/03/08 15:36:01]

Yeap.
Fine with leaving it alone given hard-to-test. 
Could not quite follow here, but 535 lands in the middle of a comment

// may be in the past, meaning the next time through this function we'll

Something/one updated BitmapImage after you grabbed the URL maybe, or the
comment was the thing you wanted to point out to me?
and "hard-to-test".  Re: crrev.com/2728193004 PTAL :)

[scroggo_chromium, 2017/03/08 20:53:21]

Someone updated. A more permanent link, at an arbitrary commit:

https://chromium.googlesource.com/chromium/src/+/396fddbbebaa3e693fdcf2b6f6fd...

This line looks like it would break with your suggestion here. (More below.)
Already looked, as stated in my comment above. "That passes for me [either way]"
(when run as a layout test - it fails for me either way when I just open the
page). It does not cover the case I am concerned about, though. As I understand
it, your test moves off the first frame, and then verifies that we draw the
first frame to the canvas. My concern is about when we're catching up. If
frameAfterNext is frame 0, and frameIsCompleteAtIndex(0) never returns true,
we'll break out of this loop, when we should have executed it.

[Noel Gordon, 2017/03/13 12:16:17]

(discussed on that bug how the "open the page case" will fail unless you deal
with the browser's same origin policy).
When frameIsCompleteAtIndex(index) meant what is says (the frame was decoded)
the catch-up code you referred to worked.  When frameIsCompleteAtIndex(index)
was later changed to mean the frame was fully received, the the catch-up code
still "worked", but with very different visual results for users in some cases.

More comments about frameIsCompleteAtIndex() below, but agree, let's stick with
the concept of frame-is-fully-received at index so we don't change meaning for
the catch-up code you referred to (with the perma-link).

+    return ImageDecoder::frameIsCompleteAtIndex(index);
+
+  // For first frames of animated images, PNGImageReader exposes whether it is
+  // fully received through firstFrameFullyReceived().
+  if (!index)
+    return m_reader->firstFrameFullyReceived();
+
+  // Non-first frames are reported by |m_reader| once it has parsed all data
+  // for that frame, so return whether the index is below the reported frame
+  // count.
+  return index < m_reader->frameCount();
 }
 
-inline bool isComplete(const PNGImageDecoder* decoder) {
-  return decoder->frameIsCompleteAtIndex(0);
+float PNGImageDecoder::frameDurationAtIndex(size_t index) const {
+  if (index < m_frameBufferCache.size())
+    return m_frameBufferCache[index].duration();
+  return 0;
 }
 
-void PNGImageDecoder::decode(bool onlySize) {
-  if (failed())
+void PNGImageDecoder::frameComplete() {

[Noel Gordon, 2017/03/06 05:21:46]

Could we move this up to just after rowAvailable() please, to keep all the
callbacks from libpng together?

[scroggo_chromium, 2017/03/07 20:25:36]

Done.

+  if (m_currentFrame >= m_frameBufferCache.size())
     return;
 
-  if (!m_reader)
-    m_reader = WTF::makeUnique<PNGImageReader>(this, m_offset);
+  if (m_reader->interlaceBuffer())
+    m_reader->clearInterlaceBuffer();
 
-  // 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();
+  ImageFrame& buffer = m_frameBufferCache[m_currentFrame];
+  if (buffer.getStatus() == ImageFrame::FrameEmpty)
+    longjmp(JMPBUF(m_reader->pngPtr()), 1);

[Noel Gordon, 2017/03/06 05:21:46]

Add a return; after this longjmp (just like we do in rowAvailable) ?

[scroggo_chromium, 2017/03/07 20:25:36]

Done.

+
+  if (!m_currentBufferSawAlpha)
+    correctAlphaWhenFrameBufferSawNoAlpha(m_currentFrame);
 
-  // If decoding is done or failed, we don't need the PNGImageReader anymore.
-  if (isComplete(this) || failed())
-    m_reader.reset();
+  buffer.setStatus(ImageFrame::FrameComplete);
 }
 
 }  // namespace blink