Changeset 58040 in webkit


Ignore:
Timestamp:
Apr 21, 2010 7:17:19 PM (14 years ago)
Author:
hamaji@chromium.org
Message:

2010-04-21 Shinichiro Hamaji <​hamaji@chromium.org>

Reviewed by Darin Adler.

Call sites of TextIterator constructor are difficult to read
​https://bugs.webkit.org/show_bug.cgi?id=37909

Now we use enum parameters instead of boolean parameters and
boolean version of constructors are eliminated.

This change also changes the names of boolean members so they are
now third person singular.

No new tests because this is just a refactoring.

  • WebCore.base.exp:
  • editing/TextIterator.cpp: (WebCore::TextIterator::TextIterator): (WebCore::TextIterator::advance): (WebCore::TextIterator::handleReplacedElement): (WebCore::TextIterator::shouldRepresentNodeOffsetZero): (WebCore::TextIterator::shouldEmitSpaceBeforeAndAfterNode): (WebCore::TextIterator::handleNonTextNode): (WebCore::TextIterator::exitNode): (WebCore::TextIterator::emitCharacter): (WebCore::TextIterator::emitText): (WebCore::CharacterIterator::CharacterIterator): (WebCore::TextIterator::rangeLength): (WebCore::TextIterator::rangeFromLocationAndLength): (WebCore::findPlainText):
  • editing/TextIterator.h: (WebCore::):
  • editing/VisibleSelection.cpp: (WebCore::VisibleSelection::appendTrailingWhitespace):
  • editing/visible_units.cpp: (WebCore::nextBoundary):
