Changeset 135972 in webkit


Ignore:
Timestamp:
Nov 27, 2012, 9:27:34 PM (12 years ago)
Author:
msaboff@apple.com
Message:

TextIterator unnecessarily converts 8 bit strings to 16 bits
​https://bugs.webkit.org/show_bug.cgi?id=103295

Reviewed by Brent Fulgham.

Source/WebCore:

Changed TextIterator to use the contained string instead of calling characters() on that string.
Other sources of text, like emitCharacter() still use the contained UChar* buffer.
Added appendTextToStringBuilder() to append the text contents of the current iterator to a string builder
irrespective of the source of the text.

No new tests as functionality covered by existing tests.

  • WebCore.exp.in: Updated plainText export and eliminated plainTextToMallocAllocatedBuffer export
  • accessibility/AccessibilityObject.cpp:

(WebCore::AccessibilityObject::stringForVisiblePositionRange): Updated to use TextIterator::appendTextToStringBuilder()

  • editing/TextIterator.cpp:

(WebCore::TextIterator::characterAt): New function to return the indexed character of the current TextIterator
(WebCore::TextIterator::appendTextToStringBuilder): Added method to append whatever the current text to a StringBuilder
(WebCore::TextIterator::emitText): Eliminated accessing the character data via characters().
(WebCore::TextIterator::rangeFromLocationAndLength): Changed to use characterAt().
(WebCore::plainText): Combined with plainTextToMallocAllocatedBuffer().

  • editing/TextIterator.h:

(WebCore::TextIterator::startOffset): New getter.
(WebCore::TextIterator::string): New getter.
(WebCore::TextIterator::characters): Updated to use correct test source.

  • page/ContextMenuController.cpp:

(WebCore::selectionContainsPossibleWord): Changed to use characterAt().

Source/WebKit/mac:

Updated _stringForRange to use plainText() instead of removed plainTextToMallocAllocatedBuffer().

  • WebView/WebFrame.mm:

(-[WebFrame _stringForRange:]):

