Prevent synchronous layout while painting the last frame of a gif

third_party/WebKit/Source/platform/graphics/BitmapImage.cpp

Index: third_party/WebKit/Source/platform/graphics/BitmapImage.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/BitmapImage.cpp b/third_party/WebKit/Source/platform/graphics/BitmapImage.cpp
index 6bd22c923ea3ab9b77ed1e4398427c6af54cfd5c..db76479a5733277d1ba3209480b5b57a23f9c5c3 100644
--- a/third_party/WebKit/Source/platform/graphics/BitmapImage.cpp
+++ b/third_party/WebKit/Source/platform/graphics/BitmapImage.cpp
@@ -521,8 +521,14 @@ void BitmapImage::startAnimation(CatchUpAnimation catchUpIfNecessary)
                 break;
 
             // Yes; skip over it without notifying our observers.
-            if (!internalAdvanceAnimation(true))
+            if (!internalAdvanceAnimation(SkipFramesToCatchUp)) {
+                // If we can no longer advance the animation (i.e., it has hit
+                // the last frame), post a task to notify observers immediately
+                // so they redraw the last frame.
+                m_frameTimer = wrapUnique(new Timer<BitmapImage>(this, &BitmapImage::notifyObserversOfAnimationAdvance));
+                m_frameTimer->startOneShot(0, BLINK_FROM_HERE);
                 return;
+            }
             m_desiredFrameStartTime = frameAfterNextStartTime;
             nextFrame = frameAfterNext;
         }
@@ -573,7 +579,7 @@ void BitmapImage::advanceTime(double deltaTimeInSeconds)
 
 void BitmapImage::advanceAnimation(TimerBase*)
 {
-    internalAdvanceAnimation(false);
+    internalAdvanceAnimation();
     // At this point the image region has been marked dirty, and if it's
     // onscreen, we'll soon make a call to draw(), which will call
     // startAnimation() again to keep the animation moving.
@@ -581,18 +587,18 @@ void BitmapImage::advanceAnimation(TimerBase*)
 
 void BitmapImage::advanceAnimationWithoutCatchUp(TimerBase*)
 {
-    if (internalAdvanceAnimation(false))
+    if (internalAdvanceAnimation())
         startAnimation(DoNotCatchUp);
 }
 
-bool BitmapImage::internalAdvanceAnimation(bool skippingFrames)
+bool BitmapImage::internalAdvanceAnimation(AnimationAdvancement advancement)
 {
     // Stop the animation.
     stopAnimation();
 
     // See if anyone is still paying attention to this animation.  If not, we don't
     // advance and will remain suspended at the current frame until the animation is resumed.
-    if (!skippingFrames && getImageObserver()->shouldPauseAnimation(this))
+    if (advancement != SkipFramesToCatchUp && getImageObserver()->shouldPauseAnimation(this))
         return false;
 
     ++m_currentFrame;
@@ -615,12 +621,16 @@ bool BitmapImage::internalAdvanceAnimation(bool skippingFrames)
             m_currentFrame = 0;
     }
 
-    // We need to draw this frame if we advanced to it while not skipping, or if
-    // while trying to skip frames we hit the last frame and thus had to stop.
-    if (skippingFrames != advancedAnimation)
+    // We need to draw this frame if we advanced to it while not skipping.
+    if (advancement != SkipFramesToCatchUp && advancedAnimation)

[pdr., 2016/08/22 18:27:08]

For ease of review:
This line is the primary change in this patch. Instead of synchronously calling
animationAdvanced here, we defer it.

I found WebKit actually made the same change over a year ago:
https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/Bitmap...,
https://trac.webkit.org/changeset/183732

This change isn't a regression from
https://crrev.com/45eef1a2a0f721e6782c4c92b28de3835f94ec46, we just put a
bugtrap in that patch and ended up catching something real.

         getImageObserver()->animationAdvanced(this);
 

[chrishtr, 2016/08/22 22:55:51]

Put lines 528-529 down here?

     return advancedAnimation;
 }
 
+void BitmapImage::notifyObserversOfAnimationAdvance(TimerBase*)
+{
+    getImageObserver()->animationAdvanced(this);
+}
+
 } // namespace blink