Changeset 21611 in webkit


Ignore:
Timestamp:
May 20, 2007, 2:14:40 PM (18 years ago)
Author:
darin
Message:

Reviewed by Oliver Hunt.

  • fix <rdar://problem/5193416> REGRESSION: Selection on large pages extremely slow
  • dom/Document.cpp: (WebCore::Document::removeMarkers): Added an early exit for the common case where there are no markers. Changed code to iterate over all the nodes in the range instead of using TextIterator, which is more efficient.


  • page/Frame.cpp: (WebCore::Frame::respondToChangedSelection): Added checks for editable, so we don't bother doing work related to spell checking and grammar checking when changing the selection in non-editable text. Also rearranged the code so we only compute the old word boundaries and sentence boundaries when actually needed, and don't do the sentence range checks unless grammar checking is enabled.
  • platform/TextBreakIteratorICU.cpp: (WebCore::setUpIterator): Don't take a locale parameter. Always pass in currentTextBreakLocaleID. (WebCore::characterBreakIterator): Removed local parameter. (WebCore::wordBreakIterator): Ditto. (WebCore::lineBreakIterator): Ditto. (WebCore::sentenceBreakIterator): Ditto.
  • platform/mac/TextBreakIteratorInternalICUMac.mm: (WebCore::getTextBreakLocale): Broke out the code to actually get the locale. (WebCore::currentTextBreakLocaleID): This function now handles only the caching and calls getTextBreakLocale to actually figure it out.
  • editing/visible_units.cpp: Added lots of FIXME comments, but no code change.
