Save a bitmap copy when advancing to dependent GIF and WebP animation frames

third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp

Index: third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp
diff --git a/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp b/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp
index 72f2c834ae95c11df7e2e03cee1975a0e5591c08..c2bc6be760868b6d3dbad9739ad3e2f856909dcb 100644
--- a/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp
@@ -356,12 +356,18 @@ bool GIFImageDecoder::initFrameBuffer(size_t frameIndex)
         if (!buffer->setSize(size().width(), size().height()))
             return setFailed();
     } else {
-        const ImageFrame* prevBuffer = &m_frameBufferCache[requiredPreviousFrameIndex];
+        ImageFrame* prevBuffer = &m_frameBufferCache[requiredPreviousFrameIndex];
         ASSERT(prevBuffer->getStatus() == ImageFrame::FrameComplete);
 
-        // Preserve the last frame as the starting state for this frame.
-        if (!buffer->copyBitmapData(*prevBuffer))
-            return setFailed();
+        if (buffer->getDisposalMethod() == ImageFrame::DisposeOverwritePrevious) {
+            // Preserve the last frame as the starting state for this frame as
+            // the next frame would also use it.

[Peter Kasting, 2016/07/19 03:04:17]

Nit: How about:

The next frame will also use |prevBuffer| as its starting state, so we can't
take over its image data as we do below.  Copy the data instead.

[aleksandar.stojiljkovic, 2016/07/19 11:10:40]

Done.

+            if (!buffer->copyBitmapData(*prevBuffer))
+                return setFailed();
+        } else {
+            // Use the required frame as the starting state for this frame.

[Peter Kasting, 2016/07/19 03:04:17]

Nit: How about:

This is the only frame to use |prevBuffer| as its starting state, and we'll
always clear the old frame data after initializing this frame anyway, so we can
save time by just moving its data over.

Also, if the above is true, why is it true (that we will always clear)? Is it
because of ImageFrameGenerator::decode() calling clearCacheExceptFrame()?  If so
why do we also need |m_purgeAggressively|? I'm confused about all this.

[aleksandar.stojiljkovic, 2016/07/19 11:10:40]

Done.

+            buffer->takeBitmapData(*prevBuffer);
+        }
 
         if (prevBuffer->getDisposalMethod() == ImageFrame::DisposeOverwriteBgcolor) {
             // We want to clear the previous frame to transparent, without