Filed by: archaeopteryx [at] coole-files.de https://treeherder.mozilla.org/logviewer.html#?job_id=38829765&repo=mozilla-inbound https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-inbound-linux-debug/1478616095/mozilla-inbound_ubuntu32_vm-debug_test-web-platform-tests-e10s-10-bm04-tests1-linux32-build58.txt.gz

Hi Olli, I am guessing this failure is affected by bug 1315570. There is no change in dom/animation code when the first failure happened. <https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=73a97f8ffebac71fb4b5d505e2aa0da35a013d8c> Is there any possibility that the change delays animation's finish event? If so, is there anything we can do to avoid the delay?

Affected by bug 1315570? That hasn't landed. You probably mean bug 1306591. That may have revealed a bug in animations code or in the test. Vsync (refreshDriver/animationFrameCallbacks) may be run now at different time. But it could have been run at that time also before, but just not as often. It is after all somewhat random when vsync runs.

finish event is dispatched async here http://searchfox.org/mozilla-central/rev/846adaea6ccd1428780ed47d0d94da18478acdbb/dom/animation/Animation.cpp#1393 and any random code may run between that call and and when the DOM event actually is dispatched.

As far as I see, the test is buggy at least. Nothing guarantees assert_equals(receivedEvents.length, 1, 'length of array should be one'); should be actually true (per HTML spec's processing model). If browser does something heavy between animation frames (waitForAnimationFrames(2);), the finish event may be dispatched later.

Thank you, Olli for your deep insight! IIUC, we will have to a) In case that test case expects an event: just wait for the event. b) In case that test case expects that no event receives: wait for an requestIdleCallback, and then check the event.

I filed https://github.com/w3c/web-platform-tests/issues/4198

https://treeherder.mozilla.org/#/jobs?repo=try&revision=63ef431f19747b5a26b35276a989ae1075affef2 If it's not a good idea to use requestIdleCallback in web animation's tests (neither Edge nor Safari supports it yet), I will land only the part 2. Brian?

(In reply to Hiroyuki Ikezoe (:hiro) from comment #10) > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=63ef431f19747b5a26b35276a989ae1075affef2 > > If it's not a good idea to use requestIdleCallback in web animation's tests > (neither Edge nor Safari supports it yet), I will land only the part 2. > Brian? Oops, part 1 does a) what I commented in comment 5, part 2 does b).

(In reply to Hiroyuki Ikezoe (:hiro) from comment #11) > (In reply to Hiroyuki Ikezoe (:hiro) from comment #10) > > https://treeherder.mozilla.org/#/ > > jobs?repo=try&revision=63ef431f19747b5a26b35276a989ae1075affef2 > > > > If it's not a good idea to use requestIdleCallback in web animation's tests > > (neither Edge nor Safari supports it yet), I will land only the part 2. > > Brian? > > Oops, part 1 does a) what I commented in comment 5, part 2 does b). Opposite... 1 does b), 2 does a).

I agree we shouldn't use requestIdleCallback yet. https://hg.mozilla.org/try/rev/a3225012cbf624d591cd8ca17a18d20a7fdc5899 looks good to me. I wonder if we need to check the event time though--that seems orthogonal to fixing this orange? Perhaps we could just have: anim.ready.then(function() { anim.onfinish = t.step_func(function(event) { assert_unreached('onfinish event should not be fired during endDelay'); }); anim.currentTime = 110000; // during endDelay return waitForAnimationFrames(2); }).then(t.step_func(function() { anim.onfinish = t.step_func(function(event) { t.done(); }); anim.currentTime = 130000; // after endTime })); What do you think?

Ah, exactly. The check is totally out of scope of this test case.

Comment on attachment 8812654 [details] Bug 1316050 - Wait for a single finish event itself instead of waiting two requestAnimationFrame and checking the event. https://reviewboard.mozilla.org/r/94318/#review94526 Thanks for taking care of this!

Pushed by hiikezoe@mozilla-japan.org: https://hg.mozilla.org/integration/autoland/rev/0ac505253b26 Wait for a single finish event itself instead of waiting two requestAnimationFrame and checking the event. r=birtles