Crash in WebCore::RenderStyle::colorIncludingFallback.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Jun 2017 19:36:45 +0000 (19:36 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Jun 2017 19:36:45 +0000 (19:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173347
<rdar://problem/32675317>

Reviewed by Chris Dumez.

Source/WebCore:

Starting an SVG image animation synchronously might trigger recursive style recalc.
We should kick off the animation on a zero timer to reduce callstack complexity.

Test: svg/as-image/svg-css-animation.html

* loader/cache/CachedImage.cpp:
(WebCore::CachedImage::didAddClient):
* platform/graphics/Image.cpp:
(WebCore::Image::Image):
(WebCore::Image::startAnimationAsynchronously):
* platform/graphics/Image.h:

LayoutTests:

* svg/animations/animated-svg-image-removed-from-document-paused.html: animations are not started synchronously anymore.
* svg/as-image/svg-css-animation-expected.txt: Added.
* svg/as-image/svg-css-animation.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@218284 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/svg/animations/animated-svg-image-removed-from-document-paused.html
LayoutTests/svg/as-image/svg-css-animation-expected.txt [new file with mode: 0644]
LayoutTests/svg/as-image/svg-css-animation.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/cache/CachedImage.cpp
Source/WebCore/platform/graphics/Image.cpp
Source/WebCore/platform/graphics/Image.h

index 53fc38f..2a40180 100644 (file)
@@ -1,3 +1,15 @@
+2017-06-14  Zalan Bujtas  <zalan@apple.com>
+
+        Crash in WebCore::RenderStyle::colorIncludingFallback.
+        https://bugs.webkit.org/show_bug.cgi?id=173347
+        <rdar://problem/32675317>
+
+        Reviewed by Chris Dumez.
+
+        * svg/animations/animated-svg-image-removed-from-document-paused.html: animations are not started synchronously anymore.
+        * svg/as-image/svg-css-animation-expected.txt: Added.
+        * svg/as-image/svg-css-animation.html: Added.
+
 2017-06-14  Matt Lewis  <jlewis3@apple.com>
 
         Fixed typo error for re-baselined editing/execCommand/strikethrough-uses-strike-tag.html.
index e054338..890e594 100644 (file)
@@ -29,12 +29,16 @@ onload = function() {
 
                 evalAndLog("document.body.appendChild(imageA)");
                 document.body.offsetWidth; // Force layout.
-                shouldBeTrue("internals.isImageAnimating(imageA)");
-                evalAndLog("document.body.appendChild(imageB)");
-                document.body.offsetWidth; // Force layout.
-                shouldBeTrue("internals.isImageAnimating(imageB)");
+                setTimeout(function() {
+                    shouldBeTrue("internals.isImageAnimating(imageA)");
 
-                finishJSTest();
+                    evalAndLog("document.body.appendChild(imageB)");
+                    document.body.offsetWidth; // Force layout.
+                    setTimeout(function() {
+                        shouldBeTrue("internals.isImageAnimating(imageB)");
+                        finishJSTest();
+                    }, 30);
+                }, 30);
             }, 30);
         }, 30);
     }, 30);
