Commit 29df4b01 authored by darin's avatar darin
Browse files

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.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@21611 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 685241da
2007-05-20 Darin Adler <darin@apple.com>
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.
2007-05-20 Adam Treat <adam@staikos.net>
 
Add -f to cp to deal with some obscure build environments.
......@@ -727,7 +757,7 @@
(WebCore::FrameLoader::mainReceivedCompleteError): Make sure to always check whether the overall
load completed, even if we think the current frame load is already complete.
 
2007-05-16 dethbakin <bdakin@apple.com>
2007-05-16 Beth Dakin <bdakin@apple.com>
 
Reviewed by Hyatt.
 
......@@ -2761,20 +2761,27 @@ void Document::addMarker(Range *range, DocumentMarker::MarkerType type, String d
}
}
void Document::removeMarkers(Range *range, DocumentMarker::MarkerType markerType)
void Document::removeMarkers(Range* range, DocumentMarker::MarkerType markerType)
{
// Use a TextIterator to visit the potentially multiple nodes the range covers.
for (TextIterator markedText(range); !markedText.atEnd(); markedText.advance()) {
RefPtr<Range> textPiece = markedText.range();
int exception = 0;
unsigned startOffset = textPiece->startOffset(exception);
unsigned length = textPiece->endOffset(exception) - startOffset + 1;
removeMarkers(textPiece->startContainer(exception), startOffset, length, markerType);
}
if (m_markers.isEmpty())
return;
ExceptionCode ec = 0;
Node* startContainer = range->startContainer(ec);
int startOffset = range->startOffset(ec);
Node* endContainer = range->endContainer(ec);
int endOffset = range->endOffset(ec);
Node* pastEndNode = range->pastEndNode();
for (Node* node = range->startNode(); node != pastEndNode; node = node->traverseNextNode())
removeMarkers(node,
node == startContainer ? startOffset : 0,
node == endContainer ? endOffset : INT_MAX,
markerType);
}
// Markers are stored in order sorted by their location. They do not overlap each other, as currently
// required by the drawing code in RenderText.cpp.
// Markers are stored in order sorted by their location.
// They do not overlap each other, as currently required by the drawing code in RenderText.cpp.
void Document::addMarker(Node *node, DocumentMarker newMarker)
{
......@@ -2871,7 +2878,7 @@ void Document::copyMarkers(Node *srcNode, unsigned startOffset, int length, Node
dstNode->renderer()->repaint();
}
void Document::removeMarkers(Node *node, unsigned startOffset, int length, DocumentMarker::MarkerType markerType)
void Document::removeMarkers(Node* node, unsigned startOffset, int length, DocumentMarker::MarkerType markerType)
{
if (length <= 0)
return;
......@@ -3007,7 +3014,6 @@ Vector<IntRect> Document::renderedRectsForMarkers(DocumentMarker::MarkerType mar
return result;
}
void Document::removeMarkers(Node* node)
{
MarkerMap::iterator i = m_markers.find(node);
......
......@@ -521,45 +521,61 @@ VisiblePosition nextLinePosition(const VisiblePosition &visiblePosition, int x)
static unsigned startSentenceBoundary(const UChar* characters, unsigned length)
{
TextBreakIterator* iterator = sentenceBreakIterator(characters, length);
// FIXME: The following function can return -1; we don't handle that.
return textBreakPreceding(iterator, length);
}
VisiblePosition startOfSentence(const VisiblePosition &c)
{
// FIXME: The sentence break iterator will not stop at paragraph boundaries.
// Thus this treats many large documents as one giant sentence.
return previousBoundary(c, startSentenceBoundary);
}
static unsigned endSentenceBoundary(const UChar* characters, unsigned length)
{
// FIXME: It's unclear why this starts searching from the end of the buffer.
// The word equivalent starts at the start of the buffer, and I think that's correct.
TextBreakIterator* iterator = sentenceBreakIterator(characters, length);
// FIXME: The following function can return -1; we don't handle that.
int start = textBreakPreceding(iterator, length);
return textBreakFollowing(iterator, start);
}
VisiblePosition endOfSentence(const VisiblePosition &c)
{
// FIXME: The sentence break iterator will not stop at paragraph boundaries.
// Thus this treats many large documents as one giant sentence.
return nextBoundary(c, endSentenceBoundary);
}
static unsigned previousSentencePositionBoundary(const UChar* characters, unsigned length)
{
// FIXME: This is identical to startSentenceBoundary. I'm pretty sure that's not right.
TextBreakIterator* iterator = sentenceBreakIterator(characters, length);
// FIXME: The following function can return -1; we don't handle that.
return textBreakPreceding(iterator, length);
}
VisiblePosition previousSentencePosition(const VisiblePosition &c)
{
// FIXME: The sentence break iterator will not stop at paragraph boundaries.
// Thus this treats many large documents as one giant sentence.
return previousBoundary(c, previousSentencePositionBoundary);
}
static unsigned nextSentencePositionBoundary(const UChar* characters, unsigned length)
{
// FIXME: This is too close to endSentenceBoundary. I'm pretty sure it's not right.
TextBreakIterator* iterator = sentenceBreakIterator(characters, length);
// FIXME: The following function can return -1; we don't handle that.
return textBreakFollowing(iterator, 0);
}
VisiblePosition nextSentencePosition(const VisiblePosition &c)
{
// FIXME: The sentence break iterator will not stop at paragraph boundaries.
// Thus this treats many large documents as one giant sentence.
return nextBoundary(c, nextSentencePositionBoundary);
}
......
/* This file is part of the KDE project
*
/*
* Copyright (C) 1998, 1999 Torben Weis <weis@kde.org>
* 1999 Lars Knoll <knoll@kde.org>
* 1999 Antti Koivisto <koivisto@kde.org>
* 2000 Simon Hausmann <hausmann@kde.org>
* 2000 Stefan Schimanski <1Stein@gmx.de>
* 2001 George Staikos <staikos@kde.org>
* Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
* Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
* Copyright (C) 2005 Alexey Proskuryakov <ap@nypop.com>
* Copyright (C) 2007 Trolltech ASA
*
......@@ -35,92 +34,51 @@
#include "CSSComputedStyleDeclaration.h"
#include "CSSProperty.h"
#include "CSSPropertyNames.h"
#include "Cache.h"
#include "CachedCSSStyleSheet.h"
#include "Chrome.h"
#include "DOMWindow.h"
#include "DocLoader.h"
#include "DocumentType.h"
#include "EditingText.h"
#include "EditorClient.h"
#include "Event.h"
#include "EventNames.h"
#include "FloatRect.h"
#include "FocusController.h"
#include "Frame.h"
#include "FrameLoadRequest.h"
#include "FrameLoader.h"
#include "FrameView.h"
#include "GraphicsContext.h"
#include "HTMLDocument.h"
#include "HTMLFormElement.h"
#include "HTMLFrameElementBase.h"
#include "HTMLGenericFormElement.h"
#include "HTMLInputElement.h"
#include "HTMLNames.h"
#include "HTMLObjectElement.h"
#include "HTMLTableCellElement.h"
#include "HitTestRequest.h"
#include "HitTestResult.h"
#include "IconDatabase.h"
#include "IconLoader.h"
#include "ImageDocument.h"
#include "IndentOutdentCommand.h"
#include "Logging.h"
#include "MediaFeatureNames.h"
#include "MouseEventWithHitTestResults.h"
#include "NodeList.h"
#include "Page.h"
#include "PlatformScrollBar.h"
#include "RegularExpression.h"
#include "RenderListBox.h"
#include "RenderObject.h"
#include "RenderPart.h"
#include "RenderTableCell.h"
#include "RenderTextControl.h"
#include "RenderTheme.h"
#include "RenderView.h"
#include "SegmentedString.h"
#include "TextIterator.h"
#include "TextResourceDecoder.h"
#include "TypingCommand.h"
#include "XMLTokenizer.h"
#include "cssstyleselector.h"
#include "htmlediting.h"
#include "kjs_proxy.h"
#include "kjs_window.h"
#include "markup.h"
#include "visible_units.h"
#include "xmlhttprequest.h"
#include <math.h>
#include <sys/types.h>
#include <wtf/Platform.h>
#include "XMLNames.h"
#include "bindings/NP_jsobject.h"
#include "bindings/npruntime_impl.h"
#include "bindings/runtime_root.h"
#if !PLATFORM(WIN_OS)
#include <unistd.h>
#endif
#include "kjs_proxy.h"
#include "kjs_window.h"
#include "visible_units.h"
#if ENABLE(SVG)
#include "SVGNames.h"
#include "XLinkNames.h"
#include "SVGDocument.h"
#include "SVGDocumentExtensions.h"
#endif
#include "XMLNames.h"
using namespace std;
using KJS::JSLock;
using KJS::JSValue;
using KJS::Location;
using KJS::PausedTimeouts;
using KJS::SavedProperties;
using KJS::SavedBuiltins;
using KJS::UString;
using KJS::Window;
namespace WebCore {
......@@ -1803,44 +1761,52 @@ void Frame::scheduleClose()
chrome->closeWindowSoon();
}
void Frame::respondToChangedSelection(const Selection &oldSelection, bool closeTyping)
void Frame::respondToChangedSelection(const Selection& oldSelection, bool closeTyping)
{
if (document()) {
if (editor()->isContinuousSpellCheckingEnabled()) {
Selection oldAdjacentWords;
Selection oldSelectedSentence;
// If this is a change in selection resulting from a delete operation, oldSelection may no longer
// be in the document.
if (oldSelection.start().node() && oldSelection.start().node()->inDocument()) {
VisiblePosition oldStart(oldSelection.visibleStart());
oldAdjacentWords = Selection(startOfWord(oldStart, LeftWordIfOnBoundary), endOfWord(oldStart, RightWordIfOnBoundary));
oldSelectedSentence = Selection(startOfSentence(oldStart), endOfSentence(oldStart));
bool isContinuousSpellCheckingEnabled = editor()->isContinuousSpellCheckingEnabled();
bool isContinuousGrammarCheckingEnabled = isContinuousSpellCheckingEnabled && editor()->isGrammarCheckingEnabled();
if (isContinuousSpellCheckingEnabled) {
Selection newAdjacentWords;
Selection newSelectedSentence;
if (selectionController()->selection().isContentEditable()) {
VisiblePosition newStart(selectionController()->selection().visibleStart());
newAdjacentWords = Selection(startOfWord(newStart, LeftWordIfOnBoundary), endOfWord(newStart, RightWordIfOnBoundary));
if (isContinuousGrammarCheckingEnabled)
newSelectedSentence = Selection(startOfSentence(newStart), endOfSentence(newStart));
}
VisiblePosition newStart(selectionController()->selection().visibleStart());
Selection newAdjacentWords(startOfWord(newStart, LeftWordIfOnBoundary), endOfWord(newStart, RightWordIfOnBoundary));
Selection newSelectedSentence(startOfSentence(newStart), endOfSentence(newStart));
// When typing we check spelling elsewhere, so don't redo it here.
if (closeTyping && oldAdjacentWords != newAdjacentWords) {
editor()->markMisspellings(oldAdjacentWords);
if (oldSelectedSentence != newSelectedSentence)
editor()->markBadGrammar(oldSelectedSentence);
// If this is a change in selection resulting from a delete operation,
// oldSelection may no longer be in the document.
if (closeTyping && oldSelection.isContentEditable() && oldSelection.start().node() && oldSelection.start().node()->inDocument()) {
VisiblePosition oldStart(oldSelection.visibleStart());
Selection oldAdjacentWords = Selection(startOfWord(oldStart, LeftWordIfOnBoundary), endOfWord(oldStart, RightWordIfOnBoundary));
if (oldAdjacentWords != newAdjacentWords) {
editor()->markMisspellings(oldAdjacentWords);
if (isContinuousGrammarCheckingEnabled) {
Selection oldSelectedSentence = Selection(startOfSentence(oldStart), endOfSentence(oldStart));
if (oldSelectedSentence != newSelectedSentence)
editor()->markBadGrammar(oldSelectedSentence);
}
}
}
// This only erases a marker in the first unit (word or sentence) of the selection.
// This only erases markers that are in the first unit (word or sentence) of the selection.
// Perhaps peculiar, but it matches AppKit.
document()->removeMarkers(newAdjacentWords.toRange().get(), DocumentMarker::Spelling);
document()->removeMarkers(newSelectedSentence.toRange().get(), DocumentMarker::Grammar);
} else {
// When continuous spell checking is off, existing markers disappear after the selection changes.
if (RefPtr<Range> wordRange = newAdjacentWords.toRange())
document()->removeMarkers(wordRange.get(), DocumentMarker::Spelling);
if (RefPtr<Range> sentenceRange = newSelectedSentence.toRange())
document()->removeMarkers(sentenceRange.get(), DocumentMarker::Grammar);
}
// When continuous spell checking is off, existing markers disappear after the selection changes.
if (!isContinuousSpellCheckingEnabled)
document()->removeMarkers(DocumentMarker::Spelling);
if (!isContinuousGrammarCheckingEnabled)
document()->removeMarkers(DocumentMarker::Grammar);
}
}
editor()->respondToChangedSelection(oldSelection);
}
......
......@@ -29,18 +29,14 @@
namespace WebCore {
static TextBreakIterator* setUpIterator(bool& createdIterator, TextBreakIterator*& iterator,
UBreakIteratorType type, const UChar* string, int length, const char* locale)
UBreakIteratorType type, const UChar* string, int length)
{
if (!string)
return 0;
if (!createdIterator) {
// The locale is currently ignored when determining character cluster breaks.
// This may change in the future, according to Deborah Goldsmith.
// FIXME: Presumably we do need to pass the correct locale for word and line
// break iterators, though!
UErrorCode openStatus = U_ZERO_ERROR;
iterator = static_cast<TextBreakIterator*>(ubrk_open(type, locale, 0, 0, &openStatus));
iterator = static_cast<TextBreakIterator*>(ubrk_open(type, currentTextBreakLocaleID(), 0, 0, &openStatus));
createdIterator = true;
}
if (!iterator)
......@@ -59,7 +55,7 @@ TextBreakIterator* characterBreakIterator(const UChar* string, int length)
static bool createdCharacterBreakIterator = false;
static TextBreakIterator* staticCharacterBreakIterator;
return setUpIterator(createdCharacterBreakIterator,
staticCharacterBreakIterator, UBRK_CHARACTER, string, length, "en_us");
staticCharacterBreakIterator, UBRK_CHARACTER, string, length);
}
TextBreakIterator* wordBreakIterator(const UChar* string, int length)
......@@ -67,7 +63,7 @@ TextBreakIterator* wordBreakIterator(const UChar* string, int length)
static bool createdWordBreakIterator = false;
static TextBreakIterator* staticWordBreakIterator;
return setUpIterator(createdWordBreakIterator,
staticWordBreakIterator, UBRK_WORD, string, length, "en_us");
staticWordBreakIterator, UBRK_WORD, string, length);
}
TextBreakIterator* lineBreakIterator(const UChar* string, int length)
......@@ -75,7 +71,7 @@ TextBreakIterator* lineBreakIterator(const UChar* string, int length)
static bool createdLineBreakIterator = false;
static TextBreakIterator* staticLineBreakIterator;
return setUpIterator(createdLineBreakIterator,
staticLineBreakIterator, UBRK_LINE, string, length, "en_us");
staticLineBreakIterator, UBRK_LINE, string, length);
}
TextBreakIterator* sentenceBreakIterator(const UChar* string, int length)
......@@ -83,7 +79,7 @@ TextBreakIterator* sentenceBreakIterator(const UChar* string, int length)
static bool createdSentenceBreakIterator = false;
static TextBreakIterator* staticSentenceBreakIterator;
return setUpIterator(createdSentenceBreakIterator,
staticSentenceBreakIterator, UBRK_SENTENCE, string, length, currentTextBreakLocaleID());
staticSentenceBreakIterator, UBRK_SENTENCE, string, length);
}
int textBreakFirst(TextBreakIterator* bi)
......
......@@ -23,22 +23,15 @@
namespace WebCore {
static const int maxLocaleStringLength = 32;
// This code was swiped from the CarbonCore UnicodeUtilities. One change from that is to use the empty
// string instead of the "old locale model" as the ultimate fallback. This change is per the UnicodeUtilities
// engineer.
//
// NOTE: this abviously could be fairly expensive to do. If it turns out to be a bottleneck, it might
// help to instead put a call in the iterator initializer to set the current text break locale. Unfortunately,
// we can not cache it across calls to our API since the result can change without our knowing (AFAIK
// there are no notifiers for AppleTextBreakLocale and/or AppleLanguages changes).
const char* currentTextBreakLocaleID()
static void getTextBreakLocale(char localeStringBuffer[maxLocaleStringLength])
{
const int localeStringLength = 32;
static char localeStringBuffer[localeStringLength] = { 0 };
char* localeString = &localeStringBuffer[0];
// Empty string means "root locale", which what we use if we can't use a pref.
// Empty string means "root locale", which is what we use if we can't use a pref.
// We get the parts string from AppleTextBreakLocale pref.
// If that fails then look for the first language in the AppleLanguages pref.
CFStringRef prefLocaleStr = (CFStringRef)CFPreferencesCopyValue(CFSTR("AppleTextBreakLocale"),
......@@ -54,18 +47,26 @@ const char* currentTextBreakLocaleID()
CFRelease(appleLangArr);
}
}
if (prefLocaleStr) {
// Canonicalize pref string in case it is not in the canonical format. This call is only available on Tiger and newer.
// Canonicalize pref string in case it is not in the canonical format.
CFStringRef canonLocaleCFStr = CFLocaleCreateCanonicalLanguageIdentifierFromString(kCFAllocatorDefault, prefLocaleStr);
if (canonLocaleCFStr) {
CFStringGetCString(canonLocaleCFStr, localeString, localeStringLength, kCFStringEncodingASCII);
CFStringGetCString(canonLocaleCFStr, localeStringBuffer, maxLocaleStringLength, kCFStringEncodingASCII);
CFRelease(canonLocaleCFStr);
}
CFRelease(prefLocaleStr);
}
return localeString;
}
const char* currentTextBreakLocaleID()
{
static char localeStringBuffer[maxLocaleStringLength];
static bool gotTextBreakLocale = false;
if (!gotTextBreakLocale) {
getTextBreakLocale(localeStringBuffer);
gotTextBreakLocale = true;
}
return localeStringBuffer;
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment