Title: [127474] trunk
Revision
127474
Author
[email protected]
Date
2012-09-04 11:31:31 -0700 (Tue, 04 Sep 2012)

Log Message

ASSERTion failure when SVG element is removed from document and readded
https://bugs.webkit.org/show_bug.cgi?id=95517
<rdar://problem/12175583>

Reviewed by Brady Eidson.

Previously, SVG animations would cease to animate when their parent
<svg> element was removed and re-added to the document.

Instead, to match Firefox and Opera, we should continue the animation
with the same beginTime (i.e. the animation continues as if it had never
been removed from the document).

Test: svg/animations/reinserting-svg-into-document.html

* svg/SVGSVGElement.cpp:
(WebCore::SVGSVGElement::insertedInto): Don't call begin() on an already-started SMILTimeContainer().
* svg/animation/SMILTimeContainer.cpp:
(WebCore::SMILTimeContainer::isStarted): Added.
* svg/animation/SMILTimeContainer.h: Add isStarted().
* svg/animation/SVGSMILElement.cpp:
(WebCore::SVGSMILElement::insertedInto): Always reschedule SVGSMILElements with
their parent SMILTimeContainer when they are inserted into the document, otherwise
they are lost (and never again update) when their subtree is removed and then
readded to the document.

Add a test that ensures that removing an SVG element from the document
and readding it continues SMIL animations.

* svg/animations/reinserting-svg-into-document.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (127473 => 127474)


--- trunk/LayoutTests/ChangeLog	2012-09-04 18:08:19 UTC (rev 127473)
+++ trunk/LayoutTests/ChangeLog	2012-09-04 18:31:31 UTC (rev 127474)
@@ -1,3 +1,16 @@
+2012-09-04  Tim Horton  <[email protected]>
+
+        ASSERTion failure when SVG element is removed from document and readded
+        https://bugs.webkit.org/show_bug.cgi?id=95517
+        <rdar://problem/12175583>
+
+        Reviewed by Brady Eidson.
+
+        Add a test that ensures that removing an SVG element from the document
+        and readding it continues SMIL animations.
+
+        * svg/animations/reinserting-svg-into-document.html: Added.
+
 2012-09-04  Vincent Scheib  <[email protected]>
 
         [Chromium] Allow asyncronous response of pointer lock requests in layout tests.

Added: trunk/LayoutTests/svg/animations/reinserting-svg-into-document-expected.txt (0 => 127474)


--- trunk/LayoutTests/svg/animations/reinserting-svg-into-document-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/svg/animations/reinserting-svg-into-document-expected.txt	2012-09-04 18:31:31 UTC (rev 127474)
@@ -0,0 +1,5 @@
+Reinserting SVG animation into document should continue the animation
+
+PASS rect.x.animVal.value is 30
+PASS rect.x.animVal.value is 60
+

Added: trunk/LayoutTests/svg/animations/reinserting-svg-into-document.html (0 => 127474)


--- trunk/LayoutTests/svg/animations/reinserting-svg-into-document.html	                        (rev 0)
+++ trunk/LayoutTests/svg/animations/reinserting-svg-into-document.html	2012-09-04 18:31:31 UTC (rev 127474)
@@ -0,0 +1,43 @@
+<html>
+<script src=""
+<script src=""
+<script>
+function load() {
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+    }
+
+    svg = document.getElementById("svg");
+    rect = document.getElementById("rect");
+
+    setTimeout(function () {
+        svg.setCurrentTime(1);
+
+        document.body.removeChild(svg);
+        document.body.appendChild(svg);
+
+        setTimeout(function () {
+            shouldBeCloseEnough("rect.x.animVal.value", "30", 1);
+
+            svg.setCurrentTime(2);
+
+            shouldBeCloseEnough("rect.x.animVal.value", "60", 1);
+
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }, 0);
+    }, 0);
+};
+</script>
+<body _onload_="load()">
+    <h1>Reinserting SVG animation into document should continue the animation</h1>
+    <p id="description"></p>
+    <div id="console"></div>
+    <svg id="svg" xmlns="http://www.w3.org/2000/svg">
+        <rect id="rect" x="0" y="0" width="20" height="20">
+            <animate attributeName="x" begin="0" from="0" to="90" dur="3s" fill="freeze" />
+        </rect>
+    </svg>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (127473 => 127474)


--- trunk/Source/WebCore/ChangeLog	2012-09-04 18:08:19 UTC (rev 127473)
+++ trunk/Source/WebCore/ChangeLog	2012-09-04 18:31:31 UTC (rev 127474)
@@ -1,3 +1,31 @@
+2012-09-04  Tim Horton  <[email protected]>
+
+        ASSERTion failure when SVG element is removed from document and readded
+        https://bugs.webkit.org/show_bug.cgi?id=95517
+        <rdar://problem/12175583>
+
+        Reviewed by Brady Eidson.
+
+        Previously, SVG animations would cease to animate when their parent
+        <svg> element was removed and re-added to the document.
+
+        Instead, to match Firefox and Opera, we should continue the animation
+        with the same beginTime (i.e. the animation continues as if it had never
+        been removed from the document).
+
+        Test: svg/animations/reinserting-svg-into-document.html
+
+        * svg/SVGSVGElement.cpp:
+        (WebCore::SVGSVGElement::insertedInto): Don't call begin() on an already-started SMILTimeContainer().
+        * svg/animation/SMILTimeContainer.cpp: 
+        (WebCore::SMILTimeContainer::isStarted): Added.
+        * svg/animation/SMILTimeContainer.h: Add isStarted().
+        * svg/animation/SVGSMILElement.cpp:
+        (WebCore::SVGSMILElement::insertedInto): Always reschedule SVGSMILElements with
+        their parent SMILTimeContainer when they are inserted into the document, otherwise
+        they are lost (and never again update) when their subtree is removed and then
+        readded to the document.
+
 2012-09-04  Andrei Bucur  <[email protected]>
 
         [CSS Regions] Destroying a render named flow thread without unregistering left-over content nodes triggered an assertion.

Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeNodeMac.mm (127473 => 127474)


--- trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeNodeMac.mm	2012-09-04 18:08:19 UTC (rev 127473)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeNodeMac.mm	2012-09-04 18:31:31 UTC (rev 127474)
@@ -70,15 +70,22 @@
     if (state->changedProperties() & (ScrollingTreeState::ScrollLayer | ScrollingTreeState::ContentsSize | ScrollingTreeState::ViewportRect))
         updateMainFramePinState(scrollPosition());
 
-    if ((state->changedProperties() & ScrollingTreeState::ShouldUpdateScrollLayerPositionOnMainThread) && shouldUpdateScrollLayerPositionOnMainThread()) {
-        // We're transitioning to the slow "update scroll layer position on the main thread" mode.
-        // Initialize the probable main thread scroll position with the current scroll layer position.
-        if (state->changedProperties() & ScrollingTreeState::RequestedScrollPosition)
-            m_probableMainThreadScrollPosition = state->requestedScrollPosition();
-        else {
-            CGPoint scrollLayerPosition = m_scrollLayer.get().position;
-            m_probableMainThreadScrollPosition = IntPoint(-scrollLayerPosition.x, -scrollLayerPosition.y);
+    if ((state->changedProperties() & ScrollingTreeState::ShouldUpdateScrollLayerPositionOnMainThread)) {
+        bool updateScrollLayerPositionOnMainThread = shouldUpdateScrollLayerPositionOnMainThread();
+        
+        if (updateScrollLayerPositionOnMainThread) {
+            // We're transitioning to the slow "update scroll layer position on the main thread" mode.
+            // Initialize the probable main thread scroll position with the current scroll layer position.
+            if (state->changedProperties() & ScrollingTreeState::RequestedScrollPosition)
+                m_probableMainThreadScrollPosition = state->requestedScrollPosition();
+            else {
+                CGPoint scrollLayerPosition = m_scrollLayer.get().position;
+                m_probableMainThreadScrollPosition = IntPoint(-scrollLayerPosition.x, -scrollLayerPosition.y);
+            }
         }
+        
+        if (scrollingTree()->scrollingPeformanceLoggingEnabled())
+            printf("SCROLLING: Switching to %s scrolling mode. Time: %f\n", updateScrollLayerPositionOnMainThread ? "slow" : "fast", WTF::monotonicallyIncreasingTime());
     }
 }
 

Modified: trunk/Source/WebCore/svg/SVGSVGElement.cpp (127473 => 127474)


--- trunk/Source/WebCore/svg/SVGSVGElement.cpp	2012-09-04 18:08:19 UTC (rev 127473)
+++ trunk/Source/WebCore/svg/SVGSVGElement.cpp	2012-09-04 18:31:31 UTC (rev 127474)
@@ -480,7 +480,7 @@
         // Animations are started at the end of document parsing and after firing the load event,
         // but if we miss that train (deferred programmatic element insertion for example) we need
         // to initialize the time container here.
-        if (!document()->parsing() && !document()->processingLoadEvent() && document()->loadEventFinished())
+        if (!document()->parsing() && !document()->processingLoadEvent() && document()->loadEventFinished() && !timeContainer()->isStarted())
             timeContainer()->begin();
     }
     return SVGStyledLocatableElement::insertedInto(rootParent);

Modified: trunk/Source/WebCore/svg/animation/SMILTimeContainer.cpp (127473 => 127474)


--- trunk/Source/WebCore/svg/animation/SMILTimeContainer.cpp	2012-09-04 18:08:19 UTC (rev 127473)
+++ trunk/Source/WebCore/svg/animation/SMILTimeContainer.cpp	2012-09-04 18:31:31 UTC (rev 127474)
@@ -84,6 +84,11 @@
     return m_pauseTime;
 }
 
+bool SMILTimeContainer::isStarted() const
+{
+    return m_beginTime;
+}
+
 void SMILTimeContainer::begin()
 {
     ASSERT(!m_beginTime);

Modified: trunk/Source/WebCore/svg/animation/SMILTimeContainer.h (127473 => 127474)


--- trunk/Source/WebCore/svg/animation/SMILTimeContainer.h	2012-09-04 18:08:19 UTC (rev 127473)
+++ trunk/Source/WebCore/svg/animation/SMILTimeContainer.h	2012-09-04 18:31:31 UTC (rev 127474)
@@ -55,6 +55,7 @@
 
     bool isActive() const;
     bool isPaused() const;
+    bool isStarted() const;
     
     void begin();
     void pause();

Modified: trunk/Source/WebCore/svg/animation/SVGSMILElement.cpp (127473 => 127474)


--- trunk/Source/WebCore/svg/animation/SVGSMILElement.cpp	2012-09-04 18:08:19 UTC (rev 127473)
+++ trunk/Source/WebCore/svg/animation/SVGSMILElement.cpp	2012-09-04 18:31:31 UTC (rev 127474)
@@ -212,11 +212,11 @@
     if (!fastHasAttribute(SVGNames::beginAttr))
         m_beginTimes.append(SMILTimeWithOrigin());
 
-    if (m_isWaitingForFirstInterval) {
+    if (m_isWaitingForFirstInterval)
         resolveFirstInterval();
-        reschedule();
-    }
 
+    reschedule();
+
     return InsertionDone;
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to