REGRESSION(r96257): Deleting a large amount of text is very slow.
authorenrica@apple.com <enrica@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Apr 2012 18:51:14 +0000 (18:51 +0000)
committerenrica@apple.com <enrica@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Apr 2012 18:51:14 +0000 (18:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=83983
<rdar://problem/10826076>

Reviewed by Ryosuke Niwa.

The change in r96257 did not cause the performance regression per se,
but exposed a problem in the way we calculate the offset in container
node when the anchorType is PositionIsOffsetInAnchor.
The offset was computed as the minimum between the given offset and
lastOffsetInNode. If the container has a very large number of children,
we walk the entire list of child nodes in the container simply to find
out how many they are.
Looking through the entire editing code, I found other 2 cases (one
is only an ASSERT) where we could do a similar optimization.

No new tests. No behavior change, only performance optimization.

* dom/Position.cpp:
(WebCore::Position::computeOffsetInContainerNode):
* dom/Position.h:
(WebCore::minOffsetForNode):
(WebCore::offsetIsBeforeLastNodeOffset):
* editing/ApplyStyleCommand.cpp:
(WebCore::ApplyStyleCommand::removeInlineStyle):
(WebCore::ApplyStyleCommand::mergeEndWithNextIfIdentical):

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

Source/WebCore/ChangeLog
Source/WebCore/dom/Position.cpp
Source/WebCore/dom/Position.h
Source/WebCore/editing/ApplyStyleCommand.cpp

index 7477085..373dafb 100644 (file)
@@ -1,3 +1,32 @@
+2012-04-27  Enrica Casucci  <enrica@apple.com>
+
+        REGRESSION(r96257): Deleting a large amount of text is very slow.
+        https://bugs.webkit.org/show_bug.cgi?id=83983
+        <rdar://problem/10826076>
+        
+        Reviewed by Ryosuke Niwa.
+
+        The change in r96257 did not cause the performance regression per se,
+        but exposed a problem in the way we calculate the offset in container
+        node when the anchorType is PositionIsOffsetInAnchor.
+        The offset was computed as the minimum between the given offset and
+        lastOffsetInNode. If the container has a very large number of children,
+        we walk the entire list of child nodes in the container simply to find
+        out how many they are.
+        Looking through the entire editing code, I found other 2 cases (one
+        is only an ASSERT) where we could do a similar optimization.
+
+        No new tests. No behavior change, only performance optimization.
+
+        * dom/Position.cpp:
+        (WebCore::Position::computeOffsetInContainerNode):
+        * dom/Position.h:
+        (WebCore::minOffsetForNode):
+        (WebCore::offsetIsBeforeLastNodeOffset):
+        * editing/ApplyStyleCommand.cpp:
+        (WebCore::ApplyStyleCommand::removeInlineStyle):
+        (WebCore::ApplyStyleCommand::mergeEndWithNextIfIdentical):
+
 2012-04-27  Julien Chaffraix  <jchaffraix@webkit.org>
 
         NULL-deref in RenderBox::clippedOverflowRectForRepaint
index 31ab445..fd78eb9 100644 (file)
@@ -177,7 +177,7 @@ int Position::computeOffsetInContainerNode() const
     case PositionIsAfterChildren:
         return lastOffsetInNode(m_anchorNode.get());
     case PositionIsOffsetInAnchor:
-        return std::min(lastOffsetInNode(m_anchorNode.get()), m_offset);
+        return minOffsetForNode(m_anchorNode.get(), m_offset);
     case PositionIsBeforeAnchor:
         return m_anchorNode->nodeIndex();
     case PositionIsAfterAnchor:
index f1e7ef3..4e244a0 100644 (file)
@@ -286,6 +286,31 @@ inline Position lastPositionInNode(Node* anchorNode)
     return Position(anchorNode, Position::PositionIsAfterChildren);
 }
 
+inline int minOffsetForNode(Node* anchorNode, int offset)
+{
+    if (anchorNode->offsetInCharacters())
+        return std::min(offset, anchorNode->maxCharacterOffset());
+
+    int newOffset = 0;
+    for (Node* node = anchorNode->firstChild(); node && newOffset < offset; node = node->nextSibling())
+        newOffset++;
+    
+    return newOffset;
+}
+
+inline bool offsetIsBeforeLastNodeOffset(int offset, Node* anchorNode)
+{
+    if (anchorNode->offsetInCharacters())
+        return offset < anchorNode->maxCharacterOffset();
+
+    int currentOffset = 0;
+    for (Node* node = anchorNode->firstChild(); node && currentOffset < offset; node = node->nextSibling())
+        currentOffset++;
+    
+    
+    return offset < currentOffset;
+}
+
 } // namespace WebCore
 
 #ifndef NDEBUG
index 4306768..403ca57 100644 (file)
@@ -1063,8 +1063,7 @@ void ApplyStyleCommand::removeInlineStyle(EditingStyle* style, const Position &s
                     // Since elem must have been fully selected, and it is at the end
                     // of the selection, it is clear we can set the new e offset to
                     // the max range offset of prev.
-                    ASSERT(s.anchorType() == Position::PositionIsAfterAnchor
-                           || s.offsetInContainerNode() >= lastOffsetInNode(s.containerNode()));
+                    ASSERT(s.anchorType() == Position::PositionIsAfterAnchor || !offsetIsBeforeLastNodeOffset(s.offsetInContainerNode(), s.containerNode()));
                     e = lastPositionInOrAfterNode(prev.get());
                 }
             }
@@ -1227,7 +1226,7 @@ bool ApplyStyleCommand::mergeEndWithNextIfIdentical(const Position& start, const
     int endOffset = end.computeOffsetInContainerNode();
 
     if (isAtomicNode(endNode)) {
-        if (endOffset < lastOffsetInNode(endNode))
+        if (offsetIsBeforeLastNodeOffset(endOffset, endNode))
             return false;
 
         unsigned parentLastOffset = end.deprecatedNode()->parentNode()->childNodes()->length() - 1;