Location:
trunk/WebCore
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r21610 r21611  
     12007-05-20  Darin Adler  <darin@apple.com>
     2
     3        Reviewed by Oliver Hunt.
     4
     5        - fix <rdar://problem/5193416> REGRESSION: Selection on large pages extremely slow
     6
     7        * dom/Document.cpp: (WebCore::Document::removeMarkers): Added an early exit for the common
     8        case where there are no markers. Changed code to iterate over all the nodes in the range
     9        instead of using TextIterator, which is more efficient.
     10       
     11        * page/Frame.cpp: (WebCore::Frame::respondToChangedSelection): Added checks for editable,
     12        so we don't bother doing work related to spell checking and grammar checking when changing
     13        the selection in non-editable text. Also rearranged the code so we only compute the old
     14        word boundaries and sentence boundaries when actually needed, and don't do the sentence
     15        range checks unless grammar checking is enabled.
     16
     17        * platform/TextBreakIteratorICU.cpp:
     18        (WebCore::setUpIterator): Don't take a locale parameter. Always pass in currentTextBreakLocaleID.
     19        (WebCore::characterBreakIterator): Removed local parameter.
     20        (WebCore::wordBreakIterator): Ditto.
     21        (WebCore::lineBreakIterator): Ditto.
     22        (WebCore::sentenceBreakIterator): Ditto.
     23
     24        * platform/mac/TextBreakIteratorInternalICUMac.mm:
     25        (WebCore::getTextBreakLocale): Broke out the code to actually get the locale.
     26        (WebCore::currentTextBreakLocaleID): This function now handles only the caching and calls
     27        getTextBreakLocale to actually figure it out.
     28
     29        * editing/visible_units.cpp: Added lots of FIXME comments, but no code change.
     30
    1312007-05-20  Adam Treat  <adam@staikos.net>
    232
    … …  
    728758        load completed, even if we think the current frame load is already complete.
    729759
    730 2007-05-16  dethbakin  <bdakin@apple.com>
     7602007-05-16  Beth Dakin  <bdakin@apple.com>
    731761
    732762        Reviewed by Hyatt.

  • trunk/WebCore/dom/Document.cpp

    r21467 r21611  
    27622762}
    27632763
    2764 void Document::removeMarkers(Range *range, DocumentMarker::MarkerType markerType)
    2765 {
    2766     // Use a TextIterator to visit the potentially multiple nodes the range covers.
    2767     for (TextIterator markedText(range); !markedText.atEnd(); markedText.advance()) {
    2768         RefPtr<Range> textPiece = markedText.range();
    2769         int exception = 0;
    2770         unsigned startOffset = textPiece->startOffset(exception);
    2771         unsigned length = textPiece->endOffset(exception) - startOffset + 1;
    2772         removeMarkers(textPiece->startContainer(exception), startOffset, length, markerType);
    2773     }
    2774 }
    2775 
    2776 // Markers are stored in order sorted by their location.  They do not overlap each other, as currently
    2777 // required by the drawing code in RenderText.cpp.
     2764void Document::removeMarkers(Range* range, DocumentMarker::MarkerType markerType)
     2765{
     2766    if (m_markers.isEmpty())
     2767        return;
     2768
     2769    ExceptionCode ec = 0;
     2770    Node* startContainer = range->startContainer(ec);
     2771    int startOffset = range->startOffset(ec);
     2772    Node* endContainer = range->endContainer(ec);
     2773    int endOffset = range->endOffset(ec);
     2774
     2775    Node* pastEndNode = range->pastEndNode();
     2776    for (Node* node = range->startNode(); node != pastEndNode; node = node->traverseNextNode())
     2777        removeMarkers(node,
     2778            node == startContainer ? startOffset : 0,
     2779            node == endContainer ? endOffset : INT_MAX,
     2780            markerType);
     2781}
     2782
     2783// Markers are stored in order sorted by their location.
     2784// They do not overlap each other, as currently required by the drawing code in RenderText.cpp.
    27782785
    27792786void Document::addMarker(Node *node, DocumentMarker newMarker)
    … …  
    28722879}
    28732880
    2874 void Document::removeMarkers(Node *node, unsigned startOffset, int length, DocumentMarker::MarkerType markerType)
     2881void Document::removeMarkers(Node* node, unsigned startOffset, int length, DocumentMarker::MarkerType markerType)
    28752882{
    28762883    if (length <= 0)
    … …  
    30083015}
    30093016
    3010 
    30113017void Document::removeMarkers(Node* node)
    30123018{

  • trunk/WebCore/editing/visible_units.cpp

    r21374 r21611  
    522522{
    523523    TextBreakIterator* iterator = sentenceBreakIterator(characters, length);
     524    // FIXME: The following function can return -1; we don't handle that.
    524525    return textBreakPreceding(iterator, length);
    525526}
    … …  
    527528VisiblePosition startOfSentence(const VisiblePosition &c)
    528529{
     530    // FIXME: The sentence break iterator will not stop at paragraph boundaries.
     531    // Thus this treats many large documents as one giant sentence.
    529532    return previousBoundary(c, startSentenceBoundary);
    530533}
    … …  
    532535static unsigned endSentenceBoundary(const UChar* characters, unsigned length)
    533536{
     537    // FIXME: It's unclear why this starts searching from the end of the buffer.
     538    // The word equivalent starts at the start of the buffer, and I think that's correct.
    534539    TextBreakIterator* iterator = sentenceBreakIterator(characters, length);
     540    // FIXME: The following function can return -1; we don't handle that.
    535541    int start = textBreakPreceding(iterator, length);
    536542    return textBreakFollowing(iterator, start);
    … …  
    539545VisiblePosition endOfSentence(const VisiblePosition &c)
    540546{
     547    // FIXME: The sentence break iterator will not stop at paragraph boundaries.
     548    // Thus this treats many large documents as one giant sentence.
    541549    return nextBoundary(c, endSentenceBoundary);
    542550}
    … …  
    544552static unsigned previousSentencePositionBoundary(const UChar* characters, unsigned length)
    545553{
     554    // FIXME: This is identical to startSentenceBoundary. I'm pretty sure that's not right.
    546555    TextBreakIterator* iterator = sentenceBreakIterator(characters, length);
     556    // FIXME: The following function can return -1; we don't handle that.
    547557    return textBreakPreceding(iterator, length);
    548558}
    … …  
    550560VisiblePosition previousSentencePosition(const VisiblePosition &c)
    551561{
     562    // FIXME: The sentence break iterator will not stop at paragraph boundaries.
     563    // Thus this treats many large documents as one giant sentence.
    552564    return previousBoundary(c, previousSentencePositionBoundary);
    553565}
    … …  
    555567static unsigned nextSentencePositionBoundary(const UChar* characters, unsigned length)
    556568{
     569    // FIXME: This is too close to endSentenceBoundary. I'm pretty sure it's not right.
    557570    TextBreakIterator* iterator = sentenceBreakIterator(characters, length);
     571    // FIXME: The following function can return -1; we don't handle that.
    558572    return textBreakFollowing(iterator, 0);
    559573}
    … …  
    561575VisiblePosition nextSentencePosition(const VisiblePosition &c)
    562576{
     577    // FIXME: The sentence break iterator will not stop at paragraph boundaries.
     578    // Thus this treats many large documents as one giant sentence.
    563579    return nextBoundary(c, nextSentencePositionBoundary);
    564580}

  • trunk/WebCore/page/Frame.cpp

    r21476 r21611  
    1 /* This file is part of the KDE project
    2  *
     1/*
    32 * Copyright (C) 1998, 1999 Torben Weis <weis@kde.org>
    43 *                     1999 Lars Knoll <knoll@kde.org>
    … …  
    76 *                     2000 Stefan Schimanski <1Stein@gmx.de>
    87 *                     2001 George Staikos <staikos@kde.org>
    9  * Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
     8 * Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
    109 * Copyright (C) 2005 Alexey Proskuryakov <ap@nypop.com>
    1110 * Copyright (C) 2007 Trolltech ASA
    … …  
    3635#include "CSSProperty.h"
    3736#include "CSSPropertyNames.h"
    38 #include "Cache.h"
    3937#include "CachedCSSStyleSheet.h"
    40 #include "Chrome.h"
    4138#include "DOMWindow.h"
    4239#include "DocLoader.h"
    … …  
    4441#include "EditingText.h"
    4542#include "EditorClient.h"
    46 #include "Event.h"
    4743#include "EventNames.h"
    48 #include "FloatRect.h"
    4944#include "FocusController.h"
    50 #include "Frame.h"
    51 #include "FrameLoadRequest.h"
    5245#include "FrameLoader.h"
    5346#include "FrameView.h"
    5447#include "GraphicsContext.h"
     48#include "HTMLDocument.h"
    5549#include "HTMLFormElement.h"
    5650#include "HTMLFrameElementBase.h"
    5751#include "HTMLGenericFormElement.h"
    58 #include "HTMLInputElement.h"
    5952#include "HTMLNames.h"
    60 #include "HTMLObjectElement.h"
    6153#include "HTMLTableCellElement.h"
    62 #include "HitTestRequest.h"
    63 #include "HitTestResult.h"
    64 #include "IconDatabase.h"
    65 #include "IconLoader.h"
    66 #include "ImageDocument.h"
    67 #include "IndentOutdentCommand.h"
    6854#include "Logging.h"
    6955#include "MediaFeatureNames.h"
    70 #include "MouseEventWithHitTestResults.h"
    7156#include "NodeList.h"
    7257#include "Page.h"
    73 #include "PlatformScrollBar.h"
    7458#include "RegularExpression.h"
    75 #include "RenderListBox.h"
    76 #include "RenderObject.h"
    7759#include "RenderPart.h"
    7860#include "RenderTableCell.h"
    … …  
    8062#include "RenderTheme.h"
    8163#include "RenderView.h"
    82 #include "SegmentedString.h"
    8364#include "TextIterator.h"
    8465#include "TextResourceDecoder.h"
    85 #include "TypingCommand.h"
    86 #include "XMLTokenizer.h"
    87 #include "cssstyleselector.h"
    88 #include "htmlediting.h"
    89 #include "kjs_proxy.h"
    90 #include "kjs_window.h"
    91 #include "markup.h"
    92 #include "visible_units.h"
    93 #include "xmlhttprequest.h"
    94 #include <math.h>
    95 #include <sys/types.h>
    96 #include <wtf/Platform.h>
    97 
     66#include "XMLNames.h"
    9867#include "bindings/NP_jsobject.h"
    9968#include "bindings/npruntime_impl.h"
    10069#include "bindings/runtime_root.h"
    101 
    102 #if !PLATFORM(WIN_OS)
    103 #include <unistd.h>
    104 #endif
     70#include "kjs_proxy.h"
     71#include "kjs_window.h"
     72#include "visible_units.h"
    10573
    10674#if ENABLE(SVG)
    10775#include "SVGNames.h"
    10876#include "XLinkNames.h"
    109 #include "SVGDocument.h"
    110 #include "SVGDocumentExtensions.h"
    11177#endif
    11278
    113 #include "XMLNames.h"
    114 
    11579using namespace std;
    11680
    11781using KJS::JSLock;
    118 using KJS::JSValue;
    119 using KJS::Location;
    120 using KJS::PausedTimeouts;
    121 using KJS::SavedProperties;
    122 using KJS::SavedBuiltins;
    123 using KJS::UString;
    12482using KJS::Window;
    12583
    … …  
    18041762}
    18051763
    1806 void Frame::respondToChangedSelection(const Selection &oldSelection, bool closeTyping)
     1764void Frame::respondToChangedSelection(const Selection& oldSelection, bool closeTyping)
    18071765{
    18081766    if (document()) {
    1809         if (editor()->isContinuousSpellCheckingEnabled()) {
    1810             Selection oldAdjacentWords;
    1811             Selection oldSelectedSentence;
    1812            
    1813             // If this is a change in selection resulting from a delete operation, oldSelection may no longer
    1814             // be in the document.
    1815             if (oldSelection.start().node() && oldSelection.start().node()->inDocument()) {
     1767        bool isContinuousSpellCheckingEnabled = editor()->isContinuousSpellCheckingEnabled();
     1768        bool isContinuousGrammarCheckingEnabled = isContinuousSpellCheckingEnabled && editor()->isGrammarCheckingEnabled();
     1769        if (isContinuousSpellCheckingEnabled) {
     1770            Selection newAdjacentWords;
     1771            Selection newSelectedSentence;
     1772            if (selectionController()->selection().isContentEditable()) {
     1773                VisiblePosition newStart(selectionController()->selection().visibleStart());
     1774                newAdjacentWords = Selection(startOfWord(newStart, LeftWordIfOnBoundary), endOfWord(newStart, RightWordIfOnBoundary));
     1775                if (isContinuousGrammarCheckingEnabled)
     1776                    newSelectedSentence = Selection(startOfSentence(newStart), endOfSentence(newStart));
     1777            }
     1778
     1779            // When typing we check spelling elsewhere, so don't redo it here.
     1780            // If this is a change in selection resulting from a delete operation,
     1781            // oldSelection may no longer be in the document.
     1782            if (closeTyping && oldSelection.isContentEditable() && oldSelection.start().node() && oldSelection.start().node()->inDocument()) {
    18161783                VisiblePosition oldStart(oldSelection.visibleStart());
    1817                 oldAdjacentWords = Selection(startOfWord(oldStart, LeftWordIfOnBoundary), endOfWord(oldStart, RightWordIfOnBoundary));   
    1818                 oldSelectedSentence = Selection(startOfSentence(oldStart), endOfSentence(oldStart));   
     1784                Selection oldAdjacentWords = Selection(startOfWord(oldStart, LeftWordIfOnBoundary), endOfWord(oldStart, RightWordIfOnBoundary));   
     1785                if (oldAdjacentWords != newAdjacentWords) {
     1786                    editor()->markMisspellings(oldAdjacentWords);
     1787                    if (isContinuousGrammarCheckingEnabled) {
     1788                        Selection oldSelectedSentence = Selection(startOfSentence(oldStart), endOfSentence(oldStart));   
     1789                        if (oldSelectedSentence != newSelectedSentence)
     1790                            editor()->markBadGrammar(oldSelectedSentence);
     1791                    }
     1792                }
    18191793            }
    1820            
    1821             VisiblePosition newStart(selectionController()->selection().visibleStart());
    1822             Selection newAdjacentWords(startOfWord(newStart, LeftWordIfOnBoundary), endOfWord(newStart, RightWordIfOnBoundary));
    1823             Selection newSelectedSentence(startOfSentence(newStart), endOfSentence(newStart));
    1824            
    1825             // When typing we check spelling elsewhere, so don't redo it here.
    1826             if (closeTyping && oldAdjacentWords != newAdjacentWords) {
    1827                 editor()->markMisspellings(oldAdjacentWords);
    1828                
    1829                 if (oldSelectedSentence != newSelectedSentence)
    1830                     editor()->markBadGrammar(oldSelectedSentence);
    1831             }
    1832            
    1833             // This only erases a marker in the first unit (word or sentence) of the selection.
     1794
     1795            // This only erases markers that are in the first unit (word or sentence) of the selection.
    18341796            // Perhaps peculiar, but it matches AppKit.
    1835             document()->removeMarkers(newAdjacentWords.toRange().get(), DocumentMarker::Spelling);
    1836             document()->removeMarkers(newSelectedSentence.toRange().get(), DocumentMarker::Grammar);
    1837         } else {
    1838             // When continuous spell checking is off, existing markers disappear after the selection changes.
     1797            if (RefPtr<Range> wordRange = newAdjacentWords.toRange())
     1798                document()->removeMarkers(wordRange.get(), DocumentMarker::Spelling);
     1799            if (RefPtr<Range> sentenceRange = newSelectedSentence.toRange())
     1800                document()->removeMarkers(sentenceRange.get(), DocumentMarker::Grammar);
     1801        }
     1802
     1803        // When continuous spell checking is off, existing markers disappear after the selection changes.
     1804        if (!isContinuousSpellCheckingEnabled)
    18391805            document()->removeMarkers(DocumentMarker::Spelling);
     1806        if (!isContinuousGrammarCheckingEnabled)
    18401807            document()->removeMarkers(DocumentMarker::Grammar);
    1841         }
    1842     }
    1843    
     1808    }
     1809
    18441810    editor()->respondToChangedSelection(oldSelection);
    18451811}

  • trunk/WebCore/platform/TextBreakIteratorICU.cpp

    r21270 r21611  
    3030
    3131static TextBreakIterator* setUpIterator(bool& createdIterator, TextBreakIterator*& iterator,
    32     UBreakIteratorType type, const UChar* string, int length, const char* locale)
     32    UBreakIteratorType type, const UChar* string, int length)
    3333{
    3434    if (!string)
    … …  
    3636
    3737    if (!createdIterator) {
    38         // The locale is currently ignored when determining character cluster breaks.
    39         // This may change in the future, according to Deborah Goldsmith.
    40         // FIXME: Presumably we do need to pass the correct locale for word and line
    41         // break iterators, though!
    4238        UErrorCode openStatus = U_ZERO_ERROR;
    43         iterator = static_cast<TextBreakIterator*>(ubrk_open(type, locale, 0, 0, &openStatus));
     39        iterator = static_cast<TextBreakIterator*>(ubrk_open(type, currentTextBreakLocaleID(), 0, 0, &openStatus));
    4440        createdIterator = true;
    4541    }
    … …  
    6056    static TextBreakIterator* staticCharacterBreakIterator;
    6157    return setUpIterator(createdCharacterBreakIterator,
    62         staticCharacterBreakIterator, UBRK_CHARACTER, string, length, "en_us");
     58        staticCharacterBreakIterator, UBRK_CHARACTER, string, length);
    6359}
    6460
    … …  
    6864    static TextBreakIterator* staticWordBreakIterator;
    6965    return setUpIterator(createdWordBreakIterator,
    70         staticWordBreakIterator, UBRK_WORD, string, length, "en_us");
     66        staticWordBreakIterator, UBRK_WORD, string, length);
    7167}
    7268
    … …  
    7672    static TextBreakIterator* staticLineBreakIterator;
    7773    return setUpIterator(createdLineBreakIterator,
    78         staticLineBreakIterator, UBRK_LINE, string, length, "en_us");
     74        staticLineBreakIterator, UBRK_LINE, string, length);
    7975}
    8076
    … …  
    8480    static TextBreakIterator* staticSentenceBreakIterator;
    8581    return setUpIterator(createdSentenceBreakIterator,
    86         staticSentenceBreakIterator, UBRK_SENTENCE, string, length, currentTextBreakLocaleID());
     82        staticSentenceBreakIterator, UBRK_SENTENCE, string, length);
    8783}
    8884

  • trunk/WebCore/platform/mac/TextBreakIteratorInternalICUMac.mm

    r21446 r21611  
    2424namespace WebCore {
    2525
     26static const int maxLocaleStringLength = 32;
     27
    2628// This code was swiped from the CarbonCore UnicodeUtilities. One change from that is to use the empty
    2729// string instead of the "old locale model" as the ultimate fallback. This change is per the UnicodeUtilities
    2830// engineer.
    29 //
    30 // NOTE: this abviously could be fairly expensive to do.  If it turns out to be a bottleneck, it might
    31 // help to instead put a call in the iterator initializer to set the current text break locale.  Unfortunately,
    32 // we can not cache it across calls to our API since the result can change without our knowing (AFAIK
    33 // there are no notifiers for AppleTextBreakLocale and/or AppleLanguages changes).
    34 const char* currentTextBreakLocaleID()
     31static void getTextBreakLocale(char localeStringBuffer[maxLocaleStringLength])
    3532{
    36     const int localeStringLength = 32;
    37     static char localeStringBuffer[localeStringLength] = { 0 };
    38     char* localeString = &localeStringBuffer[0];
    39    
    40     // Empty string means "root locale", which what we use if we can't use a pref.
    41    
     33    // Empty string means "root locale", which is what we use if we can't use a pref.
     34
    4235    // We get the parts string from AppleTextBreakLocale pref.
    4336    // If that fails then look for the first language in the AppleLanguages pref.
    … …  
    5548        }
    5649    }
    57    
    5850    if (prefLocaleStr) {
    59         // Canonicalize pref string in case it is not in the canonical format. This call is only available on Tiger and newer.
     51        // Canonicalize pref string in case it is not in the canonical format.
    6052        CFStringRef canonLocaleCFStr = CFLocaleCreateCanonicalLanguageIdentifierFromString(kCFAllocatorDefault, prefLocaleStr);
    6153        if (canonLocaleCFStr) {
    62             CFStringGetCString(canonLocaleCFStr, localeString, localeStringLength, kCFStringEncodingASCII);
     54            CFStringGetCString(canonLocaleCFStr, localeStringBuffer, maxLocaleStringLength, kCFStringEncodingASCII);
    6355            CFRelease(canonLocaleCFStr);
    6456        }
    6557        CFRelease(prefLocaleStr);
    6658    }
    67    
    68     return localeString;
     59}
     60
     61const char* currentTextBreakLocaleID()
     62{
     63    static char localeStringBuffer[maxLocaleStringLength];
     64    static bool gotTextBreakLocale = false;
     65    if (!gotTextBreakLocale) {
     66        getTextBreakLocale(localeStringBuffer);
     67        gotTextBreakLocale = true;
     68    }
     69    return localeStringBuffer;
    6970}
    7071
Note: See TracChangeset for help on using the changeset viewer.