Bug 227720 – Make sure SVG SMIL animations do not run in processes without any pages

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Chris Dumez]

Make sure SVG SMIL animations do not run in processes without any pages.

We have seen traces / logging indicating that SVG SMIL animations may run in cached WebProcesses, thus using CPU unnecessarily. Cached WebProcesses have no WebPage so this patch adds assertions to catch cases where those animations run when there is no Page (and early returns in release to avoid doing unnecessary work).

[Chris Dumez]

Created attachment 432971 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 432971 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Make sure SVG SMIL animations do not run in processes without any pages.

Perhaps we should instead ensure that SMIL animations don't run in cached SVGImages (or those that don't have a connection to a live Page)? The approach in this patch seems a bit indirect.

> Source/WebCore/ChangeLog:10
> +        We have seen traces / logging indicating that SVG SMIL animations may run in cached WebProcesses, thus

Would be nice if I could find them via a radar chain referenced from this changelog.

[Chris Dumez]

(In reply to Simon Fraser (smfr) from comment #2)
> Comment on attachment 432971 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=432971&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        Make sure SVG SMIL animations do not run in processes without any pages.
> 
> Perhaps we should instead ensure that SMIL animations don't run in cached
> SVGImages (or those that don't have a connection to a live Page)? The
> approach in this patch seems a bit indirect.

I made the change to SMILTimeContainer because this is the code we see running in the traces when the process is cached (no WebPage), on a recurring timer. CachedImage already has logic to stop animations when it no longer has any clients.
I don't know where the logic bug is but adding the assertions in SMILTimeContainer seems to me like the surest way to find it?

> > Source/WebCore/ChangeLog:10
> > +        We have seen traces / logging indicating that SVG SMIL animations may run in cached WebProcesses, thus
> 
> Would be nice if I could find them via a radar chain referenced from this
> changelog.

Oh indeed, this is a fix for <rdar://79625708>. I'll update the changelog.

[Chris Dumez]

<rdar://79625708>

[Chris Dumez]

Created attachment 433158 [details]
Patch

[EWS]

Committed r279768 (239538@main): <https://commits.webkit.org/239538@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433158 [details].