Bug 209239

Summary: REGRESSION(r257417): imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-replacement.html is flaky failing.
Product: WebKit Reporter: Jason Lawrence <Lawrence.j>
Component: AnimationsAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, ddkilzer, dino, graouts, graouts, simon.fraser, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Mac   
OS: macOS 10.14   
Bug Depends on: 208069    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Jason Lawrence 2020-03-18 10:57:40 PDT 
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
Comment 1 Radar WebKit Bug Importer 2020-03-18 10:58:08 PDT 
<rdar://problem/60591358>
Comment 2 Jason Lawrence 2020-03-18 11:10:34 PDT 
I have marked this test as failing while this issue is investigated.
https://trac.webkit.org/changeset/258650/webkit
Comment 3 Antoine Quint 2020-03-19 09:54:37 PDT 
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.
Comment 4 Antoine Quint 2020-03-19 10:54:09 PDT 
When the test fails, the Document only ever sees a single timeline in m_timelines and thus fails to update the animation correctly.
Comment 5 Antoine Quint 2020-03-19 11:00:24 PDT 
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.
Comment 6 Antoine Quint 2020-03-21 14:11:10 PDT 
Created attachment 394177 [details]
Patch
Comment 7 David Kilzer (:ddkilzer) 2020-03-21 22:52:54 PDT 
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
Comment 8 Antoine Quint 2020-03-22 15:28:34 PDT 
Created attachment 394233 [details]
Patch for landing
Comment 9 EWS 2020-03-22 15:57:27 PDT 
Committed r258823: <https://trac.webkit.org/changeset/258823>

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