Closed Bug 1123936 Opened 5 years ago Closed 5 years ago
_animation _actors _02 .js | The animation is currently running - Got pending, expected running
1.38 KB, patch
|Details | Diff | Splinter Review|
39 bytes, text/x-review-board-request
Sending over to pbrosset
Assignee: nobody → pbrosset
Component: Developer Tools → Developer Tools: Inspector
That's an easy fix. Recently web animations have had a new thing whereby they go through a pending state before starting to run, and there's a ready promise that consumers can wait for in that case. This promise is already available on the AnimationPlayerActor, so it's just a matter of adding a step to the intermittent failing test to wait for this promise to resolve.
Some background for Brian: the test fails since bug 927349 landed. That bug made animations pass by a pending state shortly before starting, and also exposed a 'ready' promise on AnimationPlayer objects. The failing test assumed the animation was running no matter what. Since bug 1113091, I've exposed the ready promise on AnimationPlayerActor objects. So the test just has to wait until the front.ready promise resolves. Pending try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a3931b9c52f
Attachment #8552985 - Flags: review?(bgrinstead)
That's right, this intermittent made it to 37. I'll request uplift once it gets reviewed and lands on central. The try push is green so far.
Attachment #8552985 - Flags: review?(bgrinstead) → review+
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
Sorry, this is a bit of a mess. The patch in this bug was the right idea, but the fix was incorrect: yield player.ready instead of yield player.ready() So this doesn't make anything fail, and since the bug was intermittent to start with, we didn't realize that the patch didn't solve the issue right away. Also, I have actually fixed this issue for good on nightly since then (in bug 1122466 apparently). So this intermittent remains in Aurora. I don't know if it's a good idea to uplift bug 1122466 now. I'd rather just provide a new patch here just for Aurora (a patch that just adds the () parens after yield player.ready). Ryan, is that something I can do?
If you're worried about uplifting bug 1122466, then a simple test-only fix sounds like a great idea :)
/r/4097 - Bug 1123936 - Aurora-only fix for browser_animation_actors_02.js; r=me Pull down this commit: hg pull review -r a34c2a30cb9abe612254fabdd14e52e536b249bc
Attachment #8567139 - Flags: review?(pbrosset)
Attachment #8567139 - Flags: review?(pbrosset) → review+
Comment on attachment 8567139 [details] MozReview Request: bz://1123936/pbrosset https://reviewboard.mozilla.org/r/4095/#review3259 Ship It!
So, I've made an Aurora-only patch (since that particular change already exists on nightly). It's a test only change that will have absolutely no impacts except from fixing the intermittent on Aurora, so I've gone ahead and R+'d it myself. I hope that's ok. I think it is because that exact fix already exists on Nightly and has been R+'d there for some time.
You need to log in before you can comment on or make changes to this bug.