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

RESOLVED FIXED in Firefox 66

Status

()

enhancement
P3
normal
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: birtles, Assigned: birtles)

Tracking

unspecified
mozilla66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

5 months ago

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).

Assignee

Comment 1

5 months ago

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.

Assignee

Comment 3

5 months ago

Oops, pushed an empty patch.

Assignee

Comment 5

5 months ago

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.

Assignee

Updated

5 months ago
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED

Comment 7

5 months ago
Pushed by bbirtles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2f4920fa4ac7
Don't register animations with playbackRate == 0 with a timeline; r=hiro

Comment 8

5 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.