Merge r163579 - Make adoption agency use the task queue

https://bugs.webkit.org/show_bug.cgi?id=109445 Reviewed by Ryosuke Niwa. Source/WebCore: Tests: fast/parser/adoption-agency-crash-01.html fast/parser/adoption-agency-crash-02.html fast/parser/adoption-agency-crash-03.html * html/parser/HTMLConstructionSite.cpp: (WebCore::insert): (WebCore::executeInsertTask): (WebCore::executeReparentTask): (WebCore::executeInsertAlreadyParsedChildTask): (WebCore::executeTakeAllChildrenTask): (WebCore::executeTask): (WebCore::HTMLConstructionSite::attachLater): (WebCore::HTMLConstructionSite::executeQueuedTasks): (WebCore::HTMLConstructionSite::insertTextNode): (WebCore::HTMLConstructionSite::reparent): (WebCore::HTMLConstructionSite::insertAlreadyParsedChild): (WebCore::HTMLConstructionSite::takeAllChildren): (WebCore::HTMLConstructionSite::fosterParent): * html/parser/HTMLConstructionSite.h: (WebCore::HTMLConstructionSiteTask::HTMLConstructionSiteTask): (WebCore::HTMLConstructionSiteTask::oldParent): * html/parser/HTMLTreeBuilder.cpp: (WebCore::HTMLTreeBuilder::callTheAdoptionAgency): LayoutTests: * TestExpectations: * fast/parser/adoption-agency-crash-01-expected.txt: Added. * fast/parser/adoption-agency-crash-01.html: Added. * fast/parser/adoption-agency-crash-02-expected.txt: Added. * fast/parser/adoption-agency-crash-02.html: Added. * fast/parser/adoption-agency-crash-03-expected.txt: Added. * fast/parser/adoption-agency-crash-03.html: Added.
WebKit · Apr 14, 2014 · a6079ab · a6079ab
1 parent 7fd44b9
commit a6079ab
Show file tree

Hide file tree

Showing 12 changed files with 206 additions and 38 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,18 @@
+2014-02-04  Jeffrey Pfau  <jpfau@apple.com>
+
+        Make adoption agency use the task queue
+        https://bugs.webkit.org/show_bug.cgi?id=109445
+
+        Reviewed by Ryosuke Niwa.
+
+        * TestExpectations:
+        * fast/parser/adoption-agency-crash-01-expected.txt: Added.
+        * fast/parser/adoption-agency-crash-01.html: Added.
+        * fast/parser/adoption-agency-crash-02-expected.txt: Added.
+        * fast/parser/adoption-agency-crash-02.html: Added.
+        * fast/parser/adoption-agency-crash-03-expected.txt: Added.
+        * fast/parser/adoption-agency-crash-03.html: Added.
+
 2014-02-04  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Move characterAt index checks from InlineIterator to RenderText

diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
@@ -91,3 +91,7 @@ webkit.org/b/126166 [ Debug ] js/dfg-uint32array-overflow-values.html [ Skip ]
 fast/harness/sample-fail-mismatch-reftest.html [ WontFix ImageOnlyFailure ]
 
 webkit.org/b/127697 fast/writing-mode/ruby-text-logical-left.html [ Skip ]
