WebCore:
authorjustin.garcia@apple.com <justin.garcia@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 May 2008 19:30:44 +0000 (19:30 +0000)
committerjustin.garcia@apple.com <justin.garcia@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 May 2008 19:30:44 +0000 (19:30 +0000)
2008-05-14  Justin Garcia  <justin.garcia@apple.com>

        Reviewed by Darin.

        <rdar://problem/5914803> Improve performance of WebCore::Editor::setComposition

        * editing/Editor.cpp:
        (WebCore::Editor::confirmComposition): Remove the previous composition
        when we insert the new one, not with a separate, slower, delete operation.
        (WebCore::Editor::setComposition): Ditto.
        * editing/InsertTextCommand.cpp:
        (WebCore::InsertTextCommand::performTrivialReplace): Remove the selected
        text with a low level operation that doesn't perform a layout and insert
        the new text in a way that won't trigger a layout from the removal.
        (WebCore::InsertTextCommand::input): Call the optimized replace.
        * editing/InsertTextCommand.h:
        * editing/htmlediting.cpp:
        (WebCore::isTabSpanNode): Check to see if the node is a span, to avoid
        the expense of getAttribute in the common case.
        * page/Frame.cpp:
        (WebCore::Frame::selectionLayoutChanged): Selection::start() and end()
        will already be at VisiblePosition deepEquivalents. Selection::validate()
        ensures this.

LayoutTests:

2008-05-14  Justin Garcia  <justin.garcia@apple.com>

        Reviewed by Darin.

        <rdar://problem/5914803> Improve performance of WebCore::Editor::setComposition

        Replacing all the text in a node is now much less likely to remove it
        and cause selection changes.

        * platform/mac/editing/input/text-input-controller-expected.txt:
        * platform/mac/editing/inserting/4959067-expected.txt:
        * platform/mac/editing/style/style-3681552-fix-001-expected.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@33449 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/mac/editing/input/text-input-controller-expected.txt
LayoutTests/platform/mac/editing/inserting/4959067-expected.txt
LayoutTests/platform/mac/editing/style/style-3681552-fix-001-expected.txt
WebCore/ChangeLog
WebCore/editing/Editor.cpp
WebCore/editing/InsertTextCommand.cpp
WebCore/editing/InsertTextCommand.h
WebCore/editing/htmlediting.cpp
WebCore/page/Frame.cpp

index 7f12bcc..4afe47c 100644 (file)
@@ -1,3 +1,16 @@
+2008-05-14  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Darin.
+
+        <rdar://problem/5914803> Improve performance of WebCore::Editor::setComposition
+        
+        Replacing all the text in a node is now much less likely to remove it
+        and cause selection changes.
+
+        * platform/mac/editing/input/text-input-controller-expected.txt:
+        * platform/mac/editing/inserting/4959067-expected.txt:
+        * platform/mac/editing/style/style-3681552-fix-001-expected.txt:
+
 2008-05-14  Alexey Proskuryakov  <ap@webkit.org>
 
         Reviewed by Darin.
index c5d3918..8e4a64a 100644 (file)
@@ -4,8 +4,7 @@ EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 7 of #text > DIV > BODY > HTML > #document toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 7 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: shouldInsertText:to be deleted replacingDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 7 of #text > DIV > BODY > HTML > #document givenAction:WebViewInsertActionTyped
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 13 of #text > DIV > BODY > HTML > #document to 13 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 7 of #text > DIV > BODY > HTML > #document toDOMRange:range from 13 of #text > DIV > BODY > HTML > #document to 13 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 13 of #text > DIV > BODY > HTML > #document to 13 of #text > DIV > BODY > HTML > #document toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 13 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
@@ -18,21 +17,9 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 7 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 7 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
-EDITING DELEGATE: shouldInsertText:Success replacingDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document givenAction:WebViewInsertActionTyped
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 7 of #text > DIV > BODY > HTML > #document to 7 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldInsertText:Success replacingDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 7 of #text > DIV > BODY > HTML > #document givenAction:WebViewInsertActionTyped
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 7 of #text > DIV > BODY > HTML > #document toDOMRange:range from 7 of #text > DIV > BODY > HTML > #document to 7 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 7 of #text > DIV > BODY > HTML > #document to 7 of #text > DIV > BODY > HTML > #document toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 7 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
index 5008644..048cbb3 100644 (file)
@@ -10,10 +10,8 @@ layer at (0,0) size 800x600
         RenderBlock {UL} at (0,0) size 784x18
           RenderListItem {LI} at (40,0) size 744x18
             RenderListMarker at (-17,0) size 7x18: bullet
