[Web Animations] Separate setting a timeline's current time from updating its animations
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Jan 2020 18:59:42 +0000 (18:59 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Jan 2020 18:59:42 +0000 (18:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=206880

Reviewed by Dean Jackson.

While we must always update the current time of all timelines if a new animation frame has been requested,
regardless of the reason (rAF callback, animation servicing, etc.), we should only update timelines' animations
if at least one timeline has requested an update. We used to decide this at the DocumentTimeline level, but
this needs to be coordinated at the Document level to consider all timelines at once.

This is required for an upcoming patch where we make changes to the way we schedule animations to correctly
support mixed accelerated and non-accelerated properties.

No new tests since this shouldn't yield any visible behavior change.

* animation/DocumentTimeline.cpp:
(WebCore::DocumentTimeline::updateCurrentTime):
(WebCore::DocumentTimeline::updateAnimationsAndSendEvents):
* animation/DocumentTimeline.h:
* dom/Document.cpp:
(WebCore::Document::updateAnimationsAndSendEvents):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@255260 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/animation/DocumentTimeline.cpp
Source/WebCore/animation/DocumentTimeline.h
Source/WebCore/dom/Document.cpp

index f065e01..002d562 100644 (file)
@@ -1,3 +1,27 @@
+2020-01-28  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Separate setting a timeline's current time from updating its animations
+        https://bugs.webkit.org/show_bug.cgi?id=206880
+
+        Reviewed by Dean Jackson.
+
+        While we must always update the current time of all timelines if a new animation frame has been requested,
+        regardless of the reason (rAF callback, animation servicing, etc.), we should only update timelines' animations
+        if at least one timeline has requested an update. We used to decide this at the DocumentTimeline level, but
+        this needs to be coordinated at the Document level to consider all timelines at once.
+
+        This is required for an upcoming patch where we make changes to the way we schedule animations to correctly
+        support mixed accelerated and non-accelerated properties.
+
+        No new tests since this shouldn't yield any visible behavior change.
+
+        * animation/DocumentTimeline.cpp:
+        (WebCore::DocumentTimeline::updateCurrentTime):
+        (WebCore::DocumentTimeline::updateAnimationsAndSendEvents):
+        * animation/DocumentTimeline.h:
+        * dom/Document.cpp:
+        (WebCore::Document::updateAnimationsAndSendEvents):
+
 2020-01-28  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][IFC] Decouple Display::LineBox and Layout::LineBoxBuilder
index 6be4e64..d417b89 100644 (file)
@@ -335,15 +335,18 @@ bool DocumentTimeline::shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionS
     return !m_animations.isEmpty() || !m_acceleratedAnimationsPendingRunningStateChange.isEmpty();
 }
 
-void DocumentTimeline::updateAnimationsAndSendEvents(DOMHighResTimeStamp timestamp)
+void DocumentTimeline::updateCurrentTime(DOMHighResTimeStamp timestamp)
 {
     // We need to freeze the current time even if no animation is running.
     // document.timeline.currentTime may be called from a rAF callback and
     // it has to match the rAF timestamp.
     if (!m_isSuspended)
         cacheCurrentTime(timestamp);
+}
 
-    if (m_isSuspended || !m_animationResolutionScheduled || !shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState())
+void DocumentTimeline::updateAnimationsAndSendEvents()
+{
+    if (m_isSuspended || !shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState())
         return;
 
     // Updating animations and sending events may invalidate the timing of some animations, so we must set the m_animationResolutionScheduled
index 2bb749e..c602c92 100644 (file)
@@ -72,7 +72,9 @@ public:
 
     void enqueueAnimationPlaybackEvent(AnimationPlaybackEvent&);
     
-    void updateAnimationsAndSendEvents(DOMHighResTimeStamp timestamp);
+    bool scheduledUpdate() const { return m_animationResolutionScheduled; }
+    void updateCurrentTime(DOMHighResTimeStamp timestamp);
+    void updateAnimationsAndSendEvents();
 
     WEBCORE_EXPORT Seconds animationInterval() const;
     WEBCORE_EXPORT void suspendAnimations();
index b4da24e..795762c 100644 (file)
@@ -6353,12 +6353,21 @@ void Document::resumeScriptedAnimationControllerCallbacks()
 void Document::updateAnimationsAndSendEvents(DOMHighResTimeStamp timestamp)
 {
     ASSERT(!m_timelines.hasNullReferences());
+
     // We need to copy m_timelines before iterating over its members since calling updateAnimationsAndSendEvents() may mutate m_timelines.
     Vector<RefPtr<DocumentTimeline>> timelines;
-    for (auto& timeline : m_timelines)
+    bool shouldUpdateAnimations = false;
+    for (auto& timeline : m_timelines) {
+        if (!shouldUpdateAnimations && timeline.scheduledUpdate())
+            shouldUpdateAnimations = true;
         timelines.append(&timeline);
-    for (auto& timeline : timelines)
-        timeline->updateAnimationsAndSendEvents(timestamp);
+    }
+
+    for (auto& timeline : timelines) {
+        timeline->updateCurrentTime(timestamp);
+        if (shouldUpdateAnimations)
+            timeline->updateAnimationsAndSendEvents();
+    }
 }
 
 void Document::serviceRequestAnimationFrameCallbacks(DOMHighResTimeStamp timestamp)