<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[244276] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/244276">244276</a></dd>
<dt>Author</dt> <dd>said@apple.com</dd>
<dt>Date</dt> <dd>2019-04-15 11:54:21 -0700 (Mon, 15 Apr 2019)</dd>
</dl>

<h3>Log Message</h3>
<pre>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.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoresvgSVGUseElementcpp">trunk/Source/WebCore/svg/SVGUseElement.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestssvgcustomusedisallowedelementclearcorrespondingelementexpectedtxt">trunk/LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element-expected.txt</a></li>
<li><a href="#trunkLayoutTestssvgcustomusedisallowedelementclearcorrespondingelementhtml">trunk/LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element.html</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (244275 => 244276)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog      2019-04-15 18:47:38 UTC (rev 244275)
+++ trunk/LayoutTests/ChangeLog 2019-04-15 18:54:21 UTC (rev 244276)
</span><span class="lines">@@ -1,3 +1,13 @@
</span><ins>+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.
+
</ins><span class="cx"> 2019-04-15  Devin Rousso  <drousso@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Web Inspector: DOMDebugger: "Attribute Modified" breakpoints pause after the modification occurs for the style attribute
</span></span></pre></div>
<a id="trunkLayoutTestssvgcustomusedisallowedelementclearcorrespondingelementexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element-expected.txt (0 => 244276)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,3 @@
</span><ins>+Test passes if it does not assert in debug builds.
+
+
</ins></span></pre></div>
<a id="trunkLayoutTestssvgcustomusedisallowedelementclearcorrespondingelementhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/custom/use-disallowed-element-clear-corresponding-element.html (0 => 244276)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,25 @@
</span><ins>+<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>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (244275 => 244276)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,31 @@
</span><ins>+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):
+
</ins><span class="cx"> 2019-04-15  Devin Rousso  <drousso@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Web Inspector: DOMDebugger: "Attribute Modified" breakpoints pause after the modification occurs for the style attribute
</span></span></pre></div>
<a id="trunkSourceWebCoresvgSVGUseElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/svg/SVGUseElement.cpp (244275 => 244276)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -321,6 +321,8 @@
</span><span class="cx">     for (auto& clone : clones) {
</span><span class="cx">         for (auto& descendant : descendantsOfType<SVGElement>(*clone))
</span><span class="cx">             descendant.setCorrespondingElement(nullptr);
</span><ins>+        if (is<SVGElement>(clone))
+            downcast<SVGElement>(*clone).setCorrespondingElement(nullptr);
</ins><span class="cx">         clone->parentNode()->removeChild(*clone);
</span><span class="cx">     }
</span><span class="cx"> }
</span></span></pre>
</div>
</div>

</body>
</html>