Bug 206615

Summary: REGRESSION: [ Mac WK2 ] animations/suspend-resume-animation-events.html is a flaky failure
Product: WebKit Reporter: Jacob Uphoff <jacob_uphoff>
Component: AnimationsAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, darin, dino, ryanhaddad, tsavell, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch ap: review+

Jacob Uphoff
Reported 2020-01-22 15:13:19 PST
This is a flakey failure on Mac wk2 I did not try to reproduce, however failure goes back as far as I can see the history. History: https://results.webkit.org/?suite=layout-tests&test=animations%2Fsuspend-resume-animation-events.html Diff: --- /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/animations/suspend-resume-animation-events-expected.txt +++ /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/animations/suspend-resume-animation-events-actual.txt @@ -4,5 +4,4 @@ start animation move iteration animation move -end animation move
Attachments
Patch (2.21 KB, patch)
2020-01-25 06:58 PST, Darin Adler 		ap: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2020-01-22 15:44:27 PST
<rdar://problem/58814195>
Radar WebKit Bug Importer
Comment 2 2020-01-22 15:44:27 PST
<rdar://problem/58814196>
Ryan Haddad
Comment 3 2020-01-23 10:03:49 PST
This became very frequent starting on 1/7/2020, around seemingly unrelated commit r254181. Maybe this commit adding animations logging could be related? Add some more Animations logging ​https://bugs.webkit.org/show_bug.cgi?id=205890 https://trac.webkit.org/changeset/254177/webkit
Darin Adler
Comment 4 2020-01-25 06:55:38 PST
This test is inherently racy and timing-dependent. If the 350ms timeout fires too soon, we would move on to the next test without seeing the end of the animation. I think we can make it much less racy by having each timer set the subsequent one instead of setting all three timers at once. I’ll try that, but no easy way to test if it worked.
Darin Adler
Comment 5 2020-01-25 06:58:07 PST
Created attachment 388773 [details] Patch
Alexey Proskuryakov
Comment 6 2020-01-25 11:54:59 PST
Comment on attachment 388773 [details] Patch The change looks harmless with regards to what this tests check for, and likely beneficial for test reliability. However, I do not understand what is wrong with the test as is. The animation starts before any setTimeouts, and is shorter. Wouldn’t it be an actual WebKit bug if there is raciness?
Darin Adler
Comment 7 2020-01-25 11:58:41 PST
(In reply to Alexey Proskuryakov from comment #6) > The animation starts before any setTimeouts, and is shorter. Wouldn’t it be > an actual WebKit bug if there is raciness? As far as I know, WebKit does not attempt to have a single unified timeline between JavaScript timers and webpage animation. Unless there’s something I’m not aware of. Could be a neat feature and might help webpages work more reliably. Not sure I’d call it a "bug" exactly.
Darin Adler
Comment 8 2020-01-25 12:05:13 PST
I’m going to land this change, even though I’m not sure it will eliminate the flakiness. If this attempt fails then we might need to find another way to write the test. And I think it’s exciting to contemplate an architecture change where we unify JavaScript timers with animation timing and possibly even other time bases that affect web pages.
Darin Adler
Comment 9 2020-01-25 12:07:25 PST
Committed r255124: <https://trac.webkit.org/changeset/255124>
Note You need to log in before you can comment on or make changes to this bug.