Crash happens when calling removeEventListener for an SVG element which has an instan...
authorsaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Jul 2015 20:10:03 +0000 (20:10 +0000)
committersaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Jul 2015 20:10:03 +0000 (20:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=147290

Reviewed by Daniel Bates.

Source/WebCore:

When the shadow tree is built for a <use> element, all the SVG elements
are allowed to be cloned in the shadow tree but later some of the elements
are disallowed and removed. Make sure, when disallowing an element in the
shadow tree, to reset the correspondingElement relationship between all
the disallowed descendant SVG elements and all their original elements.

Test: svg/custom/remove-event-listener-shadow-disallowed-element.svg

*svg/SVGElement.cpp:
(WebCore::SVGElement::setCorrespondingElement)
* svg/SVGUseElement.cpp:
(WebCore::removeDisallowedElementsFromSubtree):

LayoutTests:

Make sure we do not crash when when calling removeEventListener() for an
element which is cloned under a disallowed parent inside the shadow tree
of another <use> element.

* svg/custom/remove-event-listener-shadow-disallowed-element-expected.txt: Added.
* svg/custom/remove-event-listener-shadow-disallowed-element.svg: Added.

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

LayoutTests/ChangeLog
LayoutTests/svg/custom/remove-event-listener-shadow-disallowed-element-expected.txt [new file with mode: 0644]
LayoutTests/svg/custom/remove-event-listener-shadow-disallowed-element.svg [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/svg/SVGElement.cpp
Source/WebCore/svg/SVGUseElement.cpp

index 2cb5e0a..ffa5c91 100644 (file)
@@ -1,3 +1,17 @@
+2015-07-28  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        Crash happens when calling removeEventListener for an SVG element which has an instance inside a <defs> element of shadow tree
+        https://bugs.webkit.org/show_bug.cgi?id=147290
+
+        Reviewed by Daniel Bates.
+
+        Make sure we do not crash when when calling removeEventListener() for an
+        element which is cloned under a disallowed parent inside the shadow tree
+        of another <use> element.
+
+        * svg/custom/remove-event-listener-shadow-disallowed-element-expected.txt: Added.
+        * svg/custom/remove-event-listener-shadow-disallowed-element.svg: Added.
+
 2015-07-27  David Hyatt  <hyatt@apple.com>
 
         ASSERTION FAILED: !currBox->needsLayout() loading bing maps (and apple.com/music and nytimes)
diff --git a/LayoutTests/svg/custom/remove-event-listener-shadow-disallowed-element-expected.txt b/LayoutTests/svg/custom/remove-event-listener-shadow-disallowed-element-expected.txt
new file mode 100644 (file)
index 0000000..a567c4b
--- /dev/null
@@ -0,0 +1 @@
+Pass.
diff --git a/LayoutTests/svg/custom/remove-event-listener-shadow-disallowed-element.svg b/LayoutTests/svg/custom/remove-event-listener-shadow-disallowed-element.svg
new file mode 100644 (file)
index 0000000..064c550
--- /dev/null
@@ -0,0 +1,37 @@
+<svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+
+  <defs>
+    <svg id="green-svg" width="100" height="100">
+      <defs>
+         <rect id="green-rect" fill="green" width="100" height="100"/>
+      </defs>
+      <use id="use-green-rect" xlink:href="#green-rect"/>
+    </svg>
+  </defs>  
+
+  <use id="use-green-svg" x="10" y="10" xlink:href="#green-svg"/>
+  <text>Pass.</text>
+
+  <script>
+   if (window.testRunner)
+      testRunner.dumpAsText();
+    
+    var onAlert = function() {
+      alert("test");
+    }
+
+    // This removeEventListener() has to be called before the onload fires. Before
+    // onload fires, the shadow tree of the SVGUseElement is not built. So this
+    // call should affect the original element only. Once the shadow tree is built,
+    // the SVGUseElement calls updateShadowTree() and this is where we want to make
+    // sure the eventListeners of all the cloned elements in the shadow tree are
+    // updated correctly and the disallowed cloned elements and their descendants
+    // are disconnected from their corresponding elements.
+    document.getElementById("green-rect").addEventListener("click", onAlert);
+    
+    window.addEventListener("load", function () {
+      document.getElementById("green-rect").removeEventListener("click", onAlert);
+    }, false);
+  </script>
+
+</svg>
\ No newline at end of file
index 3ff267d..6e07150 100644 (file)
@@ -1,3 +1,23 @@
+2015-07-28  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        Crash happens when calling removeEventListener for an SVG element which has an instance inside a <defs> element of shadow tree
+        https://bugs.webkit.org/show_bug.cgi?id=147290
+
+        Reviewed by Daniel Bates.
+
+        When the shadow tree is built for a <use> element, all the SVG elements
+        are allowed to be cloned in the shadow tree but later some of the elements
+        are disallowed and removed. Make sure, when disallowing an element in the
+        shadow tree, to reset the correspondingElement relationship between all
+        the disallowed descendant SVG elements and all their original elements.
+        
+        Test: svg/custom/remove-event-listener-shadow-disallowed-element.svg
+
+        *svg/SVGElement.cpp:
+        (WebCore::SVGElement::setCorrespondingElement)
+        * svg/SVGUseElement.cpp:
+        (WebCore::removeDisallowedElementsFromSubtree):
+
 2015-07-28  Chris Dumez  <cdumez@apple.com>
 
         Unreviewed, follow-up nit fix after r187489.
index 8a8516f..51fc6fe 100644 (file)
@@ -502,7 +502,8 @@ void SVGElement::setCorrespondingElement(SVGElement* correspondingElement)
         if (SVGElement* oldCorrespondingElement = m_svgRareData->correspondingElement())
             oldCorrespondingElement->m_svgRareData->instances().remove(this);
     }
-    ensureSVGRareData().setCorrespondingElement(correspondingElement);
+    if (m_svgRareData || correspondingElement)
+        ensureSVGRareData().setCorrespondingElement(correspondingElement);
     if (correspondingElement)
         correspondingElement->ensureSVGRareData().instances().add(this);
 }
index 6897cdc..704fe4a 100644 (file)
@@ -329,8 +329,11 @@ static void removeDisallowedElementsFromSubtree(SVGElement& subtree)
         }
         ++it;
     }
-    for (auto* element : disallowedElements)
+    for (auto* element : disallowedElements) {
+        for (auto& descendant : descendantsOfType<SVGElement>(*element))
+            descendant.setCorrespondingElement(nullptr);
         element->parentNode()->removeChild(element);
+    }
 }
 
 static void associateClonesWithOriginals(SVGElement& clone, SVGElement& original)