[webkit-reviews] review denied: [Bug 123170] Keyframe animations do multiple style recalcs before starting : [Attachment 214906] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Ralph T
<ralpht+bugs at gmail.com>'s request for review:
Bug 123170: Keyframe animations do multiple style recalcs before starting
https://bugs.webkit.org/show_bug.cgi?id=123170

Attachment 214906: Patch
https://bugs.webkit.org/attachment.cgi?id=214906&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=214906&action=review


> Source/WebCore/ChangeLog:16
> +	   (WebCore::KeyframeAnimation::animate):
> +	   Reorder advancing the state machine and calling
AnimationBase::fireAnimationEventsIfNeeded. This allows AnimationBase's state
machine to advance past WaitForStyle after the first recalc. This order matches
what ImplicitAnimation::animate does for transitions.

If this is a behavior change, you should be able to make a testcase for it.

> Source/WebCore/page/animation/KeyframeAnimation.cpp:69
> +#if USE(ACCELERATED_COMPOSITING)
> +    // Force the compositor to schedule a layer flush here so that it occurs
before the
> +    // animation timer, which will be scheduled later (but after the
animation has been
> +    // added to the layer in the composited case).
> +    if (m_object && toRenderBoxModelObject(m_object)->layer())
> +	  
toRenderBoxModelObject(m_object)->layer()->compositor().scheduleLayerFlush(fals
e);
> +#endif

You can't just do this for all animations, and this seems wrong. We'll schedule
a layer flush when we push the animations to the GraphicsLayerCA, which
probably happens in the same runloop anyway.