Intermittent /web-animations/interfaces/AnimationEffectTiming/endDelay.html | onfinish event is fired currentTime is after endTime - assert_equals: length of array should be one expected 1 but got 0

RESOLVED FIXED in Firefox 52

Status

()

Core
DOM: Animation
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Treeherder Bug Filer, Assigned: hiro)

Tracking

({intermittent-failure})

unspecified
mozilla53
intermittent-failure
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox51 unaffected, firefox52 fixed, firefox53 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Comment 1

2 years ago
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?
Flags: needinfo?(bugs)

Comment 2

2 years ago
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.
Flags: needinfo?(bugs)

Comment 3

2 years ago
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.

Comment 4

2 years ago
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.
(Assignee)

Comment 5

2 years ago
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.

Updated

2 years ago
Blocks: 1244635

Comment 7

2 years ago
43 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 24
* autoland: 13
* mozilla-central: 4
* try: 2

Platform breakdown:
* linux32: 29
* linux64: 11
* osx-10-10: 2
* windows7-32-vm: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1316050&startday=2016-11-07&endday=2016-11-13&tree=all

Comment 8

2 years ago
18 failures in 144 pushes (0.125 failures/push) were associated with this bug yesterday.  

Repository breakdown:
* mozilla-inbound: 8
* autoland: 7
* try: 2
* mozilla-aurora: 1

Platform breakdown:
* linux32: 15
* osx-10-10: 3

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1316050&startday=2016-11-15&endday=2016-11-15&tree=all
Priority: -- → P1

Comment 9

2 years ago
20 failures in 147 pushes (0.136 failures/push) were associated with this bug yesterday.  

Repository breakdown:
* autoland: 11
* try: 4
* mozilla-inbound: 4
* mozilla-aurora: 1

Platform breakdown:
* linux32: 16
* osx-10-10: 2
* linux64: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1316050&startday=2016-11-17&endday=2016-11-17&tree=all
(Assignee)

Updated

2 years ago
Assignee: nobody → hiikezoe
Status: NEW → ASSIGNED
(Assignee)

Comment 10

2 years ago
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?
Flags: needinfo?(bbirtles)
(Assignee)

Comment 11

2 years ago
(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).
(Assignee)

Comment 12

2 years ago
(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).

Comment 13

2 years ago
23 failures in 113 pushes (0.204 failures/push) were associated with this bug yesterday.  

Repository breakdown:
* autoland: 11
* mozilla-inbound: 6
* mozilla-central: 4
* mozilla-aurora: 2

Platform breakdown:
* linux32: 21
* windows7-32-vm: 1
* linux64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1316050&startday=2016-11-18&endday=2016-11-18&tree=all

Comment 14

2 years ago
102 failures in 715 pushes (0.143 failures/push) were associated with this bug in the last 7 days. 

This is the #12 most frequent failure this week. 

** This failure happened more than 50 times this week! Resolving this bug is a high priority. **

Repository breakdown:
* autoland: 43
* mozilla-inbound: 32
* mozilla-central: 10
* mozilla-aurora: 9
* try: 8

Platform breakdown:
* linux32: 85
* linux64: 8
* osx-10-10: 7
* windows7-32-vm: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1316050&startday=2016-11-14&endday=2016-11-20&tree=all
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?
Flags: needinfo?(bbirtles)
(Assignee)

Comment 16

2 years ago
Ah, exactly.  The check is totally out of scope of this test case.
Comment hidden (mozreview-request)

Comment 18

2 years ago
mozreview-review
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!
Attachment #8812654 - Flags: review?(bbirtles) → review+

Comment 19

2 years ago
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
status-firefox51: --- → unaffected
status-firefox52: --- → affected
status-firefox53: --- → affected

Comment 20

2 years ago
30 failures in 113 pushes (0.265 failures/push) were associated with this bug yesterday.  

Repository breakdown:
* mozilla-inbound: 11
* autoland: 11
* try: 3
* mozilla-central: 3
* mozilla-aurora: 2

Platform breakdown:
* linux32: 26
* linux64: 3
* windows7-32-vm: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1316050&startday=2016-11-21&endday=2016-11-21&tree=all

Comment 21

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0ac505253b26
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53

Comment 22

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/239c6400a45d
status-firefox52: affected → fixed
Flags: in-testsuite+

Comment 23

2 years ago
41 failures in 623 pushes (0.066 failures/push) were associated with this bug in the last 7 days. 

This is the #36 most frequent failure this week. 

Repository breakdown:
* mozilla-inbound: 15
* autoland: 11
* mozilla-aurora: 9
* try: 3
* mozilla-central: 3

Platform breakdown:
* linux32: 36
* linux64: 3
* windows7-32-vm: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1316050&startday=2016-11-21&endday=2016-11-27&tree=all
You need to log in before you can comment on or make changes to this bug.