Internals should always cause a layout before calling into TextIterator

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

Reviewed by Antti Koivisto.

Source/WebCore:

Inspired by https://chromium.googlesource.com/chromium/blink/+/5fee5da7b04a710171c79bd6e87eca3533188e45.

Force a layout in the constructors of TextIterator, and SimplifiedBackwardsTextIterator and remove
superfluous calls to updateLayout() in other places.

As much as I hate for a constructor to have a side effect like this, I couldn't think of a better place
to update the layout. Unfortunately, we're slowly moving away from manually createing TextIterator and
wrapping them in a static function.

* editing/TextIterator.cpp:
(WebCore::TextIterator::TextIterator):
(WebCore::SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator):
(WebCore::TextIterator::rangeFromLocationAndLength):
(WebCore::findPlainText):

LayoutTests:

Progression.

* platform/mac/editing/input/caret-primary-bidi-expected.txt:

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 5bcada380729d3df3817b0aafb4da976f9b1e1d0..7bc8e17160300b5c59c018b976d42d236da894ee 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2013-09-06  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Internals should always cause a layout before calling into TextIterator
+        https://bugs.webkit.org/show_bug.cgi?id=120891
+
+        Reviewed by Antti Koivisto.
+
+        Progression.
+
+        * platform/mac/editing/input/caret-primary-bidi-expected.txt:
+
 2013-09-09  Mark Lam  <mark.lam@apple.com>
 
         Change some remaining fast/* files to use pre and post js files in LayoutTests/resources.

diff --git a/LayoutTests/platform/mac/editing/input/caret-primary-bidi-expected.txt b/LayoutTests/platform/mac/editing/input/caret-primary-bidi-expected.txt
index 40722f5cba03f01352a3a38b12239c34c0f49be3..caab34f81ae9be1d397516c47d4d30855eba5d81 100644 (file)
--- a/LayoutTests/platform/mac/editing/input/caret-primary-bidi-expected.txt
+++ b/LayoutTests/platform/mac/editing/input/caret-primary-bidi-expected.txt
@@ -1,4 +1,4 @@
-0: 124,508,0,28
+0: 8,564,0,28
 1: 21,564,0,28
 2: 36,564,0,28
 3: 48,564,0,28

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 36df841931a660bbb74bf4e7c0b8967b73cb3109..d68f3881db01c00c9dfd108eaf7a5eea36f56177 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2013-09-06  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Internals should always cause a layout before calling into TextIterator
+        https://bugs.webkit.org/show_bug.cgi?id=120891
+
+        Reviewed by Antti Koivisto.
+
+        Inspired by https://chromium.googlesource.com/chromium/blink/+/5fee5da7b04a710171c79bd6e87eca3533188e45.
+
+        Force a layout in the constructors of TextIterator, and SimplifiedBackwardsTextIterator and remove
+        superfluous calls to updateLayout() in other places.
+
+        As much as I hate for a constructor to have a side effect like this, I couldn't think of a better place
+        to update the layout. Unfortunately, we're slowly moving away from manually createing TextIterator and
+        wrapping them in a static function.
+
+        * editing/TextIterator.cpp:
+        (WebCore::TextIterator::TextIterator):
+        (WebCore::SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator):
+        (WebCore::TextIterator::rangeFromLocationAndLength):
+        (WebCore::findPlainText):
+
 2013-09-09  David Hyatt  <hyatt@apple.com>
 
         Move layoutBlock and layoutBlockChildren into RenderBlockFlow

diff --git a/Source/WebCore/editing/TextIterator.cpp b/Source/WebCore/editing/TextIterator.cpp
index 83c3adbb1309ba3384ac066a53906032b93a3d85..9db530ffc20be940d9f9ccfa903cc67c6f4e7699 100644 (file)
--- a/Source/WebCore/editing/TextIterator.cpp
+++ b/Source/WebCore/editing/TextIterator.cpp
@@ -293,6 +293,8 @@ TextIterator::TextIterator(const Range* r, TextIteratorBehavior behavior)
     if (!r)
         return;
 
+    r->ownerDocument().updateLayoutIgnorePendingStylesheets();
+
     // get and validate the range endpoints
     Node* startContainer = r->startContainer();
     if (!startContainer)
@@ -1118,6 +1120,8 @@ SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator(const Range* r,
     if (!r)
         return;
 
+    r->ownerDocument().updateLayoutIgnorePendingStylesheets();
+
     Node* startNode = r->startContainer();
     if (!startNode)
         return;
@@ -2427,7 +2431,6 @@ PassRefPtr<Range> TextIterator::rangeFromLocationAndLength(ContainerNode* scope,
             // FIXME: This is a workaround for the fact that the end of a run is often at the wrong
             // position for emitted '\n's.
             if (len == 1 && it.characterAt(0) == '\n') {
-                scope->document().updateLayoutIgnorePendingStylesheets();
                 it.advance();
                 if (!it.atEnd()) {
                     RefPtr<Range> range = it.range();
@@ -2593,9 +2596,6 @@ tryAgain:
 
 PassRefPtr<Range> findPlainText(const Range* range, const String& target, FindOptions options)
 {
-    // CharacterIterator requires renderers to be up-to-date
-    range->ownerDocument().updateLayout();
-
     // First, find the text.
     size_t matchStart;
     size_t matchLength;