Title: [244276] trunk
Revision
244276
Author
s...@apple.com
Date
2019-04-15 11:54:21 -0700 (Mon, 15 Apr 2019)

Log Message

ASSERT fires when removing a disallowed clone from the shadow tree without reseting its corresponding element
https://bugs.webkit.org/show_bug.cgi?id=196895

Reviewed by Darin Adler.

Source/WebCore:

When cloning elements to the shadow tree of an SVGUseElement, the
corresponding element links are set from the clones to the originals.
Later some of the elements may be disallowed to exist in the shadow tree.
For example the SVGPatternElement is disallowed and has to be removed
even after cloning. The problem is the corresponding elements are not
reset to null. Usually this is not a problem because the removed elements
will be deleted and the destructor of SVGElement will reset the corresponding
element links. However in some cases, the cloned element is referenced
from another SVGElement, for example the target of a SVGTRefElement. In
this case the clone won't be deleted but it will be linked to the original
and the event listeners won't be copied from the original. When the
original is deleted, its event listeners have to be removed. The event
listeners of the clones also ave to be removed. But because the event
listeners of the original were not copied when cloning, the assertion in
SVGElement::removeEventListener() fires.

Test: svg/custom/use-disallowed-element-clear-corresponding-element.html

* svg/SVGUseElement.cpp:
(WebCore::disassociateAndRemoveClones):

LayoutTests:

* svg/custom/use-disallowed-element-clear-corresponding-element-expected.txt: Added.
* svg/custom/use-disallowed-element-clear-corresponding-element.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (244275 => 244276)


--- trunk/LayoutTests/ChangeLog	2019-04-15 18:47:38 UTC (rev 244275)
+++ trunk/LayoutTests/ChangeLog	2019-04-15 18:54:21 UTC (rev 244276)
@@ -1,3 +1,13 @@
+2019-04-15  Said Abou-Hallawa  <s...@apple.com>
+
+        ASSERT fires when removing a disallowed clone from the shadow tree without reseting its corresponding element
+        https://bugs.webkit.org/show_bug.cgi?id=196895
+
+        Reviewed by Darin Adler.
+
+        * svg/custom/use-disallowed-element-clear-corresponding-element-expected.txt: Added.
+        * svg/custom/use-disallowed-element-clear-corresponding-element.html: Added.
+
 2019-04-15  Devin Rousso  <drou...@apple.com>
 
         Web Inspector: DOMDebugger: "Attribute Modified" breakpoints pause after the modification occurs for the style attribute

Added: trunk/LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element-expected.txt (0 => 244276)


--- trunk/LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element-expected.txt	2019-04-15 18:54:21 UTC (rev 244276)
@@ -0,0 +1,3 @@
+Test passes if it does not assert in debug builds.
+
+

Added: trunk/LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element.html (0 => 244276)


--- trunk/LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element.html	                        (rev 0)
+++ trunk/LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element.html	2019-04-15 18:54:21 UTC (rev 244276)
@@ -0,0 +1,25 @@
+<body>
+    <p>Test passes if it does not assert in debug builds.</p>
+    <svg id="svg">
+        <pattern id="svg-pattern-1">
+            <use id="svg-use" href=""
+        </pattern>
+        <pattern id="svg-pattern-2" href=""
+        </pattern>
+    </svg>
+    <shadow id="shadow">
+        <svg>
+            <tref href=""
+            <use href=""
+        </svg>
+    </shadow>
+    <script>
+        if (window.testRunner)
+            testRunner.dumpAsText();
+        window.addEventListener('load', function() {
+            var svgPattern2 = document.getElementById("svg-pattern-2");
+            var shadow = document.getElementById("shadow");
+            svgPattern2.after(shadow);
+        }, false);
+    </script>
+</body>

Modified: trunk/Source/WebCore/ChangeLog (244275 => 244276)


--- trunk/Source/WebCore/ChangeLog	2019-04-15 18:47:38 UTC (rev 244275)
+++ trunk/Source/WebCore/ChangeLog	2019-04-15 18:54:21 UTC (rev 244276)
@@ -1,3 +1,31 @@
+2019-04-15  Said Abou-Hallawa  <s...@apple.com>
+
+        ASSERT fires when removing a disallowed clone from the shadow tree without reseting its corresponding element
+        https://bugs.webkit.org/show_bug.cgi?id=196895
+
+        Reviewed by Darin Adler.
+
+        When cloning elements to the shadow tree of an SVGUseElement, the
+        corresponding element links are set from the clones to the originals.
+        Later some of the elements may be disallowed to exist in the shadow tree.
+        For example the SVGPatternElement is disallowed and has to be removed 
+        even after cloning. The problem is the corresponding elements are not
+        reset to null. Usually this is not a problem because the removed elements
+        will be deleted and the destructor of SVGElement will reset the corresponding
+        element links. However in some cases, the cloned element is referenced
+        from another SVGElement, for example the target of a SVGTRefElement. In
+        this case the clone won't be deleted but it will be linked to the original
+        and the event listeners won't be copied from the original. When the
+        original is deleted, its event listeners have to be removed. The event
+        listeners of the clones also ave to be removed. But because the event
+        listeners of the original were not copied when cloning, the assertion in
+        SVGElement::removeEventListener() fires.
+
+        Test: svg/custom/use-disallowed-element-clear-corresponding-element.html
+
+        * svg/SVGUseElement.cpp:
+        (WebCore::disassociateAndRemoveClones):
+
 2019-04-15  Devin Rousso  <drou...@apple.com>
 
         Web Inspector: DOMDebugger: "Attribute Modified" breakpoints pause after the modification occurs for the style attribute

Modified: trunk/Source/WebCore/svg/SVGUseElement.cpp (244275 => 244276)


--- trunk/Source/WebCore/svg/SVGUseElement.cpp	2019-04-15 18:47:38 UTC (rev 244275)
+++ trunk/Source/WebCore/svg/SVGUseElement.cpp	2019-04-15 18:54:21 UTC (rev 244276)
@@ -321,6 +321,8 @@
     for (auto& clone : clones) {
         for (auto& descendant : descendantsOfType<SVGElement>(*clone))
             descendant.setCorrespondingElement(nullptr);
+        if (is<SVGElement>(clone))
+            downcast<SVGElement>(*clone).setCorrespondingElement(nullptr);
         clone->parentNode()->removeChild(*clone);
     }
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to