Title: [187504] trunk
Revision
187504
Author
[email protected]
Date
2015-07-28 13:10:03 -0700 (Tue, 28 Jul 2015)

Log Message

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.

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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (187503 => 187504)


--- trunk/LayoutTests/ChangeLog	2015-07-28 19:48:03 UTC (rev 187503)
+++ trunk/LayoutTests/ChangeLog	2015-07-28 20:10:03 UTC (rev 187504)
@@ -1,3 +1,17 @@
+2015-07-28  Said Abou-Hallawa  <[email protected]>
+
+        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  <[email protected]>
 
         ASSERTION FAILED: !currBox->needsLayout() loading bing maps (and apple.com/music and nytimes)

Added: trunk/LayoutTests/svg/custom/remove-event-listener-shadow-disallowed-element-expected.txt (0 => 187504)


--- trunk/LayoutTests/svg/custom/remove-event-listener-shadow-disallowed-element-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/svg/custom/remove-event-listener-shadow-disallowed-element-expected.txt	2015-07-28 20:10:03 UTC (rev 187504)
@@ -0,0 +1 @@
+Pass.

Added: trunk/LayoutTests/svg/custom/remove-event-listener-shadow-disallowed-element.svg (0 => 187504)


--- trunk/LayoutTests/svg/custom/remove-event-listener-shadow-disallowed-element.svg	                        (rev 0)
+++ trunk/LayoutTests/svg/custom/remove-event-listener-shadow-disallowed-element.svg	2015-07-28 20:10:03 UTC (rev 187504)
@@ -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=""
+    </svg>
+  </defs>  
+
+  <use id="use-green-svg" x="10" y="10" xlink:href=""
+  <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

Modified: trunk/Source/WebCore/ChangeLog (187503 => 187504)


--- trunk/Source/WebCore/ChangeLog	2015-07-28 19:48:03 UTC (rev 187503)
+++ trunk/Source/WebCore/ChangeLog	2015-07-28 20:10:03 UTC (rev 187504)
@@ -1,3 +1,23 @@
+2015-07-28  Said Abou-Hallawa  <[email protected]>
+
+        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  <[email protected]>
 
         Unreviewed, follow-up nit fix after r187489.

Modified: trunk/Source/WebCore/svg/SVGElement.cpp (187503 => 187504)


--- trunk/Source/WebCore/svg/SVGElement.cpp	2015-07-28 19:48:03 UTC (rev 187503)
+++ trunk/Source/WebCore/svg/SVGElement.cpp	2015-07-28 20:10:03 UTC (rev 187504)
@@ -502,7 +502,8 @@
         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);
 }

Modified: trunk/Source/WebCore/svg/SVGUseElement.cpp (187503 => 187504)


--- trunk/Source/WebCore/svg/SVGUseElement.cpp	2015-07-28 19:48:03 UTC (rev 187503)
+++ trunk/Source/WebCore/svg/SVGUseElement.cpp	2015-07-28 20:10:03 UTC (rev 187504)
@@ -329,8 +329,11 @@
         }
         ++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)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to