Skip to content

Commit 2f89cb3

Browse files
committed
[scroll-animations] WPT test scroll-animations/view-timelines/animation-events.html is a flaky failure
https://bugs.webkit.org/show_bug.cgi?id=284299 Reviewed by Anne van Kesteren. The Web Animations spec says that pending animation events should be enqueued per document, and we made the choice years ago to host the event queue on the `DocumentTimeline` created for each document. When document timelines were the only timeline types, running the "update animation and send events" procedure [0] would only ever run if the document timeline needed an update. When we started supporting multiple types of timelines, this meant we could run that procedure when a scroll or view timeline needed an update but the document timeline did not. As such, we change the step where we process events in the "update animation and send events" procedure to not only look for a document timeline among the timelines needing an update, but always looking at the document timeline created for the document. A better fix would be to stop enqueuing events on `DocumentTimeline` and instead on `AnimationTimelinesController` but the given the scope of that potential change and how simple this fix is, it seems like a better solution for the time being. [0] https://drafts.csswg.org/web-animations-1/#update-animations-and-send-events * LayoutTests/TestExpectations: * Source/WebCore/animation/AnimationTimelinesController.cpp: (WebCore::AnimationTimelinesController::updateAnimationsAndSendEvents): Canonical link: https://commits.webkit.org/294068@main
1 parent bf340ec commit 2f89cb3

File tree

2 files changed

+15
-16
lines changed

2 files changed

+15
-16
lines changed

LayoutTests/TestExpectations

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7055,8 +7055,6 @@ webkit.org/b/283701 imported/w3c/web-platform-tests/scroll-animations/css/deferr
70557055
webkit.org/b/283702 imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/constructor-no-document.html [ Skip ]
70567056
webkit.org/b/283107 imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/updating-the-finished-state.html [ Skip ]
70577057

7058-
webkit.org/b/284299 imported/w3c/web-platform-tests/scroll-animations/view-timelines/animation-events.html [ Pass Failure ]
7059-
70607058
# webkit.org/b/263872 [scroll-animations] some WPT tests are ImageOnlyFailure
70617059
imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/layout-changes-on-percentage-based-timeline.html [ ImageOnlyFailure ]
70627060
imported/w3c/web-platform-tests/scroll-animations/view-timelines/range-boundary.html [ ImageOnlyFailure ]

Source/WebCore/animation/AnimationTimelinesController.cpp

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -202,22 +202,23 @@ void AnimationTimelinesController::updateAnimationsAndSendEvents(ReducedResoluti
202202
// 3. Perform a microtask checkpoint.
203203
protectedDocument()->eventLoop().performMicrotaskCheckpoint();
204204

205-
// 4. Let events to dispatch be a copy of doc's pending animation event queue.
206-
// 5. Clear doc's pending animation event queue.
207-
AnimationEvents events;
208-
for (auto& timeline : timelinesToUpdate) {
209-
if (RefPtr documentTimeline = dynamicDowncast<DocumentTimeline>(timeline))
210-
events.appendVector(documentTimeline->prepareForPendingAnimationEventsDispatch());
211-
}
205+
if (RefPtr documentTimeline = m_document->existingTimeline()) {
206+
// FIXME: pending animation events should be owned by this controller rather
207+
// than the document timeline.
212208

213-
// 6. Perform a stable sort of the animation events in events to dispatch as follows.
214-
std::stable_sort(events.begin(), events.end(), [] (const Ref<AnimationEventBase>& lhs, const Ref<AnimationEventBase>& rhs) {
215-
return compareAnimationEventsByCompositeOrder(lhs.get(), rhs.get());
216-
});
209+
// 4. Let events to dispatch be a copy of doc's pending animation event queue.
210+
// 5. Clear doc's pending animation event queue.
211+
auto events = documentTimeline->prepareForPendingAnimationEventsDispatch();
217212

218-
// 7. Dispatch each of the events in events to dispatch at their corresponding target using the order established in the previous step.
219-
for (auto& event : events)
220-
event->target()->dispatchEvent(event);
213+
// 6. Perform a stable sort of the animation events in events to dispatch as follows.
214+
std::stable_sort(events.begin(), events.end(), [] (const Ref<AnimationEventBase>& lhs, const Ref<AnimationEventBase>& rhs) {
215+
return compareAnimationEventsByCompositeOrder(lhs.get(), rhs.get());
216+
});
217+
218+
// 7. Dispatch each of the events in events to dispatch at their corresponding target using the order established in the previous step.
219+
for (auto& event : events)
220+
event->target()->dispatchEvent(event);
221+
}
221222

222223
// This will cancel any scheduled invalidation if we end up removing all animations.
223224
for (auto& animation : animationsToRemove) {

0 commit comments

Comments
 (0)