Ignore:
Timestamp:
Feb 12, 2017 8:21:03 PM (10 months ago)
Author:
rniwa@webkit.org
Message:

parserRemoveChild should unload subframes
​https://bugs.webkit.org/show_bug.cgi?id=168151

Reviewed by Darin Adler.

Source/WebCore:

Fix the bug that the adoption agency algorithm does not unload subframes as it disconnects nodes.

Also moved calls to nodeWillBeRemoved inside NoEventDispatchAssertion to expand on r211965.

Tests: fast/parser/adoption-agency-clear-focus-range.html

fast/parser/adoption-agency-unload-iframe-1.html
fast/parser/adoption-agency-unload-iframe-2.html

  • dom/ContainerNode.cpp:

(WebCore::ContainerNode::takeAllChildrenFrom): Rewritten using idioms used in removeChildren and parserAppendChild.

Disconnect all subframes first since this can synchronously dispatch an unload event. Then update DOM ranges,
the focused element, and other states in the document.

Second, use the regular removeBetween, notifyChildNodeRemoved, childrenChanged sequence of calls to disconnect nodes
instead of a single call to removeDetachedChildren to properly disconnect child nodes since those nodes may have
already come live due to execution of synchronous scripts prior to the adoption agency algorithm has run, or in
response to the unload event we just dispatched.

Third, append these nodes using parserAppendChild to avoid dispatching mutation events.

(WebCore::willRemoveChild): Removed the call to nodeWillBeRemoved. It's now called within NoEventDispatchAssertion
in each call site of willRemoveChild and willRemoveChildren.
(WebCore::willRemoveChildren): Ditto.
(WebCore::ContainerNode::removeChild): Call nodeWillBeRemoved inside NoEventDispatchAssertion.
(WebCore::ContainerNode::replaceAllChildren): Call nodeWillBeRemoved inside NoEventDispatchAssertion.
(WebCore::ContainerNode::parserRemoveChild): Disconnect subframes and update document's states.

  • html/parser/HTMLConstructionSite.cpp:

(WebCore::executeTakeAllChildrenAndReparentTask): Add a release assert that new parent does not already have a parent.

LayoutTests:

Add two W3C-style testharness tests for unloading iframes inside the adoption agency algorithm.

Also added a test to make sure ContainerNode::takeAllChildrenFrom adjusts the focused element and DOM ranges.

  • fast/css/stylesheet-candidate-nodes-crash-expected.txt: Rebaselined. The difference comes from the fact

iframe now is unloaded in parserRemoveChild as expected and then reloaded in parserAppendChild inside
insertErrorMessageBlock as opposed to after the parser had completed as if the iframe had never been detached.

  • fast/parser/adoption-agency-clear-focus-range-expected.txt: Added.
  • fast/parser/adoption-agency-clear-focus-range.html: Added.
  • fast/parser/adoption-agency-unload-iframe-1-expected.txt: Added.
  • fast/parser/adoption-agency-unload-iframe-1.html: Added.
  • fast/parser/adoption-agency-unload-iframe-2-expected.txt: Added.
  • fast/parser/adoption-agency-unload-iframe-2.html: Added.
File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/dom/ContainerNode.cpp

    r211965 r212218  
    131131    }
    132132
    133     // FIXME: We need to do notifyMutationObserversNodeWillDetach() for each child,
    134     // probably inside removeDetachedChildrenInContainer.
    135 
    136     oldParent->removeDetachedChildren();
    137 
     133    disconnectSubframesIfNeeded(*oldParent, DescendantsOnly);
     134    {
     135        NoEventDispatchAssertion assertNoEventDispatch;
     136
     137        oldParent->document().nodeChildrenWillBeRemoved(*oldParent);
     138
     139        WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
     140        while (RefPtr<Node> child = oldParent->m_firstChild) {
     141            oldParent->removeBetween(nullptr, child->nextSibling(), *child);
     142            notifyChildNodeRemoved(*oldParent, *child);
     143        }
     144        ChildChange change = { AllChildrenRemoved, nullptr, nullptr, ChildChangeSourceParser };
     145        childrenChanged(change);
     146    }
     147
     148    // FIXME: assert that we don't dispatch events here since this container node is still disconnected.
    138149    for (auto& child : children) {
    139         destroyRenderTreeIfNeeded(child);
    140 
    141         // FIXME: We need a no mutation event version of adoptNode.
    142         auto adoptedChild = document().adoptNode(child).releaseReturnValue();
    143         parserAppendChild(adoptedChild);
    144         // FIXME: Together with adoptNode above, the tree scope might get updated recursively twice
    145         // (if the document changed or oldParent was in a shadow tree, AND *this is in a shadow tree).
    146         // Can we do better?
    147         treeScope().adoptIfNeeded(adoptedChild);
     150        RELEASE_ASSERT(!child->parentNode() && &child->treeScope() == &treeScope());
     151        ASSERT(!ensurePreInsertionValidity(child, nullptr).hasException());
     152        treeScope().adoptIfNeeded(child);
     153        parserAppendChild(child);
    148154    }
    149155}
    … …  
    487493    if (is<ContainerNode>(child))
    488494        disconnectSubframesIfNeeded(downcast<ContainerNode>(child), RootAndDescendants);
    489 
    490     if (child.parentNode() != &container)
    491         return;
    492 
    493     child.document().nodeWillBeRemoved(child); // e.g. mutation event listener can create a new range.
    494495}
    495496
    … …  
    509510
    510511    disconnectSubframesIfNeeded(container, DescendantsOnly);
    511 
    512     container.document().nodeChildrenWillBeRemoved(container);
    513 
    514512}
    515513
    … …  
    535533    willRemoveChild(*this, child);
    536534
    537     // Mutation events might have moved this child into a different parent.
     535    // Mutation events in willRemoveChild might have moved this child into a different parent.
    538536    if (child->parentNode() != this)
    539537        return Exception { NOT_FOUND_ERR };
    … …  
    542540        WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
    543541        NoEventDispatchAssertion assertNoEventDispatch;
     542
     543        document().nodeWillBeRemoved(child);
    544544
    545545        Node* prev = child->previousSibling();
    … …  
    592592void ContainerNode::parserRemoveChild(Node& oldChild)
    593593{
     594    disconnectSubframesIfNeeded(*this, DescendantsOnly);
     595
    594596    NoEventDispatchAssertion assertNoEventDispatch;
     597
     598    document().nodeChildrenWillBeRemoved(*this);
    595599
    596600    ASSERT(oldChild.parentNode() == this);
    … …  
    599603    Node* prev = oldChild.previousSibling();
    600604    Node* next = oldChild.nextSibling();
    601 
    602     oldChild.updateAncestorConnectedSubframeCountForRemoval();
    603605
    604606    ChildListMutationScope(*this).willRemoveChild(oldChild);
    … …  
    644646        {
    645647            NoEventDispatchAssertion assertNoEventDispatch;
     648
     649            document().nodeChildrenWillBeRemoved(*this);
     650
    646651            while (RefPtr<Node> child = m_firstChild) {
    647652                removeBetween(nullptr, child->nextSibling(), *child);
    … …  
    686691        WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
    687692        NoEventDispatchAssertion assertNoEventDispatch;
     693
     694        document().nodeChildrenWillBeRemoved(*this);
    688695
    689696        while (RefPtr<Node> child = m_firstChild) {
Note: See TracChangeset for help on using the changeset viewer.