Closed
Bug 1161320
Opened 9 years ago
Closed 9 years ago
Assertion failure: "An animation in the play-pending state should have either a resolved hold time or resolved start time"
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla41
People
(Reporter: jruderman, Assigned: birtles)
References
Details
(Keywords: assertion, testcase)
Attachments
(4 files, 1 obsolete file)
Assertion failure: mHoldTime.IsNull() != mStartTime.IsNull() (An animation in the play-pending state should have either a resolved hold time or resolved start time (but not both)), at dom/animation/Animation.cpp:654
Reporter | ||
Comment 1•9 years ago
|
||
Assignee | ||
Comment 2•9 years ago
|
||
My current theory here is a conflict between zero-duration intervals and finishing behavior. It goes something like this: 1. We start the animation of zero duration and immediately hit the end and apply finishing behaviour. At this point, the start time is resolved, and the hold time is zero. 2. Later we pause the animation. This puts us in the pause-pending state. At this point, the start time is resolved, and the hold time is zero (no change). 3. Then we play the animation whilst still in the pause-pending animation (by removing the style sheet). At this point, the start time is resolved, and the hold time is zero (no change). This is because we detect the case of an aborted pause and *don't* clear the start time in this case. 4. Then, on a subsequent tick, we go to complete the play operation. We call ResumeAt and hit this assertion because both start time and hold time are set. I *think* we could also hit this case without a zero-duration interval by seeking past the end of the active interval and doing pause() followed by play(). The call to play() here would trigger finishing behaviour which would cause us to fill in the hold time. Because it is an aborted pause we also wouldn't clear the start time. I can probably make simplified test cases of the above two patterns. As for the fix, for the first case we could simply remove the assertion and I think things would behave as expected. For the second case, I'm not sure what the expected behavior should be. Should it seek back to the start of the interval? Probably it should. In that case, maybe during DoPlay we should clear the start time if we also set the hold time for an aborted pause? e.g. Change: if (!abortedPause) { mStartTime.SetNull(); } to: if (!abortedPause || !mHoldTime.IsNull()) { mStartTime.SetNull(); }
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•9 years ago
|
||
/r/8399 - Bug 1161320 - Fix conflict between finishing and aborting a pause Pull down this commit: hg pull -r acdf8b6ac0e44cde1e97cfea48c0094a8652b31c https://reviewboard-hg.mozilla.org/gecko/
Attachment #8603165 -
Flags: review?(jwatt)
Assignee | ||
Comment 4•9 years ago
|
||
Comment on attachment 8603165 [details] MozReview Request: bz://1161320/birtles /r/8399 - Bug 1161320 - Fix conflict between finishing and aborting a pause /r/8401 - Simplify test in DoPlay Pull down these commits: hg pull -r 83f980d6cbc34375274f30f6527992b4fec7f639 https://reviewboard-hg.mozilla.org/gecko/
Assignee | ||
Comment 5•9 years ago
|
||
I've updated the spec accordingly: https://github.com/w3c/web-animations/commit/1a6ddc9a52c6d3bae17d123288c3a9172c9dc4cc
Comment 6•9 years ago
|
||
Comment on attachment 8603165 [details] MozReview Request: bz://1161320/birtles https://reviewboard.mozilla.org/r/8397/#review7301
Attachment #8603165 -
Flags: review?(jwatt) → review+
https://hg.mozilla.org/mozilla-central/rev/93a79ddab910
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Assignee | ||
Comment 9•9 years ago
|
||
Attachment #8603165 -
Attachment is obsolete: true
Attachment #8620228 -
Flags: review+
Attachment #8620229 -
Flags: review+
Assignee | ||
Comment 10•9 years ago
|
||
Assignee | ||
Comment 11•9 years ago
|
||
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•