+
+# These will be fixed soon
+[ Debug ] fast/parser/adoption-agency-crash-01.html [ Crash ]
+[ Debug ] fast/parser/adoption-agency-crash-03.html [ Crash ]
diff --git a/LayoutTests/fast/parser/adoption-agency-crash-01-expected.txt b/LayoutTests/fast/parser/adoption-agency-crash-01-expected.txt
@@ -0,0 +1,2 @@
+PASS
+
diff --git a/LayoutTests/fast/parser/adoption-agency-crash-01.html b/LayoutTests/fast/parser/adoption-agency-crash-01.html
@@ -0,0 +1,6 @@
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+document.write('<a><p><iframe onload="document.write(\'<script>void(0)<\/script></a>\');"></iframe><script>document.body.innerHTML = \'PASS\';<\/script>');
+</script>
diff --git a/LayoutTests/fast/parser/adoption-agency-crash-02-expected.txt b/LayoutTests/fast/parser/adoption-agency-crash-02-expected.txt
@@ -0,0 +1 @@
+PASS
diff --git a/LayoutTests/fast/parser/adoption-agency-crash-02.html b/LayoutTests/fast/parser/adoption-agency-crash-02.html
@@ -0,0 +1,6 @@
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+document.write('<a><p><iframe onload="document.write(\'<script>document.body.innerHTML = &quot;PASS&quot;;<\/script></a>\');"></iframe>');
+</script>
diff --git a/LayoutTests/fast/parser/adoption-agency-crash-03-expected.txt b/LayoutTests/fast/parser/adoption-agency-crash-03-expected.txt
@@ -0,0 +1,2 @@
+PASS 1 of 2
+PASS 2 of 2
diff --git a/LayoutTests/fast/parser/adoption-agency-crash-03.html b/LayoutTests/fast/parser/adoption-agency-crash-03.html
@@ -0,0 +1,5 @@
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+<iframe onload="document.write('<a><blockquote>PASS 2 of 2<iframe onload=&quot;document.write(\'<a>\')&quot;></iframe><script>document.body.innerHTML = \'PASS 1 of 2\';</script>');">
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,34 @@
+2014-02-04  Jeffrey Pfau  <jpfau@apple.com>
+
+        Make adoption agency use the task queue
+        https://bugs.webkit.org/show_bug.cgi?id=109445
+
+        Reviewed by Ryosuke Niwa.
+
+        Tests: fast/parser/adoption-agency-crash-01.html
+               fast/parser/adoption-agency-crash-02.html
+               fast/parser/adoption-agency-crash-03.html
+
+        * html/parser/HTMLConstructionSite.cpp:
+        (WebCore::insert):
+        (WebCore::executeInsertTask):
+        (WebCore::executeReparentTask):
+        (WebCore::executeInsertAlreadyParsedChildTask):
+        (WebCore::executeTakeAllChildrenTask):
+        (WebCore::executeTask):
+        (WebCore::HTMLConstructionSite::attachLater):
+        (WebCore::HTMLConstructionSite::executeQueuedTasks):
+        (WebCore::HTMLConstructionSite::insertTextNode):
+        (WebCore::HTMLConstructionSite::reparent):
+        (WebCore::HTMLConstructionSite::insertAlreadyParsedChild):
+        (WebCore::HTMLConstructionSite::takeAllChildren):
+        (WebCore::HTMLConstructionSite::fosterParent):
+        * html/parser/HTMLConstructionSite.h:
+        (WebCore::HTMLConstructionSiteTask::HTMLConstructionSiteTask):
+        (WebCore::HTMLConstructionSiteTask::oldParent):
+        * html/parser/HTMLTreeBuilder.cpp:
+        (WebCore::HTMLTreeBuilder::callTheAdoptionAgency):
+
 2014-02-04  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Move characterAt index checks from InlineIterator to RenderText

diff --git a/Source/WebCore/html/parser/HTMLConstructionSite.cpp b/Source/WebCore/html/parser/HTMLConstructionSite.cpp
@@ -79,30 +79,86 @@ static inline bool isAllWhitespace(const String& string)
     return string.isAllSpecialCharacters<isHTMLSpace>();
 }
 
-static inline void executeTask(HTMLConstructionSiteTask& task)
+static inline void insert(HTMLConstructionSiteTask& task)
 {
 #if ENABLE(TEMPLATE_ELEMENT)
     if (task.parent->hasTagName(templateTag))
         task.parent = toHTMLTemplateElement(task.parent.get())->content();
 #endif
 
+    if (ContainerNode* parent = task.child->parentNode())
+        parent->parserRemoveChild(*task.child);
+
     if (task.nextChild)
         task.parent->parserInsertBefore(task.child.get(), task.nextChild.get());
     else
         task.parent->parserAppendChild(task.child.get());
+}
+
+static inline void executeInsertTask(HTMLConstructionSiteTask& task)
+{
+    ASSERT(task.operation == HTMLConstructionSiteTask::Insert);
+
+    insert(task);
 
     task.child->beginParsingChildren();
 
     if (task.selfClosing)
         task.child->finishParsingChildren();
 }
 