-            RenderInline {FONT} at (0,0) size 21x18 [color=#0000EE]
-              RenderInline {SPAN} at (0,0) size 21x18
-                RenderInline {A} at (0,0) size 21x18
-                  RenderText {#text} at (0,0) size 21x18
-                    text run at (0,0) width 21: "foo"
+            RenderInline {A} at (0,0) size 21x18 [color=#0000EE]
+              RenderText {#text} at (0,0) size 21x18
+                text run at (0,0) width 21: "foo"
       RenderBlock {UL} at (0,68) size 784x0
-caret: position 3 of child 0 {#text} of child 0 {A} of child 0 {SPAN} of child 0 {FONT} of child 0 {LI} of child 1 {UL} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
+caret: position 3 of child 0 {#text} of child 0 {A} of child 0 {LI} of child 1 {UL} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
index 01f5880..69325bd 100644 (file)
@@ -13,8 +13,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 1 of #text > I > SPAN > DIV > BODY > HTML > #document to 1 of #text > I > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of #text > I > SPAN > DIV > BODY > HTML > #document to 1 of #text > I > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 1 of #text > I > SPAN > DIV > BODY > HTML > #document to 1 of #text > I > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > I > SPAN > DIV > BODY > HTML > #document to 1 of #text > I > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 2 of #text > I > SPAN > DIV > BODY > HTML > #document to 2 of #text > I > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
index c7c1cf8..f55ecab 100644 (file)
@@ -1,3 +1,27 @@
+2008-05-14  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Darin.
+
+        <rdar://problem/5914803> Improve performance of WebCore::Editor::setComposition
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::confirmComposition): Remove the previous composition
+        when we insert the new one, not with a separate, slower, delete operation.
+        (WebCore::Editor::setComposition): Ditto.
+        * editing/InsertTextCommand.cpp:
+        (WebCore::InsertTextCommand::performTrivialReplace): Remove the selected
+        text with a low level operation that doesn't perform a layout and insert
+        the new text in a way that won't trigger a layout from the removal.
+        (WebCore::InsertTextCommand::input): Call the optimized replace.
+        * editing/InsertTextCommand.h:
+        * editing/htmlediting.cpp:
+        (WebCore::isTabSpanNode): Check to see if the node is a span, to avoid 
+        the expense of getAttribute in the common case.
+        * page/Frame.cpp:
+        (WebCore::Frame::selectionLayoutChanged): Selection::start() and end()
+        will already be at VisiblePosition deepEquivalents. Selection::validate()
+        ensures this.
+
 2008-05-14  Adam Roben  <aroben@apple.com>
 
         Make the Inspector able to handle being reloaded
index 96cdb26..66d3cda 100644 (file)
@@ -1152,9 +1152,10 @@ void Editor::confirmComposition(const String& text, bool preserveSelection)
         return;
     }
     
-    // If there is a composition to replace, remove it with a deletion that will be part of the
-    // same Undo step as the next and previous insertions.
-    TypingCommand::deleteSelection(m_frame->document(), false);
+    // If text is empty, then delete the old composition here.  If text is non-empty, InsertTextCommand::input
+    // will delete the old composition with an optimized replace operation.
+    if (text.isEmpty())
+        TypingCommand::deleteSelection(m_frame->document(), false);
 
     m_compositionNode = 0;
     m_customCompositionUnderlines.clear();
@@ -1178,9 +1179,10 @@ void Editor::setComposition(const String& text, const Vector<CompositionUnderlin
         return;
     }
     
-    // If there is a composition to replace, remove it with a deletion that will be part of the
-    // same Undo step as the next and previous insertions.
-    TypingCommand::deleteSelection(m_frame->document(), false);
+    // If text is empty, then delete the old composition here.  If text is non-empty, InsertTextCommand::input
+    // will delete the old composition with an optimized replace operation.
+    if (text.isEmpty())
+        TypingCommand::deleteSelection(m_frame->document(), false);
 
     m_compositionNode = 0;
     m_customCompositionUnderlines.clear();
index 8be8d1e..f139fd2 100644 (file)
@@ -79,6 +79,39 @@ Position InsertTextCommand::prepareForTextInsertion(const Position& p)
     return pos;
 }
 
+// This avoids the expense of a full fledged delete operation, and avoids a layout that typically results
+// from text removal.
+bool InsertTextCommand::performTrivialReplace(const String& text, bool selectInsertedText)
+{
+    if (!endingSelection().isRange())
+        return false;
+    
+    if (text.contains('\t') || text.contains(' ') || text.contains('\n'))
+        return false;
+    
+    Position start = endingSelection().start();
+    Position end = endingSelection().end();
+    
+    if (start.node() != end.node() || !start.node()->isTextNode() || isTabSpanTextNode(start.node()))
+        return false;
+        
+    replaceTextInNode(static_cast<Text*>(start.node()), start.offset(), end.offset() - start.offset(), text);
+    
+    Position endPosition(start.node(), start.offset() + text.length());
+    
+    // We could have inserted a part of composed character sequence,
+    // so we are basically treating ending selection as a range to avoid validation.
+    // <http://bugs.webkit.org/show_bug.cgi?id=15781>
+    Selection forcedEndingSelection;
+    forcedEndingSelection.setWithoutValidation(start, endPosition);
+    setEndingSelection(forcedEndingSelection);
+    
+    if (!selectInsertedText)
+        setEndingSelection(Selection(endingSelection().visibleEnd()));
+    
+    return true;
+}
+
 void InsertTextCommand::input(const String& originalText, bool selectInsertedText)
 {
     String text = originalText;
@@ -97,8 +130,11 @@ void InsertTextCommand::input(const String& originalText, bool selectInsertedTex
     
     // Delete the current selection.
     // FIXME: This delete operation blows away the typing style.
-    if (endingSelection().isRange())
+    if (endingSelection().isRange()) {
+        if (performTrivialReplace(text, selectInsertedText))
+            return;
         deleteSelection(false, true, true, false);
+    }
     
     // Insert the character at the leftmost candidate.
     Position startPosition = endingSelection().start().upstream();
index 8b25c8f..43d4ec7 100644 (file)
@@ -46,6 +46,8 @@ private:
 
     Position prepareForTextInsertion(const Position&);
     Position insertTab(const Position&);
+    
+    bool performTrivialReplace(const String&, bool selectInsertedText);
 
     unsigned m_charactersAdded;
 };
index 6ef8f8f..12c013a 100644 (file)
@@ -783,7 +783,7 @@ PassRefPtr<Element> createElement(Document* document, const String& tagName)
 
 bool isTabSpanNode(const Node *node)
 {
-    return (node && node->isElementNode() && static_cast<const Element *>(node)->getAttribute("class") == AppleTabSpanClass);
+    return node && node->hasTagName(spanTag) && node->isElementNode() && static_cast<const Element *>(node)->getAttribute(classAttr) == AppleTabSpanClass;
 }
 
 bool isTabSpanTextNode(const Node *node)
index 5fa6273..40a7886 100644 (file)
@@ -610,10 +610,10 @@ void Frame::selectionLayoutChanged()
         // Example: foo <a>bar</a>.  Imagine that a line wrap occurs after 'foo', and that 'bar' is selected.   If we pass [foo, 3]
         // as the start of the selection, the selection painting code will think that content on the line containing 'foo' is selected
         // and will fill the gap before 'bar'.
-        Position startPos = selection.visibleStart().deepEquivalent();
+        Position startPos = selection.start();
         if (startPos.downstream().isCandidate())
             startPos = startPos.downstream();
-        Position endPos = selection.visibleEnd().deepEquivalent();
+        Position endPos = selection.end();
         if (endPos.upstream().isCandidate())
             endPos = endPos.upstream();