Prevent synchronous layout while painting the last frame of a gif

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

 /*
  * Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
  * Copyright (C) 2004, 2005, 2006, 2008 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 503 matching lines @@
         // See if we've also passed the time for frames after that to start, in
         // case we need to skip some frames entirely.  Remember not to advance
         // to an incomplete frame.
         for (size_t frameAfterNext = (nextFrame + 1) % frameCount(); frameIsCompleteAtIndex(frameAfterNext); frameAfterNext = (nextFrame + 1) % frameCount()) {
             // Should we skip the next frame?
             double frameAfterNextStartTime = m_desiredFrameStartTime + frameDurationAtIndex(nextFrame);
             if (time < frameAfterNextStartTime)
                 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;
         }
 
         // Post a task to advance the frame immediately. m_desiredFrameStartTime
         // may be in the past, meaning the next time through this function we'll
         // kick off the next advancement sooner than this frame's duration would
         // suggest.
         m_frameTimer = wrapUnique(new Timer<BitmapImage>(this, &BitmapImage::advanceAnimationWithoutCatchUp));
         m_frameTimer->startOneShot(0, BLINK_FROM_HERE);
@@ skipping 30 matching lines @@
 void BitmapImage::advanceTime(double deltaTimeInSeconds)
 {
     if (m_desiredFrameStartTime)
         m_desiredFrameStartTime -= deltaTimeInSeconds;
     else
         m_desiredFrameStartTime = monotonicallyIncreasingTime() - 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.
 }
 
 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;
     bool advancedAnimation = true;
     if (m_currentFrame >= frameCount()) {
         ++m_repetitionsComplete;
 
         // Get the repetition count again.  If we weren't able to get a
         // repetition count before, we should have decoded the whole image by
         // now, so it should now be available.
         // Note that we don't need to special-case cAnimationLoopOnce here
         // because it is 0 (see comments on its declaration in ImageAnimation.h).
         if ((repetitionCount(true) != cAnimationLoopInfinite && m_repetitionsComplete > m_repetitionCount)
             || (m_animationPolicy == ImageAnimationPolicyAnimateOnce && m_repetitionsComplete > 0)) {
             m_animationFinished = true;
             m_desiredFrameStartTime = 0;
             --m_currentFrame;
             advancedAnimation = false;
         } else
             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