Skip to content

Commit

Permalink
[scroll-animations] setting a new progress-based timeline on an anima…
Browse files Browse the repository at this point in the history
…tion should reset its `ready` promise

https://bugs.webkit.org/show_bug.cgi?id=285667
rdar://142608438

Reviewed by NOBODY (OOPS!).

We were failing some scroll-animations WPT tests related to switching timelines dynamically because these tests
relied on the animation's `ready` promise being reset upon setting a new progress-based timeline. While the spec
doesn't currently state that this should occur, it makes sense that it would if the animation was not already
pending since a new start time will be computed in the next animation frame.

A spec issue was filed at w3c/csswg-drafts#11465 to clear this up.

So when we schedule a pending play task, we also reset the `ready` promise in case the animation was not already
in the pending state.

* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/css/animation-timeline-ignored.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/setting-timeline.tentative-expected.txt:
* Source/WebCore/animation/WebAnimation.cpp:
(WebCore::WebAnimation::setTimeline):
  • Loading branch information
graouts committed Jan 9, 2025
1 parent 0089d8a commit fbc67a1
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

FAIL Changing animation-timeline changes the timeline (sanity check) assert_equals: expected "140px" but got "0px"
FAIL animation-timeline ignored after setting timeline with JS (ScrollTimeline from JS) assert_equals: expected "180px" but got "0px"
FAIL animation-timeline ignored after setting timeline with JS (ScrollTimeline from CSS) assert_equals: expected "140px" but got "0px"
PASS Changing animation-timeline changes the timeline (sanity check)
PASS animation-timeline ignored after setting timeline with JS (ScrollTimeline from JS)
PASS animation-timeline ignored after setting timeline with JS (ScrollTimeline from CSS)
PASS animation-timeline ignored after setting timeline with JS (document timeline)
FAIL animation-timeline ignored after setting timeline with JS (null) assert_equals: expected "120px" but got "0px"

Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@
PASS Setting a scroll timeline on a play-pending animation synchronizes currentTime of the animation with the scroll position.
PASS Setting a scroll timeline on a pause-pending animation fixes the currentTime of the animation based on the scroll position once resumed
PASS Setting a scroll timeline on a reversed play-pending animation synchronizes the currentTime of the animation with the scroll position.
FAIL Setting a scroll timeline on a running animation synchronizes the currentTime of the animation with the scroll position. assert_approx_equals: Timeline's currentTime aligns with the scroll position even when paused expected a number but got a "object"
PASS Setting a scroll timeline on a running animation synchronizes the currentTime of the animation with the scroll position.
PASS Setting a scroll timeline on a paused animation fixes the currentTime of the animation based on the scroll position when resumed
PASS Setting a scroll timeline on a reversed paused animation fixes the currentTime of the animation based on the scroll position when resumed
PASS Transitioning from a scroll timeline to a document timeline on a running animation preserves currentTime
PASS Transitioning from a scroll timeline to a document timeline on a pause-pending animation preserves currentTime
PASS Transition from a scroll timeline to a document timeline on a reversed paused animation maintains correct currentTime
PASS Transitioning from a scroll timeline to a null timeline on a running animation preserves current progress.
FAIL Switching from a null timeline to a scroll timeline on an animation with a resolved start time preserved the play state promise_test: Unhandled rejection with value: object "TypeError: null is not an object (evaluating 'actual.unit')"
FAIL Switching from one scroll timeline to another updates currentTime promise_test: Unhandled rejection with value: object "TypeError: null is not an object (evaluating 'actual.unit')"
PASS Switching from a null timeline to a scroll timeline on an animation with a resolved start time preserved the play state
PASS Switching from one scroll timeline to another updates currentTime
PASS Switching from a document timeline to a scroll timeline updates currentTime when unpaused via CSS.
PASS Switching from a document timeline to a scroll timeline and updating currentTime preserves the progress while paused.
FAIL Switching from a document timeline to a scroll timeline on an infinite duration animation. assert_approx_equals: values do not match for "undefined" expected 100 +/- 0.125 but got 200
FAIL Changing from a scroll-timeline to a view-timeline updates start time. promise_test: Unhandled rejection with value: object "TypeError: null is not an object (evaluating 'actual.unit')"
PASS Changing from a scroll-timeline to a view-timeline updates start time.

6 changes: 6 additions & 0 deletions Source/WebCore/animation/WebAnimation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,13 @@ void WebAnimation::setTimeline(RefPtr<AnimationTimeline>&& timeline)
if (previousPlayState == PlayState::Finished || previousPlayState == PlayState::Running) {
// 5. If previous play state is "finished" or "running":
// Schedule a pending play task.
// FIXME: re-creating the ready promise is not part of the spec but Chrome implements this
// behavior and it makes sense since the new start time won't be computed until the timeline
// is updated. This is covered by https://github.com/w3c/csswg-drafts/issues/11465.
auto wasAlreadyPending = pending();
m_timeToRunPendingPlayTask = TimeToRunPendingTask::WhenReady;
if (!wasAlreadyPending)
m_readyPromise = makeUniqueRef<ReadyPromise>(*this, &WebAnimation::readyPromiseResolve);
} else if (previousPlayState == PlayState::Paused && previousProgress) {
// 6. If previous play state is "paused" and previous progress is resolved:
// Set hold time to previous progress * end time.
Expand Down

0 comments on commit fbc67a1

Please sign in to comment.