The test at imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events.html is flaky.
This test is no longer flaky after fixing bug 207364 (r256619).
(In reply to Antoine Quint from comment #1) > This test is no longer flaky after fixing bug 207364 (r256619). The test is not fixed, as reported in https://bugs.webkit.org/show_bug.cgi?id=207788 https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fweb-animations%2Ftiming-model%2Ftimelines%2Fupdate-and-send-events.html&platform=mac
<rdar://problem/59470821>
*** Bug 207788 has been marked as a duplicate of this bug. ***
Marked the test as flaky again in http://trac.webkit.org/r256645.
*** Bug 207790 has been marked as a duplicate of this bug. ***
This is the error I'm seeing: -FAIL All timelines are updated before running microtasks promise_test: Unhandled rejection with value: object "AbortError: The operation was aborted." +TIMEOUT All timelines are updated before running microtasks Test timed out
If that failing test runs in isolation over many iterations, it never times out. However, if it's run with its preceding test, it eventually times out.
We can simply fix the last test to be a PASS and this will resolve the issue. To do this, we must make sure we run the "update animations and send events" procedure with all timelines in a coordinated fashion such that there is only a single micro task point.
Retitling to better capture the scope of the work to unflake this test.
Created attachment 397182 [details] Patch
Created attachment 397183 [details] Patch
Comment on attachment 397183 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397183&action=review > Source/WebCore/ChangeLog:13 > + So far, although we did manage multiple animation timelines per document, we mostly operated under the assumption that there really was a single timeline. > + In this patch we make the "update animations and send events" procedure, which is central to the lifecycle of animations, work with multiple timelines such > + that a single microtask checkpoint is performed even with multiple timelines, whereas we would perform one per timeline before. To do this, we move much of > + the logic DocumentTimeline::updateAnimationsAndSendEvents() to DocumentTimelinesController where each step is run across each timeline, rather than running > + all steps for each timeline one after the other, respecting the single microtask checkpoint in the middle of the process. Why are you wrapping the lines at such a high column? I think you want at least half this width. > Source/WebCore/animation/DocumentTimelinesController.cpp:71 > + Vector<RefPtr<DocumentTimeline>> protectedTimelines; > + for (auto& timeline : m_timelines) > + protectedTimelines.append(&timeline); Vector<RefPtr<DocumentTimeline>> protectedTimelines { m_timelines };
(In reply to Dean Jackson from comment #13) > Comment on attachment 397183 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=397183&action=review > > > Source/WebCore/ChangeLog:13 > Why are you wrapping the lines at such a high column? I think you want at > least half this width. I don't know! I don't really understand why we have to wrap them in the first place. > > Source/WebCore/animation/DocumentTimelinesController.cpp:71 > > + Vector<RefPtr<DocumentTimeline>> protectedTimelines; > > + for (auto& timeline : m_timelines) > > + protectedTimelines.append(&timeline); > > Vector<RefPtr<DocumentTimeline>> protectedTimelines { m_timelines }; Note that m_timelines is a WeakHashSet<DocumentTimeline>, so I don't think that can work.
Committed r260525: <https://trac.webkit.org/changeset/260525>
Comment on attachment 397183 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397183&action=review > Source/WebCore/animation/AnimationTimeline.h:53 > + AnimationCollection relevantAnimations() const { return m_animations; } OK as-is. Optimal solution is to return const lvalue ref. See bug #211309. > Source/WebCore/animation/AnimationTimeline.h:54 > + Vector<WeakPtr<WebAnimation>> allAnimations() const { return m_allAnimations; } This is unused? Can it be removed? If not then optimal solution is to return const lvalue ref.
Committed r261218: <https://trac.webkit.org/changeset/261218>
(In reply to Daniel Bates from comment #16) > > Source/WebCore/animation/AnimationTimeline.h:54 > > + Vector<WeakPtr<WebAnimation>> allAnimations() const { return m_allAnimations; } > > This is unused? Can it be removed? If not then optimal solution is to return > const lvalue ref. It was removed in r261218.