Changeset 212647 in webkit

Timestamp:

Feb 20, 2017 9:29:20 AM (7 years ago)

Author:

matthew_hanson@apple.com

Message:

Merge r212621. rdar://problem/30563318

Location:

branches/safari-603-branch

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/parser/adoption-agency-unload-iframe-3-expected.txt (added)
• LayoutTests/fast/parser/adoption-agency-unload-iframe-3.html (added)
• LayoutTests/fast/parser/adoption-agency-unload-iframe-4-expected.txt (added)
• LayoutTests/fast/parser/adoption-agency-unload-iframe-4.html (added)
• LayoutTests/fast/parser/xml-error-unload-iframe-expected.txt (added)
• LayoutTests/fast/parser/xml-error-unload-iframe.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/ContainerNode.cpp (modified) (2 diffs)
• Source/WebCore/html/parser/HTMLConstructionSite.cpp (modified) (2 diffs)
• Source/WebCore/xml/XMLErrors.cpp (modified) (1 diff)
• Source/WebCore/xml/parser/XMLDocumentParser.cpp (modified) (2 diffs)

branches/safari-603-branch/LayoutTests/ChangeLog

@@ +1 @@
+2017-02-20  Matthew Hanson  <matthew_hanson@apple.com>
+
+        Merge r212621. rdar://problem/30563318
+
+    2017-02-18  Ryosuke Niwa  <rniwa@webkit.org>
+
+            REGRESSION(r212218): Assertion failures in and after parserRemoveChild
+            https://bugs.webkit.org/show_bug.cgi?id=168458
+
+            Reviewed by Antti Koivisto.
+
+            Add tests to make sure parserAppendChild aren't called when a node removed by parserRemoveChild
+            had already been been inserted elsewhere by scripts.
+
+            * fast/parser/adoption-agency-unload-iframe-3-expected.txt: Added.
+            * fast/parser/adoption-agency-unload-iframe-3.html: Added.
+            * fast/parser/adoption-agency-unload-iframe-4-expected.txt: Added.
+            * fast/parser/adoption-agency-unload-iframe-4.html: Added.
+            * fast/parser/xml-error-unload-iframe-expected.txt: Added.
+            * fast/parser/xml-error-unload-iframe.html: Added.
+
 2017-02-20  Matthew Hanson  <matthew_hanson@apple.com>
 

branches/safari-603-branch/Source/WebCore/ChangeLog

@@ +1 @@
+2017-02-20  Matthew Hanson  <matthew_hanson@apple.com>
+
+        Merge r212621. rdar://problem/30563318
+
+    2017-02-18  Ryosuke Niwa  <rniwa@webkit.org>
+
+            REGRESSION(r212218): Assertion failures in and after parserRemoveChild
+            https://bugs.webkit.org/show_bug.cgi?id=168458
+
+            Reviewed by Antti Koivisto.
+
+            The bug was caused by parserRemoveChild not preceeding to remove oldChild even when
+            oldChild had been inserted elsewhere during unload evnets of the disconnected frames.
+            Fixed the bug by checking this condition and exiting early.
+
+            Also fixed various callers of parserRemoveChild to not call parserAppendChild when
+            the removed node had already been inserted elsewhere by scripts.
+
+            Tests: fast/parser/adoption-agency-unload-iframe-3.html
+                   fast/parser/adoption-agency-unload-iframe-4.html
+                   fast/parser/xml-error-unload-iframe.html
+
+            * dom/ContainerNode.cpp:
+            (WebCore::ContainerNode::parserRemoveChild): Exit early when the node had been
+            inserted elsewhere while firing unload events. Also moved the call to
+            notifyRemovePendingSheetIfNeeded outside NoEventDispatchAssertion since it can
+            synchrnously fire a focus event.
+            (WebCore::ContainerNode::parserAppendChild): Moved adoptNode call to inside
+            NoEventDispatchAssertion since adoptNode call here should never mutate DOM.
+            * html/parser/HTMLConstructionSite.cpp:
+            (WebCore::executeReparentTask): Added an early exit when the node had already been
+            inserted elsewhere.
+            (WebCore::executeInsertAlreadyParsedChildTask): Ditto.
+            * xml/XMLErrors.cpp:
+            (WebCore::XMLErrors::insertErrorMessageBlock): Ditto.
+            * xml/parser/XMLDocumentParser.cpp:
+            (WebCore::XMLDocumentParser::end): Fixed a crash unveiled by one of the test cases.
+            Exit early when insertErrorMessageBlock detached the parser (by author scripts).
+            (WebCore::XMLDocumentParser::finish): Keep the parser alive until we exit.
+
 2017-02-20  Matthew Hanson  <matthew_hanson@apple.com>
 

