Summary: | [ Mac iOS ] animations/animation-direction-normal.html is a flaky failure | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Truitt Savell <tsavell> | ||||
Component: | Animations | Assignee: | Antoine Quint <graouts> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | dino, graouts, graouts, jacob_uphoff, simon.fraser, webkit-bot-watchers-bugzilla, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Truitt Savell
2020-04-07 14:53:41 PDT
This test disables the use of a pause API and thus relies exclusively on timers. Presumably this is to check the runtime behavior of the test, but then this makes the test flaky by design since it relies on timers. We should probably nuke such tests, I don't think they serve much value. I'll still try something to improve the situation and use a requestAnimationFrame() loop rather than set timers. That won't help either. This test needs to be rewritten completely. *** Bug 209362 has been marked as a duplicate of this bug. *** Created attachment 397350 [details]
Patch
Comment on attachment 397350 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397350&action=review > LayoutTests/animations/resources/animation-test.js:34 > + this._target = options.target; "target" is a bit generic. Maybe targetProperty? > LayoutTests/animations/resources/animation-test.js:91 > + // Reset the animation. > + animation.cancel(); > + > + // Make sure it's in the paused state for seeking. > + animation.pause(); Is the cancel then pause really necessary? (In reply to Simon Fraser (smfr) from comment #7) > Comment on attachment 397350 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=397350&action=review > > > LayoutTests/animations/resources/animation-test.js:34 > > + this._target = options.target; > > "target" is a bit generic. Maybe targetProperty? This is an element and it matches the terminology for KeyframeEffect::target. > > LayoutTests/animations/resources/animation-test.js:91 > > + // Reset the animation. > > + animation.cancel(); > > + > > + // Make sure it's in the paused state for seeking. > > + animation.pause(); > > Is the cancel then pause really necessary? Yeah, I think we can get by with the cancel() call only. Committed r260577: <https://trac.webkit.org/changeset/260577> |