Bug 170043 – Animated SVG images are not paused in pages loaded in the background

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

[Chris Dumez]

SMIL animations inside SVG image do not get paused in background tabs.

Example:
https://pr.gg/svgExamples/smilAnimationImage.html (first animation)

The issue is that SVGImage has its own Page object (SVGImage::m_page) and this page contains the SMIL animation (not the main page). When the tab goes into the background, the main Page knows it is no longer visible and it pauses its SVG animations. However, the Page inside the SVGImage does not currently know it is no longer visible and its SVG animations keep going.

I saw this issue on Yahoo News and reduced it.

[Radar WebKit Bug Importer]

<rdar://problem/31234412>

[Chris Dumez]

I think this is the image causing trouble on Yahoo News:
https://s.yimg.com/os/yc/pt/icons/fuji-loader-v2-blue.0.svg

Seems to be used in a CSS background property.

[Chris Dumez]

Created attachment 305758 [details]
Patch

[Build Bot]

Comment on attachment 305758 [details]
Patch

Attachment 305758 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3434905

New failing tests:
svg/animations/animations-paused-in-background-page-iframe.html
svg/animations/animations-paused-in-background-page.html

[Build Bot]

Created attachment 305765 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 305758 [details]
Patch

Attachment 305758 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3434904

New failing tests:
svg/animations/animations-paused-in-background-page-iframe.html
svg/animations/animations-paused-in-background-page.html

[Build Bot]

Created attachment 305768 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 305758 [details]
Patch

Attachment 305758 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3434969

New failing tests:
svg/animations/animations-paused-in-background-page-iframe.html
svg/animations/animations-paused-in-background-page.html

[Build Bot]

Created attachment 305770 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Chris Dumez]

Created attachment 305771 [details]
Patch

[Chris Dumez]

Comment on attachment 305771 [details]
Patch

Clearing flags on attachment: 305771

Committed r214561: <http://trac.webkit.org/changeset/214561>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Said Abou-Hallawa]

Comment on attachment 305771 [details]
Patch

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

> LayoutTests/svg/animations/animations-paused-in-background-page-expected.txt:10
> +Setting page visibility to hidden
> +PASS internals.isImageAnimating(testImage) became false
> +Setting page visibility to visible
> +PASS internals.isImageAnimating(testImage) became false

These expected results do not seem to be correct. The image is not animating regardless whether the page is visible or not.

> LayoutTests/svg/animations/animations-paused-in-background-page.html:14
> +    if (window.testRunner)
> +        testRunner.setPageVisibility('visible');
> +    shouldBecomeEqual("internals.isImageAnimating(testImage)", "false", finishJSTest);

How can this happen? The page visibility is changed to 'visible' but the image is still not animating?

[Chris Dumez]

