Changeset 212621 in webkit


Ignore:
Timestamp:
Feb 19, 2017 10:22:02 PM (7 years ago)
Author:
rniwa@webkit.org
Message:

REGRESSION(r212218): Assertion failures in and after parserRemoveChild
​https://bugs.webkit.org/show_bug.cgi?id=168458

Reviewed by Antti Koivisto.

Source/WebCore:

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.

LayoutTests:

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.
Location:
trunk
Files:
6 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r212617 r212621  
     12017-02-18  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        REGRESSION(r212218): Assertion failures in and after parserRemoveChild
     4        https://bugs.webkit.org/show_bug.cgi?id=168458
     5
     6        Reviewed by Antti Koivisto.
     7
     8        Add tests to make sure parserAppendChild aren't called when a node removed by parserRemoveChild
     9        had already been been inserted elsewhere by scripts.
     10
     11        * fast/parser/adoption-agency-unload-iframe-3-expected.txt: Added.
     12        * fast/parser/adoption-agency-unload-iframe-3.html: Added.
     13        * fast/parser/adoption-agency-unload-iframe-4-expected.txt: Added.
     14        * fast/parser/adoption-agency-unload-iframe-4.html: Added.
     15        * fast/parser/xml-error-unload-iframe-expected.txt: Added.
     16        * fast/parser/xml-error-unload-iframe.html: Added.
     17
    1182017-02-19  Chris Dumez  <cdumez@apple.com>
    219

  • trunk/Source/WebCore/ChangeLog

    r212617 r212621  
     12017-02-18  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        REGRESSION(r212218): Assertion failures in and after parserRemoveChild
     4        https://bugs.webkit.org/show_bug.cgi?id=168458
     5
     6        Reviewed by Antti Koivisto.
     7
     8        The bug was caused by parserRemoveChild not preceeding to remove oldChild even when
     9        oldChild had been inserted elsewhere during unload evnets of the disconnected frames.
     10        Fixed the bug by checking this condition and exiting early.
     11
     12        Also fixed various callers of parserRemoveChild to not call parserAppendChild when
     13        the removed node had already been inserted elsewhere by scripts.
     14
     15        Tests: fast/parser/adoption-agency-unload-iframe-3.html
     16               fast/parser/adoption-agency-unload-iframe-4.html
     17               fast/parser/xml-error-unload-iframe.html
     18
     19        * dom/ContainerNode.cpp:
     20        (WebCore::ContainerNode::parserRemoveChild): Exit early when the node had been
     21        inserted elsewhere while firing unload events. Also moved the call to
     22        notifyRemovePendingSheetIfNeeded outside NoEventDispatchAssertion since it can
     23        synchrnously fire a focus event.
     24        (WebCore::ContainerNode::parserAppendChild): Moved adoptNode call to inside
     25        NoEventDispatchAssertion since adoptNode call here should never mutate DOM.
     26        * html/parser/HTMLConstructionSite.cpp:
     27        (WebCore::executeReparentTask): Added an early exit when the node had already been
     28        inserted elsewhere.
     29        (WebCore::executeInsertAlreadyParsedChildTask): Ditto.
     30        * xml/XMLErrors.cpp:
     31        (WebCore::XMLErrors::insertErrorMessageBlock): Ditto.
     32        * xml/parser/XMLDocumentParser.cpp:
     33        (WebCore::XMLDocumentParser::end): Fixed a crash unveiled by one of the test cases.
     34        Exit early when insertErrorMessageBlock detached the parser (by author scripts).
     35        (WebCore::XMLDocumentParser::finish): Keep the parser alive until we exit.
     36
    1372017-02-19  Chris Dumez  <cdumez@apple.com>
    238

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

    r212614 r212621  
    593593{
    594594    disconnectSubframesIfNeeded(*this, DescendantsOnly);
    595 
    596     NoEventDispatchAssertion assertNoEventDispatch;
    597 
    598     document().nodeChildrenWillBeRemoved(*this);
    599 
    600     ASSERT(oldChild.parentNode() == this);
    601     ASSERT(!oldChild.isDocumentFragment());
    602 
    603     Node* prev = oldChild.previousSibling();
    604     Node* next = oldChild.nextSibling();
    605 
    606     ChildListMutationScope(*this).willRemoveChild(oldChild);
    607     oldChild.notifyMutationObserversNodeWillDetach();
    608 
    609     removeBetween(prev, next, oldChild);
    610 
    611     notifyChildRemoved(oldChild, prev, next, ChildChangeSourceParser);
     595    if (oldChild.parentNode() != this)
     596        return;
     597
     598    {
     599        NoEventDispatchAssertion assertNoEventDispatch;
     600
     601        document().nodeChildrenWillBeRemoved(*this);
     602
     603        ASSERT(oldChild.parentNode() == this);
     604        ASSERT(!oldChild.isDocumentFragment());
     605
     606        Node* prev = oldChild.previousSibling();
     607        Node* next = oldChild.nextSibling();
     608
     609        ChildListMutationScope(*this).willRemoveChild(oldChild);
     610        oldChild.notifyMutationObserversNodeWillDetach();
     611
     612        removeBetween(prev, next, oldChild);
     613
     614        notifyChildRemoved(oldChild, prev, next, ChildChangeSourceParser);
     615    }
    612616}
    613617
    … …  
    769773    ASSERT(!hasTagName(HTMLNames::templateTag));
    770774
    771     if (&document() != &newChild.document())
    772         document().adoptNode(newChild);
    773 
    774775    {
    775776        NoEventDispatchAssertion assertNoEventDispatch;
     777
     778        if (&document() != &newChild.document())
     779            document().adoptNode(newChild);
     780
    776781        appendChildCommon(newChild);
    777782        treeScope().adoptIfNeeded(newChild);

  • trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp

    r212218 r212621  
    132132        parent->parserRemoveChild(*task.child);
    133133
     134    if (task.child->parentNode())
     135        return;
     136
    134137    task.parent->parserAppendChild(*task.child);
    135138}
    … …  
    141144    if (ContainerNode* parent = task.child->parentNode())
    142145        parent->parserRemoveChild(*task.child);
     146
     147    if (task.child->parentNode())
     148        return;
     149
    143150    insert(task);
    144151}

  • trunk/Source/WebCore/xml/XMLErrors.cpp

    r209627 r212621  
    141141
    142142        m_document.parserRemoveChild(*documentElement);
     143        if (!documentElement->parentNode())
     144            body->parserAppendChild(*documentElement);
    143145
    144         body->parserAppendChild(*documentElement);
    145146        m_document.parserAppendChild(rootElement);
    146147

  • trunk/Source/WebCore/xml/parser/XMLDocumentParser.cpp

    r210319 r212621  
    196196        return;
    197197
    198     if (m_sawError)
     198    if (m_sawError) {
    199199        insertErrorMessageBlock();
    200     else {
     200        if (isDetached()) // Inserting an error message may have ran arbitrary scripts.
     201            return;
     202    } else {
    201203        updateLeafTextNode();
    202204        document()->styleScope().didChangeStyleSheetEnvironment();
    … …  
    215217    // makes sense to call any methods on DocumentParser once it's been stopped.
    216218    // However, FrameLoader::stop calls DocumentParser::finish unconditionally.
     219
     220    Ref<XMLDocumentParser> protectedThis(*this);
    217221
    218222    if (m_parserPaused)
Note: See TracChangeset for help on using the changeset viewer.