Changeset 71288 in webkit


Ignore:
Timestamp:
Nov 3, 2010 5:14:01 PM (13 years ago)
Author:
rniwa@webkit.org
Message:

2010-11-01 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Darin Adler.

Crash in ApplyStyleCommand::removeStyleFromRunBeforeApplyingStyle
​https://bugs.webkit.org/show_bug.cgi?id=48581

The crash was caused by RemoveNodePreservingChildrenCommand's calling removeNode
on m_node without checking that m_node has a parent and it's still in the document.
Fixed the crash by adding an early exit in CompositeEditCommand::removeNode and
deploying RefPtr in several places of ApplyStyleCommand.cpp.

Test: editing/style/iframe-onload-crash.html

  • editing/ApplyStyleCommand.cpp: (WebCore::ApplyStyleCommand::applyInlineStyleToNodeRange): (WebCore::ApplyStyleCommand::removeStyleFromRunBeforeApplyingStyle): (WebCore::ApplyStyleCommand::removeInlineStyleFromElement):
  • editing/ApplyStyleCommand.h:
  • editing/CompositeEditCommand.cpp: (WebCore::CompositeEditCommand::removeNode):

2010-10-29 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Darin Adler.

Crash in ApplyStyleCommand::removeStyleFromRunBeforeApplyingStyle
​https://bugs.webkit.org/show_bug.cgi?id=48581

