Rework timeline/frame scheduling logic for SVGImage

third_party/WebKit/Source/core/svg/graphics/SVGImageChromeClient.cpp

Index: third_party/WebKit/Source/core/svg/graphics/SVGImageChromeClient.cpp
diff --git a/third_party/WebKit/Source/core/svg/graphics/SVGImageChromeClient.cpp b/third_party/WebKit/Source/core/svg/graphics/SVGImageChromeClient.cpp
index 808438d18b3f2b60fae56ea0b2412429c4fdf910..fcf11b51487917198b4017fe1abe183b71b1012e 100644
--- a/third_party/WebKit/Source/core/svg/graphics/SVGImageChromeClient.cpp
+++ b/third_party/WebKit/Source/core/svg/graphics/SVGImageChromeClient.cpp
@@ -28,9 +28,7 @@
 
 #include "core/svg/graphics/SVGImageChromeClient.h"
 
-#include "core/frame/FrameView.h"
 #include "core/svg/graphics/SVGImage.h"
-#include "platform/ScriptForbiddenScope.h"
 #include "platform/graphics/ImageObserver.h"
 #include "wtf/CurrentTime.h"
 
@@ -41,6 +39,7 @@ static const double animationFrameDelay = 0.025;
 SVGImageChromeClient::SVGImageChromeClient(SVGImage* image)
     : m_image(image)
     , m_animationTimer(this, &SVGImageChromeClient::animationTimerFired)
+    , m_timelineState(Running)
 {
 }
 
@@ -66,6 +65,29 @@ void SVGImageChromeClient::invalidateRect(const IntRect& r)
         m_image->getImageObserver()->changedInRect(m_image, r);
 }
 
+void SVGImageChromeClient::suspendAnimation()
+{
+    // Preserve SuspendedWithFramePending if set.
+    m_timelineState = std::max(m_timelineState, Suspended);
+}
+
+void SVGImageChromeClient::resumeAnimation()
+{
+    bool havePendingFrame = m_timelineState == SuspendedWithFramePending;
+    m_timelineState = Running;
+
+    // If an animation frame was requested while animations were blocked, kick
+    // off an animation timer right away.
+    if (!havePendingFrame)
+        return;
+    // In the unlikely event that we suspend and resume within a single frame,
+    // the timer could still be running, so just leave it at the current
+    // timeout.
+    if (m_animationTimer.isActive())

[chrishtr, 2016/05/20 18:26:15]

Why not just call scheduleAnimation() directly? The Widget parameter is unused
anyway.

[fs, 2016/05/20 19:23:58]

That would schedule a timer with a delay of |animationFrameDelay| rather than 0.

[chrishtr, 2016/05/20 19:55:18]

Does that matter?

[fs, 2016/05/20 20:03:06]

I guess we can try it and see.

[chrishtr, 2016/05/23 20:02:51]

I suppose it doesn't matter that much. Only a few lines of code.

[fs, 2016/05/23 20:58:34]

I've shuffled this around a bit now, and this is now calling scheduleAnimation
directly.

+        return;
+    m_animationTimer.startOneShot(0, BLINK_FROM_HERE);
+}
+
 void SVGImageChromeClient::scheduleAnimation(Widget*)
 {
     // Because a single SVGImage can be shared by multiple pages, we can't key
@@ -79,7 +101,14 @@ void SVGImageChromeClient::scheduleAnimation(Widget*)
     // animations, but prefer a fixed, jittery, frame-delay if there're any
     // animations. Checking for pending/active animations could be more
     // stringent.
-    double fireTime = m_image->hasAnimations() ? animationFrameDelay : 0;
+    double fireTime = 0;
+    if (m_image->hasAnimations()) {
+        if (m_timelineState >= Suspended) {
+            m_timelineState = SuspendedWithFramePending;
+            return;
+        }
+        fireTime = animationFrameDelay;
+    }
     m_animationTimer.startOneShot(fireTime, BLINK_FROM_HERE);
 }
 
@@ -97,16 +126,7 @@ void SVGImageChromeClient::animationTimerFired(Timer<SVGImageChromeClient>*)
     if (ThreadHeap::willObjectBeLazilySwept(m_image->getImageObserver()))
         return;
 
-    // serviceScriptedAnimations runs requestAnimationFrame callbacks, but SVG
-    // images can't have any so we assert there's no script.
-    ScriptForbiddenScope forbidScript;
-
-    // The calls below may trigger GCs, so set up the required persistent
-    // reference on the ImageResource which owns this SVGImage. By transitivity,
-    // that will keep this SVGImageChromeClient object alive.
-    Persistent<ImageObserver> protect(m_image->getImageObserver());
-    m_image->frameView()->page()->animator().serviceScriptedAnimations(monotonicallyIncreasingTime());
-    m_image->frameView()->updateAllLifecyclePhases();
+    m_image->serviceAnimations(monotonicallyIncreasingTime());
 }
 
 } // namespace blink