branches/safari-603-branch/Source/WebCore/dom/ContainerNode.cpp

@@ -599 +599 @@
 {
     disconnectSubframesIfNeeded(*this, DescendantsOnly);
-
-    NoEventDispatchAssertion assertNoEventDispatch;
-
-    document().nodeChildrenWillBeRemoved(*this);
-
-    ASSERT(oldChild.parentNode() == this);
-    ASSERT(!oldChild.isDocumentFragment());
-
-    Node* prev = oldChild.previousSibling();
-    Node* next = oldChild.nextSibling();
-
-    ChildListMutationScope(*this).willRemoveChild(oldChild);
-    oldChild.notifyMutationObserversNodeWillDetach();
-
-    removeBetween(prev, next, oldChild);
-
-    notifyChildRemoved(oldChild, prev, next, ChildChangeSourceParser);
-    document().notifyRemovePendingSheetIfNeeded();
+    if (oldChild.parentNode() != this)
+        return;
+
+    {
+        NoEventDispatchAssertion assertNoEventDispatch;
+
+        document().nodeChildrenWillBeRemoved(*this);
+
+        ASSERT(oldChild.parentNode() == this);
+        ASSERT(!oldChild.isDocumentFragment());
+
+        Node* prev = oldChild.previousSibling();
+        Node* next = oldChild.nextSibling();
+
+        ChildListMutationScope(*this).willRemoveChild(oldChild);
+        oldChild.notifyMutationObserversNodeWillDetach();
+
+        removeBetween(prev, next, oldChild);
+
+        notifyChildRemoved(oldChild, prev, next, ChildChangeSourceParser);
+        document().notifyRemovePendingSheetIfNeeded();
+    }
 }
 
@@ -720 +724 @@
     ASSERT(!hasTagName(HTMLNames::templateTag));
 
-    if (&document() != &newChild.document())
-        document().adoptNode(newChild);
-
     {
         NoEventDispatchAssertion assertNoEventDispatch;
+
+        if (&document() != &newChild.document())
+            document().adoptNode(newChild);
+
         appendChildCommon(newChild);
         treeScope().adoptIfNeeded(newChild);

branches/safari-603-branch/Source/WebCore/html/parser/HTMLConstructionSite.cpp

@@ -134 +134 @@
         parent->parserRemoveChild(*task.child);
 
+    if (task.child->parentNode())
+        return;
+
     task.parent->parserAppendChild(*task.child);
 }
@@ -140 +143 @@
 {
     ASSERT(task.operation == HTMLConstructionSiteTask::InsertAlreadyParsedChild);
+
+    if (task.child->parentNode())
+        return;
 
     insert(task);

branches/safari-603-branch/Source/WebCore/xml/XMLErrors.cpp

@@ -141 +141 @@
 
         m_document.parserRemoveChild(*documentElement);
+        if (!documentElement->parentNode())
+            body->parserAppendChild(*documentElement);
 
-        body->parserAppendChild(*documentElement);
         m_document.parserAppendChild(rootElement);
 

branches/safari-603-branch/Source/WebCore/xml/parser/XMLDocumentParser.cpp

@@ -196 +196 @@
         return;
 
-    if (m_sawError)
+    if (m_sawError) {
         insertErrorMessageBlock();
-    else {
+        if (isDetached()) // Inserting an error message may have ran arbitrary scripts.
+            return;
+    } else {
         updateLeafTextNode();
         document()->styleScope().didChangeStyleSheetEnvironment();
@@ -215 +217 @@
     // makes sense to call any methods on DocumentParser once it's been stopped.
     // However, FrameLoader::stop calls DocumentParser::finish unconditionally.
+
+    Ref<XMLDocumentParser> protectedThis(*this);
 
     if (m_parserPaused)