Location:
trunk/WebCore
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r58038 r58040  
     12010-04-21  Shinichiro Hamaji  <hamaji@chromium.org>
     2
     3        Reviewed by Darin Adler.
     4
     5        Call sites of TextIterator constructor are difficult to read
     6        https://bugs.webkit.org/show_bug.cgi?id=37909
     7
     8        Now we use enum parameters instead of boolean parameters and
     9        boolean version of constructors are eliminated.
     10
     11        This change also changes the names of boolean members so they are
     12        now third person singular.
     13
     14        No new tests because this is just a refactoring.
     15
     16        * WebCore.base.exp:
     17        * editing/TextIterator.cpp:
     18        (WebCore::TextIterator::TextIterator):
     19        (WebCore::TextIterator::advance):
     20        (WebCore::TextIterator::handleReplacedElement):
     21        (WebCore::TextIterator::shouldRepresentNodeOffsetZero):
     22        (WebCore::TextIterator::shouldEmitSpaceBeforeAndAfterNode):
     23        (WebCore::TextIterator::handleNonTextNode):
     24        (WebCore::TextIterator::exitNode):
     25        (WebCore::TextIterator::emitCharacter):
     26        (WebCore::TextIterator::emitText):
     27        (WebCore::CharacterIterator::CharacterIterator):
     28        (WebCore::TextIterator::rangeLength):
     29        (WebCore::TextIterator::rangeFromLocationAndLength):
     30        (WebCore::findPlainText):
     31        * editing/TextIterator.h:
     32        (WebCore::):
     33        * editing/VisibleSelection.cpp:
     34        (WebCore::VisibleSelection::appendTrailingWhitespace):
     35        * editing/visible_units.cpp:
     36        (WebCore::nextBoundary):
     37
    1382010-04-21  Jesus Sanchez-Palencia  <jesus@webkit.org>
    239

  • trunk/WebCore/WebCore.base.exp

    r58029 r58040  
    262262__ZN7WebCore12TextIterator26rangeFromLocationAndLengthEPNS_7ElementEiib
    263263__ZN7WebCore12TextIterator7advanceEv
    264 __ZN7WebCore12TextIteratorC1EPKNS_5RangeEbb
     264__ZN7WebCore12TextIteratorC1EPKNS_5RangeENS_20TextIteratorBehaviorE
    265265__ZN7WebCore12WorkerThread17workerThreadCountEv
    266266__ZN7WebCore12applyCommandEN3WTF10PassRefPtrINS_11EditCommandEEE

  • trunk/WebCore/editing/TextIterator.cpp

    r57940 r58040  
    255255    , m_textLength(0)
    256256    , m_lastCharacter(0)
    257     , m_emitCharactersBetweenAllVisiblePositions(false)
    258     , m_enterTextControls(false)
     257    , m_emitsCharactersBetweenAllVisiblePositions(false)
     258    , m_entersTextControls(false)
    259259    , m_emitsTextWithoutTranscoding(false)
    260260{
    261 }
    262 
    263 TextIterator::TextIterator(const Range* r, bool emitCharactersBetweenAllVisiblePositions, bool enterTextControls)
    264     : m_startContainer(0)
    265     , m_startOffset(0)
    266     , m_endContainer(0)
    267     , m_endOffset(0)
    268     , m_positionNode(0)
    269     , m_textCharacters(0)
    270     , m_textLength(0)
    271     , m_emitCharactersBetweenAllVisiblePositions(emitCharactersBetweenAllVisiblePositions)
    272     , m_enterTextControls(enterTextControls)
    273     , m_emitsTextWithoutTranscoding(false)
    274 {
    275     init(r);
    276261}
    277262
    … …  
    284269    , m_textCharacters(0)
    285270    , m_textLength(0)
    286     , m_emitCharactersBetweenAllVisiblePositions(behavior & TextIteratorBehaviorEmitCharactersBetweenAllVisiblePositions)
    287     , m_enterTextControls(behavior & TextIteratorBehaviorEnterTextControls)
    288     , m_emitsTextWithoutTranscoding(behavior & TextIteratorBehaviorEmitsTextsWithoutTranscoding)
    289 {
    290     init(r);
    291 }
    292 
    293 void TextIterator::init(const Range* r)
     271    , m_emitsCharactersBetweenAllVisiblePositions(behavior & TextIteratorEmitsCharactersBetweenAllVisiblePositions)
     272    , m_entersTextControls(behavior & TextIteratorEntersTextControls)
     273    , m_emitsTextWithoutTranscoding(behavior & TextIteratorEmitsTextsWithoutTranscoding)
    294274{
    295275    if (!r)
    … …  
    327307
    328308    // initialize node processing state
    329     m_needAnotherNewline = false;
     309    m_needsAnotherNewline = false;
    330310    m_textBox = 0;
    331311
    332312    // initialize record of previous node processing
    333     m_haveEmitted = false;
     313    m_hasEmitted = false;
    334314    m_lastTextNode = 0;
    335315    m_lastTextNodeEndedWithCollapsedSpace = false;
    … …  
    352332
    353333    // handle remembered node that needed a newline after the text node's newline
    354     if (m_needAnotherNewline) {
     334    if (m_needsAnotherNewline) {
    355335        // Emit the extra newline, and position it *inside* m_node, after m_node's
    356336        // contents, in case it's a block, in the same way that we position the first
    … …  
    358338        // break begins.
    359339        // FIXME: It would be cleaner if we emitted two newlines during the last
    360         // iteration, instead of using m_needAnotherNewline.
     340        // iteration, instead of using m_needsAnotherNewline.
    361341        Node* baseNode = m_node->lastChild() ? m_node->lastChild() : m_node;
    362342        emitCharacter('\n', baseNode->parentNode(), baseNode, 1, 1);
    363         m_needAnotherNewline = false;
     343        m_needsAnotherNewline = false;
    364344        return;
    365345    }
    … …  
    586566    }
    587567
    588     if (m_enterTextControls && renderer->isTextControl()) {
     568    if (m_entersTextControls && renderer->isTextControl()) {
    589569        if (HTMLElement* innerTextElement = toRenderTextControl(renderer)->innerTextElement()) {
    590570            m_node = innerTextElement->shadowTreeRootNode();
    … …  
    595575    }
    596576
    597     m_haveEmitted = true;
    598 
    599     if (m_emitCharactersBetweenAllVisiblePositions) {
     577    m_hasEmitted = true;
     578
     579    if (m_emitsCharactersBetweenAllVisiblePositions) {
    600580        // We want replaced elements to behave like punctuation for boundary
    601581        // finding, and to simply take up space for the selection preservation
    … …  
    759739bool TextIterator::shouldRepresentNodeOffsetZero()
    760740{
    761     if (m_emitCharactersBetweenAllVisiblePositions && m_node->renderer() && m_node->renderer()->isTable())
     741    if (m_emitsCharactersBetweenAllVisiblePositions && m_node->renderer() && m_node->renderer()->isTable())
    762742        return true;
    763743       
    … …  
    768748   
    769749    // Otherwise, show the position if we have emitted any characters
    770     if (m_haveEmitted)
     750    if (m_hasEmitted)
    771751        return true;
    772752   
    … …  
    790770    // If we started as m_startContainer offset 0 and the current node is a descendant of
    791771    // the start container, we already had enough context to correctly decide whether to
    792     // emit after a preceding block. We chose not to emit (m_haveEmitted is false),
     772    // emit after a preceding block. We chose not to emit (m_hasEmitted is false),
    793773    // so don't second guess that now.
    794774    // NOTE: Is this really correct when m_node is not a leftmost descendant? Probably
    … …  
    814794bool TextIterator::shouldEmitSpaceBeforeAndAfterNode(Node* node)
    815795{
    816     return node->renderer() && node->renderer()->isTable() && (node->renderer()->isInline() || m_emitCharactersBetweenAllVisiblePositions);
     796    return node->renderer() && node->renderer()->isTable() && (node->renderer()->isInline() || m_emitsCharactersBetweenAllVisiblePositions);
    817797}
    818798
    … …  
    841821    if (shouldEmitNewlineForNode(m_node))
    842822        emitCharacter('\n', m_node->parentNode(), m_node, 0, 1);
    843     else if (m_emitCharactersBetweenAllVisiblePositions && m_node->renderer() && m_node->renderer()->isHR())
     823    else if (m_emitsCharactersBetweenAllVisiblePositions && m_node->renderer() && m_node->renderer()->isHR())
    844824        emitCharacter(' ', m_node->parentNode(), m_node, 0, 1);
    845825    else
    … …  
    852832{
    853833    // prevent emitting a newline when exiting a collapsed block at beginning of the range
    854     // FIXME: !m_haveEmitted does not necessarily mean there was a collapsed block... it could
     834    // FIXME: !m_hasEmitted does not necessarily mean there was a collapsed block... it could
    855835    // have been an hr (e.g.). Also, a collapsed block could have height (e.g. a table) and
    856836    // therefore look like a blank line.
    857     if (!m_haveEmitted)
     837    if (!m_hasEmitted)
    858838        return;
    859839       
    … …  
    876856            emitCharacter('\n', baseNode->parentNode(), baseNode, 1, 1);
    877857            // remember whether to later add a newline for the current node
    878             ASSERT(!m_needAnotherNewline);
    879             m_needAnotherNewline = addNewline;
     858            ASSERT(!m_needsAnotherNewline);
     859            m_needsAnotherNewline = addNewline;
    880860        } else if (addNewline)
    881861            // insert a newline with a position following this block's contents.
    … …  
    890870void TextIterator::emitCharacter(UChar c, Node* textNode, Node* offsetBaseNode, int textStartOffset, int textEndOffset)
    891871{
    892     m_haveEmitted = true;
     872    m_hasEmitted = true;
    893873   
    894874    // remember information with which to construct the TextIterator::range()
    … …  
    924904
    925905    m_lastTextNodeEndedWithCollapsedSpace = false;
    926     m_haveEmitted = true;
     906    m_hasEmitted = true;
    927907}
    928908
    … …  
    11751155}
    11761156
    1177 CharacterIterator::CharacterIterator(const Range* r, bool emitCharactersBetweenAllVisiblePositions, bool enterTextControls)
     1157CharacterIterator::CharacterIterator(const Range* r, TextIteratorBehavior behavior)
    11781158    : m_offset(0)
    11791159    , m_runOffset(0)
    11801160    , m_atBreak(true)
    1181     , m_textIterator(r, emitCharactersBetweenAllVisiblePositions, enterTextControls)
     1161    , m_textIterator(r, behavior)
    11821162{
    11831163    while (!atEnd() && m_textIterator.length() == 0)
    … …  
    20141994{
    20151995    int length = 0;
    2016     for (TextIterator it(r, forSelectionPreservation); !it.atEnd(); it.advance())
     1996    for (TextIterator it(r, forSelectionPreservation ? TextIteratorEmitsCharactersBetweenAllVisiblePositions : TextIteratorDefaultBehavior); !it.atEnd(); it.advance())
    20171997        length += it.length();
    20181998   
    … …  
    20362016    RefPtr<Range> textRunRange;
    20372017
    2038     TextIterator it(rangeOfContents(scope).get(), forSelectionPreservation);
     2018    TextIterator it(rangeOfContents(scope).get(), forSelectionPreservation ? TextIteratorEmitsCharactersBetweenAllVisiblePositions : TextIteratorDefaultBehavior);
    20392019   
    20402020    // FIXME: the atEnd() check shouldn't be necessary, workaround for <http://bugs.webkit.org/show_bug.cgi?id=6289>.
    … …  
    21392119    Vector<UChar> textBuffer;
    21402120    textBuffer.reserveInitialCapacity(cMaxSegmentSize);
    2141     for (TextIterator it(r, isDisplayString ? TextIteratorBehaviorDefault : TextIteratorBehaviorEmitsTextsWithoutTranscoding); !it.atEnd(); it.advance()) {
     2121    for (TextIterator it(r, isDisplayString ? TextIteratorDefaultBehavior : TextIteratorEmitsTextsWithoutTranscoding); !it.atEnd(); it.advance()) {
    21422122        if (textBuffer.size() && textBuffer.size() + it.length() > cMaxSegmentSize) {
    21432123            UChar* newSegmentBuffer = static_cast<UChar*>(malloc(textBuffer.size() * sizeof(UChar)));
    … …  
    22592239    size_t matchLength;
    22602240    {
    2261         CharacterIterator findIterator(range, false, true);
     2241        CharacterIterator findIterator(range, TextIteratorEntersTextControls);
    22622242        matchLength = findPlainText(findIterator, target, forward, caseSensitive, matchStart);
    22632243        if (!matchLength)
    … …  
    22662246
    22672247    // Then, find the document position of the start and the end of the text.
    2268     CharacterIterator computeRangeIterator(range, false, true);
     2248    CharacterIterator computeRangeIterator(range, TextIteratorEntersTextControls);
    22692249    return characterSubrange(computeRangeIterator, matchStart, matchLength);
    22702250}

  • trunk/WebCore/editing/TextIterator.h

    r57940 r58040  
    7070
    7171enum TextIteratorBehavior {
    72     TextIteratorBehaviorDefault = 0,
    73     TextIteratorBehaviorEmitCharactersBetweenAllVisiblePositions = 1 << 0,
    74     TextIteratorBehaviorEnterTextControls = 1 << 1,
    75     TextIteratorBehaviorEmitsTextsWithoutTranscoding = 1 << 2,
     72    TextIteratorDefaultBehavior = 0,
     73    TextIteratorEmitsCharactersBetweenAllVisiblePositions = 1 << 0,
     74    TextIteratorEntersTextControls = 1 << 1,
     75    TextIteratorEmitsTextsWithoutTranscoding = 1 << 2,
    7676};
    7777
    … …  
    7979public:
    8080    TextIterator();
    81     explicit TextIterator(const Range*, bool emitCharactersBetweenAllVisiblePositions = false, bool enterTextControls = false);
    82     TextIterator(const Range*, TextIteratorBehavior);
     81    explicit TextIterator(const Range*, TextIteratorBehavior = TextIteratorDefaultBehavior);
    8382
    8483    bool atEnd() const { return !m_positionNode; }
    … …  
    9695   
    9796private:
    98     void init(const Range*);
    9997    void exitNode();
    10098    bool shouldRepresentNodeOffsetZero();
    … …  
    133131    // Used when there is still some pending text from the current node; when these
    134132    // are false and 0, we go back to normal iterating.
    135     bool m_needAnotherNewline;
     133    bool m_needsAnotherNewline;
    136134    InlineTextBox* m_textBox;
    137135   
    … …  
    149147   
    150148    // Used when deciding whether to emit a "positioning" (e.g. newline) before any other content
    151     bool m_haveEmitted;
     149    bool m_hasEmitted;
    152150   
    153151    // Used by selection preservation code.  There should be one character emitted between every VisiblePosition
    … …  
    155153    // FIXME <rdar://problem/6028818>: This functionality should eventually be phased out when we rewrite
    156154    // moveParagraphs to not clone/destroy moved content.
    157     bool m_emitCharactersBetweenAllVisiblePositions;
    158     bool m_enterTextControls;
     155    bool m_emitsCharactersBetweenAllVisiblePositions;
     156    bool m_entersTextControls;
    159157
    160158    // Used when we want texts for copying, pasting, and transposing.
    … …  
    223221public:
    224222    CharacterIterator();
    225     explicit CharacterIterator(const Range*, bool emitCharactersBetweenAllVisiblePositions = false, bool enterTextControls = false);
     223    explicit CharacterIterator(const Range*, TextIteratorBehavior = TextIteratorDefaultBehavior);
    226224   
    227225    void advance(int numCharacters);

  • trunk/WebCore/editing/VisibleSelection.cpp

    r56825 r58040  
    227227        return;
    228228
    229     CharacterIterator charIt(searchRange.get(), true);
     229    CharacterIterator charIt(searchRange.get(), TextIteratorEmitsCharactersBetweenAllVisiblePositions);
    230230
    231231    for (; charIt.length(); charIt.advance(1)) {

  • trunk/WebCore/editing/visible_units.cpp

    r55613 r58040  
    208208    searchRange->selectNodeContents(boundary, ec);
    209209    searchRange->setStart(start.node(), start.deprecatedEditingOffset(), ec);
    210     TextIterator it(searchRange.get(), true);
     210    TextIterator it(searchRange.get(), TextIteratorEmitsCharactersBetweenAllVisiblePositions);
    211211    unsigned next = 0;
    212212    bool inTextSecurityMode = start.node() && start.node()->renderer() && start.node()->renderer()->style()->textSecurity() != TSNONE;
    … …  
    239239    } else if (next != prefixLength) {
    240240        // Use the character iterator to translate the next value into a DOM position.
    241         CharacterIterator charIt(searchRange.get(), true);
     241        CharacterIterator charIt(searchRange.get(), TextIteratorEmitsCharactersBetweenAllVisiblePositions);
    242242        charIt.advance(next - prefixLength - 1);
    243243        pos = charIt.range()->endPosition();
Note: See TracChangeset for help on using the changeset viewer.