<!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>[189906] trunk/Source/WebCore</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/189906">189906</a></dd>
<dt>Author</dt> <dd>rniwa@webkit.org</dd>
<dt>Date</dt> <dd>2015-09-16 23:12:06 -0700 (Wed, 16 Sep 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>removeShadow shouldn't call ChildNodeRemovalNotifier with the shadow host as the removal point
https://bugs.webkit.org/show_bug.cgi?id=149244
Reviewed by Antti Koivisto.
Since a shadow host is in a different tree than nodes in its shadow tree, it's incorrect to call
removedFrom with the shadow host as the removal point. This causes HTMLSlotElement::removedFrom
which will be added in the bug 149241 to call methods on a wrong ShadowRoot.
We still keep the ad-hoc behavior of using the shadow host as the insertion/removal point when
calling insertedInto and removedFrom on the shadow root itself to update the InDocument node flag.
We may want to re-visit this design in the future.
No new tests since I couldn't quite create a reduction. However, tests I'm adding in the bug 149241
will crash without this change.
I separated this patch from the bug 149241 to isolate the high-risk code change here.
* dom/Element.cpp:
(WebCore::Element::addShadowRoot): Call insertedInto on ShadowRoot, and then call it on all its
children separately with the insertion point set to the shadow root since insertedInto relies on
insertion point's inDocument flag to be true when the shadow host is in the document.
(WebCore::Element::removeShadowRoot): Ditto in the reverse order.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoredomElementcpp">trunk/Source/WebCore/dom/Element.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (189905 => 189906)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-09-17 05:40:02 UTC (rev 189905)
+++ trunk/Source/WebCore/ChangeLog 2015-09-17 06:12:06 UTC (rev 189906)
</span><span class="lines">@@ -1,3 +1,29 @@
</span><ins>+2015-09-16 Ryosuke Niwa <rniwa@webkit.org>
+
+ removeShadow shouldn't call ChildNodeRemovalNotifier with the shadow host as the removal point
+ https://bugs.webkit.org/show_bug.cgi?id=149244
+
+ Reviewed by Antti Koivisto.
+
+ Since a shadow host is in a different tree than nodes in its shadow tree, it's incorrect to call
+ removedFrom with the shadow host as the removal point. This causes HTMLSlotElement::removedFrom
+ which will be added in the bug 149241 to call methods on a wrong ShadowRoot.
+
+ We still keep the ad-hoc behavior of using the shadow host as the insertion/removal point when
+ calling insertedInto and removedFrom on the shadow root itself to update the InDocument node flag.
+ We may want to re-visit this design in the future.
+
+ No new tests since I couldn't quite create a reduction. However, tests I'm adding in the bug 149241
+ will crash without this change.
+
+ I separated this patch from the bug 149241 to isolate the high-risk code change here.
+
+ * dom/Element.cpp:
+ (WebCore::Element::addShadowRoot): Call insertedInto on ShadowRoot, and then call it on all its
+ children separately with the insertion point set to the shadow root since insertedInto relies on
+ insertion point's inDocument flag to be true when the shadow host is in the document.
+ (WebCore::Element::removeShadowRoot): Ditto in the reverse order.
+
</ins><span class="cx"> 2015-09-16 Gyuyoung Kim <gyuyoung.kim@webkit.org>
</span><span class="cx">
</span><span class="cx"> Remove all uses of PassRefPtr in WebCore/inspector
</span></span></pre></div>
<a id="trunkSourceWebCoredomElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/Element.cpp (189905 => 189906)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/Element.cpp 2015-09-17 05:40:02 UTC (rev 189905)
+++ trunk/Source/WebCore/dom/Element.cpp 2015-09-17 06:12:06 UTC (rev 189906)
</span><span class="lines">@@ -1615,11 +1615,19 @@
</span><span class="cx"> shadowRoot.setHost(this);
</span><span class="cx"> shadowRoot.setParentTreeScope(&treeScope());
</span><span class="cx">
</span><del>- NodeVector postInsertionNotificationTargets;
- notifyChildNodeInserted(*this, shadowRoot, postInsertionNotificationTargets);
</del><ins>+ auto shadowRootInsertionResult = shadowRoot.insertedInto(*this);
+ ASSERT_UNUSED(shadowRootInsertionResult, shadowRootInsertionResult == InsertionDone);
+ if (auto* firstChild = shadowRoot.firstChild()) {
+ NodeVector postInsertionNotificationTargets;
+ {
+ NoEventDispatchAssertion assertNoEventDispatch;
+ for (auto* child = firstChild; child; child = child->nextSibling())
+ notifyChildNodeInserted(shadowRoot, *child, postInsertionNotificationTargets);
+ }
</ins><span class="cx">
</span><del>- for (auto& target : postInsertionNotificationTargets)
- target->finishedInsertingSubtree();
</del><ins>+ for (auto& target : postInsertionNotificationTargets)
+ target->finishedInsertingSubtree();
+ }
</ins><span class="cx">
</span><span class="cx"> resetNeedsNodeRenderingTraversalSlowPath();
</span><span class="cx">
</span><span class="lines">@@ -1646,7 +1654,13 @@
</span><span class="cx"> oldRoot->setHost(nullptr);
</span><span class="cx"> oldRoot->setParentTreeScope(&document());
</span><span class="cx">
</span><del>- notifyChildNodeRemoved(*this, *oldRoot);
</del><ins>+ {
+ NoEventDispatchAssertion assertNoEventDispatch;
+ for (auto* child = oldRoot->firstChild(); child; child = child->nextSibling())
+ notifyChildNodeRemoved(*oldRoot, *child);
+ }
+
+ oldRoot->removedFrom(*this);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> RefPtr<ShadowRoot> Element::createShadowRoot(ExceptionCode& ec)
</span></span></pre>
</div>
</div>
</body>
</html>