[webkit-reviews] review granted: [Bug 192604] [Web Animations] CSSAnimationController should be created lazily : [Attachment 357125] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Antoine Quint <graouts at apple.com>'s
request for review:
Bug 192604: [Web Animations] CSSAnimationController should be created lazily
https://bugs.webkit.org/show_bug.cgi?id=192604

Attachment 357125: Patch

https://bugs.webkit.org/attachment.cgi?id=357125&action=review




--- Comment #12 from Dean Jackson <dino at apple.com> ---
Comment on attachment 357125
  --> https://bugs.webkit.org/attachment.cgi?id=357125
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=357125&action=review

> Source/WebCore/ChangeLog:8
> +	   Frame used to expose an animation() method that returned a reference
as it created its CSSAnimationController at creation time.

improve wording - "created its controller at creation time"

> Source/WebCore/ChangeLog:11
> +	   We add a new Frame::existingAnimationController() method which
returns a pointer and Frame::animationController() which creates
> +	   a RefPtr<CSSAnimationController> lazily. We then replace the vast
majority of calls to Frame::animation() to
Frame::existingAnimationController().
> +	   This matches the pattern used in the Web Animations codepath to
access the DocumentTimeline.

What do we gain by doing this?

> Source/WebCore/page/FrameView.cpp:629
> +    if
(!RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled()
) {
> +	   if (auto* animationController =
frame().existingAnimationController())
> +	       ASSERT(!animationController->hasAnimations());
> +    }

Put #if DEBUG around all this since you only end up with an ASSERT. The
compiler probably removes it all anyway, but you might as well do it.