1127157 - Better document and perhaps rename AnimationPlayer::CanThrottle()

Status: NEW

(Core :: DOM: Core & HTML, defect)

Description

[Jonathan Watt [:jwatt]]

I find the AnimationPlayer::CanThrottle() function confusing. If I understand correctly, it's not so much throttling (as in, reducing the frequency of) the animated style updates, but rather is skipping them altogether until some future point when they actually have to happen.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: proposed change

Brian, what do you think of this? If this seems good to you I guess I should probably change the names of EnsureStyleRule_IsNotThrottled/EnsureStyleRule_IsThrottled (replacing Throttled with Skipped). Possibly also AnimationPlayerCollection's CanThrottleTransformChanges/CanThrottleAnimation (not sure yet to what extent they're related)?

Comment 2

[Brian Birtles (:birtles)]

(In reply to Jonathan Watt [:jwatt] from comment #1)
> Created attachment 8556217 [details] [diff] [review]
> proposed change
> 
> Brian, what do you think of this?

It makes sense to me, but I'd like to get David's opinion too.

Comment 3

[Brian Birtles (:birtles)]

Also, Jonathan, did you see bug 1084220? We should fix the naming there too.

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Seems fine with me, as long as the term is fully replaced, and we don't still have a bunch of things using "Throttle" to mean the same thing.  (I think that covers every use of "throttle" in layout/style/* and layout/base/{RestyleManager,nsPresContext,nsDisplayList}.*, **some** (all but the last) of the uses in nsPresShell.cpp, and **none** of the uses in nsRefreshDriver.)

Comment 5

[Brian Birtles (:birtles)]

Comment on attachment 8556217 [details] [diff] [review]
proposed change

Review of attachment 8556217 [details] [diff] [review]:
-----------------------------------------------------------------

Everything looks fine but we should fix all the references pointed out in comment 4 (and comment 3). Clearing review request for now since I should probably have another look after all those other references have been adjusted.