Infinite loop if a <use> element references its ancestor and the DOMNodeInserted...
authorsaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Jun 2018 19:24:45 +0000 (19:24 +0000)
committersaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Jun 2018 19:24:45 +0000 (19:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186925

Reviewed by Antti Koivisto.

Source/WebCore:

This patches fixes two issues:
-- SVGTRefTargetEventListener should not assume it has to be attached to
target when its handleEvent() is called.
Because SVGTRefTargetEventListener::handleEvent() references the target
element, we just return if the listener is detached.

-- The <use> element should not clone its shadow tree if it references one
of its ancestors. The DOMNodeInserted of any node in the target element
tree may issue a document command. This document command will cause the
shadow tree to be re-cloned so this will cause infinite loop to happen.

Test: svg/dom/svg-use-infinite-loop-cloning.html

* svg/SVGTRefElement.cpp:
(WebCore::SVGTRefTargetEventListener::handleEvent):
* svg/SVGUseElement.cpp:
(WebCore::SVGUseElement::updateShadowTree):

LayoutTests:

* svg/dom/svg-use-infinite-loop-cloning-expected.txt: Added.
* svg/dom/svg-use-infinite-loop-cloning.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/svg/dom/svg-use-infinite-loop-cloning-expected.txt [new file with mode: 0644]
LayoutTests/svg/dom/svg-use-infinite-loop-cloning.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/svg/SVGTRefElement.cpp
Source/WebCore/svg/SVGUseElement.cpp

index 2697546..c5b9b95 100644 (file)
@@ -1,3 +1,13 @@
+2018-06-25  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        Infinite loop if a <use> element references its ancestor and the DOMNodeInserted event handler of one its ancestor's descents updates the document style
+        https://bugs.webkit.org/show_bug.cgi?id=186925
+
+        Reviewed by Antti Koivisto.
+
+        * svg/dom/svg-use-infinite-loop-cloning-expected.txt: Added.
+        * svg/dom/svg-use-infinite-loop-cloning.html: Added.
+
 2018-06-29  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [WPE] Three CSS Grid Layout tests crash due to valueless std::optional access
diff --git a/LayoutTests/svg/dom/svg-use-infinite-loop-cloning-expected.txt b/LayoutTests/svg/dom/svg-use-infinite-loop-cloning-expected.txt
new file mode 100644 (file)
index 0000000..44311f8
--- /dev/null
@@ -0,0 +1,3 @@
+This test passes if it doesn't crash.
+
+
diff --git a/LayoutTests/svg/dom/svg-use-infinite-loop-cloning.html b/LayoutTests/svg/dom/svg-use-infinite-loop-cloning.html
new file mode 100644 (file)
index 0000000..e0e6b5f
--- /dev/null
@@ -0,0 +1,36 @@
+<script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+
+    function gc() {
+        if (window.GCController)
+            return GCController.collect();
+
+        // Force garbage collection.
+        for(var i=0;i<100;i++)
+            a = new Uint8Array(1024*1024);
+    }
+
+    function onNodeInsertedTspan() {
+        switchElement.setAttribute("y", "0 1 100");
+        document.execCommand("justifyCenter", false);
+        gc();
+    }
+
+    function onLoadUseElement() {
+        tspanElement.addEventListener("DOMNodeInserted", onNodeInsertedTspan);
+        document.execCommand("hiliteColor", false, "red");
+    }
+</script>
+<body>
+    <p>This test passes if it doesn't crash.</p>
+    <svg id="svgElement">
+        <switch id="switchElement">
+            <tref id="terfElement_1" xlink:href="#terfElement_2" />
+            <tref id="terfElement_2">
+                <tspan id="tspanElement" />
+            </tref>
+            <use id="useElement_1" xlink:href="#svgElement" onload="onLoadUseElement()" />
+        </switch>
+    </svg>
+</body>
index 6c1af8f..fd52945 100644 (file)
@@ -1,3 +1,28 @@
+2018-06-25  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        Infinite loop if a <use> element references its ancestor and the DOMNodeInserted event handler of one its ancestor's descents updates the document style
+        https://bugs.webkit.org/show_bug.cgi?id=186925
+
+        Reviewed by Antti Koivisto.
+
+        This patches fixes two issues:
+        -- SVGTRefTargetEventListener should not assume it has to be attached to
+        target when its handleEvent() is called.
+        Because SVGTRefTargetEventListener::handleEvent() references the target
+        element, we just return if the listener is detached.
+
+        -- The <use> element should not clone its shadow tree if it references one
+        of its ancestors. The DOMNodeInserted of any node in the target element
+        tree may issue a document command. This document command will cause the 
+        shadow tree to be re-cloned so this will cause infinite loop to happen.
+
+        Test: svg/dom/svg-use-infinite-loop-cloning.html
+
+        * svg/SVGTRefElement.cpp:
+        (WebCore::SVGTRefTargetEventListener::handleEvent):
+        * svg/SVGUseElement.cpp:
+        (WebCore::SVGUseElement::updateShadowTree):
+
 2018-06-29  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [WPE] Three CSS Grid Layout tests crash due to valueless std::optional access
index 70edf6a..c160b07 100644 (file)
@@ -120,7 +120,8 @@ bool SVGTRefTargetEventListener::operator==(const EventListener& listener) const
 
 void SVGTRefTargetEventListener::handleEvent(ScriptExecutionContext&, Event& event)
 {
-    ASSERT(isAttached());
+    if (!isAttached())
+        return;
 
     if (event.type() == eventNames().DOMSubtreeModifiedEvent && &m_trefElement != event.target())
         m_trefElement.updateReferencedText(m_target.get());
index 525292d..072c590 100644 (file)
@@ -260,6 +260,9 @@ void SVGUseElement::updateShadowTree()
         return;
     }
 
+    if (isDescendantOf(target))
+        return;
+    
     {
         auto& shadowRoot = ensureUserAgentShadowRoot();
         cloneTarget(shadowRoot, *target);