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..f56e64b721161be477d12b705314fd8831891bf6 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,256 @@
 #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),
+      m_colorSpaceSet(false),
+      m_hasAlphaChannel(false),
+      m_currentBufferSawAlpha(false) {}
 
-inline blink::PNGImageDecoder* imageDecoder(png_structp png) {
-  return static_cast<blink::PNGImageDecoder*>(png_get_progressive_ptr(png));
+PNGImageDecoder::~PNGImageDecoder() {}
+
+size_t PNGImageDecoder::decodeFrameCount() {
+  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 ImageFrame::FrameComplete. Therefore, it is
+  // OK that the do-while loop always appends |index| to |m_framesToDecode|,
+  // without checking for its status.
+  //
+  // 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.
+// See crbug.com/267883
+int PNGImageDecoder::repetitionCount() const {
+  if (m_reader->parseCompleted() && 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;
+}
+
+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));
+}
 
-  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; }
+// 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];
+
+  // Return true if the frame is already initialised.
+  if (buffer->getStatus() != ImageFrame::FrameEmpty)
+    return true;
+
+  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]);
+  unsigned colorChannels = m_hasAlphaChannel ? 4 : 3;
+  png_structp png = m_reader->pngPtr();
+  if (PNG_INTERLACE_ADAM7 == png_get_interlace_type(png, m_reader->infoPtr())) {
+    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(true);
+  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 transparent black after initialisation.
+  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 transparent, without affecting
+    // 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);
+  m_currentBufferSawAlpha = false;

[scroggo_chromium, 2016/11/29 16:30:52]

We already know the frame rectangle here. Could we (should we) set this to true
if it does not cover the full frame? I guess not necessarily, since we also need
to check the required frame if it has one?

[joostouwerling, 2016/12/02 16:08:42]

This variable is semantically used to store whether there are (semi-)
transparent pixels in the the frame data. The logic of whether this means the
frame is actually transparent is done in complete(). And indeed, the required
frame could be opaque outside this frame's rect, which could result in an opaque
frame if the frame data does not have any transparent pixels. 

+  return true;
+}
 
 inline float pngFixedToFloat(png_fixed_point x) {
   return ((float)x) * 0.00001f;
@@ -223,17 +364,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;
@@ -261,7 +406,8 @@ void PNGImageDecoder::headerAvailable() {
       colorType == PNG_COLOR_TYPE_GRAY_ALPHA)
     png_set_gray_to_rgb(png);
 
-  if ((colorType & PNG_COLOR_MASK_COLOR) && !m_ignoreColorSpace) {
+  if ((colorType & PNG_COLOR_MASK_COLOR) && !m_ignoreColorSpace &&
+      !m_colorSpaceSet) {
     // We only support color profiles for color PALETTE and RGB[A] PNG.
     // Supporting color profiles for gray-scale images is slightly tricky, at
     // least using the CoreGraphics ICC library, because we expand gray-scale
@@ -272,6 +418,10 @@ void PNGImageDecoder::headerAvailable() {
     if (colorSpace) {
       setColorSpaceAndComputeTransform(colorSpace);
     }
+
+    // For animated PNGs, we only need to set the color space once, since frames
+    // don't have their own color space. Set |m_colorSpaceSet| to true.
+    m_colorSpaceSet = true;
   }
 
   if (!hasEmbeddedColorSpace()) {
@@ -308,20 +458,7 @@ void PNGImageDecoder::headerAvailable() {
   channels = png_get_channels(png, info);
   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
-  }
+  m_hasAlphaChannel = (channels == 4);
 }
 
 void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
@@ -330,75 +467,58 @@ 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()));
+  if (!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.
+  ImageFrame& buffer = m_frameBufferCache[m_currentFrame];
+  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.
-     */
-
-  bool hasAlpha = m_reader->hasAlpha();
+   *
+   * 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_hasAlphaChannel;
   png_bytep row = rowBuffer;
 
   if (png_bytep interlaceBuffer = m_reader->interlaceBuffer()) {
@@ -409,9 +529,9 @@ 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);
+  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) {
@@ -433,20 +553,42 @@ void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
       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];
+    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;
-           dstPixel++, srcPtr += 4) {
-        buffer.setRGBARaw(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2], srcPtr[3]);
-        alphaMask &= srcPtr[3];
+      if (buffer.premultiplyAlpha()) {
+        for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
+             dstPixel++, srcPtr += 4) {
+          buffer.blendRGBAPremultiply(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 && !m_currentBufferSawAlpha)
+      m_currentBufferSawAlpha = true;
+
   } else {
     for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
          dstPixel++, srcPtr += 3) {
@@ -462,38 +604,76 @@ void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
     }
   }
 
