[webkit-reviews] review granted: [Bug 209370] REGRESSION: CSS animations inside an embedded SVG image do not animate : [Attachment 395907] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 209370: REGRESSION: CSS animations inside an embedded SVG image do not
animate
https://bugs.webkit.org/show_bug.cgi?id=209370

Attachment 395907: Patch

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




--- Comment #36 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 395907
  --> https://bugs.webkit.org/attachment.cgi?id=395907
Patch

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

> Source/WebCore/loader/cache/CachedResourceLoader.h:121
> +    Vector<SVGImage*> allCachedSVGImages() const;

I think this should return Vector<Ref<SVGImage>>

> Source/WebCore/page/ChromeClient.h:319
> +    virtual bool scheduleTimedRenderingUpdate() { return false; }

Why do we need to expose the "timed" version here?

> Source/WebCore/page/Page.cpp:1370
> +	   for (auto* image :
document.cachedResourceLoader().allCachedSVGImages()) {

This becomes for (auto image : ...

> Source/WebCore/svg/graphics/SVGImage.h:72
> +    Page* page() { return m_page.get(); }

Because this is so special, I would call it internalPage() or something. It's
not the Page that the image lives in.