CrashTracer: com.apple.WebKit.WebContent at WebCore: WebCore::Document::updateStyleIf...
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Jan 2018 19:46:38 +0000 (19:46 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Jan 2018 19:46:38 +0000 (19:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=182299
<rdar://problem/36853088>

Reviewed by Simon Fraser.

Source/WebCore:

Mostly speculative fix for the case where a scrollTo moves an
animated SVG image into view, causing its animation to restart during
a paint operation. This was causing a release ASSERT, so we now defer
the resumption of the animation into a timer.

Test: svg/animated-svgImage-scroll.html

* rendering/RenderElement.cpp:
(WebCore::RenderElement::repaintForPausedImageAnimationsIfNeeded): Enqueue the
animation if it is an SVGImage.
* svg/graphics/SVGImage.cpp: Add a timer to enqueue animation starts.
(WebCore::SVGImage::SVGImage):
(WebCore::SVGImage::startAnimationTimerFired):
(WebCore::SVGImage::enqueueStartAnimation):
(WebCore::SVGImage::stopAnimation):
* svg/graphics/SVGImage.h:

LayoutTests:

* svg/animated-svgImage-scroll-expected.txt: Added.
* svg/animated-svgImage-scroll.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/svg/animated-svgImage-scroll-expected.txt [new file with mode: 0644]
LayoutTests/svg/animated-svgImage-scroll.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/svg/graphics/SVGImage.cpp
Source/WebCore/svg/graphics/SVGImage.h

index ac5df43..99f730b 100644 (file)
@@ -1,3 +1,14 @@
+2018-01-30  Dean Jackson  <dino@apple.com>
+
+        CrashTracer: com.apple.WebKit.WebContent at WebCore: WebCore::Document::updateStyleIfNeeded
+        https://bugs.webkit.org/show_bug.cgi?id=182299
+        <rdar://problem/36853088>
+
+        Reviewed by Simon Fraser.
+
+        * svg/animated-svgImage-scroll-expected.txt: Added.
+        * svg/animated-svgImage-scroll.html: Added.
+
 2018-01-30  Frederic Wang  <fwang@igalia.com>
 
         Unreviewed test gardening.
diff --git a/LayoutTests/svg/animated-svgImage-scroll-expected.txt b/LayoutTests/svg/animated-svgImage-scroll-expected.txt
new file mode 100644 (file)
index 0000000..8dd7aad
--- /dev/null
@@ -0,0 +1,8 @@
+Before text
+
+Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
+
+Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
+
+
+After text
diff --git a/LayoutTests/svg/animated-svgImage-scroll.html b/LayoutTests/svg/animated-svgImage-scroll.html
new file mode 100644 (file)
index 0000000..47a2b58
--- /dev/null
@@ -0,0 +1,41 @@
+<style>
+    #container {
+        overflow: scroll;
+        width: 300px;
+        height: 200px;
+    }
+    #expander {
+        background-color: blue;
+        height: 500px;
+    }
+</style>
+<p>Before text</p>
+<div id="container">
+    <p>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+    <p>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+    <img src="resources/animated.svg" width="200" height="100"></img>
+    <p id="expander"></p>
+</div>
+<p>After text</p>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+function run() {
+    let container = document.getElementById("container");
+    container.scrollTo(0, 1000);
+    setTimeout(function () {
+        let expander = document.getElementById("expander");
+        expander.style.height = "1px";
+        if (window.testRunner) {
+            setTimeout(function () {
+                testRunner.notifyDone();
+            }, 0);
+        }
+    }, 0);
+}
+
+window.addEventListener("load", run, false);
+</script>
\ No newline at end of file
index de27d88..f091944 100644 (file)
@@ -1,3 +1,28 @@
+2018-01-30  Dean Jackson  <dino@apple.com>
+
+        CrashTracer: com.apple.WebKit.WebContent at WebCore: WebCore::Document::updateStyleIfNeeded
+        https://bugs.webkit.org/show_bug.cgi?id=182299
+        <rdar://problem/36853088>
+
+        Reviewed by Simon Fraser.
+
+        Mostly speculative fix for the case where a scrollTo moves an
+        animated SVG image into view, causing its animation to restart during
+        a paint operation. This was causing a release ASSERT, so we now defer
+        the resumption of the animation into a timer.
+
+        Test: svg/animated-svgImage-scroll.html
+
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::repaintForPausedImageAnimationsIfNeeded): Enqueue the
+        animation if it is an SVGImage.
+        * svg/graphics/SVGImage.cpp: Add a timer to enqueue animation starts.
+        (WebCore::SVGImage::SVGImage):
+        (WebCore::SVGImage::startAnimationTimerFired):
+        (WebCore::SVGImage::enqueueStartAnimation):
+        (WebCore::SVGImage::stopAnimation):
+        * svg/graphics/SVGImage.h:
+
 2018-01-30  Chris Dumez  <cdumez@apple.com>
 
         Service worker registration soft updates happen too frequently
