Title: [212140] trunk/Source/WebCore
Revision
212140
Author
rn...@webkit.org
Date
2017-02-10 13:15:51 -0800 (Fri, 10 Feb 2017)

Log Message

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):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (212139 => 212140)


--- 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)
@@ -1,3 +1,33 @@
+2017-02-10  Ryosuke Niwa  <rn...@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):
+
 2017-02-10  Dave Hyatt  <hy...@apple.com>
 
         [CSS Parser] Make intercap property values serialize correctly

Modified: trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp (212139 => 212140)


--- 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)
@@ -105,9 +105,7 @@
     if (is<HTMLTemplateElement>(*task.parent))
         task.parent = &downcast<HTMLTemplateElement>(*task.parent).content();
 
-    if (ContainerNode* parent = task.child->parentNode())
-        parent->parserRemoveChild(*task.child);
-
+    ASSERT(!task.child->parentNode());
     if (task.nextChild)
         task.parent->parserInsertBefore(*task.child, *task.nextChild);
     else
@@ -140,16 +138,19 @@
 {
     ASSERT(task.operation == HTMLConstructionSiteTask::InsertAlreadyParsedChild);
 
+    if (ContainerNode* parent = task.child->parentNode())
+        parent->parserRemoveChild(*task.child);
     insert(task);
 }
 
-static inline void executeTakeAllChildrenTask(HTMLConstructionSiteTask& task)
+static inline void executeTakeAllChildrenAndReparentTask(HTMLConstructionSiteTask& task)
 {
-    ASSERT(task.operation == HTMLConstructionSiteTask::TakeAllChildren);
+    ASSERT(task.operation == HTMLConstructionSiteTask::TakeAllChildrenAndReparent);
 
-    task.parent->takeAllChildrenFrom(task.oldParent());
-    // Notice that we don't need to manually attach the moved children
-    // because takeAllChildrenFrom does that work for us.
+    auto* furthestBlock = task.oldParent();
+    task.parent->takeAllChildrenFrom(furthestBlock);
+
+    furthestBlock->parserAppendChild(*task.parent);
 }
 
 static inline void executeTask(HTMLConstructionSiteTask& task)
@@ -165,8 +166,8 @@
     case HTMLConstructionSiteTask::Reparent:
         executeReparentTask(task);
         return;
-    case HTMLConstructionSiteTask::TakeAllChildren:
-        executeTakeAllChildrenTask(task);
+    case HTMLConstructionSiteTask::TakeAllChildrenAndReparent:
+        executeTakeAllChildrenAndReparentTask(task);
         return;
     }
     ASSERT_NOT_REACHED();
@@ -599,30 +600,21 @@
     m_taskQueue.append(WTFMove(task));
 }
 
-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));
-}
-
 void HTMLConstructionSite::insertAlreadyParsedChild(HTMLStackItem& newParent, HTMLElementStack::ElementRecord& child)
 {
+    HTMLConstructionSiteTask task(HTMLConstructionSiteTask::InsertAlreadyParsedChild);
     if (causesFosterParenting(newParent)) {
-        fosterParent(child.element());
-        return;
-    }
-
-    HTMLConstructionSiteTask task(HTMLConstructionSiteTask::InsertAlreadyParsedChild);
-    task.parent = &newParent.node();
+        findFosterSite(task);
+        ASSERT(task.parent);
+    } else
+        task.parent = &newParent.node();
     task.child = &child.element();
     m_taskQueue.append(WTFMove(task));
 }
 
-void HTMLConstructionSite::takeAllChildren(HTMLStackItem& newParent, HTMLElementStack::ElementRecord& oldParent)
+void HTMLConstructionSite::takeAllChildrenAndReparent(HTMLStackItem& newParent, HTMLElementStack::ElementRecord& oldParent)
 {
-    HTMLConstructionSiteTask task(HTMLConstructionSiteTask::TakeAllChildren);
+    HTMLConstructionSiteTask task(HTMLConstructionSiteTask::TakeAllChildrenAndReparent);
     task.parent = &newParent.node();
     task.child = &oldParent.node();
     m_taskQueue.append(WTFMove(task));

Modified: trunk/Source/WebCore/html/parser/HTMLConstructionSite.h (212139 => 212140)


--- 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)
@@ -41,7 +41,7 @@
         Insert,
         InsertAlreadyParsedChild,
         Reparent,
-        TakeAllChildren,
+        TakeAllChildrenAndReparent,
     };
 
     explicit HTMLConstructionSiteTask(Operation op)
@@ -119,12 +119,11 @@
     void insertHTMLBodyStartTagInBody(AtomicHTMLToken&&);
 
     void reparent(HTMLElementStack::ElementRecord& newParent, HTMLElementStack::ElementRecord& child);
-    void reparent(HTMLElementStack::ElementRecord& newParent, HTMLStackItem& child);
     // insertAlreadyParsedChild assumes that |child| has already been parsed (i.e., we're just
     // moving it around in the tree rather than parsing it for the first time). That means
     // this function doesn't call beginParsingChildren / finishParsingChildren.
     void insertAlreadyParsedChild(HTMLStackItem& newParent, HTMLElementStack::ElementRecord& child);
-    void takeAllChildren(HTMLStackItem& newParent, HTMLElementStack::ElementRecord& oldParent);
+    void takeAllChildrenAndReparent(HTMLStackItem& newParent, HTMLElementStack::ElementRecord& oldParent);
 
     Ref<HTMLStackItem> createElementFromSavedToken(HTMLStackItem&);
 

Modified: trunk/Source/WebCore/html/parser/HTMLTreeBuilder.cpp (212139 => 212140)


--- 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)
@@ -1496,10 +1496,8 @@
         m_tree.insertAlreadyParsedChild(commonAncestor.get(), *lastNode);
         // 11.
         auto newItem = m_tree.createElementFromSavedToken(formattingElementRecord->stackItem());
-        // 12.
-        m_tree.takeAllChildren(newItem, *furthestBlock);
-        // 13.
-        m_tree.reparent(*furthestBlock, newItem);
+        // 12. & 13.
+        m_tree.takeAllChildrenAndReparent(newItem, *furthestBlock);
         // 14.
         m_tree.activeFormattingElements().swapTo(*formattingElement, newItem.copyRef(), bookmark);
         // 15.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to