1518982 - Animations with playbackRate == 0 should not be registered with the timeline

Status: RESOLVED FIXED

(Core :: DOM: Animation, enhancement, P3)

Description

[Brian Birtles (:birtles)]

As Hiro points out, given:

const anim = div.animate(..., { duration: 1000, delay: 1000 });
anim.playbackRate = 0;
anim = undefined;

Then as of bug 1518403 the animation will no longer be returned by getAnimations() but will still be registered with the AnimationTimeline since NeedsTicks() will return true since anim.playState will be "running". As a result we end up effectively leaking anim.

Note that anim.effect will NOT be registered with the EffectSet since anim should not be relevant at this point (not current or in effect).

The suggested solution is to make sure animations with playbackRate == 0 are not registered with a timeline (i.e. NeedsTicks() returns false).

Comment 1

[Brian Birtles (:birtles)]

I'm not sure how to write an automated test for this. I guess as long as no existing tests break with this change then it should be fine.

Comment 2

[Brian Birtles (:birtles)]

Let's see how this goes:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd976100753820576c63bf8364241bc251f2dfc3

Comment 3

[Brian Birtles (:birtles)]

Oops, pushed an empty patch.

Comment 4

[Brian Birtles (:birtles)]

Try again:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=66b6957ae8e56b0f0bdcacd2a74c78fb6851aeb7

Comment 5

[Brian Birtles (:birtles)]

I'm going to tentatively put this up for review because I won't be around when the try run ends but the relevant tests seem to pass locally.

Comment 6

[Brian Birtles (:birtles)]

Attachment: Bug 1518982 - Don't register animations with playbackRate == 0 with a timeline; r?hiro

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2f4920fa4ac7
Don't register animations with playbackRate == 0 with a timeline; r=hiro

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/2f4920fa4ac7