Bug 206615 - REGRESSION: [ Mac WK2 ] animations/suspend-resume-animation-events.html is a flaky failure
Summary: REGRESSION: [ Mac WK2 ] animations/suspend-resume-animation-events.html is a ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-01-22 15:13 PST by Jacob Uphoff
Modified: 2020-01-25 12:07 PST (History)
7 users (show)

See Also:


Attachments
Patch (2.21 KB, patch)
2020-01-25 06:58 PST, Darin Adler
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jacob Uphoff 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
Comment 1 Radar WebKit Bug Importer 2020-01-22 15:44:27 PST
<rdar://problem/58814195>
Comment 2 Radar WebKit Bug Importer 2020-01-22 15:44:27 PST
<rdar://problem/58814196>
Comment 3 Ryan Haddad 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
Comment 4 Darin Adler 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.
Comment 5 Darin Adler 2020-01-25 06:58:07 PST
Created attachment 388773 [details]
Patch
Comment 6 Alexey Proskuryakov 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?
Comment 7 Darin Adler 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.
Comment 8 Darin Adler 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.
Comment 9 Darin Adler 2020-01-25 12:07:25 PST
Committed r255124: <https://trac.webkit.org/changeset/255124>