<!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>[212140] 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/212140">212140</a></dd>
<dt>Author</dt> <dd>rniwa@webkit.org</dd>
<dt>Date</dt> <dd>2017-02-10 13:15:51 -0800 (Fri, 10 Feb 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>HTMLConstructionSiteTask::Insert should never be called on a node with a parent
https://bugs.webkit.org/show_bug.cgi?id=168099
Reviewed by Sam Weinig.
insertAlreadyParsedChild always use HTMLConstructionSiteTask::InsertAlreadyParsedChild instead
of using HTMLConstructionSiteTask::Insert when fostering a child.
Also combine the step to take all children and re-parenting into a single task instead of
separately issuing TakeAllChildren and Reparent tasks.
No new tests since this is a refactoring.
* html/parser/HTMLConstructionSite.cpp:
(WebCore::insert): Now asserts that the child node never have a parent.
(WebCore::executeInsertAlreadyParsedChildTask): Moved the code to remove the parent here.
(WebCore::executeTakeAllChildrenAndReparentTask): Renamed from executeTakeAllChildrenTask
now that this function also does the reparenting.
(WebCore::executeTask):
(WebCore::HTMLConstructionSite::reparent): Removed the variant only used with takeAllChildren.
(WebCore::HTMLConstructionSite::insertAlreadyParsedChild): Always use InsertAlreadyParsedChild
instead of calling fosterParent which uses Insert when fostering parents.
(WebCore::HTMLConstructionSite::takeAllChildrenAndReparent): Renamed from takeAllChildren.
* html/parser/HTMLConstructionSite.h:
(WebCore::HTMLConstructionSiteTask:Operation):
* html/parser/HTMLTreeBuilder.cpp:
(WebCore::HTMLTreeBuilder::callTheAdoptionAgency):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorehtmlparserHTMLConstructionSitecpp">trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp</a></li>
<li><a href="#trunkSourceWebCorehtmlparserHTMLConstructionSiteh">trunk/Source/WebCore/html/parser/HTMLConstructionSite.h</a></li>
<li><a href="#trunkSourceWebCorehtmlparserHTMLTreeBuildercpp">trunk/Source/WebCore/html/parser/HTMLTreeBuilder.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (212139 => 212140)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2017-02-10 21:08:32 UTC (rev 212139)
+++ trunk/Source/WebCore/ChangeLog 2017-02-10 21:15:51 UTC (rev 212140)
</span><span class="lines">@@ -1,3 +1,33 @@
</span><ins>+2017-02-10 Ryosuke Niwa <rniwa@webkit.org>
+
+ HTMLConstructionSiteTask::Insert should never be called on a node with a parent
+ https://bugs.webkit.org/show_bug.cgi?id=168099
+
+ Reviewed by Sam Weinig.
+
+ insertAlreadyParsedChild always use HTMLConstructionSiteTask::InsertAlreadyParsedChild instead
+ of using HTMLConstructionSiteTask::Insert when fostering a child.
+
+ Also combine the step to take all children and re-parenting into a single task instead of
+ separately issuing TakeAllChildren and Reparent tasks.
+
+ No new tests since this is a refactoring.
+
+ * html/parser/HTMLConstructionSite.cpp:
+ (WebCore::insert): Now asserts that the child node never have a parent.
+ (WebCore::executeInsertAlreadyParsedChildTask): Moved the code to remove the parent here.
+ (WebCore::executeTakeAllChildrenAndReparentTask): Renamed from executeTakeAllChildrenTask
+ now that this function also does the reparenting.
+ (WebCore::executeTask):
+ (WebCore::HTMLConstructionSite::reparent): Removed the variant only used with takeAllChildren.
+ (WebCore::HTMLConstructionSite::insertAlreadyParsedChild): Always use InsertAlreadyParsedChild
+ instead of calling fosterParent which uses Insert when fostering parents.
+ (WebCore::HTMLConstructionSite::takeAllChildrenAndReparent): Renamed from takeAllChildren.
+ * html/parser/HTMLConstructionSite.h:
+ (WebCore::HTMLConstructionSiteTask:Operation):
+ * html/parser/HTMLTreeBuilder.cpp:
+ (WebCore::HTMLTreeBuilder::callTheAdoptionAgency):
+
</ins><span class="cx"> 2017-02-10 Dave Hyatt <hyatt@apple.com>
</span><span class="cx">
</span><span class="cx"> [CSS Parser] Make intercap property values serialize correctly
</span></span></pre></div>
<a id="trunkSourceWebCorehtmlparserHTMLConstructionSitecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp (212139 => 212140)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp 2017-02-10 21:08:32 UTC (rev 212139)
+++ trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp 2017-02-10 21:15:51 UTC (rev 212140)
</span><span class="lines">@@ -105,9 +105,7 @@
</span><span class="cx"> if (is<HTMLTemplateElement>(*task.parent))
</span><span class="cx"> task.parent = &downcast<HTMLTemplateElement>(*task.parent).content();
</span><span class="cx">
</span><del>- if (ContainerNode* parent = task.child->parentNode())
- parent->parserRemoveChild(*task.child);
-
</del><ins>+ ASSERT(!task.child->parentNode());
</ins><span class="cx"> if (task.nextChild)
</span><span class="cx"> task.parent->parserInsertBefore(*task.child, *task.nextChild);
</span><span class="cx"> else
</span><span class="lines">@@ -140,16 +138,19 @@
</span><span class="cx"> {
</span><span class="cx"> ASSERT(task.operation == HTMLConstructionSiteTask::InsertAlreadyParsedChild);
</span><span class="cx">
</span><ins>+ if (ContainerNode* parent = task.child->parentNode())
+ parent->parserRemoveChild(*task.child);
</ins><span class="cx"> insert(task);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static inline void executeTakeAllChildrenTask(HTMLConstructionSiteTask& task)
</del><ins>+static inline void executeTakeAllChildrenAndReparentTask(HTMLConstructionSiteTask& task)
</ins><span class="cx"> {
</span><del>- ASSERT(task.operation == HTMLConstructionSiteTask::TakeAllChildren);
</del><ins>+ ASSERT(task.operation == HTMLConstructionSiteTask::TakeAllChildrenAndReparent);
</ins><span class="cx">
</span><del>- task.parent->takeAllChildrenFrom(task.oldParent());
- // Notice that we don't need to manually attach the moved children
- // because takeAllChildrenFrom does that work for us.
</del><ins>+ auto* furthestBlock = task.oldParent();
+ task.parent->takeAllChildrenFrom(furthestBlock);
+
+ furthestBlock->parserAppendChild(*task.parent);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> static inline void executeTask(HTMLConstructionSiteTask& task)
</span><span class="lines">@@ -165,8 +166,8 @@
</span><span class="cx"> case HTMLConstructionSiteTask::Reparent:
</span><span class="cx"> executeReparentTask(task);
</span><span class="cx"> return;
</span><del>- case HTMLConstructionSiteTask::TakeAllChildren:
- executeTakeAllChildrenTask(task);
</del><ins>+ case HTMLConstructionSiteTask::TakeAllChildrenAndReparent:
+ executeTakeAllChildrenAndReparentTask(task);
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx"> ASSERT_NOT_REACHED();
</span><span class="lines">@@ -599,30 +600,21 @@
</span><span class="cx"> m_taskQueue.append(WTFMove(task));
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void HTMLConstructionSite::reparent(HTMLElementStack::ElementRecord& newParent, HTMLStackItem& child)
-{
- HTMLConstructionSiteTask task(HTMLConstructionSiteTask::Reparent);
- task.parent = &newParent.node();
- task.child = &child.element();
- m_taskQueue.append(WTFMove(task));
-}
-
</del><span class="cx"> void HTMLConstructionSite::insertAlreadyParsedChild(HTMLStackItem& newParent, HTMLElementStack::ElementRecord& child)
</span><span class="cx"> {
</span><ins>+ HTMLConstructionSiteTask task(HTMLConstructionSiteTask::InsertAlreadyParsedChild);
</ins><span class="cx"> if (causesFosterParenting(newParent)) {
</span><del>- fosterParent(child.element());
- return;
- }
-
- HTMLConstructionSiteTask task(HTMLConstructionSiteTask::InsertAlreadyParsedChild);
- task.parent = &newParent.node();
</del><ins>+ findFosterSite(task);
+ ASSERT(task.parent);
+ } else
+ task.parent = &newParent.node();
</ins><span class="cx"> task.child = &child.element();
</span><span class="cx"> m_taskQueue.append(WTFMove(task));
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void HTMLConstructionSite::takeAllChildren(HTMLStackItem& newParent, HTMLElementStack::ElementRecord& oldParent)
</del><ins>+void HTMLConstructionSite::takeAllChildrenAndReparent(HTMLStackItem& newParent, HTMLElementStack::ElementRecord& oldParent)
</ins><span class="cx"> {
</span><del>- HTMLConstructionSiteTask task(HTMLConstructionSiteTask::TakeAllChildren);
</del><ins>+ HTMLConstructionSiteTask task(HTMLConstructionSiteTask::TakeAllChildrenAndReparent);
</ins><span class="cx"> task.parent = &newParent.node();
</span><span class="cx"> task.child = &oldParent.node();
</span><span class="cx"> m_taskQueue.append(WTFMove(task));
</span></span></pre></div>
<a id="trunkSourceWebCorehtmlparserHTMLConstructionSiteh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/html/parser/HTMLConstructionSite.h (212139 => 212140)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/html/parser/HTMLConstructionSite.h 2017-02-10 21:08:32 UTC (rev 212139)
+++ trunk/Source/WebCore/html/parser/HTMLConstructionSite.h 2017-02-10 21:15:51 UTC (rev 212140)
</span><span class="lines">@@ -41,7 +41,7 @@
</span><span class="cx"> Insert,
</span><span class="cx"> InsertAlreadyParsedChild,
</span><span class="cx"> Reparent,
</span><del>- TakeAllChildren,
</del><ins>+ TakeAllChildrenAndReparent,
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> explicit HTMLConstructionSiteTask(Operation op)
</span><span class="lines">@@ -119,12 +119,11 @@
</span><span class="cx"> void insertHTMLBodyStartTagInBody(AtomicHTMLToken&&);
</span><span class="cx">
</span><span class="cx"> void reparent(HTMLElementStack::ElementRecord& newParent, HTMLElementStack::ElementRecord& child);
</span><del>- void reparent(HTMLElementStack::ElementRecord& newParent, HTMLStackItem& child);
</del><span class="cx"> // insertAlreadyParsedChild assumes that |child| has already been parsed (i.e., we're just
</span><span class="cx"> // moving it around in the tree rather than parsing it for the first time). That means
</span><span class="cx"> // this function doesn't call beginParsingChildren / finishParsingChildren.
</span><span class="cx"> void insertAlreadyParsedChild(HTMLStackItem& newParent, HTMLElementStack::ElementRecord& child);
</span><del>- void takeAllChildren(HTMLStackItem& newParent, HTMLElementStack::ElementRecord& oldParent);
</del><ins>+ void takeAllChildrenAndReparent(HTMLStackItem& newParent, HTMLElementStack::ElementRecord& oldParent);
</ins><span class="cx">
</span><span class="cx"> Ref<HTMLStackItem> createElementFromSavedToken(HTMLStackItem&);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorehtmlparserHTMLTreeBuildercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/html/parser/HTMLTreeBuilder.cpp (212139 => 212140)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/html/parser/HTMLTreeBuilder.cpp 2017-02-10 21:08:32 UTC (rev 212139)
+++ trunk/Source/WebCore/html/parser/HTMLTreeBuilder.cpp 2017-02-10 21:15:51 UTC (rev 212140)
</span><span class="lines">@@ -1496,10 +1496,8 @@
</span><span class="cx"> m_tree.insertAlreadyParsedChild(commonAncestor.get(), *lastNode);
</span><span class="cx"> // 11.
</span><span class="cx"> auto newItem = m_tree.createElementFromSavedToken(formattingElementRecord->stackItem());
</span><del>- // 12.
- m_tree.takeAllChildren(newItem, *furthestBlock);
- // 13.
- m_tree.reparent(*furthestBlock, newItem);
</del><ins>+ // 12. & 13.
+ m_tree.takeAllChildrenAndReparent(newItem, *furthestBlock);
</ins><span class="cx"> // 14.
</span><span class="cx"> m_tree.activeFormattingElements().swapTo(*formattingElement, newItem.copyRef(), bookmark);
</span><span class="cx"> // 15.
</span></span></pre>
</div>
</div>
</body>
</html>