Bug 209239 – REGRESSION(r257417): imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-replacement.html is flaky failing.

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Jason Lawrence]

imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-replacement.html 

Description:
This test is flaky failing on Mac. When looking at just the recent set of flaky failures, they started on 03/02/2020.

History:
https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fweb-animations%2Ftiming-model%2Ftimelines%2Fupdate-and-send-events-replacement.html&platform=mac&recent=false&flavor=wk2&flavor=wk1&limit=50000

Diff:
--- /Volumes/Data/slave/mojave-release-tests-wk1/build/layout-test-results/imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-replacement-expected.txt
+++ /Volumes/Data/slave/mojave-release-tests-wk1/build/layout-test-results/imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-replacement-actual.txt
@@ -11,7 +11,7 @@
 PASS Removes an animation after updating its fill mode 
 PASS Removes an animation after updating another animation's effect to one with different timing 
 PASS Removes an animation after updating its effect to one with different timing 
-PASS Removes an animation after updating another animation's timeline 
+FAIL Removes an animation after updating another animation's timeline assert_equals: expected "removed" but got "active"
 PASS Removes an animation after updating its timeline 
 PASS Removes an animation after updating another animation's effect's properties 
 PASS Removes an animation after updating its effect's properties

[Radar WebKit Bug Importer]

<rdar://problem/60591358>

[Jason Lawrence]

I have marked this test as failing while this issue is investigated.
https://trac.webkit.org/changeset/258650/webkit

[Antoine Quint]

The failure can be reproduced like so:

rwt imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-replacement.html --iterations=5000 --exit-after-n-failures=1 --force -f

The test can be reduced to the single flaky failing test.

[Antoine Quint]

When the test fails, the Document only ever sees a single timeline in m_timelines and thus fails to update the animation correctly.

[Antoine Quint]

OK, this is due to r257417, the fix for bug 208069. In this test, we have the following:

  animB.timeline = new DocumentTimeline({
    originTime:
      document.timeline.currentTime - 100 * MS_PER_SEC - animB.startTime,
  });

But the DocumentTimeline is not referenced by anything other than the animation and so it gets deref'd, no longer being considered. We need to revisit bug 208069.

[Antoine Quint]

Created attachment 394177 [details]
Patch

[David Kilzer (:ddkilzer)]

Comment on attachment 394177 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=394177&action=review

> Source/WebCore/ChangeLog:20
> +        WebAnimation a weak reference, in some cases, if CG kicks in, the timeline would be dereferenced and the test would fail. We restore that relationship

CG => GC

[Antoine Quint]

Created attachment 394233 [details]
Patch for landing

[EWS]

Committed r258823: <https://trac.webkit.org/changeset/258823>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394233 [details].