Closed Bug 1134381 Opened 5 years ago Closed 5 years ago

Skip pausing if an animation is already paused

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: birtles, Assigned: birtles)

Details

Attachments

(1 file, 1 obsolete file)

Currently when we update CSS animations we build up a list of animations ("players" actually) and call play on them all. Then we compare the list of new players against the old list of players. When we find a match, we copy the (possibly updated) timing and keyframes from the new to the old and drop the new.

For many updates, however, we'll end up dropping most or all of the new players because they all match. Calling play on these objects only to drop them not only makes debugging confusing it's also a bit wasteful now that we end up putting such players in the "pending players" hashmap only to remove them from that hashmap a moment later.

We should probably defer calling play on those new players until we know we're going to use them.
While we're looking into this, I wonder if we can now add an assertion to the dtor for CSSAnimationPlayer/CSSTransitionPlayer that it has been cancelled (e.g. by checking if it is in the idle state). That, however, won't work if we allow these objects to be constructed by script as well.
After looking into this, I don't think we actually need anything to do here. What happens is that we create a list of new players, compare it to the old players, copy over the start times / play state from the old players to the new players, then swap the new players for the old players. Hence, we really do need to play the players unless we totally rework the animation building to update animations in place.

Also, we won't add animations to the pending players to the pending players hashmap if they are already playing since AnimationPlayer::DoPlay returns early if the player is already playing.

What we do need to do, however, is apply the same treatment to pausing such that we ignore a pause if we are already paused. The spec needs to be updated to do this too.
(In reply to Brian Birtles (:birtles) from comment #2)
> The spec needs to be updated to do this too.

Fixed: https://github.com/w3c/web-animations/commit/98ebcbb6ed85df70c73c3648db7cc95200c58d30
Hijacking this bug to be just about the pause case
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
Summary: Don't play animations until we know they are going to be used → Skip pausing if an animation is already paused
Attachment #8591492 - Flags: review?(jwatt)
Attachment #8591492 - Attachment is obsolete: true
Attachment #8591492 - Flags: review?(jwatt)
Attachment #8591503 - Flags: review?(jwatt) → review+
https://hg.mozilla.org/mozilla-central/rev/2ae6b31b1eb2
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.