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

third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp

Index: third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp
diff --git a/third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp b/third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp
index 6a4db8fa2d711b56af64eac34f940d4fd870781b..209971bdd699e2816da63c07590d2214b1385a23 100644
--- a/third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp
@@ -96,6 +96,20 @@ bool ImageFrame::copyBitmapData(const ImageFrame& other)
     return other.m_bitmap.copyTo(&m_bitmap, other.m_bitmap.colorType());
 }
 
+bool ImageFrame::takeBitmapDataIfWritable(ImageFrame* other)
+{
+    DCHECK(other);
+    if (this == other)
+        return false;
+    if (other->m_bitmap.isImmutable())

[Peter Kasting, 2016/08/26 19:04:04]

Nit: Just combine these conditionals

[aleksandar.stojiljkovic, 2016/08/26 21:53:51]

Done.

+        return false;
+    m_hasAlpha = other->m_hasAlpha;
+    m_bitmap.reset();
+    m_bitmap.swap(other->m_bitmap);
+    other->m_status = FrameEmpty;
+    return true;
+}
+
 bool ImageFrame::setSizeAndColorProfile(int newWidth, int newHeight, const ICCProfile& newIccProfile)
 {
     // setSizeAndColorProfile() should only be called once, it leaks memory otherwise.
@@ -119,6 +133,17 @@ bool ImageFrame::hasAlpha() const
     return m_hasAlpha;
 }
 
+const SkBitmap& ImageFrame::bitmap(bool setImmutableIfDone)

[aleksandar.stojiljkovic, 2016/08/26 18:53:41]

pkasting@
The fix for the failing tests is simpler than anticipated:
- ImageFrameGenerator is the only customer of ImageFrame::bitmap() in cases
referred from the bugs listed here. ImageFrameGenerator doesn't increase ref
count (it just calls getPixels) and it is not interested if bitmap is mutable or
not... When decoder and ImageFrame are used by ImageFrameGenerator, there is no
need to get the bitmap immutable flag set (to call setImmutable). 

The other clients of ImageFrame::bitmap() are kept the same - setImmutable is
deferred and set when client makes a request to access bitmap via
ImageFrame::bitmap(bool setImmutableIfDone = true).

+{
+    if (m_status == FrameComplete && setImmutableIfDone && !m_bitmap.isImmutable()) {
+        // We don't set the status to immutable until there is a request to
+        // access the bitmap.  This is because mutable bitmap can be reused as
+        // the next frame by calling takeBitmapDataIfWritable.
+        m_bitmap.setImmutable();
+    }
+    return m_bitmap;
+}
+
 void ImageFrame::setHasAlpha(bool alpha)
 {
     m_hasAlpha = alpha;
@@ -134,7 +159,6 @@ void ImageFrame::setStatus(Status status)
         // Send pending pixels changed notifications now, because we can't do this after
         // the bitmap has been marked immutable.
         notifyBitmapIfPixelsChanged();
-        m_bitmap.setImmutable(); // Tell the bitmap it's done.
     }
 }