Closed Bug 1264107 Opened 9 years ago Closed 7 years ago

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

Categories

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

48 Branch
defect

Tracking

()

RESOLVED FIXED
Tracking Status
firefox48 --- affected

People

(Reporter: KWierso, Assigned: hiro)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

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)
(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)
(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
(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
This bug might have been fixed between the end of this April and the beginning of this May, and not uplifted to 48.
Oh, is this possible to be caused by the same reason of bug 1283026?
(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.
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.
(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+
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.
Keywords: leave-open
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
Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.
Priority: -- → P3
Closing since I believe the commit in comment 21 fixed this intermittent failure.
Assignee: nobody → hikezoe
Status: NEW → RESOLVED
Closed: 7 years 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.

Attachment

General

Created:
Updated:
Size: