Closed Bug 1297285 Opened 3 years ago Closed 3 years ago

Intermittent /web-animations/timing-model/animations/updating-the-finished-state.html | Updating the finished state when playing past end - assert_equals: Hold time is set to target end clamping current time expected 100000 but got 99999.232704

Categories

(Core :: DOM: Animation, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox50 --- unaffected
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: birtles)

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Component: web-platform-tests → DOM: Animation
Product: Testing → Core
Version: Version 3 → Trunk
I'm not quite sure what is happening here. Perhaps we have a quirk in the refresh driver where sometimes we can get less than 16ms between ticks (perhaps we change timers?). Or, in this case less than 1ms? Given the infrequency of this, I don't like my chance of reproducing it locally so it's hard to know exactly what is going on.

We could, of course, just make the test wait two frames though it would be a shame to add browser-specific workarounds into a cross-browser test like this.

We could also compare performance.now() to make sure at least 1ms has passed but that would clutter the code quite a bit.
Priority: -- → P2
I think we could just add a waitFramesWithMinDelay helper here that takes the number of animation frames plus some minimum delay. If after waiting x frames the difference in time as measured using document.timeline.currentTime was less than the min delay, it would keep calling requestAnimationFrame until the minimum delay had passed.

Alternatively, and probably preferably, we could just add a method that only takes a delay (but which uses rAF to wait instead of setTimeout since it's animation time we care about).
Yeah, I am also convinced this is the right approach for this test case.  Though I would like to know what was exactly going on there in the failure case, it's my job. \o/

By the way, we should also fix a similar test case in file_animation_currenttime.html in a new bug, we are currently waiting two frames there!

http://hg.mozilla.org/mozilla-central/file/ea104eeb14cc/dom/animation/test/css-animations/file_animation-currenttime.html#l282
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
My guess is that we occasionally hit a case where we change internal timers in the refresh driver and end up with a frame less than 16ms (or, in fact, less than 1ms in this case).
Comment on attachment 8797892 [details]
Bug 1297285 - Make update-the-finished-state.html not depend on frame timing;

https://reviewboard.mozilla.org/r/83488/#review82096
Attachment #8797892 - Flags: review?(hiikezoe) → review+
Pushed by bbirtles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/388256e2c54f
Make update-the-finished-state.html not depend on frame timing; r=hiro
https://hg.mozilla.org/mozilla-central/rev/388256e2c54f
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.