Changeset 79854 in webkit


Ignore:
Timestamp:
Feb 28, 2011 5:24:25 AM (13 years ago)
Author:
rniwa@webkit.org
Message:

2011-02-28 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Kent Tamura.

Range::processContents needs cleanup
​https://bugs.webkit.org/show_bug.cgi?id=51006

Refactored Range::processContents. Extracted childOfCommonRootBeforeOffset from processContents
which is used to find processStart and processEnd respectively. In the case of processStart,
we use the next sibling of the node returned by childOfCommonRootBeforeOffset when m_start is not
the common root because copying m_start's ancestors will result in processing too much contents.

Also extracted processNodes and deleteCharacterData from processContents and processContentsBetweenOffsets.

In addition, lengthOfContentsInNode was modified to return the correct length instead of
numeric_limits<unsigned>::max() because the convention that processContentsBetweenOffsets automatically
corrects the length when endOffset is numeric_limits<unsigned>::max() seemed more confusing than
having two switch statements that need to be consistent.

Historically, lengthOfContentsInNode was introduced in r78413 as a build fix because unsigned const
LengthOfContentsInNode added in r78409 violated WebKit C++ rules and caused build failures on Mac and
other ports.

  • dom/Range.cpp: (WebCore::childOfCommonRootBeforeOffset): Extracted from processContents. (WebCore::lengthOfContentsInNode): Added. (WebCore::Range::processContents): Calls childOfCommonRootBeforeOffset, lengthOfContentsInNode, and processNodes. (WebCore::deleteCharacterData): Added. (WebCore::Range::processContentsBetweenOffsets): Calls deleteCharacterData and processNodes. (WebCore::Range::processNodes): Extracted from processContents and processContentsBetweenOffsets. (WebCore::Range::processAncestorsAndTheirSiblings):
  • dom/Range.h:
Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r79853 r79854  
     12011-02-28  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Kent Tamura.
     4
     5        Range::processContents needs cleanup
     6        https://bugs.webkit.org/show_bug.cgi?id=51006
     7
     8        Refactored Range::processContents.  Extracted childOfCommonRootBeforeOffset from processContents
     9        which is used to find processStart and processEnd respectively.  In the case of processStart,
     10        we use the next sibling of the node returned by childOfCommonRootBeforeOffset when m_start is not
     11        the common root because copying m_start's ancestors will result in processing too much contents.
     12
     13        Also extracted processNodes and deleteCharacterData from processContents and processContentsBetweenOffsets.
     14
     15        In addition, lengthOfContentsInNode was modified to return the correct length instead of
     16        numeric_limits<unsigned>::max() because the convention that processContentsBetweenOffsets automatically
     17        corrects the length when endOffset is numeric_limits<unsigned>::max() seemed more confusing than
     18        having two switch statements that need to be consistent.
     19
     20        Historically, lengthOfContentsInNode was introduced in r78413 as a build fix because unsigned const
     21        LengthOfContentsInNode added in r78409 violated WebKit C++ rules and caused build failures on Mac and
     22        other ports.
     23
     24        * dom/Range.cpp:
     25        (WebCore::childOfCommonRootBeforeOffset): Extracted from processContents.
     26        (WebCore::lengthOfContentsInNode): Added.
     27        (WebCore::Range::processContents): Calls childOfCommonRootBeforeOffset, lengthOfContentsInNode,
     28        and processNodes.
     29        (WebCore::deleteCharacterData): Added.
     30        (WebCore::Range::processContentsBetweenOffsets): Calls deleteCharacterData and processNodes.
     31        (WebCore::Range::processNodes): Extracted from processContents and processContentsBetweenOffsets.
     32        (WebCore::Range::processAncestorsAndTheirSiblings):
     33        * dom/Range.h:
     34
    1352011-02-28  Pavel Feldman  <pfeldman@chromium.org>
    236

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

    r78761 r79854  
    608608}
    609609
    610 static inline unsigned lengthOfContentsInNode() { return numeric_limits<unsigned>::max(); }
     610static inline Node* childOfCommonRootBeforeOffset(Node* container, unsigned offset, Node* commonRoot)
     611{
     612    ASSERT(container);
     613    ASSERT(commonRoot);
     614    ASSERT(commonRoot->contains(container));
     615
     616    if (container == commonRoot) {
     617        container = container->firstChild();
     618        for (unsigned i = 0; container && i < offset; i++)
     619            container = container->nextSibling();
     620    } else {
     621        while (container->parentNode() != commonRoot)
     622            container = container->parentNode();
     623    }
     624
     625    return container;
     626}
     627
     628static inline unsigned lengthOfContentsInNode(Node* node)
     629{
     630    // This switch statement must be consistent with that of Range::processContentsBetweenOffsets.
     631    switch (node->nodeType()) {
     632    case Node::TEXT_NODE:
     633    case Node::CDATA_SECTION_NODE:
     634    case Node::COMMENT_NODE:
     635        return static_cast<CharacterData*>(node)->length();
     636    case Node::PROCESSING_INSTRUCTION_NODE:
     637        return static_cast<ProcessingInstruction*>(node)->data().length();
     638    case Node::ELEMENT_NODE:
     639    case Node::ATTRIBUTE_NODE:
     640    case Node::ENTITY_REFERENCE_NODE:
     641    case Node::ENTITY_NODE:
     642    case Node::DOCUMENT_NODE:
     643    case Node::DOCUMENT_TYPE_NODE:
     644    case Node::DOCUMENT_FRAGMENT_NODE:
     645    case Node::NOTATION_NODE:
     646    case Node::XPATH_NAMESPACE_NODE:
     647        return node->childNodeCount();
     648    }
     649    ASSERT_NOT_REACHED();
     650    return 0;
     651}
    611652
    612653PassRefPtr<DocumentFragment> Range::processContents(ActionType action, ExceptionCode& ec)
    … …  
    657698    RefPtr<Node> leftContents;
    658699    if (m_start.container() != commonRoot) {
    659         leftContents = processContentsBetweenOffsets(action, 0, m_start.container(), m_start.offset(), lengthOfContentsInNode(), ec);
     700        leftContents = processContentsBetweenOffsets(action, 0, m_start.container(), m_start.offset(), lengthOfContentsInNode(m_start.container()), ec);
    660701        leftContents = processAncestorsAndTheirSiblings(action, m_start.container(), ProcessContentsForward, leftContents, commonRoot, ec);
    661702    }
    … …  
    668709
    669710    // delete all children of commonRoot between the start and end container
    670     Node* processStart; // child of commonRoot
    671     if (m_start.container() == commonRoot) {
    672         processStart = m_start.container()->firstChild();
    673         for (int i = 0; i < m_start.offset(); i++)
    674             processStart = processStart->nextSibling();
    675     } else {
    676         processStart = m_start.container();
    677         while (processStart->parentNode() != commonRoot)
    678             processStart = processStart->parentNode();
     711    Node* processStart = childOfCommonRootBeforeOffset(m_start.container(), m_start.offset(), commonRoot);
     712    if (m_start.container() != commonRoot) // processStart contains nodes before m_start.
    679713        processStart = processStart->nextSibling();
    680     }
    681     Node* processEnd; // child of commonRoot
    682     if (m_end.container() == commonRoot) {
    683         processEnd = m_end.container()->firstChild();
    684         for (int i = 0; i < m_end.offset(); i++)
    685             processEnd = processEnd->nextSibling();
    686     } else {
    687         processEnd = m_end.container();
    688         while (processEnd->parentNode() != commonRoot)
    689             processEnd = processEnd->parentNode();
    690     }
     714    Node* processEnd = childOfCommonRootBeforeOffset(m_end.container(), m_end.offset(), commonRoot);
    691715
    692716    // Collapse the range, making sure that the result is not within a node that was partially selected.
    … …  
    711735        for (Node* n = processStart; n && n != processEnd; n = n->nextSibling())
    712736            nodes.append(n);
    713         for (NodeVector::const_iterator it = nodes.begin(); it != nodes.end(); it++) {
    714             Node* n = it->get();
    715             if (action == EXTRACT_CONTENTS)
    716                 fragment->appendChild(n, ec); // will remove from commonRoot
    717             else if (action == CLONE_CONTENTS)
    718                 fragment->appendChild(n->cloneNode(true), ec);
    719             else
    720                 commonRoot->removeChild(n, ec);
    721         }
     737        processNodes(action, nodes, commonRoot, fragment, ec);
    722738    }
    723739
    … …  
    728744}
    729745
     746static inline void deleteCharacterData(PassRefPtr<CharacterData> data, unsigned startOffset, unsigned endOffset, ExceptionCode& ec)
     747{
     748    if (data->length() - endOffset)
     749        data->deleteData(endOffset, data->length() - endOffset, ec);
     750    if (startOffset)
     751        data->deleteData(0, startOffset, ec);
     752}
     753
    730754PassRefPtr<Node> Range::processContentsBetweenOffsets(ActionType action, PassRefPtr<DocumentFragment> fragment,
    731755    Node* container, unsigned startOffset, unsigned endOffset, ExceptionCode& ec)
    … …  
    733757    ASSERT(container);
    734758    ASSERT(startOffset <= endOffset);
    735    
    736     RefPtr<Node> result;
     759
     760    // This switch statement must be consistent with that of lengthOfContentsInNode.
     761    RefPtr<Node> result;   
    737762    switch (container->nodeType()) {
    738763    case Node::TEXT_NODE:
    739764    case Node::CDATA_SECTION_NODE:
    740765    case Node::COMMENT_NODE:
    741         ASSERT(endOffset <= static_cast<CharacterData*>(container)->length() || endOffset == lengthOfContentsInNode());
    742         if (endOffset == lengthOfContentsInNode())
    743             endOffset = static_cast<CharacterData*>(container)->length();
     766        ASSERT(endOffset <= static_cast<CharacterData*>(container)->length());
    744767        if (action == EXTRACT_CONTENTS || action == CLONE_CONTENTS) {
    745768            RefPtr<CharacterData> c = static_pointer_cast<CharacterData>(container->cloneNode(true));
    746             if (c->length() - endOffset)
    747                 c->deleteData(endOffset, c->length() - endOffset, ec);
    748             if (startOffset)
    749                 c->deleteData(0, startOffset, ec);
     769            deleteCharacterData(c, startOffset, endOffset, ec);
    750770            if (fragment) {
    751771                result = fragment;
    … …  
    758778        break;
    759779    case Node::PROCESSING_INSTRUCTION_NODE:
    760         ASSERT(endOffset <= static_cast<ProcessingInstruction*>(container)->data().length() || endOffset == lengthOfContentsInNode());
    761         if (endOffset == lengthOfContentsInNode())
    762             endOffset = static_cast<ProcessingInstruction*>(container)->data().length();
     780        ASSERT(endOffset <= static_cast<ProcessingInstruction*>(container)->data().length());
    763781        if (action == EXTRACT_CONTENTS || action == CLONE_CONTENTS) {
    764782            RefPtr<ProcessingInstruction> c = static_pointer_cast<ProcessingInstruction>(container->cloneNode(true));
    … …  
    801819            nodes.append(n);
    802820
    803         for (unsigned i = 0; i < nodes.size(); i++) {
    804             switch (action) {
    805             case DELETE_CONTENTS:
    806                 container->removeChild(nodes[i].get(), ec);
    807                 break;
    808             case EXTRACT_CONTENTS:
    809                 result->appendChild(nodes[i].release(), ec); // will remove n from its parent
    810                 break;
    811             case CLONE_CONTENTS:
    812                 result->appendChild(nodes[i]->cloneNode(true), ec);
    813                 break;
    814             }
    815         }
     821        processNodes(action, nodes, container, result, ec);
    816822        break;
    817823    }
    818824
    819825    return result;
     826}
     827
     828void Range::processNodes(ActionType action, Vector<RefPtr<Node> >& nodes, PassRefPtr<Node> oldContainer, PassRefPtr<Node> newContainer, ExceptionCode& ec)
     829{
     830    for (unsigned i = 0; i < nodes.size(); i++) {
     831        switch (action) {
     832        case DELETE_CONTENTS:
     833            oldContainer->removeChild(nodes[i].get(), ec);
     834            break;
     835        case EXTRACT_CONTENTS:
     836            newContainer->appendChild(nodes[i].release(), ec); // will remove n from its parent
     837            break;
     838        case CLONE_CONTENTS:
     839            newContainer->appendChild(nodes[i]->cloneNode(true), ec);
     840            break;
     841        }
     842    }
    820843}
    821844
    … …  
    839862        // Copy siblings of an ancestor of start/end containers
    840863        // FIXME: This assertion may fail if DOM is modified during mutation event
     864        // FIXME: Share code with Range::processNodes
    841865        ASSERT(!firstChildInAncestorToProcess || firstChildInAncestorToProcess->parentNode() == ancestor);
    842866        RefPtr<Node> next;

  • trunk/Source/WebCore/dom/Range.h

    r78679 r79854  
    3030#include <wtf/Forward.h>
    3131#include <wtf/RefCounted.h>
     32#include <wtf/Vector.h>
    3233
    3334namespace WebCore {
    … …  
    149150    PassRefPtr<DocumentFragment> processContents(ActionType, ExceptionCode&);
    150151    static PassRefPtr<Node> processContentsBetweenOffsets(ActionType, PassRefPtr<DocumentFragment>, Node*, unsigned startOffset, unsigned endOffset, ExceptionCode&);
     152    static void processNodes(ActionType, Vector<RefPtr<Node> >&, PassRefPtr<Node> oldContainer, PassRefPtr<Node> newContainer, ExceptionCode&);
    151153    enum ContentsProcessDirection { ProcessContentsForward, ProcessContentsBackward };
    152154    static PassRefPtr<Node> processAncestorsAndTheirSiblings(ActionType, Node* container, ContentsProcessDirection, PassRefPtr<Node> clonedContainer, Node* commonRoot, ExceptionCode&);
Note: See TracChangeset for help on using the changeset viewer.