Location:
trunk/Source
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r135971 r135972  
     12012-11-27  Michael Saboff  <msaboff@apple.com>
     2
     3        TextIterator unnecessarily converts 8 bit strings to 16 bits
     4        https://bugs.webkit.org/show_bug.cgi?id=103295
     5
     6        Reviewed by Brent Fulgham.
     7
     8        Changed TextIterator to use the contained string instead of calling characters() on that string.
     9        Other sources of text, like emitCharacter() still use the contained UChar* buffer.
     10        Added appendTextToStringBuilder() to append the text contents of the current iterator to a string builder
     11        irrespective of the source of the text.
     12
     13        No new tests as functionality covered by existing tests.
     14
     15        * WebCore.exp.in: Updated plainText export and eliminated plainTextToMallocAllocatedBuffer export
     16        * accessibility/AccessibilityObject.cpp:
     17        (WebCore::AccessibilityObject::stringForVisiblePositionRange): Updated to use TextIterator::appendTextToStringBuilder()
     18        * editing/TextIterator.cpp:
     19        (WebCore::TextIterator::characterAt): New function to return the indexed character of the current TextIterator
     20        (WebCore::TextIterator::appendTextToStringBuilder): Added method to append whatever the current text to a StringBuilder
     21        (WebCore::TextIterator::emitText): Eliminated accessing the character data via characters().
     22        (WebCore::TextIterator::rangeFromLocationAndLength): Changed to use characterAt().
     23        (WebCore::plainText): Combined with plainTextToMallocAllocatedBuffer().
     24        * editing/TextIterator.h:
     25        (WebCore::TextIterator::startOffset): New getter.
     26        (WebCore::TextIterator::string): New getter.
     27        (WebCore::TextIterator::characters): Updated to use correct test source.
     28        * page/ContextMenuController.cpp:
     29        (WebCore::selectionContainsPossibleWord): Changed to use characterAt().
     30
    1312012-11-27  Noel Gordon  <noel.gordon@gmail.com>
    232

  • trunk/Source/WebCore/WebCore.exp.in

    r135952 r135972  
    692692__ZN7WebCore31CrossOriginPreflightResultCache5emptyEv
    693693__ZN7WebCore31CrossOriginPreflightResultCache6sharedEv
    694 __ZN7WebCore32plainTextToMallocAllocatedBufferEPKNS_5RangeERjbNS_20TextIteratorBehaviorE
    695694__ZN7WebCore33stripLeadingAndTrailingHTMLSpacesERKN3WTF6StringE
    696695__ZN7WebCore37WidgetHierarchyUpdatesSuspensionScope11moveWidgetsEv
    … …  
    10951094__ZN7WebCore9makeRangeERKNS_15VisiblePositionES2_
    10961095__ZN7WebCore9pageCacheEv
    1097 __ZN7WebCore9plainTextEPKNS_5RangeENS_20TextIteratorBehaviorE
     1096__ZN7WebCore9plainTextEPKNS_5RangeENS_20TextIteratorBehaviorEb
    10981097__ZN7WebCore9toElementEN3JSC7JSValueE
    10991098__ZN7WebCore9unionRectERKN3WTF6VectorINS_9FloatRectELm0EEE

  • trunk/Source/WebCore/accessibility/AccessibilityObject.cpp

    r135680 r135972  
    841841                builder.append(listMarkerText);
    842842
    843             builder.append(it.characters(), it.length());
     843            it.appendTextToStringBuilder(builder);
    844844        } else {
    845845            // locate the node and starting offset for this replaced range

  • trunk/Source/WebCore/editing/TextIterator.cpp

    r126520 r135972  
    461461            return;
    462462    }
     463}
     464
     465UChar TextIterator::characterAt(unsigned index) const
     466{
     467    ASSERT(index < static_cast<unsigned>(length()));
     468    if (!(index < static_cast<unsigned>(length())))
     469        return 0;
     470
     471    if (!m_textCharacters)
     472        return string()[startOffset() + index];
     473
     474    return m_textCharacters[index];
     475}
     476
     477void TextIterator::appendTextToStringBuilder(StringBuilder& builder) const
     478{
     479    if (!m_textCharacters)
     480        builder.append(string(), startOffset(), length());
     481    else
     482        builder.append(characters(), length());
    463483}
    464484
    … …  
    10081028    RenderText* renderer = toRenderText(renderObject);
    10091029    m_text = m_emitsOriginalText ? renderer->originalText() : (m_emitsTextWithoutTranscoding ? renderer->textWithoutTranscoding() : renderer->text());
    1010     ASSERT(m_text.characters());
     1030    ASSERT(!m_text.isEmpty());
    10111031    ASSERT(0 <= textStartOffset && textStartOffset < static_cast<int>(m_text.length()));
    10121032    ASSERT(0 <= textEndOffset && textEndOffset <= static_cast<int>(m_text.length()));
    … …  
    10171037    m_positionStartOffset = textStartOffset;
    10181038    m_positionEndOffset = textEndOffset;
    1019     m_textCharacters = m_text.characters() + textStartOffset;
     1039    m_textCharacters = 0;
    10201040    m_textLength = textEndOffset - textStartOffset;
    10211041    m_lastCharacter = m_text[textEndOffset - 1];
    … …  
    24402460            // FIXME: This is a workaround for the fact that the end of a run is often at the wrong
    24412461            // position for emitted '\n's.
    2442             if (len == 1 && it.characters()[0] == '\n') {
     2462            if (len == 1 && it.characterAt(0) == '\n') {
    24432463                scope->document()->updateLayoutIgnorePendingStylesheets();
    24442464                it.advance();
    … …  
    25322552
    25332553// --------
    2534    
    2535 UChar* plainTextToMallocAllocatedBuffer(const Range* r, unsigned& bufferLength, bool isDisplayString, TextIteratorBehavior defaultBehavior)
    2536 {
    2537     UChar* result = 0;
    2538 
     2554
     2555String plainText(const Range* r, TextIteratorBehavior defaultBehavior, bool isDisplayString)
     2556{
    25392557    // The initial buffer size can be critical for performance: https://bugs.webkit.org/show_bug.cgi?id=81192
    25402558    static const unsigned cMaxSegmentSize = 1 << 15;
    2541     bufferLength = 0;
    2542     typedef pair<UChar*, unsigned> TextSegment;
    2543     OwnPtr<Vector<TextSegment> > textSegments;
    2544     Vector<UChar> textBuffer;
    2545     textBuffer.reserveInitialCapacity(cMaxSegmentSize);
     2559
     2560    unsigned bufferLength = 0;
     2561    StringBuilder builder;
     2562    builder.reserveCapacity(cMaxSegmentSize);
    25462563    TextIteratorBehavior behavior = defaultBehavior;
    25472564    if (!isDisplayString)
    … …  
    25492566   
    25502567    for (TextIterator it(r, behavior); !it.atEnd(); it.advance()) {
    2551         if (textBuffer.size() && textBuffer.size() + it.length() > cMaxSegmentSize) {
    2552             UChar* newSegmentBuffer = static_cast<UChar*>(malloc(textBuffer.size() * sizeof(UChar)));
    2553             if (!newSegmentBuffer)
    2554                 goto exit;
    2555             memcpy(newSegmentBuffer, textBuffer.data(), textBuffer.size() * sizeof(UChar));
    2556             if (!textSegments)
    2557                 textSegments = adoptPtr(new Vector<TextSegment>);
    2558             textSegments->append(make_pair(newSegmentBuffer, (unsigned)textBuffer.size()));
    2559             textBuffer.clear();
    2560         }
    2561         textBuffer.append(it.characters(), it.length());
     2568        if (builder.capacity() < builder.length() + it.length())
     2569            builder.reserveCapacity(builder.capacity() + cMaxSegmentSize);
     2570
     2571        it.appendTextToStringBuilder(builder);
    25622572        bufferLength += it.length();
    25632573    }
    25642574
    25652575    if (!bufferLength)
    2566         return 0;
    2567 
    2568     // Since we know the size now, we can make a single buffer out of the pieces with one big alloc
    2569     result = static_cast<UChar*>(malloc(bufferLength * sizeof(UChar)));
    2570     if (!result)
    2571         goto exit;
    2572 
    2573     {
    2574         UChar* resultPos = result;
    2575         if (textSegments) {
    2576             unsigned size = textSegments->size();
    2577             for (unsigned i = 0; i < size; ++i) {
    2578                 const TextSegment& segment = textSegments->at(i);
    2579                 memcpy(resultPos, segment.first, segment.second * sizeof(UChar));
    2580                 resultPos += segment.second;
    2581             }
    2582         }
    2583         memcpy(resultPos, textBuffer.data(), textBuffer.size() * sizeof(UChar));
    2584     }
    2585 
    2586 exit:
    2587     if (textSegments) {
    2588         unsigned size = textSegments->size();
    2589         for (unsigned i = 0; i < size; ++i)
    2590             free(textSegments->at(i).first);
    2591     }
    2592    
     2576        return emptyString();
     2577
     2578    String result = builder.toString();
     2579
    25932580    if (isDisplayString && r->ownerDocument())
    2594         r->ownerDocument()->displayBufferModifiedByEncoding(result, bufferLength);
    2595 
    2596     return result;
    2597 }
    2598 
    2599 String plainText(const Range* r, TextIteratorBehavior defaultBehavior)
    2600 {
    2601     unsigned length;
    2602     UChar* buf = plainTextToMallocAllocatedBuffer(r, length, false, defaultBehavior);
    2603     if (!buf)
    2604         return "";
    2605     String result(buf, length);
    2606     free(buf);
     2581        r->ownerDocument()->displayStringModifiedByEncoding(result);
     2582
    26072583    return result;
    26082584}

  • trunk/Source/WebCore/editing/TextIterator.h

    r120044 r135972  
    6161}
    6262
    63 String plainText(const Range*, TextIteratorBehavior defaultBehavior = TextIteratorDefaultBehavior);
    64 UChar* plainTextToMallocAllocatedBuffer(const Range*, unsigned& bufferLength, bool isDisplayString, TextIteratorBehavior = TextIteratorDefaultBehavior);
     63String plainText(const Range*, TextIteratorBehavior defaultBehavior = TextIteratorDefaultBehavior, bool isDisplayString = false);
    6564PassRefPtr<Range> findPlainText(const Range*, const String&, FindOptions);
    6665
    … …  
    9594   
    9695    int length() const { return m_textLength; }
    97     const UChar* characters() const { return m_textCharacters; }
     96    const UChar* characters() const { return m_textCharacters ? m_textCharacters : m_text.characters() + startOffset(); }
     97    UChar characterAt(unsigned index) const;
     98    void appendTextToStringBuilder(StringBuilder&) const;
    9899   
    99100    PassRefPtr<Range> range() const;
    … …  
    106107   
    107108private:
     109    int startOffset() const { return m_positionStartOffset; }
     110    const String& string() const { return m_text; }
    108111    void exitNode();
    109112    bool shouldRepresentNodeOffsetZero();
    … …  
    140143    mutable int m_positionStartOffset;
    141144    mutable int m_positionEndOffset;
    142     const UChar* m_textCharacters;
     145    const UChar* m_textCharacters; // If null, then use m_text for character data.
    143146    int m_textLength;
    144147    // Hold string m_textCharacters points to so we ensure it won't be deleted.

  • trunk/Source/WebCore/page/ContextMenuController.cpp

    r135394 r135972  
    703703    for (TextIterator it(frame->selection()->toNormalizedRange().get()); !it.atEnd(); it.advance()) {
    704704        int length = it.length();
    705         const UChar* characters = it.characters();
    706705        for (int i = 0; i < length; ++i)
    707             if (!(category(characters[i]) & (Separator_Space | Separator_Line | Separator_Paragraph)))
     706            if (!(category(it.characterAt(i)) & (Separator_Space | Separator_Line | Separator_Paragraph)))
    708707                return true;
    709708    }

  • trunk/Source/WebKit/mac/ChangeLog

    r135952 r135972  
     12012-11-27  Michael Saboff  <msaboff@apple.com>
     2
     3        TextIterator unnecessarily converts 8 bit strings to 16 bits
     4        https://bugs.webkit.org/show_bug.cgi?id=103295
     5
     6        Reviewed by Brent Fulgham.
     7
     8        Updated _stringForRange to use plainText() instead of removed plainTextToMallocAllocatedBuffer().
     9
     10        * WebView/WebFrame.mm:
     11        (-[WebFrame _stringForRange:]):
     12
    1132012-11-27  James Simonsen  <simonjam@chromium.org>
    214

  • trunk/Source/WebKit/mac/WebView/WebFrame.mm

    r135952 r135972  
    499499- (NSString *)_stringForRange:(DOMRange *)range
    500500{
    501     // This will give a system malloc'd buffer that can be turned directly into an NSString
    502     unsigned length;
    503     UChar* buf = plainTextToMallocAllocatedBuffer(core(range), length, true);
    504    
    505     if (!buf)
    506         return [NSString string];
    507 
    508     // Transfer buffer ownership to NSString
    509     return [[[NSString alloc] initWithCharactersNoCopy:buf length:length freeWhenDone:YES] autorelease];
     501    return plainText(core(range), TextIteratorDefaultBehavior, true);
    510502}
    511503
Note: See TracChangeset for help on using the changeset viewer.