-  if (alphaMask != 255 && !buffer.hasAlpha())
-    buffer.setHasAlpha(true);
-
   buffer.setPixelsChanged(true);
 }
 
-void PNGImageDecoder::complete() {
-  if (m_frameBufferCache.isEmpty())
-    return;
-
-  m_frameBufferCache[0].setStatus(ImageFrame::FrameComplete);
+bool PNGImageDecoder::frameIsCompleteAtIndex(size_t index) const {
+  // @TODO(joostouwerling): show complete frames even if a later frame fails.
+  if (failed())
+    return false;
+  if (index == 0)
+    return ImageDecoder::frameIsCompleteAtIndex(index);
+
+  // For non-first frames, the frame is considered complete if all frame data
+  // has been received. Non-first frames are reported by |m_reader| once it has
+  // parsed all data for that frame, so we can simply return if the index
+  // exists in |m_frameBufferCache| here.
+  return (index < m_frameBufferCache.size());
 }
 
-inline bool isComplete(const PNGImageDecoder* decoder) {
-  return decoder->frameIsCompleteAtIndex(0);
+float PNGImageDecoder::frameDurationAtIndex(size_t index) const {
+  return (index < m_frameBufferCache.size()
+              ? m_frameBufferCache[index].duration()
+              : 0);
 }
 
-void PNGImageDecoder::decode(bool onlySize) {
-  if (failed())
+void PNGImageDecoder::complete() {
+  if (m_frameBufferCache.isEmpty())
     return;
 
-  if (!m_reader)
-    m_reader = wrapUnique(new PNGImageReader(this, m_offset));
-
-  // 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();
+  // @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.
+
+  if (m_reader->interlaceBuffer())  
+    m_reader->clearInterlaceBuffer();
+ 
+  ImageFrame& buffer = m_frameBufferCache[m_currentFrame];
+  buffer.setStatus(ImageFrame::FrameComplete);
+  
+  if (!m_currentBufferSawAlpha) {

[scroggo_chromium, 2016/11/29 16:30:52]

This boolean corresponds to the current frame, but is used by PNGImageDecoder
for all frames. I think there is a corner case that might break this:

e.g.
- partially decode frame A, which has no alpha (complete() is not called)
- decode frame B, which does not depend on A and has alpha
- finish decoding frame A

decoding B will set m_currentBufferSawAlpha to true. resuming decoding frame A
will not reset it to false, so when we call complete() on A the value will be
incorrect.

We only (currently) progressively decode frame 0, so I think this could only
happen if frame A = frame 0, and frame B is a later frame.

Maybe it would be safer to drop m_currentBufferSawAlpha and store alpha on the
ImageFrame the whole time (and rely on the code you referenced earlier, which
checks for completeness)?

[joostouwerling, 2016/12/02 16:08:42]

I think your example would falsely set frame A's alpha to true, yes. Moreover,
this scenario would have invalidated the decoder as is - since it would try to
continue progressively decoding a frame when it can't: the png struct has
already been reset for frame B. This new png struct has already received the
IEND chunk and thus does not expect any new frame data.

The easy solution for this issue is to reset |m_progressiveDecodeOffset| in
PNGImageReader when a non-first frame is decoded, and completely re-decode first
frames when they're decoded later on. And now that I've looked more carefully at
it, this is probably the best solution. Trying to continue decoding where we
left off is tricky because:

- We may have partially decoded a row at the last decode call. So we have to
find out where that row started to re-decode that row, but I don't immediately
see how.
- We have to keep track in PNGImageReader that we need to create a new png
struct when |m_progressiveDecodeOffset| is > 0, but another frame has been
decoded already.
- Libpng has to re-decode the header data anyway, since the new struct needs to
learn what's in the frame.
- We can keep a different png struct for the first frame for this scenario, but
I don't think that is worth the extra class overhead and implementation
obscurity for this edge case. 

The aforementioned solution is implemented in the new patch and I added a test
to make sure it correctly decodes the first frame.

In regard to the alpha issue, we can solve this by resetting
|m_currentBufferSawAlpha| in headerAvailable, with the change described above in
mind, since it will be called before *each* frame is decoded.

+    // The whole frame was non-transparent, so it's possible that the entire
+    // resulting buffer was non-transparent, and we can setHasAlpha(false).
+    if (buffer.originalFrameRect().contains(IntRect(IntPoint(), size()))) {
+      buffer.setHasAlpha(false);
+      buffer.setRequiredPreviousFrameIndex(kNotFound);
+    } else if (buffer.requiredPreviousFrameIndex() != kNotFound) {
+      // Tricky case.  This frame does not have alpha only if everywhere
+      // outside its rect doesn't have alpha.  To know whether this is
+      // true, we check the start state of the frame -- if it doesn't have
+      // alpha, we're safe.
+      const ImageFrame* prevBuffer =
+          &m_frameBufferCache[buffer.requiredPreviousFrameIndex()];
+      ASSERT(prevBuffer->getDisposalMethod() !=
+             ImageFrame::DisposeOverwritePrevious);
+
+      // Now, if we're at a DisposeNotSpecified or DisposeKeep frame, then

[scroggo_chromium, 2016/11/29 16:30:52]

These comments make it sound like the first block does something (check the
alpha of the prior frame) and the second block does something different, but
really the first just initializes a pointer, and the second does all the checks.
Maybe merge the comments together?

On another note, when you say "we" on this line, it is not clear to me which
frame you mean. (I think you mean prevBuffer?)

[scroggo_chromium, 2016/12/02 15:55:35]

I was just looking at GIFImageDecoder and I realized that this is copied
directly from there. (Another opportunity to share code!)

[joostouwerling, 2016/12/02 16:08:42]

Yes, this is somewhat confusion. I've merged and clarified the comments.

+      // we can say we have no alpha if that frame had no alpha.  But
+      // since in initFrameBuffer() we already copied that frame's alpha
+      // state into the current frame's, we need do nothing at all here.
+      //
+      // The only remaining case is a DisposeOverwriteBgcolor frame.  If
+      // it had no alpha, and its rect is contained in the current frame's
+      // rect, we know the current frame has no alpha.

[scroggo_chromium, 2016/11/29 16:30:52]

FWIW, I think you could take this further - if prevBuffer *does* have alpha, but
only in the rectangle which is covered by this one frame, you could still say
that this one has no alpha. But I think determining that would be tricky (-er).

[joostouwerling, 2016/12/02 16:08:42]

It is not necessarily tricky, but computationally expensive, since the runtime
is O(width * height). I don't think that is worth it.

+      if ((prevBuffer->getDisposalMethod() ==
+           ImageFrame::DisposeOverwriteBgcolor) &&
+          !prevBuffer->hasAlpha() &&
+          buffer.originalFrameRect().contains(prevBuffer->originalFrameRect()))
+        buffer.setHasAlpha(false);
+    }
+  }
 
-  // If decoding is done or failed, we don't need the PNGImageReader anymore.
-  if (isComplete(this) || failed())
-    m_reader.reset();
 }
 
 }  // namespace blink