Typing is slow in Gmail on iPads

https://bugs.webkit.org/show_bug.cgi?id=145686

Reviewed by Enrica Casucci.

The bug was caused by nextCandidate and nextVisuallyDistinctCandidate traversing through each character
in a text node without a renderer. Skip any node that doesn't have a renderer in both of those functions
and corresponding previous* functions.

It's fine to skip unrendered nodes in PositionIterator because only other clients of PositionIterator
are Position::upstream and Position::downstream and they don't care about un-rendered nodes either.

* dom/PositionIterator.cpp:
(WebCore::PositionIterator::increment):
(WebCore::PositionIterator::decrement):
* editing/htmlediting.cpp:
(WebCore::nextVisuallyDistinctCandidate):
(WebCore::previousVisuallyDistinctCandidate):

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 77ddc8290ad1bc9e6c11db75b133cfa9838c85de..53d36acae75cb8ef70cd7c886f3a12b45cc9e754 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,24 @@
+2015-06-06  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Typing is slow in Gmail on iPads
+        https://bugs.webkit.org/show_bug.cgi?id=145686
+
+        Reviewed by Enrica Casucci.
+
+        The bug was caused by nextCandidate and nextVisuallyDistinctCandidate traversing through each character
+        in a text node without a renderer. Skip any node that doesn't have a renderer in both of those functions
+        and corresponding previous* functions.
+
+        It's fine to skip unrendered nodes in PositionIterator because only other clients of PositionIterator
+        are Position::upstream and Position::downstream and they don't care about un-rendered nodes either.
+
+        * dom/PositionIterator.cpp:
+        (WebCore::PositionIterator::increment):
+        (WebCore::PositionIterator::decrement):
+        * editing/htmlediting.cpp:
+        (WebCore::nextVisuallyDistinctCandidate):
+        (WebCore::previousVisuallyDistinctCandidate):
+
 2015-06-06  Mark Lam  <mark.lam@apple.com>
 
         Returned Exception* values need to be initialized to nullptr when no exceptions are thrown.

diff --git a/Source/WebCore/dom/PositionIterator.cpp b/Source/WebCore/dom/PositionIterator.cpp
index 1f009670f2acdc9aa771368415751cad75852265..4551397ac247bacdc75ff7f5db8509429dd0ca37 100644 (file)
--- a/Source/WebCore/dom/PositionIterator.cpp
+++ b/Source/WebCore/dom/PositionIterator.cpp
@@ -66,7 +66,7 @@ void PositionIterator::increment()
         return;
     }
 
-    if (!m_anchorNode->hasChildNodes() && m_offsetInAnchor < lastOffsetForEditing(m_anchorNode))
+    if (m_anchorNode->renderer() && !m_anchorNode->hasChildNodes() && m_offsetInAnchor < lastOffsetForEditing(m_anchorNode))
         m_offsetInAnchor = Position::uncheckedNextOffset(m_anchorNode, m_offsetInAnchor);
     else {
         m_nodeAfterPositionInAnchor = m_anchorNode;
@@ -98,7 +98,7 @@ void PositionIterator::decrement()
         m_anchorNode = m_anchorNode->lastChild();
         m_offsetInAnchor = m_anchorNode->hasChildNodes()? 0: lastOffsetForEditing(m_anchorNode);
     } else {
-        if (m_offsetInAnchor)
+        if (m_offsetInAnchor && m_anchorNode->renderer())
             m_offsetInAnchor = Position::uncheckedPreviousOffset(m_anchorNode, m_offsetInAnchor);
         else {
             m_nodeAfterPositionInAnchor = m_anchorNode;

diff --git a/Source/WebCore/editing/htmlediting.cpp b/Source/WebCore/editing/htmlediting.cpp
index 31febfb8a274a724373c8b3627248f8ba0538ee1..8f04d081352b963256fbad757fe135e6dcd3d750 100644 (file)
--- a/Source/WebCore/editing/htmlediting.cpp
+++ b/Source/WebCore/editing/htmlediting.cpp
@@ -242,12 +242,17 @@ Position nextCandidate(const Position& position)
 
 Position nextVisuallyDistinctCandidate(const Position& position)
 {
+    // FIXME: Use PositionIterator instead.
     Position p = position;
     Position downstreamStart = p.downstream();
     while (!p.atEndOfTree()) {
         p = p.next(Character);
         if (p.isCandidate() && p.downstream() != downstreamStart)
             return p;
+        if (auto* node = p.containerNode()) {
+            if (!node->renderer())
+                p = lastPositionInOrAfterNode(node);
+        }
     }
     return Position();
 }
@@ -265,12 +270,17 @@ Position previousCandidate(const Position& position)
 
 Position previousVisuallyDistinctCandidate(const Position& position)
 {
+    // FIXME: Use PositionIterator instead.
     Position p = position;
     Position downstreamStart = p.downstream();
     while (!p.atStartOfTree()) {
         p = p.previous(Character);
         if (p.isCandidate() && p.downstream() != downstreamStart)
             return p;
+        if (auto* node = p.containerNode()) {
+            if (!node->renderer())
+                p = firstPositionInOrBeforeNode(node);
+        }
     }
     return Position();
 }