diff --git a/LayoutTests/svg/as-image/svg-css-animation-expected.txt b/LayoutTests/svg/as-image/svg-css-animation-expected.txt
new file mode 100644 (file)
index 0000000..c2541f4
--- /dev/null
@@ -0,0 +1 @@
+PASS if no crash.
diff --git a/LayoutTests/svg/as-image/svg-css-animation.html b/LayoutTests/svg/as-image/svg-css-animation.html
new file mode 100644 (file)
index 0000000..9a298db
--- /dev/null
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This test checks animated SVG image on the body when it is reattached.</title>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+</script>
+<style>
+body {
+    background-image: url('data:image/svg+xml,%3Csvg%20version%3D%221.1%22%20class%3D%22wufoo-ad%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%20xmlns%3Axlink%3D%22http%3A%2F%2Fwww.w3.org%2F1999%2Fxlink%22%20viewBox%3D%220%200%20400%20400%22%20enable-background%3D%22new%200%200%20400%20400%22%20xml%3Aspace%3D%22preserve%22%3E%0A%3Ccircle%20id%3D%22my-circle%22%20r%3D%2230%22%20cx%3D%2250%22%20cy%3D%2250%22%20fill%3D%22orange%22%20%2F%3E%0A%20%20%3Canimate%20%0A%20%20%20%20xlink%3Ahref%3D%22%23my-circle%22%0A%20%20%20%20attributeName%3D%22fill%22%0A%20%20%20%20attributeType%20%3D%20%22css%22%0A%20%20%20%20from%3D%22red%22%0A%20%20%20%20to%3D%22blue%22%20%0A%20%20%20%20dur%3D%2210s%22%0A%20%20%20%20fill%3D%22freeze%22%20%2F%3E%0A%3C%2Fsvg%3E%0A');
+}
+</style>
+</head>
+<body>PASS if no crash.</body>
+<script>
+var body = document.body;
+var root = document.body.parentNode;
+setTimeout(function() {
+       root.removeChild(body);
+       root.appendChild(body);
+    if (window.testRunner)
+           testRunner.notifyDone();
+}, 10);
+</script>
+</html>
index 28c367d..746eb0d 100644 (file)
@@ -1,3 +1,23 @@
+2017-06-14  Zalan Bujtas  <zalan@apple.com>
+
+        Crash in WebCore::RenderStyle::colorIncludingFallback.
+        https://bugs.webkit.org/show_bug.cgi?id=173347
+        <rdar://problem/32675317>
+
+        Reviewed by Chris Dumez.
+
+        Starting an SVG image animation synchronously might trigger recursive style recalc.
+        We should kick off the animation on a zero timer to reduce callstack complexity. 
+
+        Test: svg/as-image/svg-css-animation.html
+
+        * loader/cache/CachedImage.cpp:
+        (WebCore::CachedImage::didAddClient):
+        * platform/graphics/Image.cpp:
+        (WebCore::Image::Image):
+        (WebCore::Image::startAnimationAsynchronously):
+        * platform/graphics/Image.h:
+
 2017-06-14  Brady Eidson  <beidson@apple.com>
 
         WKIconLoadingDelegate never gets asked about the default favicon if touch/touch-precomposed icons are in the <head>
index b78f0ca..b2f83a8 100644 (file)
@@ -119,7 +119,7 @@ void CachedImage::didAddClient(CachedResourceClient& client)
         static_cast<CachedImageClient&>(client).imageChanged(this);
 
     if (m_image)
-        m_image->startAnimation();
+        m_image->startAnimationAsynchronously();
 
     CachedResource::didAddClient(client);
 }
index e853bfe..cf528e2 100644 (file)
@@ -47,6 +47,7 @@ namespace WebCore {
 
 Image::Image(ImageObserver* observer)
     : m_imageObserver(observer)
+    , m_animationStartTimer(*this, &Image::startAnimation)
 {
 }
 
@@ -307,6 +308,13 @@ void Image::computeIntrinsicDimensions(Length& intrinsicWidth, Length& intrinsic
     intrinsicHeight = Length(intrinsicRatio.height(), Fixed);
 }
 
+void Image::startAnimationAsynchronously()
+{
+    if (m_animationStartTimer.isActive())
+        return;
+    m_animationStartTimer.startOneShot(0_s);
+}
+
 void Image::dump(TextStream& ts) const
 {
     if (isAnimated())
index 7064303..d54c7a0 100644 (file)
@@ -35,6 +35,7 @@
 #include "ImageOrientation.h"
 #include "ImageTypes.h"
 #include "NativeImage.h"
+#include "Timer.h"
 #include <wtf/Optional.h>
 #include <wtf/RefCounted.h>
 #include <wtf/RefPtr.h>
@@ -129,6 +130,7 @@ public:
     // Animation begins whenever someone draws the image, so startAnimation() is not normally called.
     // It will automatically pause once all observers no longer want to render the image anywhere.
     virtual void startAnimation() { }
+    void startAnimationAsynchronously();
     virtual void stopAnimation() {}
     virtual void resetAnimation() {}
     virtual void imageFrameAvailableAtIndex(size_t) { }
@@ -198,6 +200,7 @@ protected:
 private:
     RefPtr<SharedBuffer> m_encodedImageData;
     ImageObserver* m_imageObserver;
+    Timer m_animationStartTimer;
 };
 
 TextStream& operator<<(TextStream&, const Image&);