1415457 - Avoid a redundant request restyle when a CSS animation is finished by calling finish()

Status: NEW

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

Description

[Hiroyuki Ikezoe (:hiro)]

Conformant Promise handling (bug 1193394) noticed me that when we call finish() for CSS animations (and transitions I guess), we request restyle twice in two different ticks.

One comes from Animation::Finish() itself;

 KeyframeEffectReadOnly::RequestRestyle()
 Animation::UpdateTiming()
 Animation::Finish()

The other comes from PostUpdate() in Animation::Tick(). The Animation::Tick() was called for animation events dispatching in the next tick, IIUC.

I think we can avoid the latter, actually it's not a big problem though.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #0)

> The other comes from PostUpdate() in Animation::Tick(). The
> Animation::Tick() was called for animation events dispatching in the next
> tick, IIUC.
> 
> I think we can avoid the latter, actually it's not a big problem though.

Today, I did investigate this again (honestly I did forget that I've already filed this bug.).

Anyway, how the second restyle request happens is;

1) In a microtask for Animation.ready, when Finish() is called, Finish() ends up calling KeyframeEffectReadOnly::UpdateTargetRegistration() and dropping EffectSet if there is no other animations on the same element
2) In the restyling process on the same tick when Finish() was done, EffectCompositor::PreTraverseInSubtree() is called
3) In PreTraverseInSubtree() we try to call WillCompose() but we skipped it since we don't have the valid EffectSet any more
4) As a result of 3), mFinishedAtLastComposeStyle is not updated
5) Then, in Animation::Tick() on the next tick, we call PostUpdate() because we think we hasn't composed the last style

Though I am not sure there are visually problems that causes by this, I haven't try to write such test cases yet (I mean test cases causing visually faults).