Summary: | Animated SVG images are not paused in pages loaded in the background | ||
---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> |
Component: | SVG | Assignee: | Chris Dumez <cdumez> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | barraclough, buildbot, kling, koivisto, rniwa, sabouhallawa, simon.fraser, webkit-bug-importer, zimmermann |
Priority: | P2 | Keywords: | InRadar |
Version: | WebKit Nightly Build | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=172183 | ||
Bug Depends on: | 170155 | ||
Bug Blocks: | |||
Attachments: |
Description
Chris Dumez
2017-03-23 19:58:42 PDT
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. Created attachment 305758 [details]
Patch
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 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
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 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
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 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
Created attachment 305771 [details]
Patch
Comment on attachment 305771 [details] Patch Clearing flags on attachment: 305771 Committed r214561: <http://trac.webkit.org/changeset/214561> All reviewed patches have been landed. Closing bug. 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? (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. (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? (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. (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. |