Changeset 65769 in webkit


Ignore:
Timestamp:
Aug 20, 2010 5:25:43 PM (14 years ago)
Author:
tonyg@chromium.org
Message:

2010-08-20 Tony Gentilcore <​tonyg@chromium.org>

Reviewed by Adam Barth.

Crash in WebCore::Node::createRendererIfNeeded()
​https://bugs.webkit.org/show_bug.cgi?id=44129

  • html5lib/resources/adoption02.dat: A crazy DOM from ​https://bugs.webkig.org/show_bug.cgi?id=44170 which previously crashed. The expectation doesn't appear to be correct, but it behaves the same in Firefox 4, so I'm going to file a spec bug.
  • fast/parser/remove-misnested-iframe-in-beforeload-expected.txt:
  • fast/parser/remove-misnested-iframe-in-beforeload.html: Misnest a node in a table and remove it during its beforeload handler. The node should not be in the tree.
  • fast/parser/remove-misnested-iframe-parent-in-beforeload-expected.txt:
  • fast/parser/remove-misnested-iframe-parent-in-beforeload.html: Misnest a node in a table and remove its parent during its beforeload handler. The adoption agency should have already changed its parent to the one before the table and it and its parent should be removed.

2010-08-20 Tony Gentilcore <​tonyg@chromium.org>

Reviewed by Adam Barth.

Crash in WebCore::Node::createRendererIfNeeded()
​https://bugs.webkit.org/show_bug.cgi?id=44129

  • dom/ContainerNode.cpp: (WebCore::ContainerNode::insertBefore): Factor out core bit to insertBetween. (WebCore::ContainerNode::insertBetween): Factored out of insertBefore. (WebCore::ContainerNode::parserInsertBefore): Similar to insertBefore, but doesn't handle reparenting or dispatch DOM mutation events. (WebCore::ContainerNode::removeChild): Factor out core bit to removeBetween. (WebCore::ContainerNode::removeBetween): Facotred out of removeChild. (WebCore::ContainerNode::parserRemoveChild): Similar to removeChild, but doesn't handle reparenting or dispatch DOM mutation events. (WebCore::ContainerNode::addChildCommon): (WebCore::ContainerNode::parserAddChild): (WebCore::ContainerNode::legacyParserAddChild):
  • dom/ContainerNode.h:
  • dom/Node.cpp: (WebCore::Node::parserRemoveChild): (WebCore::Node::parserInsertBefore):
  • dom/Node.h:
  • html/HTMLConstructionSite.cpp: (WebCore::HTMLConstructionSite::attach): (WebCore::HTMLConstructionSite::attachAtSite): Use new parserInsertBefore.
  • html/HTMLTreeBuilder.cpp: (WebCore::HTMLTreeBuilder::passTokenToLegacyParser): (WebCore::HTMLTreeBuilder::reparentChildren): (WebCore::HTMLTreeBuilder::callTheAdoptionAgency): Was missing a key bit from the spec about removing the old parent if it exists.
