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

RESOLVED FIXED

Status

()

P3
normal
RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: KWierso, Assigned: hiro)

Tracking

({intermittent-failure})

48 Branch
intermittent-failure
Points:
---

Firefox Tracking Flags

(firefox48 affected)

Details

Attachments

(1 attachment)

(Reporter)

Updated

3 years ago
Duplicate of this bug: 1264432
(Assignee)

Comment 2

3 years ago
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.
Flags: needinfo?(boris.chiou)
(Assignee)

Comment 3

3 years ago
(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'.
(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
Flags: needinfo?(boris.chiou)
(Assignee)

Comment 5

3 years ago
(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.
(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 hidden (Intermittent Failures Robot)
Comment hidden (Intermittent Failures Robot)
(Assignee)

Comment 9

3 years ago
(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 hidden (Intermittent Failures Robot)
(Assignee)

Comment 11

3 years ago
This bug might have been fixed between the end of this April and the beginning of this May, and not uplifted to 48.
Comment hidden (Intermittent Failures Robot)
(Assignee)

Comment 13

3 years ago
Oh, is this possible to be caused by the same reason of bug 1283026?
(Assignee)

Comment 14

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

Comment 15

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

Comment 16

3 years ago
Created attachment 8767774 [details]
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/
Attachment #8767774 - Flags: review?(bbirtles)
(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 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.
Attachment #8767774 - Flags: review?(bbirtles) → review+
(Assignee)

Comment 19

3 years ago
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/
Attachment #8767774 - Attachment description: Bug 1264107 - Use shorter duration to avoid possibilities of overflow for TimeStamp calculations. → Bug 1264107 - Use shorter duration to avoid possibilities of overflow on TimeStamp calculations.
(Assignee)

Updated

3 years ago
Keywords: leave-open

Comment 20

3 years ago
Pushed by hiikezoe@mozilla-japan.org:
https://hg.mozilla.org/integration/autoland/rev/adbe2cd1bf28
Use shorter duration to avoid possibilities of overflow on TimeStamp calculations. r=birtles

Comment 22

3 years ago
Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.
Priority: -- → P3
(Assignee)

Comment 23

a year ago
Closing since I believe the commit in comment 21 fixed this intermittent failure.
Assignee: nobody → hikezoe
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.