index d14afaa..aac3502 100644 (file)
@@ -68,6 +68,7 @@
 #include "RenderTheme.h"
 #include "RenderTreeBuilder.h"
 #include "RenderView.h"
+#include "SVGImage.h"
 #include "SVGRenderSupport.h"
 #include "Settings.h"
 #include "ShadowRoot.h"
@@ -1468,8 +1469,14 @@ bool RenderElement::repaintForPausedImageAnimationsIfNeeded(const IntRect& visib
 
     repaint();
 
-    if (auto* image = cachedImage.image())
-        image->startAnimation();
+    if (auto* image = cachedImage.image()) {
+        // SVGImages might cause a layout when starting an animation, so
+        // schedule them rather than start immediately.
+        if (is<SVGImage>(image))
+            downcast<SVGImage>(image)->scheduleStartAnimation();
+        else
+            image->startAnimation();
+    }
 
     // For directly-composited animated GIFs it does not suffice to call repaint() to resume animation. We need to mark the image as changed.
     if (is<RenderBoxModelObject>(*this))
index 6c0d9de..d76e71e 100644 (file)
@@ -69,6 +69,7 @@ namespace WebCore {
 
 SVGImage::SVGImage(ImageObserver& observer)
     : Image(&observer)
+    , m_startAnimationTimer(*this, &SVGImage::startAnimationTimerFired)
 {
 }
 
@@ -377,6 +378,19 @@ void SVGImage::computeIntrinsicDimensions(Length& intrinsicWidth, Length& intrin
         intrinsicRatio = FloatSize(floatValueForLength(intrinsicWidth, 0), floatValueForLength(intrinsicHeight, 0));
 }
 
+void SVGImage::startAnimationTimerFired()
+{
+    startAnimation();
+}
+
+void SVGImage::scheduleStartAnimation()
+{
+    auto rootElement = this->rootElement();
+    if (!rootElement || !rootElement->animationsPaused())
+        return;
+    m_startAnimationTimer.startOneShot(0_s);
+}
+
 void SVGImage::startAnimation()
 {
     auto rootElement = this->rootElement();
@@ -388,6 +402,7 @@ void SVGImage::startAnimation()
 
 void SVGImage::stopAnimation()
 {
+    m_startAnimationTimer.stop();
     auto rootElement = this->rootElement();
     if (!rootElement)
         return;
index 88f7627..87e2823 100644 (file)
@@ -60,6 +60,8 @@ public:
     void resetAnimation() final;
     bool isAnimating() const final;
 
+    void scheduleStartAnimation();
+
 #if USE(CAIRO)
     NativeImagePtr nativeImageForCurrentFrame(const GraphicsContext* = nullptr) final;
 #endif
@@ -89,6 +91,8 @@ private:
     // FIXME: Implement this to be less conservative.
     bool currentFrameKnownToBeOpaque() const final { return false; }
 
+    void startAnimationTimerFired();
+
     explicit SVGImage(ImageObserver&);
     ImageDrawResult draw(GraphicsContext&, const FloatRect& fromRect, const FloatRect& toRect, CompositeOperator, BlendMode, DecodingMode, ImageOrientationDescription) final;
     ImageDrawResult drawForContainer(GraphicsContext&, const FloatSize containerSize, float containerZoom, const URL& initialFragmentURL, const FloatRect& dstRect, const FloatRect& srcRect, CompositeOperator, BlendMode);
@@ -100,6 +104,8 @@ private:
     std::unique_ptr<SVGImageChromeClient> m_chromeClient;
     std::unique_ptr<Page> m_page;
     FloatSize m_intrinsicSize;
+
+    Timer m_startAnimationTimer;
 };
 
 bool isInSVGImage(const Element*);