Changeset 218284 in webkit

Timestamp:

Jun 14, 2017 12:36:45 PM (7 years ago)

Author:

Alan Bujtas

Message:

Crash in WebCore::RenderStyle::colorIncludingFallback.
​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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/svg/animations/animated-svg-image-removed-from-document-paused.html (modified) (1 diff)
• LayoutTests/svg/as-image/svg-css-animation-expected.txt (added)
• LayoutTests/svg/as-image/svg-css-animation.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/cache/CachedImage.cpp (modified) (1 diff)
• Source/WebCore/platform/graphics/Image.cpp (modified) (2 diffs)
• Source/WebCore/platform/graphics/Image.h (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/svg/animations/animated-svg-image-removed-from-document-paused.html

@@ -30 +30 @@
                 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);

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/loader/cache/CachedImage.cpp

@@ -120 +120 @@
 
     if (m_image)
-        m_image->startAnimation();
+        m_image->startAnimationAsynchronously();
 
     CachedResource::didAddClient(client);

trunk/Source/WebCore/platform/graphics/Image.cpp

@@ -48 +48 @@
 Image::Image(ImageObserver* observer)
     : m_imageObserver(observer)
+    , m_animationStartTimer(*this, &Image::startAnimation)
 {
 }
@@ -308 +309 @@
 }
 
+void Image::startAnimationAsynchronously()
+{
+    if (m_animationStartTimer.isActive())
+        return;
+    m_animationStartTimer.startOneShot(0_s);
+}
+
 void Image::dump(TextStream& ts) const
 {

trunk/Source/WebCore/platform/graphics/Image.h

@@ -36 +36 @@
 #include "ImageTypes.h"
 #include "NativeImage.h"
+#include "Timer.h"
 #include <wtf/Optional.h>
 #include <wtf/RefCounted.h>
@@ -130 +131 @@
     // 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() {}
@@ -199 +201 @@
     RefPtr<SharedBuffer> m_encodedImageData;
     ImageObserver* m_imageObserver;
+    Timer m_animationStartTimer;
 };