- Revision
- 256619
- Author
- grao...@webkit.org
- Date
- 2020-02-14 09:52:07 -0800 (Fri, 14 Feb 2020)
Log Message
[Web Animations] Ensure CSS Transition and CSS Animation events are queued, sorted and dispatched by their timeline
https://bugs.webkit.org/show_bug.cgi?id=207364
<rdar://problem/59370413>
Reviewed by Simon Fraser.
LayoutTests/imported/w3c:
There are some progressions but also some "regressions". The progressions are real, showing the delivery of all animation events at the correct
time. However, the regressions are misleading. The fact that the "style change" tests would work was due to a design issue in the test which would
only wait one frame to detect whether a CSS Transition was started after a change made through the Web Animations API. These would work because
events were queued in the next frame, but delivered later due to the dedicated per-animation queue used, which meant the test was fooled into
thinking the CSS Transition did not start, as expected. Changing those test to use more than one frame to test for the lack of a CSS Transition
would have shown the FAIL results.
However, in order to not regress our WPT score, the issue of "style change" events will be addressed in a follow-up patch.
* web-platform-tests/css/css-transitions/CSSTransition-startTime.tentative-expected.txt:
* web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt:
* web-platform-tests/web-animations/interfaces/Animation/style-change-events-expected.txt:
* web-platform-tests/web-animations/interfaces/KeyframeEffect/style-change-events-expected.txt:
* web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-expected.txt:
Source/WebCore:
Until now, AnimationPlaybackEvent events, which are new events introduced by the Web Animations spec, were enqueued in a shared queue on the DocumentTimeline
and dispatched during the "update animations and send events" procedure. However, AnimationEvent and TransitionEvent events, dispatched by CSS Animations
and CSS Transitions, were dispatched via a dedicated per-animation queue, which meant typically that those events were dispathed one runloop after the
AnimationPlaybackEvent events.
We now remove the dedicated per-animation queue and enqueue all events in the shared DocumentTimeline queue for dispatch during the "update animations and send
events" procedure. To do this correctly, we need to do a couple of other things that ensure we don't regress tests.
First, we update the DocumentTimeline::shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState() to account for whether there are pending animation events,
guaranteeing that an animation update is scheduled should there be any.
Second, when animation events are enqueued in DocumentTimeline::enqueueAnimationEvent() we schedule an animation update if needed, since we know we now
have pending events that will need to be delivered in an upcoming update. We also maintain a flag between the start of the "update animations and send events"
procedure and the moment when the pending animation events queue is cleared prior to dispatching events so that events enqueued in the meantime do not
prematurely schedule animation resolution. The need for a new animation resolution will be checked at the end of the procedure.
Finally, declarative animations used to have a special suclass of WebAnimation::needsTick() that would check whether they had any pending events, ensuring
they would not be removed prematurely. We now reset a flag to false as WebAnimation::tick() is called (as part of the "update animations and send events"
procedure) and set it to true in case an animation is enqueued. This flag is then used in needsTick() to guarantee the animation is not removed before
the DocumentTimeline has had a chance to dispatch the enqueued event.
Note also that, for clarity, the DocumentTimeline::unscheduleAnimationResolution() was renamed to DocumentTimeline::clearTickScheduleTimer() since it wouldn't
actually cancel a previous animation resolution schedule.
* animation/CSSTransition.h: Fix a newly found build error due to the missing wtf/MonotonicTime.h header.
* animation/DeclarativeAnimation.cpp: Remove all code related to the dedicated per-animation queue and instead call the new WebAnimation::enqueueAnimationEvent()
method to enqueue events on the DocumentTimeline.
(WebCore::DeclarativeAnimation::DeclarativeAnimation):
(WebCore::DeclarativeAnimation::tick):
(WebCore::DeclarativeAnimation::enqueueDOMEvent):
* animation/DeclarativeAnimation.h:
* animation/DocumentTimeline.cpp:
(WebCore::DocumentTimeline::detachFromDocument): Ensure the pending events queue is cleared when the timeline is detached from a document, ensuring that there no
longer events that would cause a ref-cycle (DocumentTimeline -> AnimationPlaybackEvent -> WebAnimation -> DocumentTimeline).
(WebCore::DocumentTimeline::suspendAnimations):
(WebCore::DocumentTimeline::removeAnimation):
(WebCore::DocumentTimeline::scheduleAnimationResolution):
(WebCore::DocumentTimeline::clearTickScheduleTimer):
(WebCore::DocumentTimeline::shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState const):
(WebCore::DocumentTimeline::updateCurrentTime):
(WebCore::DocumentTimeline::updateAnimationsAndSendEvents):
(WebCore::DocumentTimeline::internalUpdateAnimationsAndSendEvents):
(WebCore::DocumentTimeline::scheduleNextTick):
(WebCore::DocumentTimeline::animationAcceleratedRunningStateDidChange):
(WebCore::DocumentTimeline::enqueueAnimationEvent):
* animation/DocumentTimeline.h:
* animation/WebAnimation.cpp:
(WebCore::WebAnimation::enqueueAnimationPlaybackEvent):
(WebCore::WebAnimation::enqueueAnimationEvent):
(WebCore::WebAnimation::needsTick const):
(WebCore::WebAnimation::tick):
* animation/WebAnimation.h:
LayoutTests:
Fix a couple of tests that made some incorrect assumptions.
* TestExpectations: imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events.html is no longer flaky.
* compositing/backing/animate-into-view.html: Because the "animationstart" event is now dispatched during the "update animations and send events" procedure, which happens
during page rendering _before_ rAF callbacks are serviced, we must remove the rAF callback used prior to adding the "animationstart" event listener or else we would never
get it and the test would time out.
* webanimations/css-transition-in-flight-reversal-accelerated.html: We must wait for the initial transition to start and then two frames before reversing the transition,
to be certain that the animation did start. Indeed, the "transitionstart" event will be fired right before the next rAF callback is called, as the animation starts in that
very same frame, and so progress will be 0 and the transition wouldn't be reversable until the next frame when the animation has progress > 0.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (256618 => 256619)
--- trunk/LayoutTests/ChangeLog 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/LayoutTests/ChangeLog 2020-02-14 17:52:07 UTC (rev 256619)
@@ -1,3 +1,21 @@
+2020-02-14 Antoine Quint <grao...@webkit.org>
+
+ [Web Animations] Ensure CSS Transition and CSS Animation events are queued, sorted and dispatched by their timeline
+ https://bugs.webkit.org/show_bug.cgi?id=207364
+ <rdar://problem/59370413>
+
+ Reviewed by Simon Fraser.
+
+ Fix a couple of tests that made some incorrect assumptions.
+
+ * TestExpectations: imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events.html is no longer flaky.
+ * compositing/backing/animate-into-view.html: Because the "animationstart" event is now dispatched during the "update animations and send events" procedure, which happens
+ during page rendering _before_ rAF callbacks are serviced, we must remove the rAF callback used prior to adding the "animationstart" event listener or else we would never
+ get it and the test would time out.
+ * webanimations/css-transition-in-flight-reversal-accelerated.html: We must wait for the initial transition to start and then two frames before reversing the transition,
+ to be certain that the animation did start. Indeed, the "transitionstart" event will be fired right before the next rAF callback is called, as the animation starts in that
+ very same frame, and so progress will be 0 and the transition wouldn't be reversable until the next frame when the animation has progress > 0.
+
2020-02-14 Jacob Uphoff <jacob_uph...@apple.com>
N[ iOS ] imported/w3c/IndexedDB-private-browsing/idbcursor_advance_index3.html is flaky timing out
Modified: trunk/LayoutTests/TestExpectations (256618 => 256619)
--- trunk/LayoutTests/TestExpectations 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/LayoutTests/TestExpectations 2020-02-14 17:52:07 UTC (rev 256619)
@@ -2656,7 +2656,6 @@
webkit.org/b/202107 imported/w3c/web-platform-tests/web-animations/interfaces/Animation/style-change-events.html [ Pass Failure ]
webkit.org/b/202108 imported/w3c/web-platform-tests/web-animations/interfaces/DocumentTimeline/style-change-events.html [ Pass Failure ]
-webkit.org/b/202109 imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events.html [ Pass Failure ]
webkit.org/b/157068 [ Debug ] imported/w3c/web-platform-tests/fetch/nosniff/importscripts.html [ Pass Crash ]
webkit.org/b/157068 [ Release ] imported/w3c/web-platform-tests/fetch/nosniff/importscripts.html [ Pass Failure ]
Modified: trunk/LayoutTests/compositing/backing/animate-into-view.html (256618 => 256619)
--- trunk/LayoutTests/compositing/backing/animate-into-view.html 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/LayoutTests/compositing/backing/animate-into-view.html 2020-02-14 17:52:07 UTC (rev 256619)
@@ -60,17 +60,15 @@
}
window.addEventListener('load', () => {
- requestAnimationFrame(() => {
- let animator = document.getElementById('target');
- animator.addEventListener('animationstart', () => {
- requestAnimationFrame(() => {
- dumpLayers();
- if (window.testRunner)
- testRunner.notifyDone();
- });
+ let animator = document.getElementById('target');
+ animator.addEventListener('animationstart', () => {
+ requestAnimationFrame(() => {
+ dumpLayers();
+ if (window.testRunner)
+ testRunner.notifyDone();
});
- animator.classList.add('animating');
});
+ animator.classList.add('animating');
}, false);
</script>
</head>
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (256618 => 256619)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2020-02-14 17:52:07 UTC (rev 256619)
@@ -1,3 +1,26 @@
+2020-02-14 Antoine Quint <grao...@webkit.org>
+
+ [Web Animations] Ensure CSS Transition and CSS Animation events are queued, sorted and dispatched by their timeline
+ https://bugs.webkit.org/show_bug.cgi?id=207364
+ <rdar://problem/59370413>
+
+ Reviewed by Simon Fraser.
+
+ There are some progressions but also some "regressions". The progressions are real, showing the delivery of all animation events at the correct
+ time. However, the regressions are misleading. The fact that the "style change" tests would work was due to a design issue in the test which would
+ only wait one frame to detect whether a CSS Transition was started after a change made through the Web Animations API. These would work because
+ events were queued in the next frame, but delivered later due to the dedicated per-animation queue used, which meant the test was fooled into
+ thinking the CSS Transition did not start, as expected. Changing those test to use more than one frame to test for the lack of a CSS Transition
+ would have shown the FAIL results.
+
+ However, in order to not regress our WPT score, the issue of "style change" events will be addressed in a follow-up patch.
+
+ * web-platform-tests/css/css-transitions/CSSTransition-startTime.tentative-expected.txt:
+ * web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt:
+ * web-platform-tests/web-animations/interfaces/Animation/style-change-events-expected.txt:
+ * web-platform-tests/web-animations/interfaces/KeyframeEffect/style-change-events-expected.txt:
+ * web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-expected.txt:
+
2020-02-12 Rossana Monteriso <rmonter...@igalia.com>
[css-grid] Move grid-item-auto-margins-alignment tests to WPT folder
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-transitions/CSSTransition-startTime.tentative-expected.txt (256618 => 256619)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-transitions/CSSTransition-startTime.tentative-expected.txt 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-transitions/CSSTransition-startTime.tentative-expected.txt 2020-02-14 17:52:07 UTC (rev 256619)
@@ -3,5 +3,5 @@
PASS The start time of transitions is based on when they are generated
PASS The start time of a transition can be set
PASS The start time can be set to seek a transition
-FAIL Seeking a transition using start time dispatches transition events promise_test: Unhandled rejection with value: object "Error: Timed out waiting for Promise to resolve: transitionstart"
+PASS Seeking a transition using start time dispatches transition events
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt (256618 => 256619)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animatable/animate-expected.txt 2020-02-14 17:52:07 UTC (rev 256619)
@@ -132,7 +132,7 @@
PASS Element.animate() correctly sets the Animation's timeline
PASS Element.animate() correctly sets the Animation's timeline when triggered on an element in a different document
PASS Element.animate() calls play on the Animation
-PASS Element.animate() does NOT trigger a style change event
+FAIL Element.animate() does NOT trigger a style change event assert_false: A transition should NOT have been triggered expected false got true
PASS animate() with pseudoElement parameter creates an Animation object
PASS animate() with pseudoElement parameter without content creates an Animation object
PASS animate() with pseudoElement parameter creates an Animation object for ::marker
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animation/style-change-events-expected.txt (256618 => 256619)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animation/style-change-events-expected.txt 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animation/style-change-events-expected.txt 2020-02-14 17:52:07 UTC (rev 256619)
@@ -20,7 +20,7 @@
FAIL Animation.pause produces expected style change events assert_false: A transition should NOT have been triggered expected false got true
FAIL Animation.updatePlaybackRate produces expected style change events assert_false: A transition should NOT have been triggered expected false got true
FAIL Animation.reverse produces expected style change events assert_false: A transition should NOT have been triggered expected false got true
-PASS Animation.persist produces expected style change events
+FAIL Animation.persist produces expected style change events assert_false: A transition should NOT have been triggered expected false got true
FAIL Animation.commitStyles produces expected style change events assert_true: A transition should have been triggered expected true got false
FAIL Animation.Animation constructor produces expected style change events assert_false: A transition should NOT have been triggered expected false got true
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/style-change-events-expected.txt (256618 => 256619)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/style-change-events-expected.txt 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/KeyframeEffect/style-change-events-expected.txt 2020-02-14 17:52:07 UTC (rev 256619)
@@ -1,13 +1,13 @@
FAIL All property keys are recognized assert_in_array: Test property 'pseudoElement' should be one of the properties on KeyframeEffect value "pseudoElement" not in array ["getTiming", "getComputedTiming", "updateTiming", "target", "iterationComposite", "composite", "getKeyframes", "setKeyframes", "KeyframeEffect constructor", "KeyframeEffect copy constructor"]
-PASS KeyframeEffect.getTiming does NOT trigger a style change event
-PASS KeyframeEffect.getComputedTiming does NOT trigger a style change event
-PASS KeyframeEffect.updateTiming does NOT trigger a style change event
-PASS KeyframeEffect.target does NOT trigger a style change event
-PASS KeyframeEffect.iterationComposite does NOT trigger a style change event
-PASS KeyframeEffect.composite does NOT trigger a style change event
-PASS KeyframeEffect.getKeyframes does NOT trigger a style change event
-PASS KeyframeEffect.setKeyframes does NOT trigger a style change event
-PASS KeyframeEffect.KeyframeEffect constructor does NOT trigger a style change event
-PASS KeyframeEffect.KeyframeEffect copy constructor does NOT trigger a style change event
+FAIL KeyframeEffect.getTiming does NOT trigger a style change event assert_false: A transition should NOT have been triggered expected false got true
+FAIL KeyframeEffect.getComputedTiming does NOT trigger a style change event assert_false: A transition should NOT have been triggered expected false got true
+FAIL KeyframeEffect.updateTiming does NOT trigger a style change event assert_false: A transition should NOT have been triggered expected false got true
+FAIL KeyframeEffect.target does NOT trigger a style change event assert_false: A transition should NOT have been triggered expected false got true
+FAIL KeyframeEffect.iterationComposite does NOT trigger a style change event assert_false: A transition should NOT have been triggered expected false got true
+FAIL KeyframeEffect.composite does NOT trigger a style change event assert_false: A transition should NOT have been triggered expected false got true
+FAIL KeyframeEffect.getKeyframes does NOT trigger a style change event assert_false: A transition should NOT have been triggered expected false got true
+FAIL KeyframeEffect.setKeyframes does NOT trigger a style change event assert_false: A transition should NOT have been triggered expected false got true
+FAIL KeyframeEffect.KeyframeEffect constructor does NOT trigger a style change event assert_false: A transition should NOT have been triggered expected false got true
+FAIL KeyframeEffect.KeyframeEffect copy constructor does NOT trigger a style change event assert_false: A transition should NOT have been triggered expected false got true
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-expected.txt (256618 => 256619)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-expected.txt 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-expected.txt 2020-02-14 17:52:07 UTC (rev 256619)
@@ -2,9 +2,9 @@
PASS Fires cancel event before requestAnimationFrame
PASS Fires finish event before requestAnimationFrame
FAIL Sorts finish events by composite order assert_array_equals: finish events for various animation type should be sorted by composite order property 0, expected "CSSTransition:finish" but got "ScriptAnimation:finish"
-FAIL Sorts cancel events by composite order assert_array_equals: cancel events should be sorted by composite order lengths differ, expected 5 got 3
-FAIL Queues a cancel event in transitionstart event callback assert_approx_equals: A rAF callback should happen in the same frame expected 112 +/- 0.001 but got 96
-FAIL Sorts events for the same transition assert_array_equals: Playback and CSS events for the same transition should be sorted by schedule event time and composite order lengths differ, expected 2 got 1
+FAIL Sorts cancel events by composite order assert_array_equals: cancel events should be sorted by composite order property 0, expected "CSSTransition:cancel" but got "ScriptAnimation:cancel"
+PASS Queues a cancel event in transitionstart event callback
+PASS Sorts events for the same transition
PASS Playback events with the same timeline retain the order in which they arequeued
FAIL All timelines are updated before running microtasks promise_test: Unhandled rejection with value: object "AbortError: The operation was aborted."
Modified: trunk/LayoutTests/webanimations/css-transition-in-flight-reversal-accelerated.html (256618 => 256619)
--- trunk/LayoutTests/webanimations/css-transition-in-flight-reversal-accelerated.html 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/LayoutTests/webanimations/css-transition-in-flight-reversal-accelerated.html 2020-02-14 17:52:07 UTC (rev 256619)
@@ -46,19 +46,23 @@
initialTransition = animations[0];
assert_true(initialTransition instanceof CSSTransition, "There is one animation applied to the target after starting the initial transition.");
- // Wait for the initial transition to start and then another frame before reversing the transition.
+ // Wait for the initial transition to start and then two frames before reversing the transition, to be certain that the animation did start.
+ // Indeed, the "transitionstart" event will be fired right before the next rAF callback is called, as the animation starts in that very same
+ // frame, and so progress will be 0 and the transition wouldn't be reversable until the next frame when the animation has progress > 0.
target.addEventListener("transitionstart", event => {
requestAnimationFrame(() => {
- target.classList.remove("in-flight");
- const animations = target.getAnimations();
- assert_equals(animations.length, 1, "There is one animation applied to the target after reversing the initial transition.");
+ requestAnimationFrame(() => {
+ target.classList.remove("in-flight");
+ const animations = target.getAnimations();
+ assert_equals(animations.length, 1, "There is one animation applied to the target after reversing the initial transition.");
- reversedTransition = animations[0];
- assert_true(reversedTransition instanceof CSSTransition, "There is one animation applied to the target after reversing the initial transition.");
- assert_not_equals(initialTransition, reversedTransition, "The animation applied to the target after reversing the initial transition is different than the original transition.");
+ reversedTransition = animations[0];
+ assert_true(reversedTransition instanceof CSSTransition, "There is one animation applied to the target after reversing the initial transition.");
+ assert_not_equals(initialTransition, reversedTransition, "The animation applied to the target after reversing the initial transition is different than the original transition.");
- target.remove();
- test.done();
+ target.remove();
+ test.done();
+ });
});
});
});
Modified: trunk/Source/WebCore/ChangeLog (256618 => 256619)
--- trunk/Source/WebCore/ChangeLog 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/Source/WebCore/ChangeLog 2020-02-14 17:52:07 UTC (rev 256619)
@@ -1,5 +1,66 @@
2020-02-14 Antoine Quint <grao...@webkit.org>
+ [Web Animations] Ensure CSS Transition and CSS Animation events are queued, sorted and dispatched by their timeline
+ https://bugs.webkit.org/show_bug.cgi?id=207364
+ <rdar://problem/59370413>
+
+ Reviewed by Simon Fraser.
+
+ Until now, AnimationPlaybackEvent events, which are new events introduced by the Web Animations spec, were enqueued in a shared queue on the DocumentTimeline
+ and dispatched during the "update animations and send events" procedure. However, AnimationEvent and TransitionEvent events, dispatched by CSS Animations
+ and CSS Transitions, were dispatched via a dedicated per-animation queue, which meant typically that those events were dispathed one runloop after the
+ AnimationPlaybackEvent events.
+
+ We now remove the dedicated per-animation queue and enqueue all events in the shared DocumentTimeline queue for dispatch during the "update animations and send
+ events" procedure. To do this correctly, we need to do a couple of other things that ensure we don't regress tests.
+
+ First, we update the DocumentTimeline::shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState() to account for whether there are pending animation events,
+ guaranteeing that an animation update is scheduled should there be any.
+
+ Second, when animation events are enqueued in DocumentTimeline::enqueueAnimationEvent() we schedule an animation update if needed, since we know we now
+ have pending events that will need to be delivered in an upcoming update. We also maintain a flag between the start of the "update animations and send events"
+ procedure and the moment when the pending animation events queue is cleared prior to dispatching events so that events enqueued in the meantime do not
+ prematurely schedule animation resolution. The need for a new animation resolution will be checked at the end of the procedure.
+
+ Finally, declarative animations used to have a special suclass of WebAnimation::needsTick() that would check whether they had any pending events, ensuring
+ they would not be removed prematurely. We now reset a flag to false as WebAnimation::tick() is called (as part of the "update animations and send events"
+ procedure) and set it to true in case an animation is enqueued. This flag is then used in needsTick() to guarantee the animation is not removed before
+ the DocumentTimeline has had a chance to dispatch the enqueued event.
+
+ Note also that, for clarity, the DocumentTimeline::unscheduleAnimationResolution() was renamed to DocumentTimeline::clearTickScheduleTimer() since it wouldn't
+ actually cancel a previous animation resolution schedule.
+
+ * animation/CSSTransition.h: Fix a newly found build error due to the missing wtf/MonotonicTime.h header.
+ * animation/DeclarativeAnimation.cpp: Remove all code related to the dedicated per-animation queue and instead call the new WebAnimation::enqueueAnimationEvent()
+ method to enqueue events on the DocumentTimeline.
+ (WebCore::DeclarativeAnimation::DeclarativeAnimation):
+ (WebCore::DeclarativeAnimation::tick):
+ (WebCore::DeclarativeAnimation::enqueueDOMEvent):
+ * animation/DeclarativeAnimation.h:
+ * animation/DocumentTimeline.cpp:
+ (WebCore::DocumentTimeline::detachFromDocument): Ensure the pending events queue is cleared when the timeline is detached from a document, ensuring that there no
+ longer events that would cause a ref-cycle (DocumentTimeline -> AnimationPlaybackEvent -> WebAnimation -> DocumentTimeline).
+ (WebCore::DocumentTimeline::suspendAnimations):
+ (WebCore::DocumentTimeline::removeAnimation):
+ (WebCore::DocumentTimeline::scheduleAnimationResolution):
+ (WebCore::DocumentTimeline::clearTickScheduleTimer):
+ (WebCore::DocumentTimeline::shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState const):
+ (WebCore::DocumentTimeline::updateCurrentTime):
+ (WebCore::DocumentTimeline::updateAnimationsAndSendEvents):
+ (WebCore::DocumentTimeline::internalUpdateAnimationsAndSendEvents):
+ (WebCore::DocumentTimeline::scheduleNextTick):
+ (WebCore::DocumentTimeline::animationAcceleratedRunningStateDidChange):
+ (WebCore::DocumentTimeline::enqueueAnimationEvent):
+ * animation/DocumentTimeline.h:
+ * animation/WebAnimation.cpp:
+ (WebCore::WebAnimation::enqueueAnimationPlaybackEvent):
+ (WebCore::WebAnimation::enqueueAnimationEvent):
+ (WebCore::WebAnimation::needsTick const):
+ (WebCore::WebAnimation::tick):
+ * animation/WebAnimation.h:
+
+2020-02-14 Antoine Quint <grao...@webkit.org>
+
[Web Animations] Make all animation event types inherit from the same base class
https://bugs.webkit.org/show_bug.cgi?id=207629
@@ -51,6 +112,62 @@
* dom/TransitionEvent.cpp:
(WebCore::TransitionEvent::TransitionEvent):
* dom/TransitionEvent.h:
+ * page/animation/CSSAnimationController.cpp:
+ (WebCore::CSSAnimationControllerPrivate::fireEventsAndUpdateStyle):
+
+2020-02-14 Antoine Quint <grao...@webkit.org>
+
+ [Web Animations] Make all animation event types inherit from the same base class
+ https://bugs.webkit.org/show_bug.cgi?id=207629
+
+ Reviewed by Simon Fraser.
+
+ Currently we dispatch events CSS Transitions and CSS Animations events using a dedicated event queue on DeclarativeAnimation, while the events
+ added by the Web Animations specification (of type AnimationPlaybackEvent) are dispatched using a shared queue on the DocumentTimeline that is
+ processed during the "update animations and send events procedure". The Web Animations specification dictates that all events should be dispatched
+ during that procedure, which includes sorting of such events based on their timeline time and associated animation relative composite order.
+
+ In this patch, we prepare the work towards spec compliance for animation events dispatch by making all event types (AnimationPlaybackEvent,
+ TransitionEvent and AnimationEvent) inherit from a single AnimationEventBase interface. This will allow DocumentTimeline to enqueue, sort and
+ dispatch all such events with a single queue in a future patch.
+
+ Due to CSSAnimationController, we must make the "timeline time" and "animation" parameters optional. When we drop support for CSSAnimationController
+ we'll be able to enforce stronger requirements for these.
+
+ No new test since this should not introduce any behavior change.
+
+ * Sources.txt:
+ * WebCore.xcodeproj/project.pbxproj:
+ * animation/AnimationEventBase.cpp: Added.
+ (WebCore::AnimationEventBase::AnimationEventBase):
+ * animation/AnimationEventBase.h: Added.
+ (WebCore::AnimationEventBase::create):
+ (WebCore::AnimationEventBase::isAnimationPlaybackEvent const):
+ (WebCore::AnimationEventBase::isAnimationEvent const):
+ (WebCore::AnimationEventBase::isTransitionEvent const):
+ (WebCore::AnimationEventBase::timelineTime const):
+ (WebCore::AnimationEventBase::animation const):
+ * animation/AnimationPlaybackEvent.cpp:
+ (WebCore::AnimationPlaybackEvent::AnimationPlaybackEvent):
+ (WebCore::AnimationPlaybackEvent::bindingsTimelineTime const):
+ * animation/AnimationPlaybackEvent.h:
+ * animation/CSSAnimation.cpp:
+ (WebCore::CSSAnimation::createEvent):
+ * animation/CSSAnimation.h:
+ * animation/CSSTransition.cpp:
+ (WebCore::CSSTransition::createEvent):
+ * animation/CSSTransition.h:
+ * animation/DeclarativeAnimation.cpp:
+ (WebCore::DeclarativeAnimation::enqueueDOMEvent):
+ * animation/DeclarativeAnimation.h:
+ * animation/WebAnimation.cpp:
+ (WebCore::WebAnimation::enqueueAnimationPlaybackEvent):
+ * dom/AnimationEvent.cpp:
+ (WebCore::AnimationEvent::AnimationEvent):
+ * dom/AnimationEvent.h:
+ * dom/TransitionEvent.cpp:
+ (WebCore::TransitionEvent::TransitionEvent):
+ * dom/TransitionEvent.h:
* page/animation/CSSAnimationController.cpp:
(WebCore::CSSAnimationControllerPrivate::fireEventsAndUpdateStyle):
Modified: trunk/Source/WebCore/animation/CSSTransition.h (256618 => 256619)
--- trunk/Source/WebCore/animation/CSSTransition.h 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/Source/WebCore/animation/CSSTransition.h 2020-02-14 17:52:07 UTC (rev 256619)
@@ -27,6 +27,7 @@
#include "CSSPropertyNames.h"
#include "DeclarativeAnimation.h"
+#include <wtf/MonotonicTime.h>
#include <wtf/Ref.h>
namespace WebCore {
Modified: trunk/Source/WebCore/animation/DeclarativeAnimation.cpp (256618 => 256619)
--- trunk/Source/WebCore/animation/DeclarativeAnimation.cpp 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/Source/WebCore/animation/DeclarativeAnimation.cpp 2020-02-14 17:52:07 UTC (rev 256619)
@@ -42,7 +42,6 @@
DeclarativeAnimation::DeclarativeAnimation(Element& owningElement, const Animation& backingAnimation)
: WebAnimation(owningElement.document())
- , m_eventQueue(MainThreadGenericEventQueue::create(owningElement))
, m_owningElement(&owningElement)
, m_backingAnimation(const_cast<Animation&>(backingAnimation))
{
@@ -63,10 +62,8 @@
// canceled using the Web Animations API and it should be disassociated from its owner element.
// From this point on, this animation is like any other animation and should not appear in the
// maps containing running CSS Transitions and CSS Animations for a given element.
- if (wasRelevant && playState() == WebAnimation::PlayState::Idle) {
+ if (wasRelevant && playState() == WebAnimation::PlayState::Idle)
disassociateFromOwningElement();
- m_eventQueue->close();
- }
}
bool DeclarativeAnimation::canHaveGlobalPosition()
@@ -90,17 +87,6 @@
m_owningElement = nullptr;
}
-bool DeclarativeAnimation::needsTick() const
-{
- return WebAnimation::needsTick() || m_eventQueue->hasPendingEvents();
-}
-
-void DeclarativeAnimation::remove()
-{
- m_eventQueue->close();
- WebAnimation::remove();
-}
-
void DeclarativeAnimation::setBackingAnimation(const Animation& backingAnimation)
{
m_backingAnimation = const_cast<Animation&>(backingAnimation);
@@ -351,7 +337,9 @@
auto time = secondsToWebAnimationsAPITime(elapsedTime) / 1000;
const auto& pseudoId = PseudoElement::pseudoElementNameForEvents(m_owningElement->pseudoId());
auto timelineTime = timeline() ? timeline()->currentTime() : WTF::nullopt;
- m_eventQueue->enqueueEvent(createEvent(eventType, time, pseudoId, timelineTime));
+ auto event = createEvent(eventType, time, pseudoId, timelineTime);
+ event->setTarget(m_owningElement);
+ enqueueAnimationEvent(WTFMove(event));
}
} // namespace WebCore
Modified: trunk/Source/WebCore/animation/DeclarativeAnimation.h (256618 => 256619)
--- trunk/Source/WebCore/animation/DeclarativeAnimation.h 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/Source/WebCore/animation/DeclarativeAnimation.h 2020-02-14 17:52:07 UTC (rev 256619)
@@ -27,7 +27,6 @@
#include "AnimationEffect.h"
#include "AnimationEffectPhase.h"
-#include "GenericEventQueue.h"
#include "WebAnimation.h"
#include <wtf/Ref.h>
@@ -65,7 +64,6 @@
void setTimeline(RefPtr<AnimationTimeline>&&) final;
void cancel() final;
- bool needsTick() const override;
void tick() override;
bool canHaveGlobalPosition() final;
@@ -85,13 +83,10 @@
void flushPendingStyleChanges() const;
AnimationEffectPhase phaseWithoutEffect() const;
void enqueueDOMEvent(const AtomString&, Seconds);
- void remove() final;
bool m_wasPending { false };
AnimationEffectPhase m_previousPhase { AnimationEffectPhase::Idle };
- UniqueRef<MainThreadGenericEventQueue> m_eventQueue;
-
Element* m_owningElement;
Ref<Animation> m_backingAnimation;
double m_previousIteration;
Modified: trunk/Source/WebCore/animation/DocumentTimeline.cpp (256618 => 256619)
--- trunk/Source/WebCore/animation/DocumentTimeline.cpp 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/Source/WebCore/animation/DocumentTimeline.cpp 2020-02-14 17:52:07 UTC (rev 256619)
@@ -26,7 +26,7 @@
#include "config.h"
#include "DocumentTimeline.h"
-#include "AnimationPlaybackEvent.h"
+#include "AnimationEventBase.h"
#include "CSSAnimation.h"
#include "CSSTransition.h"
#include "DOMWindow.h"
@@ -88,6 +88,7 @@
if (m_document)
m_document->removeTimeline(*this);
+ m_pendingAnimationEvents.clear();
m_currentTimeClearingTaskQueue.close();
m_elementsWithRunningAcceleratedAnimations.clear();
@@ -95,7 +96,7 @@
while (!animationsToRemove.isEmpty())
animationsToRemove.first()->remove();
- unscheduleAnimationResolution();
+ clearTickScheduleTimer();
m_document = nullptr;
}
@@ -234,7 +235,7 @@
applyPendingAcceleratedAnimations();
- unscheduleAnimationResolution();
+ clearTickScheduleTimer();
}
void DocumentTimeline::resumeAnimations()
@@ -327,31 +328,31 @@
{
AnimationTimeline::removeAnimation(animation);
- if (!shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState())
- unscheduleAnimationResolution();
+ if (m_animations.isEmpty())
+ clearTickScheduleTimer();
}
void DocumentTimeline::scheduleAnimationResolution()
{
- if (m_isSuspended || m_animationResolutionScheduled || !shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState())
+ if (m_isSuspended || m_animationResolutionScheduled || !m_document || !m_document->page())
return;
- if (!m_document || !m_document->page())
+ // We need some relevant animations or pending events to proceed.
+ if (!shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState())
return;
-
+
m_document->page()->renderingUpdateScheduler().scheduleTimedRenderingUpdate();
m_animationResolutionScheduled = true;
}
-void DocumentTimeline::unscheduleAnimationResolution()
+void DocumentTimeline::clearTickScheduleTimer()
{
m_tickScheduleTimer.stop();
- m_animationResolutionScheduled = false;
}
bool DocumentTimeline::shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState() const
{
- return !m_animations.isEmpty() || !m_acceleratedAnimationsPendingRunningStateChange.isEmpty();
+ return !m_animations.isEmpty() || !m_pendingAnimationEvents.isEmpty() || !m_acceleratedAnimationsPendingRunningStateChange.isEmpty();
}
void DocumentTimeline::updateCurrentTime(DOMHighResTimeStamp timestamp)
@@ -359,19 +360,23 @@
// 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)
+ if (!m_isSuspended || !shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState())
cacheCurrentTime(timestamp);
}
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
// flag to false prior to running that procedure to allow animation with timing model updates to schedule updates.
m_animationResolutionScheduled = false;
+ if (m_isSuspended)
+ return;
+
+ if (!shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState())
+ return;
+
internalUpdateAnimationsAndSendEvents();
applyPendingAcceleratedAnimations();
@@ -383,6 +388,11 @@
{
m_numberOfAnimationTimelineInvalidationsForTesting++;
+ // enqueueAnimationEvent() calls scheduleAnimationResolution() to ensure that the "update animations and send events"
+ // procedure is run and enqueued events are dispatched in the next frame. However, events that are enqueued while
+ // this procedure is running should not schedule animation resolution until the event queue has been cleared.
+ m_shouldScheduleAnimationResolutionForNewPendingEvents = false;
+
// https://drafts.csswg.org/web-animations/#update-animations-and-send-events
// 1. Update the current time of all timelines associated with doc passing now as the timestamp.
@@ -421,9 +431,10 @@
// 4. Let events to dispatch be a copy of doc's pending animation event queue.
// 5. Clear doc's pending animation event queue.
auto pendingAnimationEvents = WTFMove(m_pendingAnimationEvents);
+ m_shouldScheduleAnimationResolutionForNewPendingEvents = true;
// 6. Perform a stable sort of the animation events in events to dispatch as follows.
- std::stable_sort(pendingAnimationEvents.begin(), pendingAnimationEvents.end(), [] (const Ref<AnimationPlaybackEvent>& lhs, const Ref<AnimationPlaybackEvent>& rhs) {
+ std::stable_sort(pendingAnimationEvents.begin(), pendingAnimationEvents.end(), [] (const Ref<AnimationEventBase>& lhs, const Ref<AnimationEventBase>& rhs) {
// 1. Sort the events by their scheduled event time such that events that were scheduled to occur earlier, sort before events scheduled to occur later
// and events whose scheduled event time is unresolved sort before events with a resolved scheduled event time.
// 2. Within events with equal scheduled event times, sort by their composite order. FIXME: We don't do this.
@@ -437,8 +448,8 @@
});
// 7. Dispatch each of the events in events to dispatch at their corresponding target using the order established in the previous step.
- for (auto& pendingEvent : pendingAnimationEvents)
- pendingEvent->target()->dispatchEvent(pendingEvent);
+ for (auto& pendingAnimationEvent : pendingAnimationEvents)
+ pendingAnimationEvent->target()->dispatchEvent(pendingAnimationEvent);
// This will cancel any scheduled invalidation if we end up removing all animations.
for (auto& animation : animationsToRemove) {
@@ -547,6 +558,10 @@
void DocumentTimeline::scheduleNextTick()
{
+ // If we have pending animation events, we need to schedule an update right away.
+ if (!m_pendingAnimationEvents.isEmpty())
+ scheduleAnimationResolution();
+
// There is no tick to schedule if we don't have any relevant animations.
if (m_animations.isEmpty())
return;
@@ -665,7 +680,7 @@
if (shouldRunUpdateAnimationsAndSendEventsIgnoringSuspensionState())
scheduleAnimationResolution();
else
- unscheduleAnimationResolution();
+ clearTickScheduleTimer();
}
void DocumentTimeline::updateListOfElementsWithRunningAcceleratedAnimationsForElement(Element& element)
@@ -708,9 +723,11 @@
return m_elementsWithRunningAcceleratedAnimations.contains(&element);
}
-void DocumentTimeline::enqueueAnimationPlaybackEvent(AnimationPlaybackEvent& event)
+void DocumentTimeline::enqueueAnimationEvent(AnimationEventBase& event)
{
m_pendingAnimationEvents.append(event);
+ if (m_shouldScheduleAnimationResolutionForNewPendingEvents)
+ scheduleAnimationResolution();
}
Vector<std::pair<String, double>> DocumentTimeline::acceleratedAnimationsForElement(Element& element) const
Modified: trunk/Source/WebCore/animation/DocumentTimeline.h (256618 => 256619)
--- trunk/Source/WebCore/animation/DocumentTimeline.h 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/Source/WebCore/animation/DocumentTimeline.h 2020-02-14 17:52:07 UTC (rev 256619)
@@ -35,7 +35,7 @@
namespace WebCore {
-class AnimationPlaybackEvent;
+class AnimationEventBase;
class RenderElement;
class DocumentTimeline final : public AnimationTimeline, public CanMakeWeakPtr<DocumentTimeline>
@@ -70,7 +70,7 @@
bool runningAnimationsForElementAreAllAccelerated(Element&) const;
void detachFromDocument();
- void enqueueAnimationPlaybackEvent(AnimationPlaybackEvent&);
+ void enqueueAnimationEvent(AnimationEventBase&);
bool scheduledUpdate() const { return m_animationResolutionScheduled; }
void updateCurrentTime(DOMHighResTimeStamp timestamp);
@@ -92,11 +92,9 @@
void cacheCurrentTime(DOMHighResTimeStamp);
void maybeClearCachedCurrentTime();
void scheduleInvalidationTaskIfNeeded();
- void performInvalidationTask();
void scheduleAnimationResolution();
- void unscheduleAnimationResolution();
+ void clearTickScheduleTimer();
void internalUpdateAnimationsAndSendEvents();
- void performEventDispatchTask();
void updateListOfElementsWithRunningAcceleratedAnimationsForElement(Element&);
void transitionDidComplete(RefPtr<CSSTransition>);
void scheduleNextTick();
@@ -108,7 +106,7 @@
GenericTaskQueue<Timer> m_currentTimeClearingTaskQueue;
HashSet<RefPtr<WebAnimation>> m_acceleratedAnimationsPendingRunningStateChange;
HashSet<Element*> m_elementsWithRunningAcceleratedAnimations;
- Vector<Ref<AnimationPlaybackEvent>> m_pendingAnimationEvents;
+ Vector<Ref<AnimationEventBase>> m_pendingAnimationEvents;
RefPtr<Document> m_document;
Markable<Seconds, Seconds::MarkableTraits> m_cachedCurrentTime;
Seconds m_originTime;
@@ -116,6 +114,7 @@
bool m_isSuspended { false };
bool m_waitingOnVMIdle { false };
bool m_animationResolutionScheduled { false };
+ bool m_shouldScheduleAnimationResolutionForNewPendingEvents { true };
};
} // namespace WebCore
Modified: trunk/Source/WebCore/animation/WebAnimation.cpp (256618 => 256619)
--- trunk/Source/WebCore/animation/WebAnimation.cpp 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/Source/WebCore/animation/WebAnimation.cpp 2020-02-14 17:52:07 UTC (rev 256619)
@@ -685,13 +685,18 @@
{
auto event = AnimationPlaybackEvent::create(type, currentTime, timelineTime, this);
event->setTarget(this);
+ enqueueAnimationEvent(WTFMove(event));
+}
+void WebAnimation::enqueueAnimationEvent(Ref<AnimationEventBase>&& event)
+{
if (is<DocumentTimeline>(m_timeline)) {
// If animation has a document for timing, then append event to its document for timing's pending animation event queue along
// with its target, animation. If animation is associated with an active timeline that defines a procedure to convert timeline times
// to origin-relative time, let the scheduled event time be the result of applying that procedure to timeline time. Otherwise, the
// scheduled event time is an unresolved time value.
- downcast<DocumentTimeline>(*m_timeline).enqueueAnimationPlaybackEvent(WTFMove(event));
+ m_hasScheduledEventsDuringTick = true;
+ downcast<DocumentTimeline>(*m_timeline).enqueueAnimationEvent(WTFMove(event));
} else {
// Otherwise, queue a task to dispatch event at animation. The task source for this task is the DOM manipulation task source.
queueTaskToDispatchEvent(*this, TaskSource::DOMManipulation, WTFMove(event));
@@ -1198,11 +1203,12 @@
bool WebAnimation::needsTick() const
{
- return pending() || playState() == PlayState::Running;
+ return pending() || playState() == PlayState::Running || m_hasScheduledEventsDuringTick;
}
void WebAnimation::tick()
{
+ m_hasScheduledEventsDuringTick = false;
updateFinishedState(DidSeek::No, SynchronouslyNotify::Yes);
m_shouldSkipUpdatingFinishedStateWhenResolving = true;
Modified: trunk/Source/WebCore/animation/WebAnimation.h (256618 => 256619)
--- trunk/Source/WebCore/animation/WebAnimation.h 2020-02-14 17:49:45 UTC (rev 256618)
+++ trunk/Source/WebCore/animation/WebAnimation.h 2020-02-14 17:52:07 UTC (rev 256619)
@@ -42,7 +42,7 @@
namespace WebCore {
class AnimationEffect;
-class AnimationPlaybackEvent;
+class AnimationEventBase;
class AnimationTimeline;
class Document;
class Element;
@@ -116,7 +116,7 @@
virtual ExceptionOr<void> bindingsPlay() { return play(); }
virtual ExceptionOr<void> bindingsPause() { return pause(); }
- virtual bool needsTick() const;
+ bool needsTick() const;
virtual void tick();
Seconds timeToNextTick() const;
virtual void resolve(RenderStyle&);
@@ -135,7 +135,7 @@
void setSuspended(bool);
bool isSuspended() const { return m_isSuspended; }
bool isReplaceable() const;
- virtual void remove();
+ void remove();
void enqueueAnimationPlaybackEvent(const AtomString&, Optional<Seconds>, Optional<Seconds>);
uint64_t globalPosition() const { return m_globalPosition; }
@@ -155,6 +155,8 @@
protected:
explicit WebAnimation(Document&);
+ void enqueueAnimationEvent(Ref<AnimationEventBase>&&);
+
private:
enum class DidSeek : uint8_t { Yes, No };
enum class SynchronouslyNotify : uint8_t { Yes, No };
@@ -201,6 +203,7 @@
bool m_finishNotificationStepsMicrotaskPending;
bool m_isRelevant;
bool m_shouldSkipUpdatingFinishedStateWhenResolving;
+ bool m_hasScheduledEventsDuringTick { false };
TimeToRunPendingTask m_timeToRunPendingPlayTask { TimeToRunPendingTask::NotScheduled };
TimeToRunPendingTask m_timeToRunPendingPauseTask { TimeToRunPendingTask::NotScheduled };
ReplaceState m_replaceState { ReplaceState::Active };