Added a test to ensure removeStyleFromRunBeforeApplyingStyle doesn't crash.

  • editing/style/iframe-onload-crash-expected.txt: Added.
  • editing/style/iframe-onload-crash.html: Added.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r71282 r71288  
     12010-10-29  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        Crash in ApplyStyleCommand::removeStyleFromRunBeforeApplyingStyle
     6        https://bugs.webkit.org/show_bug.cgi?id=48581
     7
     8        Added a test to ensure removeStyleFromRunBeforeApplyingStyle doesn't crash.
     9
     10        * editing/style/iframe-onload-crash-expected.txt: Added.
     11        * editing/style/iframe-onload-crash.html: Added.
     12
    1132010-11-02  Zhenyao Mo  <zmo@google.com>
    214

  • trunk/WebCore/ChangeLog

    r71284 r71288  
     12010-11-01  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        Crash in ApplyStyleCommand::removeStyleFromRunBeforeApplyingStyle
     6        https://bugs.webkit.org/show_bug.cgi?id=48581
     7
     8        The crash was caused by RemoveNodePreservingChildrenCommand's calling removeNode
     9        on m_node without checking that m_node has a parent and it's still in the document.
     10        Fixed the crash by adding an early exit in CompositeEditCommand::removeNode and
     11        deploying RefPtr in several places of ApplyStyleCommand.cpp.
     12
     13        Test: editing/style/iframe-onload-crash.html
     14
     15        * editing/ApplyStyleCommand.cpp:
     16        (WebCore::ApplyStyleCommand::applyInlineStyleToNodeRange):
     17        (WebCore::ApplyStyleCommand::removeStyleFromRunBeforeApplyingStyle):
     18        (WebCore::ApplyStyleCommand::removeInlineStyleFromElement):
     19        * editing/ApplyStyleCommand.h:
     20        * editing/CompositeEditCommand.cpp:
     21        (WebCore::CompositeEditCommand::removeNode):
     22
    1232010-11-03  Jia Pu  <jpu@apple.com>
    224

  • trunk/WebCore/editing/ApplyStyleCommand.cpp

    r70821 r71288  
    11511151        return;
    11521152
    1153     for (Node* next; node && node != pastEndNode; node = next) {
     1153    for (RefPtr<Node> next; node && node != pastEndNode; node = next.get()) {
    11541154        next = node->traverseNextNode();
    1155        
     1155
    11561156        if (!node->renderer() || !node->isContentEditable())
    11571157            continue;
    … …  
    11841184        }
    11851185
    1186         Node* runEnd = node;
     1186        RefPtr<Node> runStart = node;
     1187        RefPtr<Node> runEnd = node;
    11871188        Node* sibling = node->nextSibling();
    11881189        while (sibling && sibling != pastEndNode && !sibling->contains(pastEndNode)
    … …  
    11941195        next = runEnd->traverseNextSibling();
    11951196
    1196         if (!removeStyleFromRunBeforeApplyingStyle(style, node, runEnd))
     1197        if (!removeStyleFromRunBeforeApplyingStyle(style, runStart, runEnd))
    11971198            continue;
    1198         addInlineStyleIfNeeded(style, node, runEnd, AddStyledElement);
     1199        addInlineStyleIfNeeded(style, runStart.get(), runEnd.get(), AddStyledElement);
    11991200    }
    12001201}
    … …  
    12061207}
    12071208
    1208 bool ApplyStyleCommand::removeStyleFromRunBeforeApplyingStyle(CSSMutableStyleDeclaration* style, Node*& runStart, Node*& runEnd)
     1209bool ApplyStyleCommand::removeStyleFromRunBeforeApplyingStyle(CSSMutableStyleDeclaration* style, RefPtr<Node>& runStart, RefPtr<Node>& runEnd)
    12091210{
    12101211    ASSERT(runStart && runEnd && runStart->parentNode() == runEnd->parentNode());
    1211     Node* pastEndNode = runEnd->traverseNextSibling();
     1212    RefPtr<Node> pastEndNode = runEnd->traverseNextSibling();
    12121213    bool needToApplyStyle = false;
    1213     for (Node* node = runStart; node && node != pastEndNode; node = node->traverseNextNode()) {
     1214    for (Node* node = runStart.get(); node && node != pastEndNode.get(); node = node->traverseNextNode()) {
    12141215        if (node->childNodeCount())
    12151216            continue;
    … …  
    12241225        return false;
    12251226
    1226     Node* next;
    1227     for (Node* node = runStart; node && node != pastEndNode; node = next) {
     1227    RefPtr<Node> next = runStart;
     1228    for (RefPtr<Node> node = next; node && node->inDocument() && node != pastEndNode; node = next) {
    12281229        next = node->traverseNextNode();
    12291230        if (!node->isHTMLElement())
    12301231            continue;
    1231        
    1232         Node* previousSibling = node->previousSibling();
    1233         Node* nextSibling = node->nextSibling();
    1234         ContainerNode* parent = node->parentNode();
    1235         removeInlineStyleFromElement(style, static_cast<HTMLElement*>(node), RemoveAlways);
     1232
     1233        RefPtr<Node> previousSibling = node->previousSibling();
     1234        RefPtr<Node> nextSibling = node->nextSibling();
     1235        RefPtr<ContainerNode> parent = node->parentNode();
     1236        removeInlineStyleFromElement(style, static_cast<HTMLElement*>(node.get()), RemoveAlways);
    12361237        if (!node->inDocument()) {
    12371238            // FIXME: We might need to update the start and the end of current selection here but need a test.
    … …  
    12461247}
    12471248
    1248 bool ApplyStyleCommand::removeInlineStyleFromElement(CSSMutableStyleDeclaration* style, HTMLElement* element, InlineStyleRemovalMode mode, CSSMutableStyleDeclaration* extractedStyle)
     1249bool ApplyStyleCommand::removeInlineStyleFromElement(CSSMutableStyleDeclaration* style, PassRefPtr<HTMLElement> element, InlineStyleRemovalMode mode, CSSMutableStyleDeclaration* extractedStyle)
    12491250{
    12501251    ASSERT(style);
    … …  
    12541255        return false;
    12551256
    1256     if (isStyledInlineElementToRemove(element)) {
     1257    if (isStyledInlineElementToRemove(element.get())) {
    12571258        if (mode == RemoveNone)
    12581259            return true;
    … …  
    12651266
    12661267    bool removed = false;
    1267     if (removeImplicitlyStyledElement(style, element, mode, extractedStyle))
     1268    if (removeImplicitlyStyledElement(style, element.get(), mode, extractedStyle))
    12681269        removed = true;
    12691270
    … …  
    12731274    // If the node was converted to a span, the span may still contain relevant
    12741275    // styles which must be removed (e.g. <b style='font-weight: bold'>)
    1275     if (removeCSSStyle(style, element, mode, extractedStyle))
     1276    if (removeCSSStyle(style, element.get(), mode, extractedStyle))
    12761277        removed = true;
    12771278

  • trunk/WebCore/editing/ApplyStyleCommand.h

    r70283 r71288  
    8080    // style-removal helpers
    8181    bool isStyledInlineElementToRemove(Element*) const;
    82     bool removeStyleFromRunBeforeApplyingStyle(CSSMutableStyleDeclaration* style, Node*& runStart, Node*& runEnd);
    83     bool removeInlineStyleFromElement(CSSMutableStyleDeclaration*, HTMLElement*, InlineStyleRemovalMode = RemoveIfNeeded, CSSMutableStyleDeclaration* extractedStyle = 0);
     82    bool removeStyleFromRunBeforeApplyingStyle(CSSMutableStyleDeclaration* style, RefPtr<Node>& runStart, RefPtr<Node>& runEnd);
     83    bool removeInlineStyleFromElement(CSSMutableStyleDeclaration*, PassRefPtr<HTMLElement>, InlineStyleRemovalMode = RemoveIfNeeded, CSSMutableStyleDeclaration* extractedStyle = 0);
    8484    inline bool shouldRemoveInlineStyleFromElement(CSSMutableStyleDeclaration* style, HTMLElement* element) {return removeInlineStyleFromElement(style, element, RemoveNone);}
    8585    bool removeImplicitlyStyledElement(CSSMutableStyleDeclaration*, HTMLElement*, InlineStyleRemovalMode, CSSMutableStyleDeclaration* extractedStyle);

  • trunk/WebCore/editing/CompositeEditCommand.cpp

    r70594 r71288  
    207207void CompositeEditCommand::removeNode(PassRefPtr<Node> node)
    208208{
     209    if (!node || !node->parentNode())
     210        return;
    209211    applyCommandToComposite(RemoveNodeCommand::create(node));
    210212}
Note: See TracChangeset for help on using the changeset viewer.