(In reply to Said Abou-Hallawa from comment #13)
> Comment on attachment 305771 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=305771&action=review
> 
> > LayoutTests/svg/animations/animations-paused-in-background-page-expected.txt:10
> > +Setting page visibility to hidden
> > +PASS internals.isImageAnimating(testImage) became false
> > +Setting page visibility to visible
> > +PASS internals.isImageAnimating(testImage) became false
> 
> These expected results do not seem to be correct. The image is not animating
> regardless whether the page is visible or not.
> 
> > LayoutTests/svg/animations/animations-paused-in-background-page.html:14
> > +    if (window.testRunner)
> > +        testRunner.setPageVisibility('visible');
> > +    shouldBecomeEqual("internals.isImageAnimating(testImage)", "false", finishJSTest);
> 
> How can this happen? The page visibility is changed to 'visible' but the
> image is still not animating?

This was already fixed in http://trac.webkit.org/r214626.

[Said Abou-Hallawa]

(In reply to Chris Dumez from comment #14)
> (In reply to Said Abou-Hallawa from comment #13)
> > Comment on attachment 305771 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=305771&action=review
> > 
> > > LayoutTests/svg/animations/animations-paused-in-background-page-expected.txt:10
> > > +Setting page visibility to hidden
> > > +PASS internals.isImageAnimating(testImage) became false
> > > +Setting page visibility to visible
> > > +PASS internals.isImageAnimating(testImage) became false
> > 
> > These expected results do not seem to be correct. The image is not animating
> > regardless whether the page is visible or not.
> > 
> > > LayoutTests/svg/animations/animations-paused-in-background-page.html:14
> > > +    if (window.testRunner)
> > > +        testRunner.setPageVisibility('visible');
> > > +    shouldBecomeEqual("internals.isImageAnimating(testImage)", "false", finishJSTest);
> > 
> > How can this happen? The page visibility is changed to 'visible' but the
> > image is still not animating?
> 
> This was already fixed in http://trac.webkit.org/r214626.

How this test was passing sometimes? In http://trac.webkit.org/r214626, you just switched the expectation from false to true. Will this new change make the test flaky but because of the opposite reason?

[Said Abou-Hallawa]

(In reply to Said Abou-Hallawa from comment #15)
> (In reply to Chris Dumez from comment #14)
> > (In reply to Said Abou-Hallawa from comment #13)
> > > Comment on attachment 305771 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=305771&action=review
> > > 
> > > > LayoutTests/svg/animations/animations-paused-in-background-page-expected.txt:10
> > > > +Setting page visibility to hidden
> > > > +PASS internals.isImageAnimating(testImage) became false
> > > > +Setting page visibility to visible
> > > > +PASS internals.isImageAnimating(testImage) became false
> > > 
> > > These expected results do not seem to be correct. The image is not animating
> > > regardless whether the page is visible or not.
> > > 
> > > > LayoutTests/svg/animations/animations-paused-in-background-page.html:14
> > > > +    if (window.testRunner)
> > > > +        testRunner.setPageVisibility('visible');
> > > > +    shouldBecomeEqual("internals.isImageAnimating(testImage)", "false", finishJSTest);
> > > 
> > > How can this happen? The page visibility is changed to 'visible' but the
> > > image is still not animating?
> > 
> > This was already fixed in http://trac.webkit.org/r214626.
> 
> How this test was passing sometimes? In http://trac.webkit.org/r214626, you
> just switched the expectation from false to true. Will this new change make
> the test flaky but because of the opposite reason?

Never mind I just realized http://trac.webkit.org/r214626 is not the fix of the flatness of https://bugs.webkit.org/show_bug.cgi?id=172183 and it was landed 7 weeks ago.

[Chris Dumez]

(In reply to Said Abou-Hallawa from comment #15)
> (In reply to Chris Dumez from comment #14)
> > (In reply to Said Abou-Hallawa from comment #13)
> > > Comment on attachment 305771 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=305771&action=review
> > > 
> > > > LayoutTests/svg/animations/animations-paused-in-background-page-expected.txt:10
> > > > +Setting page visibility to hidden
> > > > +PASS internals.isImageAnimating(testImage) became false
> > > > +Setting page visibility to visible
> > > > +PASS internals.isImageAnimating(testImage) became false
> > > 
> > > These expected results do not seem to be correct. The image is not animating
> > > regardless whether the page is visible or not.
> > > 
> > > > LayoutTests/svg/animations/animations-paused-in-background-page.html:14
> > > > +    if (window.testRunner)
> > > > +        testRunner.setPageVisibility('visible');
> > > > +    shouldBecomeEqual("internals.isImageAnimating(testImage)", "false", finishJSTest);
> > > 
> > > How can this happen? The page visibility is changed to 'visible' but the
> > > image is still not animating?
> > 
> > This was already fixed in http://trac.webkit.org/r214626.
> 
> How this test was passing sometimes? In http://trac.webkit.org/r214626, you
> just switched the expectation from false to true. Will this new change make
> the test flaky but because of the opposite reason?

There is a small delay between the point the visibility is set and the image actually starts / stops animating.

Previously, the test was sometimes passing because:
1. The page is initially hidden
2. We ask for the page to become visible
3. We poll until the image stops animating

Step 3 will sometimes find that the image is not animating because the page visibility has not been taken into consideration yet.

Since the page is initially hidden and then becomes visible, then the image's state will go from Non-animating to Animating.

If in step 3, we check that the image starts animating (Which is what we really wanted to test and what I did in the follow-up patch), then it will NOT be flaky in the other direction.

The image goes from Non-animating to Animating, and once it is animating, it keeps animating.