1264107 - Intermittent test_animation_observers.html | application crashed [@ mozilla::layers::SampleAnimations]

Status: RESOLVED FIXED

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

Description

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://treeherder.mozilla.org/logviewer.html#?job_id=25602155&repo=mozilla-inbound

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Boris, please tell me any clue you've already know about this failure in bug 1182856#c52.
It's no double this failure is caused by recent our works.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #2)
> Boris, please tell me any clue you've already know about this failure in bug
> 1182856#c52.
> It's no double this failure is caused by recent our works.

'no doubt'.

Comment 4

[Boris Chiou [:boris]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #2)
> Boris, please tell me any clue you've already know about this failure in bug
> 1182856#c52.
> It's no doubt this failure is caused by recent our works.

I saw this testfailed happened in [1] first. It's log is very similar to that in bug 1182856. I check the log, and we have this assertion: [2] MOZ_ASSERT(!animation.startTime().IsNull(),
                          "Failed to resolve start time of pending animations");

This assertion happened between two tests in test_animation_observers.html:
a. "change_duration_and_currenttime:subtree] records after animation end"
b. "change_duration_and_currenttime:subtree] records after animation restarted - number of records"
Therefore, I think it happened when we extend the duration of an animation:

"anim.effect.timing.duration = 1000 * MS_PER_SEC;" [3].


The testfailed happened in bug 1182856 again, so I think bug 1182856 might increase the possibility of this assertion. Bug 1182856 would cancel all transitions while doing restyling if we set the element as display:none. I revised AnimationsWithDestroyedFrame [4], so it also cancel all transitions in that case. Hiro, do you have any possible idea for this bug? Maybe it is a timing issue, but I'm not sure. Or we might have to do some protections in SampleAnimation.

Thank you.

[1] https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=21ad62cfce1989cfb2881db01bacc236cc5917f6
[2] https://dxr.mozilla.org/mozilla-central/rev/21bf1af375c1fa8565ae3bb2e89bd1a0809363d4/gfx/layers/composite/AsyncCompositionManager.cpp#568
[3] https://dxr.mozilla.org/mozilla-central/rev/21bf1af375c1fa8565ae3bb2e89bd1a0809363d4/dom/animation/test/chrome/test_animation_observers.html#1548
[4] http://mxr.mozilla.org/mozilla-central/source/layout/base/RestyleManager.cpp#1144

Comment 5

[Hiroyuki Ikezoe (:hiro)]