+static inline void executeReparentTask(HTMLConstructionSiteTask& task)
+{
+    ASSERT(task.operation == HTMLConstructionSiteTask::Reparent);
+
+    if (ContainerNode* parent = task.child->parentNode())
+        parent->parserRemoveChild(*task.child);
+
+    task.parent->parserAppendChild(task.child);
+}
+
+static inline void executeInsertAlreadyParsedChildTask(HTMLConstructionSiteTask& task)
+{
+    ASSERT(task.operation == HTMLConstructionSiteTask::InsertAlreadyParsedChild);
+
+    insert(task);
+}
+
+static inline void executeTakeAllChildrenTask(HTMLConstructionSiteTask& task)
+{
+    ASSERT(task.operation == HTMLConstructionSiteTask::TakeAllChildren);
+
+    task.parent->takeAllChildrenFrom(task.oldParent());
+    // Notice that we don't need to manually attach the moved children
+    // because takeAllChildrenFrom does that work for us.
+}
+
+static inline void executeTask(HTMLConstructionSiteTask& task)
+{
+    switch (task.operation) {
+    case HTMLConstructionSiteTask::Insert:
+        executeInsertTask(task);
+        return;
+    // All the cases below this point are only used by the adoption agency.
+    case HTMLConstructionSiteTask::InsertAlreadyParsedChild:
+        executeInsertAlreadyParsedChildTask(task);
+        return;
+    case HTMLConstructionSiteTask::Reparent:
+        executeReparentTask(task);
+        return;
+    case HTMLConstructionSiteTask::TakeAllChildren:
+        executeTakeAllChildrenTask(task);
+        return;
+    }
+    ASSERT_NOT_REACHED();
+}
+
 void HTMLConstructionSite::attachLater(ContainerNode* parent, PassRefPtr<Node> prpChild, bool selfClosing)
 {
     ASSERT(scriptingContentIsAllowed(m_parserContentPolicy) || !prpChild.get()->isElementNode() || !toScriptElementIfPossible(toElement(prpChild.get())));
     ASSERT(pluginContentIsAllowed(m_parserContentPolicy) || !prpChild->isPluginElement());
 
-    HTMLConstructionSiteTask task;
+    HTMLConstructionSiteTask task(HTMLConstructionSiteTask::Insert);
     task.parent = parent;
     task.child = prpChild;
     task.selfClosing = selfClosing;
@@ -117,19 +173,18 @@ void HTMLConstructionSite::attachLater(ContainerNode* parent, PassRefPtr<Node> p
         task.parent = task.parent->parentNode();
 
     ASSERT(task.parent);
-    m_attachmentQueue.append(task);
+    m_taskQueue.append(task);
 }
 
 void HTMLConstructionSite::executeQueuedTasks()
 {
-    const size_t size = m_attachmentQueue.size();
+    const size_t size = m_taskQueue.size();
     if (!size)
         return;
 
     // Copy the task queue into a local variable in case executeTask
     // re-enters the parser.
-    AttachmentQueue queue;
-    queue.swap(m_attachmentQueue);
+    TaskQueue queue = std::move(m_taskQueue);
 
     for (size_t i = 0; i < size; ++i)
         executeTask(queue[i]);
@@ -466,7 +521,7 @@ void HTMLConstructionSite::insertForeignElement(AtomicHTMLToken* token, const At
 
 void HTMLConstructionSite::insertTextNode(const String& characters, WhitespaceMode whitespaceMode)
 {
-    HTMLConstructionSiteTask task;
+    HTMLConstructionSiteTask task(HTMLConstructionSiteTask::Insert);
     task.parent = currentNode();
 
     if (shouldFosterParent())
@@ -512,6 +567,43 @@ void HTMLConstructionSite::insertTextNode(const String& characters, WhitespaceMo
     }
 }
 
+void HTMLConstructionSite::reparent(HTMLElementStack::ElementRecord& newParent, HTMLElementStack::ElementRecord& child)
+{
+    HTMLConstructionSiteTask task(HTMLConstructionSiteTask::Reparent);
+    task.parent = newParent.node();
+    task.child = child.element();
+    m_taskQueue.append(task);
+}
+
+void HTMLConstructionSite::reparent(HTMLElementStack::ElementRecord& newParent, HTMLStackItem& child)
+{
+    HTMLConstructionSiteTask task(HTMLConstructionSiteTask::Reparent);
+    task.parent = newParent.node();
+    task.child = child.element();
+    m_taskQueue.append(task);
+}
+
+void HTMLConstructionSite::insertAlreadyParsedChild(HTMLStackItem& newParent, HTMLElementStack::ElementRecord& child)
+{
+    if (newParent.causesFosterParenting()) {
+        fosterParent(child.element());
+        return;
+    }
+
+    HTMLConstructionSiteTask task(HTMLConstructionSiteTask::InsertAlreadyParsedChild);
+    task.parent = newParent.node();
+    task.child = child.element();
+    m_taskQueue.append(task);
+}
+
+void HTMLConstructionSite::takeAllChildren(HTMLStackItem& newParent, HTMLElementStack::ElementRecord& oldParent)
+{
+    HTMLConstructionSiteTask task(HTMLConstructionSiteTask::TakeAllChildren);
+    task.parent = newParent.node();
+    task.child = oldParent.node();
+    m_taskQueue.append(task);
+}
+
 PassRefPtr<Element> HTMLConstructionSite::createElement(AtomicHTMLToken* token, const AtomicString& namespaceURI)
 {
     QualifiedName tagName(nullAtom, token->name(), namespaceURI);
@@ -655,12 +747,12 @@ bool HTMLConstructionSite::shouldFosterParent() const
 
 void HTMLConstructionSite::fosterParent(PassRefPtr<Node> node)
 {
-    HTMLConstructionSiteTask task;
+    HTMLConstructionSiteTask task(HTMLConstructionSiteTask::Insert);
     findFosterSite(task);
     task.child = node;
     ASSERT(task.parent);
 
-    m_attachmentQueue.append(task);
+    m_taskQueue.append(task);
 }
 
 }
diff --git a/Source/WebCore/html/parser/HTMLConstructionSite.h b/Source/WebCore/html/parser/HTMLConstructionSite.h
@@ -38,11 +38,28 @@
 namespace WebCore {
 
 struct HTMLConstructionSiteTask {
-    HTMLConstructionSiteTask()
-        : selfClosing(false)
+    enum Operation {
+        Insert,
+        InsertAlreadyParsedChild,
+        Reparent,
+        TakeAllChildren,
+    };
+
+    explicit HTMLConstructionSiteTask(Operation op)
+        : operation(op)
+        , selfClosing(false)
+    {
+    }
+
+    ContainerNode* oldParent()
     {
+        // It's sort of ugly, but we store the |oldParent| in the |child| field
+        // of the task so that we don't bloat the HTMLConstructionSiteTask
+        // object in the common case of the Insert operation.
+        return toContainerNode(child.get());
     }
 
+    Operation operation;
     RefPtr<ContainerNode> parent;
     RefPtr<Node> nextChild;
     RefPtr<Node> child;
@@ -99,6 +116,14 @@ class HTMLConstructionSite {
     void insertHTMLHtmlStartTagInBody(AtomicHTMLToken*);
     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);
+
     PassRefPtr<HTMLStackItem> createElementFromSavedToken(HTMLStackItem*);
 
     bool shouldFosterParent() const;
@@ -160,7 +185,7 @@ class HTMLConstructionSite {
 private:
     // In the common case, this queue will have only one task because most
     // tokens produce only one DOM mutation.
-    typedef Vector<HTMLConstructionSiteTask, 1> AttachmentQueue;
+    typedef Vector<HTMLConstructionSiteTask, 1> TaskQueue;
 
     void setCompatibilityMode(Document::CompatibilityMode);
     void setCompatibilityModeFromDoctype(const String& name, const String& publicId, const String& systemId);
@@ -187,7 +212,7 @@ class HTMLConstructionSite {
     mutable HTMLElementStack m_openElements;
     mutable HTMLFormattingElementList m_activeFormattingElements;
 
-    AttachmentQueue m_attachmentQueue;
+    TaskQueue m_taskQueue;
 
     ParserContentPolicy m_parserContentPolicy;
     bool m_isParsingFragment;

diff --git a/Source/WebCore/html/parser/HTMLTreeBuilder.cpp b/Source/WebCore/html/parser/HTMLTreeBuilder.cpp
@@ -1600,39 +1600,18 @@ void HTMLTreeBuilder::callTheAdoptionAgency(AtomicHTMLToken* token)
             if (lastNode == furthestBlock)
                 bookmark.moveToAfter(nodeEntry);
             // 9.9
-            if (ContainerNode* parent = lastNode->element()->parentNode())
-                parent->parserRemoveChild(*lastNode->element());
-            node->element()->parserAppendChild(lastNode->element());
+            m_tree.reparent(*node, *lastNode);
             // 9.10
             lastNode = node;
         }
         // 10.
-        if (ContainerNode* parent = lastNode->element()->parentNode())
-            parent->parserRemoveChild(*lastNode->element());
-        if (commonAncestor->causesFosterParenting())
-            m_tree.fosterParent(lastNode->element());
-        else {
-#if ENABLE(TEMPLATE_ELEMENT)
-            if (commonAncestor->hasTagName(templateTag))
-                toHTMLTemplateElement(commonAncestor->node())->content()->parserAppendChild(lastNode->element());
-            else
-                commonAncestor->node()->parserAppendChild(lastNode->element());
-#else
-            commonAncestor->node()->parserAppendChild(lastNode->element());
-#endif
-            ASSERT(lastNode->stackItem()->isElementNode());
-            ASSERT(lastNode->element()->parentNode());
-        }
+        m_tree.insertAlreadyParsedChild(*commonAncestor, *lastNode);
         // 11.
         RefPtr<HTMLStackItem> newItem = m_tree.createElementFromSavedToken(formattingElementRecord->stackItem().get());
         // 12.
-        newItem->element()->takeAllChildrenFrom(furthestBlock->element());
+        m_tree.takeAllChildren(*newItem, *furthestBlock);
         // 13.
-        Element* furthestBlockElement = furthestBlock->element();
-        // FIXME: All this creation / parserAppendChild / attach business should
-        //        be in HTMLConstructionSite. My guess is that steps 11--15
-        //        should all be in some HTMLConstructionSite function.
-        furthestBlockElement->parserAppendChild(newItem->element());
+        m_tree.reparent(*furthestBlock, *newItem);
         // 14.
         m_tree.activeFormattingElements()->swapTo(formattingElement, newItem, bookmark);
         // 15.