Location:
trunk
Files:
4 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r65767 r65769  
     12010-08-20  Tony Gentilcore  <tonyg@chromium.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        Crash in WebCore::Node::createRendererIfNeeded()
     6        https://bugs.webkit.org/show_bug.cgi?id=44129
     7
     8        * html5lib/resources/adoption02.dat: A crazy DOM from https://bugs.webkig.org/show_bug.cgi?id=44170 which previously crashed. The expectation doesn't appear to be correct, but it behaves the same in Firefox 4, so I'm going to file a spec bug.
     9        * fast/parser/remove-misnested-iframe-in-beforeload-expected.txt:
     10        * fast/parser/remove-misnested-iframe-in-beforeload.html: Misnest a node in a table and remove it during its beforeload handler. The node should not be in the tree.
     11        * fast/parser/remove-misnested-iframe-parent-in-beforeload-expected.txt:
     12        * fast/parser/remove-misnested-iframe-parent-in-beforeload.html: Misnest a node in a table and remove its parent during its beforeload handler. The adoption agency should have already changed its parent to the one before the table and it and its parent should be removed.
     13
    1142010-08-20  Martin Robinson  <mrobinson@igalia.com>
    215

  • trunk/LayoutTests/html5lib/resources/adoption01.dat

    r63858 r65769  
    182182|           <td>
    183183|             "B"
     184
     185#data
     186<a><svg><tr><input></a>
     187#errors
     188#document
     189| <html>
     190|   <head>
     191|   <body>
     192|     <a>
     193|       <svg svg>
     194|     <svg tr>
     195|       <a>
     196|   <svg input>
     197|     <a>

  • trunk/LayoutTests/html5lib/runner-expected.txt

    r65351 r65769  
    20320312
    204204
    205 Test 12 of 12 in resources/adoption01.dat failed. Input:
     205Test 12 of 13 in resources/adoption01.dat failed. Input:
    206206<table>A<td>B</td>C</table>
    207207Got:

  • trunk/WebCore/ChangeLog

    r65768 r65769  
     12010-08-20  Tony Gentilcore  <tonyg@chromium.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        Crash in WebCore::Node::createRendererIfNeeded()
     6        https://bugs.webkit.org/show_bug.cgi?id=44129
     7
     8        * dom/ContainerNode.cpp:
     9        (WebCore::ContainerNode::insertBefore): Factor out core bit to insertBetween.
     10        (WebCore::ContainerNode::insertBetween): Factored out of insertBefore.
     11        (WebCore::ContainerNode::parserInsertBefore): Similar to insertBefore, but doesn't handle reparenting or dispatch DOM mutation events.
     12        (WebCore::ContainerNode::removeChild): Factor out core bit to removeBetween.
     13        (WebCore::ContainerNode::removeBetween): Facotred out of removeChild.
     14        (WebCore::ContainerNode::parserRemoveChild): Similar to removeChild, but doesn't handle reparenting or dispatch DOM mutation events.
     15        (WebCore::ContainerNode::addChildCommon):
     16        (WebCore::ContainerNode::parserAddChild):
     17        (WebCore::ContainerNode::legacyParserAddChild):
     18        * dom/ContainerNode.h:
     19        * dom/Node.cpp:
     20        (WebCore::Node::parserRemoveChild):
     21        (WebCore::Node::parserInsertBefore):
     22        * dom/Node.h:
     23        * html/HTMLConstructionSite.cpp:
     24        (WebCore::HTMLConstructionSite::attach):
     25        (WebCore::HTMLConstructionSite::attachAtSite): Use new parserInsertBefore.
     26        * html/HTMLTreeBuilder.cpp:
     27        (WebCore::HTMLTreeBuilder::passTokenToLegacyParser):
     28        (WebCore::HTMLTreeBuilder::reparentChildren):
     29        (WebCore::HTMLTreeBuilder::callTheAdoptionAgency): Was missing a key bit from the spec about removing the old parent if it exists.
     30
    1312010-08-20  Kinuko Yasuda  <kinuko@chromium.org>
    232

  • trunk/WebCore/dom/ContainerNode.cpp

    r63865 r65769  
    135135            break;
    136136
    137         ASSERT(!child->nextSibling());
    138         ASSERT(!child->previousSibling());
    139 
    140         // Add child before "next".
    141         forbidEventDispatch();
    142         Node* prev = next->previousSibling();
    143         ASSERT(m_lastChild != prev);
    144         next->setPreviousSibling(child);
    145         if (prev) {
    146             ASSERT(m_firstChild != next);
    147             ASSERT(prev->nextSibling() == next);
    148             prev->setNextSibling(child);
    149         } else {
    150             ASSERT(m_firstChild == next);
    151             m_firstChild = child;
    152         }
    153         child->setParent(this);
    154         child->setPreviousSibling(prev);
    155         child->setNextSibling(next.get());
    156         allowEventDispatch();
     137        insertBeforeCommon(next.get(), child);
    157138
    158139        // Send notification about the children change.
    159140        childrenChanged(false, refChildPreviousSibling.get(), next.get(), 1);
    160141        notifyChildInserted(child);
    161                
     142
    162143        // Add child to the rendering tree.
    163144        if (attached() && !child->attached() && child->parent() == this) {
    … …  
    175156    dispatchSubtreeModifiedEvent();
    176157    return true;
     158}
     159
     160void ContainerNode::insertBeforeCommon(Node* nextChild, Node* newChild)
     161{
     162    ASSERT(newChild);
     163    ASSERT(!newChild->parent()); // Use insertBefore if you need to handle reparenting (and want DOM mutation events).
     164    ASSERT(!newChild->nextSibling());
     165    ASSERT(!newChild->previousSibling());
     166
     167    forbidEventDispatch();
     168    Node* prev = nextChild->previousSibling();
     169    ASSERT(m_lastChild != prev);
     170    nextChild->setPreviousSibling(newChild);
     171    if (prev) {
     172        ASSERT(m_firstChild != nextChild);
     173        ASSERT(prev->nextSibling() == nextChild);
     174        prev->setNextSibling(newChild);
     175    } else {
     176        ASSERT(m_firstChild == nextChild);
     177        m_firstChild = newChild;
     178    }
     179    newChild->setParent(this);
     180    newChild->setPreviousSibling(prev);
     181    newChild->setNextSibling(nextChild);
     182    allowEventDispatch();
     183}
     184
     185void ContainerNode::parserInsertBefore(PassRefPtr<Node> newChild, Node* nextChild)
     186{
     187    ASSERT(newChild);
     188    ASSERT(nextChild);
     189    ASSERT(nextChild->parentNode() == this);
     190
     191    NodeVector targets;
     192    collectTargetNodes(newChild.get(), targets);
     193    if (targets.isEmpty())
     194        return;
     195
     196    if (nextChild->previousSibling() == newChild || nextChild == newChild) // nothing to do
     197        return;
     198
     199    RefPtr<Node> next = nextChild;
     200    RefPtr<Node> nextChildPreviousSibling = nextChild->previousSibling();
     201    for (NodeVector::const_iterator it = targets.begin(); it != targets.end(); ++it) {
     202        Node* child = it->get();
     203
     204        insertBeforeCommon(next.get(), child);
     205
     206        childrenChanged(true, nextChildPreviousSibling.get(), nextChild, 1);
     207        notifyChildInserted(child);
     208    }
    177209}
    178210
    … …  
    368400    // writing, there are definitely callers who call that way).
    369401
    370     forbidEventDispatch();
    371 
    372     // Remove from rendering tree
    373     if (child->attached())
    374         child->detach();
    375 
    376     // Remove the child
    377     Node *prev, *next;
    378     prev = child->previousSibling();
    379     next = child->nextSibling();
    380 
    381     if (next)
    382         next->setPreviousSibling(prev);
    383     if (prev)
    384         prev->setNextSibling(next);
    385     if (m_firstChild == child)
    386         m_firstChild = next;
    387     if (m_lastChild == child)
    388         m_lastChild = prev;
    389 
    390     child->setPreviousSibling(0);
    391     child->setNextSibling(0);
    392     child->setParent(0);
    393 
    394     allowEventDispatch();
     402    Node* prev = child->previousSibling();
     403    Node* next = child->nextSibling();
     404    removeBetween(prev, next, child.get());
    395405
    396406    // Dispatch post-removal mutation events
    … …  
    404414
    405415    return child;
     416}
     417
     418void ContainerNode::removeBetween(Node* previousChild, Node* nextChild, Node* oldChild)
     419{
     420    ASSERT(oldChild);
     421    ASSERT(oldChild->parentNode() == this);
     422
     423    forbidEventDispatch();
     424
     425    // Remove from rendering tree
     426    if (oldChild->attached())
     427        oldChild->detach();
     428
     429    if (nextChild)
     430        nextChild->setPreviousSibling(previousChild);
     431    if (previousChild)
     432        previousChild->setNextSibling(nextChild);
     433    if (m_firstChild == oldChild)
     434        m_firstChild = nextChild;
     435    if (m_lastChild == oldChild)
     436        m_lastChild = previousChild;
     437
     438    oldChild->setPreviousSibling(0);
     439    oldChild->setNextSibling(0);
     440    oldChild->setParent(0);
     441
     442    allowEventDispatch();
     443}
     444
     445void ContainerNode::parserRemoveChild(Node* oldChild)
     446{
     447    ASSERT(oldChild);
     448    ASSERT(oldChild->parentNode() == this);
     449
     450    Node* prev = oldChild->previousSibling();
     451    Node* next = oldChild->nextSibling();
     452
     453    removeBetween(prev, next, oldChild);
     454
     455    childrenChanged(true, prev, next, -1);
     456    if (oldChild->inDocument())
     457        oldChild->removedFromDocument();
     458    else
     459        oldChild->removedFromTree(true);
    406460}
    407461
    … …  
    537591void ContainerNode::addChildCommon(Node* newChild)
    538592{
    539     ASSERT(!newChild->parent()); // Use appendChild if you need to handle reparenting.
     593    ASSERT(newChild);
     594    ASSERT(!newChild->parent()); // Use appendChild if you need to handle reparenting (and want DOM mutation events).
     595
    540596    forbidEventDispatch();
    541597    Node* last = m_lastChild;
    … …  
    544600    allowEventDispatch();
    545601
     602    // FIXME: Why doesn't this use notifyChildInserted(newChild) instead?
    546603    document()->incDOMTreeVersion();
    547604    if (inDocument())
    … …  
    553610{
    554611    ASSERT(newChild);
    555     // This function is only used during parsing.
    556     // It does not send any DOM mutation events or handle reparenting.
    557 
    558612    addChildCommon(newChild.get());
    559613}
    … …  
    562616{
    563617    ASSERT(newChild);
    564     // This function is only used during parsing.
    565     // It does not send any DOM mutation events.
    566618
    567619    // Check for consistency with DTD, but only when parsing HTML.

  • trunk/WebCore/dom/ContainerNode.h

    r62687 r65769  
    4949    virtual bool appendChild(PassRefPtr<Node> newChild, ExceptionCode&, bool shouldLazyAttach = false);
    5050
     51    // These methods are only used during parsing.
     52    // They don't send DOM mutation events or handle reparenting.
     53    // However, arbitrary code may be run by beforeload handlers.
    5154    virtual ContainerNode* legacyParserAddChild(PassRefPtr<Node>);
    5255    virtual void parserAddChild(PassRefPtr<Node>);
     56    virtual void parserRemoveChild(Node*);
     57    virtual void parserInsertBefore(PassRefPtr<Node> newChild, Node* refChild);
    5358
    5459    bool hasChildNodes() const { return m_firstChild; }
    … …  
    98103    // FIXME: This should take a PassRefPtr.
    99104    void addChildCommon(Node*);
     105    void removeBetween(Node* previousChild, Node* nextChild, Node* oldChild);
     106    void insertBeforeCommon(Node* nextChild, Node* oldChild);
    100107
    101108    static void dispatchPostAttachCallbacks();

  • trunk/WebCore/dom/Node.cpp

    r65508 r65769  
    652652}
    653653
     654void Node::parserRemoveChild(PassRefPtr<Node>)
     655{
     656    ASSERT_NOT_REACHED();
     657}
     658
     659void Node::parserInsertBefore(PassRefPtr<Node>, Node*)
     660{
     661    ASSERT_NOT_REACHED();
     662}
     663
    654664bool Node::isContentEditable() const
    655665{

  • trunk/WebCore/dom/Node.h

    r65517 r65769  
    268268    // a "clean" version to use for the HTML5 version of the HTMLTreeBuilder.
    269269    virtual void parserAddChild(PassRefPtr<Node>);
     270    virtual void parserRemoveChild(PassRefPtr<Node>);
     271    virtual void parserInsertBefore(PassRefPtr<Node> newChild, Node* nextChild);
    270272
    271273    // Called by the parser when this element's close tag is reached,

  • trunk/WebCore/html/HTMLConstructionSite.cpp

    r65692 r65769  
    9393    if (shouldFosterParent()) {
    9494        fosterParent(child.get());
    95         ASSERT(child->attached() || !child->parent()->attached());
     95        ASSERT(child->attached() || !child->parent() || !child->parent()->attached());
    9696        return child.release();
    9797    }
    … …  
    119119
    120120    if (site.nextChild) {
    121         // FIXME: We need an insertElement which does not send mutation events.
    122         ExceptionCode ec = 0;
    123         site.parent->insertBefore(child, site.nextChild, ec);
    124         ASSERT(!ec);
     121        site.parent->parserInsertBefore(child, site.nextChild);
    125122        if (site.parent->attached() && !child->attached())
    126123            child->attach();

  • trunk/WebCore/html/HTMLTreeBuilder.cpp

    r65692 r65769  
    17571757    while (child) {
    17581758        Node* nextChild = child->nextSibling();
    1759         ExceptionCode ec;
    1760         newParent->appendChild(child, ec);
    1761         ASSERT(!ec);
     1759        oldParent->parserRemoveChild(child);
     1760        newParent->parserAddChild(child);
     1761        if (newParent->attached() && !child->attached())
     1762            child->attach();
    17621763        child = nextChild;
    17631764    }
    … …  
    18301831                bookmark.moveToAfter(nodeEntry);
    18311832            // 6.6
    1832             // Use appendChild instead of parserAddChild to handle possible reparenting.
    1833             ExceptionCode ec;
    1834             node->element()->appendChild(lastNode->element(), ec, true);
    1835             ASSERT(!ec);
     1833            if (Element* parent = lastNode->element()->parentElement())
     1834                parent->parserRemoveChild(lastNode->element());
     1835            node->element()->parserAddChild(lastNode->element());
     1836            if (lastNode->element()->parentElement()->attached() && !lastNode->element()->attached())
     1837                lastNode->element()->lazyAttach();
    18361838            // 6.7
    18371839            lastNode = node;
    … …  
    18391841        // 7
    18401842        const AtomicString& commonAncestorTag = commonAncestor->localName();
     1843        if (Element* parent = lastNode->element()->parentElement())
     1844            parent->parserRemoveChild(lastNode->element());
    18411845        // FIXME: If this moves to HTMLConstructionSite, this check should use
    18421846        // causesFosterParenting(tagName) instead.
    … …  
    18461850            m_tree.fosterParent(lastNode->element());
    18471851        else {
    1848             ExceptionCode ec;
    1849             commonAncestor->appendChild(lastNode->element(), ec, true);
    1850             ASSERT(!ec);
     1852            commonAncestor->parserAddChild(lastNode->element());
     1853            if (lastNode->element()->parentElement()->attached() && !lastNode->element()->attached())
     1854                lastNode->element()->lazyAttach();
    18511855        }
    18521856        // 8
Note: See TracChangeset for help on using the changeset viewer.