Closed Bug 1206345 Opened 9 years ago Closed 8 years ago

Intermittent browser_animation_actors_07.js,browser_animation_reconstructState.js | The state retrieved has key previousStartTime -

Categories

(DevTools :: Inspector: Animations, defect, P1)

defect

Tracking

(firefox43 wontfix, firefox44 wontfix, firefox45 affected, firefox46 affected, firefox48 fixed, firefox-esr45 fixed)

RESOLVED FIXED
Firefox 48
Tracking Status
firefox43 --- wontfix
firefox44 --- wontfix
firefox45 --- affected
firefox46 --- affected
firefox48 --- fixed
firefox-esr45 --- fixed

People

(Reporter: philor, Assigned: pbro)

Details

(Keywords: intermittent-failure, Whiteboard: [btpp-fix-now])

Attachments

(1 file)

      No description provided.
Summary: Intermittent browser_animation_actors_07.js | The state retrieved has key previousStartTime - → Intermittent browser_animation_actors_07.js,browser_animation_reconstructState.js | The state retrieved has key previousStartTime -
Component: Developer Tools → Developer Tools: Animation Inspector
Failing >= 10 times a week on m-c/inbound/fx-team. Moving up to P1 and assigning to myself.
Assignee: nobody → pbrosset
Priority: -- → P1
Whiteboard: [btpp-fix-now]
This test wasn't using refreshState, instead it used getCurrentState.
What refreshState does on top of getCurrentState is it stores the new state
locally.
getCurrentState isn't meant to be called externally, it is only meant to be
an actor method interceptor that reconstructs the missing state.
So the test should have used refreshState from the start.

Turns out it worked in most cases because the animation did have a startTime
when the test retrieved it, so it had a previousStartTime.
But, if for some reason, the animation has a startTime=null when the test
starts, then the value never gets to the front, and bad things happen.

Review commit: https://reviewboard.mozilla.org/r/45773/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/45773/
Attachment #8740440 - Flags: review?(jdescottes)
Attachment #8740440 - Flags: review?(jdescottes) → review+
Comment on attachment 8740440 [details]
MozReview Request: Bug 1206345 - Force a state update in browser_animation_reconstructState.js; r=jdescottes

https://reviewboard.mozilla.org/r/45773/#review42387

Looks good to me, thanks!
Tests are successful locally.
Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6905dd353e9
Thanks Julian. This test only fails on XP with PGO builds. But devtools mochitest don't run on try on XP-PGO. So I asked around and found this:
https://wiki.mozilla.org/ReleaseEngineering/TryChooser#What_if_I_want_PGO_for_my_build
But I don't think this is enough to run devtools test on this platform:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6dd277343986&group_state=expanded

Anyway, this patch doesn't break anything on other platforms/build types, so it's safe to land it.
What I did is I simulated the problem as I understood it locally, so I'm confident the patch will work as expected on XP-PGO.
https://hg.mozilla.org/mozilla-central/rev/f6622c3b8aa7
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.