ASSERT fires when removing a disallowed clone from the shadow tree without reseting...
authorsaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Apr 2019 18:54:21 +0000 (18:54 +0000)
committersaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Apr 2019 18:54:21 +0000 (18:54 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element-expected.txt [new file with mode: 0644]
LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/svg/SVGUseElement.cpp

index 91b0a72..b640b84 100644 (file)
@@ -1,3 +1,13 @@
+2019-04-15  Said Abou-Hallawa  <said@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  <drousso@apple.com>
 
         Web Inspector: DOMDebugger: "Attribute Modified" breakpoints pause after the modification occurs for the style attribute
diff --git a/LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element-expected.txt b/LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element-expected.txt
new file mode 100644 (file)
index 0000000..68fe806
--- /dev/null
@@ -0,0 +1,3 @@
+Test passes if it does not assert in debug builds.
+
+
diff --git a/LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element.html b/LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element.html
new file mode 100644 (file)
index 0000000..42ef30e
--- /dev/null
@@ -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="#svg"/>
+        </pattern>
+        <pattern id="svg-pattern-2" href="#svg-use">
+        </pattern>
+    </svg>
+    <shadow id="shadow">
+        <svg>
+            <tref href="#svg-pattern-2"/>
+            <use href="#svg-use"/>
+        </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>
index 9afcd3a..2d7ba89 100644 (file)
@@ -1,3 +1,31 @@
+2019-04-15  Said Abou-Hallawa  <said@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  <drousso@apple.com>
 
         Web Inspector: DOMDebugger: "Attribute Modified" breakpoints pause after the modification occurs for the style attribute
index a9ae353..e08ae5a 100644 (file)
@@ -321,6 +321,8 @@ static inline void disassociateAndRemoveClones(const Vector<Element*>& clones)
     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);
     }
 }