Closed
Bug 1145912
Opened 9 years ago
Closed 9 years ago
Intermittent various failures in test_animation_observers.html | [single_transition_reversed] getAnimationPlayers().length after transition reversal - got 0, expected 1 | [multiple_transitions] number of running AnimationPlayers - got 0, expected 3
Categories
(Core :: CSS Parsing and Computation, defect)
Tracking
()
RESOLVED
FIXED
mozilla39
Tracking | Status | |
---|---|---|
firefox37 | --- | unaffected |
firefox38 | --- | unaffected |
firefox39 | --- | fixed |
firefox40 | --- | fixed |
firefox-esr31 | --- | unaffected |
People
(Reporter: KWierso, Assigned: heycam)
Details
(Keywords: intermittent-failure)
Attachments
(2 files)
1.80 KB,
patch
|
birtles
:
review+
|
Details | Diff | Splinter Review |
10.23 KB,
patch
|
birtles
:
review+
|
Details | Diff | Splinter Review |
13:54:03 INFO - 691 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_cancelled_noninterpolable] records after transition start - record[0].addedAnimations length 13:54:03 INFO - 692 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_cancelled_noninterpolable] records after transition start - record[0].addedAnimations contains expected AnimationPlayer 13:54:03 INFO - 693 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_cancelled_noninterpolable] records after transition start - record[0].changedAnimations length 13:54:03 INFO - 694 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_cancelled_noninterpolable] records after transition start - record[0].removedAnimations length 13:54:03 INFO - 695 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_cancelled_noninterpolable] records after transition end - number of records 13:54:03 INFO - 696 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_cancelled_noninterpolable] records after transition end - record[0].addedAnimations length 13:54:03 INFO - 697 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_cancelled_noninterpolable] records after transition end - record[0].changedAnimations length 13:54:03 INFO - 698 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_cancelled_noninterpolable] records after transition end - record[0].removedAnimations length 13:54:03 INFO - 699 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_cancelled_noninterpolable] records after transition end - record[0].removedAnimations contains expected AnimationPlayer 13:54:03 INFO - 700 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_reversed] getAnimationPlayers().length after transition start 13:54:03 INFO - 701 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_reversed] records after transition start - number of records 13:54:03 INFO - 702 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_reversed] records after transition start - record[0].addedAnimations length 13:54:03 INFO - 703 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_reversed] records after transition start - record[0].addedAnimations contains expected AnimationPlayer 13:54:03 INFO - 704 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_reversed] records after transition start - record[0].changedAnimations length 13:54:03 INFO - 705 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_reversed] records after transition start - record[0].removedAnimations length 13:54:03 INFO - 706 INFO TEST-UNEXPECTED-FAIL | dom/animation/test/chrome/test_animation_observers.html | [single_transition_reversed] getAnimationPlayers().length after transition reversal - got 0, expected 1 13:54:03 INFO - 707 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_reversed] records after transition reversal - number of records 13:54:03 INFO - 708 INFO TEST-UNEXPECTED-FAIL | dom/animation/test/chrome/test_animation_observers.html | [single_transition_reversed] records after transition reversal - record[0].addedAnimations length - got 0, expected 1 13:54:03 INFO - 709 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_reversed] records after transition reversal - record[0].changedAnimations length 13:54:03 INFO - 710 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_reversed] records after transition reversal - record[0].removedAnimations length 13:54:03 INFO - 711 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [single_transition_reversed] records after transition reversal - record[0].removedAnimations contains expected AnimationPlayer 13:54:03 INFO - 712 INFO TEST-UNEXPECTED-FAIL | dom/animation/test/chrome/test_animation_observers.html | [single_transition_reversed] records after transition end - number of records - got 0, expected 1 13:54:03 INFO - 713 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions] getAnimationPlayers().length after transition starts 13:54:03 INFO - 714 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions] records after transition starts - number of records 13:54:03 INFO - 715 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions] records after transition starts - record[0].addedAnimations length 13:54:03 INFO - 716 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions] records after transition starts - record[0].addedAnimations contains expected AnimationPlayer 13:54:03 INFO - 717 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions] records after transition starts - record[0].addedAnimations contains expected AnimationPlayer 13:54:03 INFO - 718 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions] records after transition starts - record[0].addedAnimations contains expected AnimationPlayer 13:54:03 INFO - 719 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions] records after transition starts - record[0].changedAnimations length 13:54:03 INFO - 720 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions] records after transition starts - record[0].removedAnimations length 13:54:03 INFO - 721 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions] number of running AnimationPlayers
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → cam
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=075b33264a01
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Updated•9 years ago
|
Attachment #8581060 -
Flags: review?(bbirtles) → review+
Assignee | ||
Comment 14•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ae6881f492e
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 16•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5ae6881f492e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment hidden (Legacy TBPL/Treeherder Robot) |
Updated•9 years ago
|
status-firefox37:
--- → unaffected
status-firefox38:
--- → unaffected
status-firefox-esr31:
--- → unaffected
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 19•9 years ago
|
||
Hmm, this can still occur https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=21f057db27bd Even after bug 1145327
Assignee | ||
Comment 20•9 years ago
|
||
Maybe we should log the playState and currentTime of the three AnimationPlayers to see what's going on.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 27•9 years ago
|
||
jwatt, you've been doing animation-related work lately too, right? Any ideas?
Flags: needinfo?(jwatt)
Assignee | ||
Comment 28•9 years ago
|
||
I added some logging: https://hg.mozilla.org/integration/mozilla-inbound/rev/79d827fe12bd
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 30•9 years ago
|
||
From that failing run: TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] getAnimations().length after transition starts animation states: pending,pending,pending animation times: 0,0,0 TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] records after transition starts - number of records TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] records after transition starts - record[0].addedAnimations length TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] records after transition starts - record[0].addedAnimations contains expected Animation TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] records after transition starts - record[0].addedAnimations contains expected Animation 19:52:32 INFO - 771 INFO TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] records after transition starts - record[0].addedAnimations contains expected Animation TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] records after transition starts - record[0].changedAnimations length TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] records after transition starts - record[0].removedAnimations length animation states: pending,pending,pending animation times: 0,0,0 animation states: pending,pending,pending animation times: 0,0,0 animation states: pending,pending,pending animation times: 50000,50000,50000 TEST-UNEXPECTED-FAIL | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] number of running Animations - got 0, expected 3 Compare that to when I run it locally: TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] getAnimations().length after transition starts animation states: pending,pending,pending animation times: 0,0,0 TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] records after transition starts - number of records TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] records after transition starts - record[0].addedAnimations length TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] records after transition starts - record[0].addedAnimations contains expected Animation TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] records after transition starts - record[0].addedAnimations contains expected Animation TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] records after transition starts - record[0].addedAnimations contains expected Animation TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] records after transition starts - record[0].changedAnimations length TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] records after transition starts - record[0].removedAnimations length animation states: pending,pending,pending animation times: 0,0,0 animation states: running,running,running animation times: 15.104384999999999,15.104384999999999,15.104384999999999 animation states: running,running,running animation times: 50000,50000,50000 TEST-PASS | dom/animation/test/chrome/test_animation_observers.html | [multiple_transitions:subtree] number of running Animations So when we fail, none of the AnimationPlayers actually get to start running for some reason. When we pass, the first time that the players are running is at least two requestAnimationFrame callbacks after we've tried to start the transitions. Is that not longer enough? Should we be, say, waiting for player.ready instead?
Comment 31•9 years ago
|
||
(In reply to Cameron McCormack (:heycam) from comment #30) > So when we fail, none of the AnimationPlayers actually get to start running > for some reason. When we pass, the first time that the players are running > is at least two requestAnimationFrame callbacks after we've tried to start > the transitions. Is that not longer enough? Should we be, say, waiting for > player.ready instead? Yes, we should. There are a few reasons why it might take one than one frame for animations to start, paint suppression being one of them.
Assignee | ||
Comment 33•9 years ago
|
||
Flags: needinfo?(jwatt)
Attachment #8587042 -
Flags: review?(bbirtles)
Assignee | ||
Updated•9 years ago
|
Attachment #8587042 -
Attachment is patch: true
Comment 34•9 years ago
|
||
Comment on attachment 8587042 [details] [diff] [review] wait for animation.ready in tests where we assume the animation has started playing >@@ -248,16 +251,19 @@ function assert_records(expected, desc) > is(animations.length, 1, "getAnimations().length after transition start"); > > // Wait for the single MutationRecord for the Animation addition to > // be delivered. > yield await_frame(); > assert_records([{ added: animations, changed: [], removed: [] }], > "records after transition start"); > >+ // Wait for the animation to be playing. >+ yield animations[0].ready; >+ > // Cancel the transition by setting the current animation value. > var value = "rgb(128, 255, 0)"; > is(getComputedStyle(e).backgroundColor, value, "half-way transition value"); > e.style.backgroundColor = value; We don't actually have to wait for the animation to get started in this case since while it is waiting to start it will be frozen at its initial time--i.e. the animation style should still be set. But it doesn't hurt to have this either. >@@ -279,16 +285,19 @@ function assert_records(expected, desc) > is(animations.length, 1, "getAnimations().length after transition start"); > > // Wait for the single MutationRecord for the Animation addition to > // be delivered. > yield await_frame(); > assert_records([{ added: animations, changed: [], removed: [] }], > "records after transition start"); > >+ // Wait for the animation to be playing. >+ yield animations[0].begin; >+ Probably should be 'ready'. (And again, it's probably not strictly necessary to wait in this case.) Actually a bunch of the other cases aren't strictly needed, but they don't hurt. It's most where we're waiting a frame and expecting the animation to be running that we'd be better off waiting on the ready promise. You can still seek an animation while it's waiting to start so most of the tests should work without the extra wait. r=birtles with begin replaced with ready (and removing any waiting you think is superfluous).
Attachment #8587042 -
Flags: review?(bbirtles) → review+
Assignee | ||
Comment 35•9 years ago
|
||
(In reply to Brian Birtles (:birtles) from comment #34) > Probably should be 'ready'. Silently not waiting on a promise is a bad failure mode here. :( > Actually a bunch of the other cases aren't strictly needed, but they don't > hurt. It's most where we're waiting a frame and expecting the animation to > be running that we'd be better off waiting on the ready promise. You can > still seek an animation while it's waiting to start so most of the tests > should work without the extra wait. OK. I'll remove all except those I know are needed.
Assignee | ||
Comment 36•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a4ac07b1a28b
Comment 37•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a4ac07b1a28b
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
You need to log in
before you can comment on or make changes to this bug.
Description
•