(In reply to Boris Chiou [:boris]  from comment #4)
> (In reply to Hiroyuki Ikezoe (:hiro) from comment #2)
> > Boris, please tell me any clue you've already know about this failure in bug
> > 1182856#c52.
> > It's no doubt this failure is caused by recent our works.
> 
> I saw this testfailed happened in [1] first. It's log is very similar to
> that in bug 1182856. I check the log, and we have this assertion: [2]
> MOZ_ASSERT(!animation.startTime().IsNull(),
>                           "Failed to resolve start time of pending
> animations");
> 
> This assertion happened between two tests in test_animation_observers.html:
> a. "change_duration_and_currenttime:subtree] records after animation end"
> b. "change_duration_and_currenttime:subtree] records after animation
> restarted - number of records"
> Therefore, I think it happened when we extend the duration of an animation:
> 
> "anim.effect.timing.duration = 1000 * MS_PER_SEC;" [3].

Thanks you, Boris! This information will be helpful to solve this failure.
One more question, which of target elements was testing at that time? Pseudo?

> The testfailed happened in bug 1182856 again, so I think bug 1182856 might
> increase the possibility of this assertion. Bug 1182856 would cancel all
> transitions while doing restyling if we set the element as display:none. I
> revised AnimationsWithDestroyedFrame [4], so it also cancel all transitions
> in that case. Hiro, do you have any possible idea for this bug? Maybe it is
> a timing issue, but I'm not sure. Or we might have to do some protections in
> SampleAnimation.

I have no idea yet, but I am going to tackle this.

Comment 6

[Boris Chiou [:boris]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #5)
> Thanks you, Boris! This information will be helpful to solve this failure.
> One more question, which of target elements was testing at that time? Pseudo?

In the normal case, we run "change_duration_and_currenttime:subtree" twice: first time for _div_ and second time for _pseudoTarget_. However, in the error log, we only ran once, and then the error happened. Therefore, I think the testing target is the normal element, _div_. [1]

Thanks for tackling this bug.

[1] https://dxr.mozilla.org/mozilla-central/rev/21bf1af375c1fa8565ae3bb2e89bd1a0809363d4/dom/animation/test/chrome/test_animation_observers.html#25

Comment 7

[Intermittent Failures Robot]

14 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 9
* try: 4
* mozilla-central: 1

Platform breakdown:
* android-4-3-armv7-api15: 14

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

Comment 8

[Intermittent Failures Robot]

11 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 8
* fx-team: 2
* mozilla-central: 1

Platform breakdown:
* android-4-3-armv7-api15: 10
* linux64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1264107&startday=2016-04-25&endday=2016-05-01&tree=all

Comment 9

[Hiroyuki Ikezoe (:hiro)]

(In reply to OrangeFactor Robot from comment #8)
> 11 automation job failures were associated with this bug in the last 7 days.
> 
> Repository breakdown:
> * mozilla-inbound: 8
> * fx-team: 2
> * mozilla-central: 1
> 
> Platform breakdown:
> * android-4-3-armv7-api15: 10
> * linux64: 1

Note that this one failure on linux64 seemed to be mis-associated with this bug.

https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=1a4b7ff56f657c95591a22286f56478cefdba64b&selectedJob=8992372

Comment 10

[Intermittent Failures Robot]

5 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-beta: 5

Platform breakdown:
* android-4-3-armv7-api15: 5

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1264107&startday=2016-06-20&endday=2016-06-26&tree=all

Comment 11

[Hiroyuki Ikezoe (:hiro)]

This bug might have been fixed between the end of this April and the beginning of this May, and not uplifted to 48.

Comment 12

[Intermittent Failures Robot]

6 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-beta: 5
* autoland: 1

Platform breakdown:
* android-4-3-armv7-api15: 6

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1264107&startday=2016-06-27&endday=2016-07-03&tree=all

Comment 13

[Hiroyuki Ikezoe (:hiro)]

Oh, is this possible to be caused by the same reason of bug 1283026?

Comment 14

[Hiroyuki Ikezoe (:hiro)]

(In reply to OrangeFactor Robot from comment #12)
> 6 automation job failures were associated with this bug in the last 7 days.
> 
> Repository breakdown:
> * mozilla-beta: 5
> * autoland: 1

I am suspecting this one failure on autoland is caused by bug 1267510.

Comment 15

[Hiroyuki Ikezoe (:hiro)]

There is a 1000ms duration in test_animation_observers.html.

http://hg.mozilla.org/mozilla-central/file/f378a56b25ce/dom/animation/test/chrome/test_animation_observers.html#l1548

We can use a shorter duration there.

Comment 16

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1264107 - Use shorter duration to avoid possibilities of overflow on TimeStamp calculations.

Review commit: https://reviewboard.mozilla.org/r/62182/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62182/

Comment 17

[Brian Birtles (:birtles)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #13)
> Oh, is this possible to be caused by the same reason of bug 1283026?

Yes, possibly the same reason. That bug is apparently about having a negative playbackRate and the test that is failing doesn't set the playbackRate or use reverse(). However, it might be the same class of bug since we're getting a null startTime which could happen when we seek a long animation such that the updated start time occurs before the TimeStamp zero time of the system.

Comment 18

[Brian Birtles (:birtles)]

Comment on attachment 8767774 [details]
Bug 1264107 - Use shorter duration to avoid possibilities of overflow on TimeStamp calculations.

https://reviewboard.mozilla.org/r/62182/#review58928

Looks fine to me. We could even just use animation.finish(), followed by animation.effect.timing.duration *= 2.

Comment 19

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8767774 [details]
Bug 1264107 - Use shorter duration to avoid possibilities of overflow on TimeStamp calculations.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62182/diff/1-2/

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/adbe2cd1bf28
Use shorter duration to avoid possibilities of overflow on TimeStamp calculations. r=birtles

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/adbe2cd1bf28

Comment 22

[BMO Automation]

Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.

Comment 23

[Hiroyuki Ikezoe (:hiro)]

Closing since I believe the commit in comment 21 fixed this intermittent failure.

Comment 24

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.