Closed
Bug 1235723
Opened 9 years ago
Closed 9 years ago
Intermittent browser_animation_playerState.js | Uncaught exception - at chrome://mochitests/content/browser/devtools/server/tests/browser/browser_animation_playerState.js:93 - TypeError: player is undefined
Categories
(DevTools :: General, defect)
DevTools
General
Tracking
(firefox47 fixed)
RESOLVED
FIXED
Firefox 47
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: philor, Unassigned)
Details
(Keywords: intermittent-failure)
Attachments
(2 files)
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Updated•9 years ago
|
Flags: needinfo?(hiikezoe)
Comment 6•9 years ago
|
||
Patrick, this orange gets more frequent in these days because Telemetry or some other components spew lots of exceptions. An instance is https://treeherder.mozilla.org/logviewer.html#?job_id=17012443&repo=try
We should use longer duration for animations in devtools tests.
Flags: needinfo?(hiikezoe)
Comment 7•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/35795/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/35795/
Attachment #8721886 -
Flags: review?(pbrosset)
Comment 8•9 years ago
|
||
Try with the patch.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae4b274152de
Comment 9•9 years ago
|
||
Comment on attachment 8721886 [details]
MozReview Request: Bug 1235723 - Part 1: Use longer duration to avoid finishing animations at some point. r?pbrosset
https://reviewboard.mozilla.org/r/35795/#review32445
Thanks for working on this.
I think all changes are good except for one that will cause browser_animation_keepFinished.js not to cover a specific case anymore.
::: devtools/server/tests/browser/animation.html:67
(Diff revision 1)
> - animation: move 1s;
> + animation: move 100s;
The problem with making this particular animation longer is that there's one test that waits for it to be finished:
\devtools\server\tests\browser\browser_animation_keepFinished.js
This test adds the animation to a node, and then waits for 2 seconds to make sure it is finished, and then checks that we have received the right number of mutations.
With this change, the test will still pass, but for the wrong reason.
Indeed, the animation-inspector tool does not send the "removed" mutation events for animations that can still be used in the tool, so even when they are finished, we just capture the event on the server-side, and don't send it to the toolbox. That's why we must make sure that this animation remains short, so that we can add it, get the "added" mutation, then wait for some time, and verify that we didn't get the "removed" mutation.
Attachment #8721886 -
Flags: review?(pbrosset)
Comment 10•9 years ago
|
||
Comment on attachment 8721886 [details]
MozReview Request: Bug 1235723 - Part 1: Use longer duration to avoid finishing animations at some point. r?pbrosset
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35795/diff/1-2/
Attachment #8721886 -
Attachment description: MozReview Request: Bug 1235723 - Use longer duration to avoid finishing animations at some point. r?pbrosset → MozReview Request: Bug 1235723 - Part 1: Use longer duration to avoid finishing animations at some point. r?pbrosset
Attachment #8721886 -
Flags: review?(pbrosset)
Comment 11•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/35975/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/35975/
Attachment #8722305 -
Flags: review?(pbrosset)
Comment 12•9 years ago
|
||
https://reviewboard.mozilla.org/r/35795/#review32445
> The problem with making this particular animation longer is that there's one test that waits for it to be finished:
>
> \devtools\server\tests\browser\browser_animation_keepFinished.js
>
> This test adds the animation to a node, and then waits for 2 seconds to make sure it is finished, and then checks that we have received the right number of mutations.
>
> With this change, the test will still pass, but for the wrong reason.
>
> Indeed, the animation-inspector tool does not send the "removed" mutation events for animations that can still be used in the tool, so even when they are finished, we just capture the event on the server-side, and don't send it to the toolbox. That's why we must make sure that this animation remains short, so that we can add it, get the "added" mutation, then wait for some time, and verify that we didn't get the "removed" mutation.
Thanks. I did not notice the eole of short animation at all! This patch dropped the change for the short animation. And a subsequent patch does check that the short animation is surely finished to avoid the mistake just like I did. :-)
Comment 13•9 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #12)
> https://reviewboard.mozilla.org/r/35795/#review32445
>
> > The problem with making this particular animation longer is that there's one test that waits for it to be finished:
> >
> > \devtools\server\tests\browser\browser_animation_keepFinished.js
> >
> > This test adds the animation to a node, and then waits for 2 seconds to make sure it is finished, and then checks that we have received the right number of mutations.
> >
> > With this change, the test will still pass, but for the wrong reason.
> >
> > Indeed, the animation-inspector tool does not send the "removed" mutation events for animations that can still be used in the tool, so even when they are finished, we just capture the event on the server-side, and don't send it to the toolbox. That's why we must make sure that this animation remains short, so that we can add it, get the "added" mutation, then wait for some time, and verify that we didn't get the "removed" mutation.
>
> Thanks. I did not notice the eole of short animation at all! This patch
'the role'.
Comment 14•9 years ago
|
||
Comment on attachment 8721886 [details]
MozReview Request: Bug 1235723 - Part 1: Use longer duration to avoid finishing animations at some point. r?pbrosset
https://reviewboard.mozilla.org/r/35795/#review32647
Attachment #8721886 -
Flags: review?(pbrosset) → review+
Comment 15•9 years ago
|
||
Comment on attachment 8722305 [details]
MozReview Request: Bug 1235723 - Part 2: Check that the short animation is surely finished before proceeding test. r?pbrosset
https://reviewboard.mozilla.org/r/35975/#review32649
Attachment #8722305 -
Flags: review?(pbrosset) → review+
Comment 16•9 years ago
|
||
Comment 17•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/883d9427c0ab
https://hg.mozilla.org/mozilla-central/rev/f3c559370097
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Comment hidden (Intermittent Failures Robot) |
Comment 19•9 years ago
|
||
All of oranges in the last week happened before the patches has been landed in mozilla-central.
The last failure revision is:
https://hg.mozilla.org/mozilla-central/rev/4f64ec5486f4
push date 2016-02-25 09:42 +0000
The push date of the land is:
push date 2016-02-25 10:58 +0000
Updated•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•