WIP: Implement APNG

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

+/*
+ * Copyright (C) 2006 Apple Computer, Inc.
+ * Copyright (C) Research In Motion Limited 2009-2010. All rights reserved.
+ *
+ * Portions are Copyright (C) 2001 mozilla.org
+ *
+ * Other contributors:
+ *   Stuart Parmenter <stuart@mozilla.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 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
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * Alternatively, the contents of this file may be used under the terms
+ * of either the Mozilla Public License Version 1.1, found at
+ * http://www.mozilla.org/MPL/ (the "MPL") or the GNU General Public
+ * License Version 2.0, found at http://www.fsf.org/copyleft/gpl.html
+ * (the "GPL"), in which case the provisions of the MPL or the GPL are
+ * applicable instead of those above.  If you wish to allow use of your
+ * version of this file only under the terms of one of those two
+ * licenses (the MPL or the GPL) and not to allow others to use your
+ * version of this file under the LGPL, indicate your decision by
+ * deletingthe provisions above and replace them with the notice and
+ * other provisions required by the MPL or the GPL, as the case may be.
+ * If you do not delete the provisions above, a recipient may use your
+ * version of this file under any of the LGPL, the MPL or the GPL.
+ */
+
+#include "platform/image-decoders/png/PNGImageReader.h"
+
+#include "platform/image-decoders/png/PNGImageDecoder.h"
+#include "platform/image-decoders/FastSharedBufferReader.h"
+#include "png.h"
+#include "wtf/PtrUtil.h"
+#include <memory>
+
+#if !defined(PNG_LIBPNG_VER_MAJOR) || !defined(PNG_LIBPNG_VER_MINOR)
+#error version error: compile against a versioned libpng.
+#endif
+#if USE(QCMSLIB)
+#include "qcms.h"
+#endif
+
+#if PNG_LIBPNG_VER_MAJOR > 1 || (PNG_LIBPNG_VER_MAJOR == 1 && PNG_LIBPNG_VER_MINOR >= 4)
+#define JMPBUF(png_ptr) png_jmpbuf(png_ptr)
+#else
+#define JMPBUF(png_ptr) png_ptr->jmpbuf
+#endif
+
+namespace {
+
+inline blink::PNGImageDecoder* imageDecoder(png_structp png)
+{
+    return static_cast<blink::PNGImageDecoder*>(png_get_progressive_ptr(png));
+}
+
+void PNGAPI pngHeaderAvailable(png_structp png, png_infop)
+{
+    imageDecoder(png)->headerAvailable();
+}
+
+void PNGAPI pngRowAvailable(png_structp png, png_bytep row,
+                            png_uint_32 rowIndex, int state)
+{
+    imageDecoder(png)->rowAvailable(row, rowIndex, state);
+}
+
+void PNGAPI pngComplete(png_structp png, png_infop)
+{
+    imageDecoder(png)->complete();
+}
+
+void PNGAPI pngFailed(png_structp png, png_const_charp err)
+{
+    longjmp(JMPBUF(png), 1);
+}
+
+} // namespace
+
+namespace blink {
+
+// This is the callback function for unknown PNG chunks, which is used to
+// extract the animation chunks.
+static int readAnimationChunk(png_structp png_ptr, png_unknown_chunkp chunk)
+{
+    PNGImageReader* reader = (PNGImageReader*) png_get_user_chunk_ptr(png_ptr);
+    reader->parseAnimationChunk((const char*) chunk->name, chunk->data,
+                                chunk->size);
+    return 1;
+}
+
+PNGImageReader::PNGImageReader(PNGImageDecoder* decoder, size_t initialOffset)
+    : m_decoder(decoder)
+    , m_initialOffset(initialOffset)
+    , m_readOffset(initialOffset)
+    , m_bytesInfo(0)
+    , m_hasAlpha(false)
+    , m_idatIsPartOfAnimation(false)
+    , m_isAnimated(false)
+    , m_parsedSignature(false)
+#if USE(QCMSLIB)
+    , m_rowBuffer()
+#endif
+{
+    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);
+
+    // Keep the chunks which are of interest for APNG. We don't need to keep
+    // the fdAT chunks, since they are converted to IDAT's by the frame decoder.
+    png_byte apngChunks[] = {"acTL\0fcTL\0"};
+    png_set_keep_unknown_chunks(m_png, PNG_HANDLE_CHUNK_NEVER, apngChunks, 2);
+    png_set_read_user_chunk_fn(m_png, (png_voidp) this, readAnimationChunk);
+}
+
+PNGImageReader::~PNGImageReader()
+{
+    png_destroy_read_struct(m_png ? &m_png : 0, m_info ? &m_info : 0, 0);
+    ASSERT(!m_png && !m_info);
+}
+
+// This method reads from the FastSharedBufferReader, starting at offset,
+// and returns |length| bytes in the form of a pointer to a const png_byte*.
+// This function is used to make it easy to access data from the reader in a
+// png friendly way, and pass it to libpng for decoding.
+//
+// Pre-conditions before using this:
+// - |reader|.size() >= |readOffset| + |length|
+// - |buffer|.size() = kBufferSize >= |length|
+//
+// The reason for the last precondition is that at this point, the png signature
+// plus IHDR chunk (8B + 25B)  is the largest chunk that is read using this
+// method. If the data is not consecutive, it is stored in |buffer|, which
+// should have the size of (at least) kBufferSize.

[scroggo_chromium, 2016/10/25 14:59:07]

should -> must

+constexpr size_t kBufferSize = 33;

[scroggo_chromium, 2016/10/25 14:59:06]

nit: I think this should be static.

+const png_byte* readAsConstPngBytep(const FastSharedBufferReader& reader,
+                                    size_t readOffset, size_t length,
+                                    char* buffer)
+{
+    ASSERT(length <= kBufferSize);
+    return reinterpret_cast<const png_byte*>(
+            reader.getConsecutiveData(readOffset, length, buffer));
+}
+
+void PNGImageReader::decode(SegmentReader& data, size_t index)
+{
+    if (index >= m_frameInfo.size())
+        return;
+
+    // For non animated PNG's, we don't want to waste CPU time with recreating
+    // the png struct. It suffices to continue parsing where we left off. Since
+    // non animated only need to decode the frame once, we can use the
+    // readOffset field to enable progressive decoding.
+    if (!m_isAnimated) {
+        if (setjmp(JMPBUF(m_png))) {
+            m_decoder->setFailed();
+            return;
+        }
+        m_frameInfo[0].readOffset += processData(data, m_frameInfo[0].readOffset, 0);
+        return;
+    }
+
+    // Initialize a new png struct for this frame.
+    startFrameDecoding(data, index);
+
+    // From the frame info that was gathered during parsing, it is known at
+    // what offset the frame data starts and how many bytes are in the stream
+    // before the frame ends. Using this, we process all chunks that fall in
+    // this interval. We catch every fdAT chunk and transform it to an IDAT
+    // chunk, so libpng will decode it like a non-animated PNG image.
+    size_t offset = m_frameInfo[index].readOffset;
+    size_t endOffset = offset + m_frameInfo[index].byteLength;
+    char readBuffer[8];
+    FastSharedBufferReader reader(&data);
+
+    while (offset < endOffset) {
+        const png_byte* chunk = readAsConstPngBytep(reader, offset, 8, readBuffer);

[scroggo_chromium, 2016/10/25 14:59:07]

Doesn't your comments up above tell us that readBuffer's size must equal
kBufferSize?

[joostouwerling, 2016/10/26 15:44:17]

I modified the comment to make this more clear.

+        const png_uint_32 length = png_get_uint_32(chunk);
+
+        if (memcmp(chunk + 4, "fdAT", 4) == 0) {
+            png_byte chunkIDAT[] = {0, 0, 0, 0, 'I', 'D', 'A', 'T'};

[scroggo_chromium, 2016/10/25 14:59:07]

Can these various chunks be constexpr?

[joostouwerling, 2016/10/26 15:44:17]

This one should semantically not be constexpr, since the next line writes to it.
For the IEND chunk later on, it would mean that it needs to be cast to a
non-const array on the next line, since libpng does not accept const png bytes,
which seems like a waste of cycles to me.

+            png_save_uint_32(chunkIDAT, length - 4);

[scroggo_chromium, 2016/10/25 14:59:07]

Why do we need to subtract 4 here?

+            png_process_data(m_png, m_info, chunkIDAT, 8);
+            // Skip the first 4 bytes of the fdAT chunk, which contain the
+            // sequence number. By doing this, parsing |length| bytes will 
+            // include the CRC at the end of the chunk as well.

[scroggo_chromium, 2016/10/25 14:59:07]

Why? Sorry, I do not follow.

[joostouwerling, 2016/10/26 15:44:17]

An fdAT chunk is build up like:

|length| (4B) -> tag (4B) -> sequence number (4B) -> frame data (|length| - 4B)
-> CRC (4B)

So the actual frame data plus the CRC data is |length| bytes long. This is also
the reason |length| - 4 is written as the new length to the IDAT chunk byte
array.

+            processData(data, offset + 12, length); 
+        } else {
+            png_process_data(m_png, m_info, const_cast<png_byte*>(chunk), 8);
+            processData(data, offset + 8, length + 4);
+        }
+
+        offset += 12 + length; 
+    }
+
+    // Finish decoding by sending the IEND chunk.
+    endFrameDecoding();
+
+}
+
+void PNGImageReader::startFrameDecoding(SegmentReader& data, size_t index)
+{
+    // Each frame is processed as if it were a complete, single frame png image.
+    // To accomplish this, destroy the current |m_png| and |m_info| structs and
+    // create new ones. CRC errors are ignored, so fdAT chunks can be processed

[scroggo_chromium, 2016/10/25 14:59:07]

Is there a risk to ignoring CRC errors?

[joostouwerling, 2016/10/26 15:44:17]

They are meant as a checksum to see if the data received corresponds to what was
intended to be sent. At this point, it does not make sense to check for CRC
errors, since they would have been calculated on data we already received over
the network, since we just feed reformatted fdAT data at this point. 

That being said, we can check if the fdAT data we received is correct, either
during parsing or during decoding. It will add some processing time, since the
code now jumps |length| bytes ahead in the stream to the next frame,  but it
makes sure that not data is missed or corrupted by the network. 

[scroggo_chromium, 2016/10/26 18:25:58]

The main concern I would have is that libpng would have thrown a fatal error due
to a CRC mismatch prior to some other worse failure (i.e. libpng does not check
for some kind of problem later because such a problem would have resulted in the
fatal CRC error).

[joostouwerling, 2016/10/27 20:29:55]

This is a good question and I don't know exactly what the dangers are. 

As far as my understanding goes, libpng gradually calculates the CRC while
processing the data it is fed. At the end it checks if this matches to what the
CRC value in the stream says. It would throw an error if they differ.

I also think it is mainly used to determine if the network stream lost bytes -
if someone has malicious intents they can also modify the CRC chunk. Missing
bytes would result in a png process error, which only fails the decode.

This code only ignores CRC errors for frame data though - all data before the
IDAT chunk is verified.

+    // as IDAT's without recalculating the CRC value. 

[scroggo_chromium, 2016/10/25 14:59:07]

nit: No need for the ' here.

+    png_destroy_read_struct(m_png ? &m_png : 0, m_info ? &m_info : 0, 0);

[scroggo_chromium, 2016/10/25 14:59:07]

A couple of comments:
- Will we ever have m_info but no m_png? I think we should be able to say
something like:

  if (m_png)
    png_destroy_read_struct(&m_png, m_info ? &m_info
                                           : nullptr);

- I assume we do *not* want to do this if we've already started decoding frame
|index| and are now continuing it.

[joostouwerling, 2016/10/26 15:44:17]

Does that happen for frames with |index| > 0, since we only report them when all
data is available? For |index| = 0 for animated images, I want to take a similar
approach as for non animated pngs.

[scroggo_chromium, 2016/10/26 18:25:58]

Good question. Just because all the data is available does not mean that the
data represents a complete frame. So it's possible that we would call this a
second time for an incomplete frame, even though there's nothing more to decode.
Which is to say that it could be incomplete the first time around, and we'll
need to continue decoding the second time? sgtm

[joostouwerling, 2016/10/27 20:29:55]

I don't think that this is the right place to take that into account. The reader
only reports frames with |index| > 0 if it has seen all it's frame data. Thus,
if the decoder asks the reader to decode a frame with |index| > 0, the reader
will always signal the IEND chunk. If the decoder receives this and is unhappy
with the data it got at that point, it also knows that requesting a decode again
won't give different data.

[scroggo_chromium, 2016/10/28 14:20:32]

Just to make sure I understand: because you've provided an IEND chunk, libpng's
callback for end of image will be called, and our callback[1] sets the
ImageFrame to FrameComplete[2], so decode will not be called again[3]. Is that
right?

[1]
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/image...
[2]
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/image...
[3]
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/image...

[joostouwerling, 2016/10/28 18:41:25]

Right.

+    m_png = png_create_read_struct(PNG_LIBPNG_VER_STRING, 0, pngFailed, 0);
+    m_info = png_create_info_struct(m_png);
+    png_set_crc_action(m_png, PNG_CRC_QUIET_USE, PNG_CRC_QUIET_USE);
+    png_set_progressive_read_fn(m_png, m_decoder, pngHeaderAvailable,
+                                pngRowAvailable, pngComplete);
+   
+    if (setjmp(JMPBUF(m_png))) {
+        m_decoder->setFailed();
+        return;
+    }
+    
+    // If the frame is the size of the whole image, we don't need to modify any
+    // data in the IHDR chunk. This means it suffices to re-process all header
+    // data up to the first frame, for mimicking a png image.
+    IntRect& frameRect = m_frameInfo[index].frameRect;

[scroggo_chromium, 2016/10/25 14:59:07]

I think this can be const

+    if (frameRect.location() == IntPoint()
+        && frameRect.size() == m_decoder->size()) {
+        processData(data, m_initialOffset, m_bytesInfo); 
+        return;
+    }
+
+    // Process the IHDR chunk, but change the width and height so it reflects
+    // the frame's width and height. Image Decoder will apply the x,y offset.
+    // This step may be omitted if width and height are equal to the image size.

[scroggo_chromium, 2016/10/25 14:59:07]

Instead of "may be", I'd say that it is omitted - perhaps with a reference to
the block above.

+    FastSharedBufferReader reader(&data);
+    char readBuffer[kBufferSize];
+
+    // |headerSize| is equal to |kBufferSize|, but adds more semantic insight.
+    constexpr size_t headerSize = 33;

[scroggo_chromium, 2016/10/25 14:59:07]

nit: kHeaderSize

+    png_byte header[headerSize];
+    const png_byte* chunk = readAsConstPngBytep(reader, m_initialOffset,
+                                                headerSize, readBuffer);
+    memcpy(header, chunk, headerSize);

[scroggo_chromium, 2016/10/25 14:59:07]

Alternatively, you could always use readBuffer:

if (reinterpret_cast<char*>(chunk) != readBuffer)
  memcpy(readBuffer, chunk, kBufferSize);

We should probably go with whatever is clearer

[joostouwerling, 2016/10/26 15:44:17]

Your option saves 33B of memory but I think it's more verbose, especially since
it also needs another const_cast. I would probably go for the memory savings but
I'm not sure what the general rule of wisdom is for Chromium.

[scroggo_chromium, 2016/10/26 18:25:58]

I don't think (a constant, stack-allocated, briefly living) 33 bytes of memory
should determine our decision. I was confused by the fact that we had two
buffers, and I thought my way might be more clear, but I do not have a strong
preference.

[joostouwerling, 2016/10/28 18:41:25]

Acknowledged.

+    png_save_uint_32(header + 16, frameRect.width());

[scroggo_chromium, 2016/10/25 14:59:07]

Here I'm guessing you do *not* want the clipped width and height. We should have
tests for these cases.

[joostouwerling, 2016/10/26 15:44:17]

The width and height that are written here are non-clipped. The clipping happens
in PNGImageDecoder::initializeNewFrame() and is taken into account in
PNGImageDecoder::rowAvailable(). 

I have tests that check whether an overflowing frame is clipped correctly:
AnimatedPNGTests.frameRectIsClipped. Is that what you mean, or do I not
understand what you're at?

[scroggo_chromium, 2016/10/26 18:25:58]

Okay, I see that now. Maybe add a comment where you declare frameRect?
I just looked over that test. I don't think it's a realistic test, and I've
commented on it there.

[joostouwerling, 2016/10/28 18:41:24]

Done

+    png_save_uint_32(header + 20, frameRect.height());
+    png_process_data(m_png, m_info, header, headerSize);
+
+    // Process the rest of the header chunks. Start after the PNG signature and
+    // IHDR chunk, 33B, and process up to the first data chunk.
+    processData(data, m_initialOffset + headerSize, m_bytesInfo - headerSize);
+}
+
+void PNGImageReader::endFrameDecoding()
+{
+    png_byte IEND[12] = {0, 0, 0, 0, 'I', 'E', 'N', 'D', 174, 66, 96, 130};
+    png_process_data(m_png, m_info, IEND, 12);

[scroggo_chromium, 2016/10/25 14:59:07]

We need to be careful about our setjmps. This is called by decode, which calls
setjmp, but in between, decode calls startFrameDecoding, which also calls
setjmp. So if this longjmps, I think it will jump back to a function which has
already exited, which would be bad. Is it possible this call will longjmp?

[joostouwerling, 2016/10/26 15:44:17]

I improved this by setting the setjmp only in decode, and for both animated and
non animated images.

[scroggo_chromium, 2016/10/26 18:25:58]

sgtm

+}
+
+bool PNGImageReader::parse(SegmentReader& data,
+                           PNGImageDecoder::PNGParseQuery query)
+{
+    if (setjmp(JMPBUF(m_png)))
+        return m_decoder->setFailed();
+
+    // If the size has not been parsed, do that first, since it's necessary
+    // for both the Size and MetaData query. If parseSize returns false,
+    // it failed because of a lack of data so we can return false at this point.
+    if (!m_decoder->isDecodedSizeAvailable() && !parseSize(data))
+        return false;
+    
+    if (query == PNGImageDecoder::PNGParseQuery::PNGSizeQuery)
+        return m_decoder->isDecodedSizeAvailable();
+    
+    // For non animated images (identified by no acTL chunk before the IDAT), 
+    // we create one frame. This saves some processing time since we don't need
+    // to go over the stream to find chunks.
+    if (!m_isAnimated) {
+        if (m_frameInfo.size() == 0) {
+            FrameInfo frame;
+            frame.readOffset = m_readOffset + 8;
+            frame.frameRect = IntRect(IntPoint(), m_decoder->size());
+            frame.duration = 0;
+            frame.alphaBlend = ImageFrame::AlphaBlendSource::BlendAtopBgcolor;
+            frame.disposalMethod = ImageFrame::DisposalMethod::DisposeNotSpecified;
+            m_frameInfo.append(frame);
+            m_decoder->setMetaDataDecoded();
+        }
+        return true;
+    }
+
+    FastSharedBufferReader reader(&data);
+    char readBuffer[kBufferSize];
+
+    // At this point, the query is FrameMetaDataQuery. Loop over the data and
+    // register all frames we can find. A frame is registered on the next fcTL
+    // chunk or when the IEND chunk is found. This ensures that only complete
+    // frames are reported, unless there is an error in the stream.
+    while (reader.size() >= m_readOffset + 8) {
+        const png_byte* chunk = readAsConstPngBytep(reader, m_readOffset, 8,
+                                                    readBuffer);
+        const size_t length = png_get_uint_32(chunk);
+        const bool isFCTLChunk = memcmp(chunk + 4, "fcTL", 4) == 0;
+        const bool isIENDChunk = memcmp(chunk + 4, "IEND", 4) == 0;
+        
+        // When we find an IDAT chunk (when the IDAT is part of the animation),
+        // or an fdAT chunk, and the readOffset field of the newFrame is 0,
+        // we have found the beginning of a new block of frame data.
+        const bool isFrameData = memcmp(chunk + 4, "fdAT", 4) == 0
+            || (memcmp(chunk + 4, "IDAT", 4) == 0 && m_idatIsPartOfAnimation);
+        if (m_newFrame.readOffset == 0 && isFrameData) {
+            m_newFrame.readOffset = m_readOffset;
+
+        // An fcTL or IEND marks the end of the previous frame. Thus, the
+        // FrameInfo data in m_newFrame is submitted to the m_frameInfo vector.
+        //
+        // Furthermore, an fcTL chunk indicates a new frame is coming,
+        // so the m_newFrame variable is prepared accordingly by setting the 
+        // readOffset field to 0, which indicates that the frame control info
+        // is available but that we haven't seen any frame data yet.
+        } else if (isFCTLChunk || isIENDChunk) {
+            if (m_newFrame.readOffset != 0) {
+                m_newFrame.byteLength = m_readOffset - m_newFrame.readOffset;
+                m_frameInfo.append(m_newFrame);
+                m_newFrame.readOffset = 0;
+            }
+
+            if (reader.size() < m_readOffset + 12 + length)
+                return false;
+            
+            if (isIENDChunk) {
+                // Let the decoder know we've parsed all data, so it does not 
+                // need to query again.
+                m_decoder->setMetaDataDecoded();
+                return true;
+            }
+
+            // At this point, we're dealing with an fcTL chunk, since the above
+            // statement already returns on IEND chunks.
+            
+            // If the fcTL chunk is not 26 bytes long, we can't process it.
+            if (length != 26)
+                longjmp(JMPBUF(m_png), 1);

[scroggo_chromium, 2016/10/25 14:59:07]

Why did you longjmp instead of returning setFailed()?

+
+            chunk = readAsConstPngBytep(reader, m_readOffset + 8, length,
+                                        readBuffer);
+            parseFrameInfo(chunk);
+
+        }
+        m_readOffset += 12 + length;
+    }
+    return false; 
+}
+
+// If |length| == 0, read until the stream ends.
+// @return: number of bytes processed.
+size_t PNGImageReader::processData(SegmentReader& data, size_t offset,
+                                   size_t length)
+{
+    const char* segment;
+    size_t totalProcessedBytes = 0;
+    while (size_t segmentLength = data.getSomeData(segment, offset)) {
+        if (length > 0 && segmentLength + totalProcessedBytes > length)
+            segmentLength = length - totalProcessedBytes;
+        png_process_data(m_png, m_info,
+                         reinterpret_cast<png_byte*>(const_cast<char*>(segment)),
+                         segmentLength);
+        offset += segmentLength;
+        totalProcessedBytes += segmentLength;
+        if (totalProcessedBytes == length)
+            return length;
+    }
+    return totalProcessedBytes;
+}
+
+// This methods reads through the stream until it has parsed the image size.
+// @return true when it succeeds in parsing the size.
+//         false when:
+//         A) not enough data is provided
+//         B) decoding by libpng fails. In the this case, it will also call
+//            setFailed on m_decoder. 
+bool PNGImageReader::parseSize(SegmentReader &data)
+{
+    FastSharedBufferReader reader(&data);
+    char readBuffer[kBufferSize];
+
+    // Process the PNG signature and the IHDR with libpng, such that this code 
+    // does not need to be bothered with parsing the contents. This also enables
+    // the reader to use the existing headerAvailable callback in the decoder.
+    //
+    // When we already have decoded the signature, we don't need to do it again.
+    // By setting a flag for this we allow for byte by byte parsing.
+    if (!m_parsedSignature) {
+        if (reader.size() < m_readOffset + 8)
+            return false;
+        const png_byte* chunk = readAsConstPngBytep(reader, m_readOffset, 8,
+                                                    readBuffer);
+        png_process_data(m_png, m_info, const_cast<png_byte*>(chunk), 8);
+        m_readOffset += 8;
+        m_parsedSignature = true;
+        // Initialize the newFrame by setting the readOffset to 0.
+        m_newFrame.readOffset = 0;
+    }
+
+    // This loop peeks at the chunk tag until the IDAT chunk is found. When
+    // a different tag is encountered, pass it on to libpng for general parsing.
+    // We can peek at chunks by looking at the first 8 bytes, which contain the
+    // length and the chunk tag.
+    //
+    // When an fcTL (frame control) is encountered before the IDAT, the frame
+    // data in the IDAT chunk is part of the animation. This case is flagged
+    // and the frame info is stored by parsing the fcTL chunk.
+    while (reader.size() >= m_readOffset + 8) {
+        const png_byte* chunk = readAsConstPngBytep(reader, m_readOffset, 8,
+                                                    readBuffer);
+        const png_uint_32 length = png_get_uint_32(chunk);
+
+        // If we encounter the IDAT chunk, we're done with the png header
+        // chunks. Indicate this to libpng by sending the beginning of the IDAT
+        // chunk, which will trigger libpng to call the headerAvailable
+        // callback on m_decoder. This provides the size to the decoder.
+        if (memcmp(chunk + 4, "IDAT", 4) == 0) {
+            if (m_idatIsPartOfAnimation)
+                m_newFrame.readOffset = m_readOffset;
+            m_bytesInfo = m_readOffset;
+            png_process_data(m_png, m_info, const_cast<png_byte*>(chunk), 8);
+            return true;
+        }
+
+        // Consider the PNG image animated if an acTL chunk of the correct
+        // length is present. Parsing the acTL content is done by 
+        // parseAnimationControl, called by libpng's png_process_data.
+        if (memcmp(chunk + 4, "acTL", 4) == 0 && length == 8)
+            m_isAnimated = true;
+
+        // We don't need to check for |length| here, because the decoder will
+        // fail later on for invalid fcTL chunks.
+        if (memcmp(chunk + 4, "fcTL", 4) == 0)
+            m_idatIsPartOfAnimation = true;
+        
+        // 12 is the length, tag and crc part of the chunk, which are all 4B.
+        if (reader.size() < m_readOffset + length + 12)
+            break;  
+
+        png_process_data(m_png, m_info, const_cast<png_byte*>(chunk), 8);
+        processData(data, m_readOffset + 8, length + 4);
+        m_readOffset += length + 12;
+    }
+
+    // If we end up here, not enough data was available for the IDAT chunk
+    // So libpng would not have called headerAvailable yet.
+    return false;
+} 
+
+
+void PNGImageReader::parseAnimationChunk(const char tag[], const void* data_chunk, size_t length)
+{
+    const png_byte* data = static_cast<const png_byte*>(data_chunk);
+
+    // The number of frames as indicated in the animation control chunk (acTL) 
+    // is ignored, and the number of frames that are actually present is used.
+    // For now, when the number of indicated frames is different from the
+    // number of supplied frames, the number of supplied frames is what is
+    // provided to the decoder. Therefore, it does not add any benefit of
+    // looking at the value of the indicated framecount. A note here is that
+    // there may be optimisations available, for example, prescaling vectors.
+    if (strcmp(tag, "acTL") == 0 && length == 8) {
+        png_uint_32 repetitionCount = png_get_uint_32(data + 4);
+        m_decoder->setRepetitionCount(repetitionCount);
+    
+    // For fcTL, decoding fails if it does not have the correct length. It is
+    // impossible to make a guess about the frame if not all data is available.
+    } else if (strcmp(tag, "fcTL") == 0) {
+        if (length != 26)
+            longjmp(JMPBUF(m_png), 1);

[scroggo_chromium, 2016/10/25 14:59:06]

Again, why did you longjmp instead of calling setFailed()?

[joostouwerling, 2016/10/26 15:44:17]

The previous comment can be handled with setFailed(), but not this one, since it
is called via libpng. It needs to get back to parse() to return false to the
decoder, and at the same time prevent unnecessary further parsing. 

[scroggo_chromium, 2016/10/26 18:25:58]

Ah yes, of course.

+        parseFrameInfo(data);
+    }
+
+}
+
+size_t PNGImageReader::frameCount() const
+{
+    return m_frameInfo.size();
+}
+
+const PNGImageReader::FrameInfo& PNGImageReader::frameInfo(size_t index) const
+{
+    ASSERT(index < m_frameInfo.size());
+    return m_frameInfo[index];
+}
+
+// Extract the frame control info and store it in m_newFrame. The length check
+// on the data chunk has been done in parseAnimationChunk.
+// The fcTL specification used can be found at:
+// https://wiki.mozilla.org/APNG_Specification#.60fcTL.60:_The_Frame_Control_Chunk
+void PNGImageReader::parseFrameInfo(const png_byte* data)
+{
+    png_uint_32 width, height, xOffset, yOffset;
+    png_uint_16 delayNumerator, delayDenominator;
+    width               = png_get_uint_32(data + 4);
+    height              = png_get_uint_32(data + 8);
+    xOffset             = png_get_uint_32(data + 12);
+    yOffset             = png_get_uint_32(data + 16);
+    delayNumerator      = png_get_uint_16(data + 20);
+    delayDenominator    = png_get_uint_16(data + 22);
+
+    m_newFrame.duration         = (delayDenominator == 0) ? delayNumerator * 10
+                                    : delayNumerator * 1000 / delayDenominator;
+    m_newFrame.frameRect        = IntRect(xOffset, yOffset, width, height);
+    m_newFrame.disposalMethod   = data[24];
+    m_newFrame.alphaBlend       = data[25];
+    
+}
+
+}; // namespace blink