Changeset 155378 in webkit

Timestamp:

Sep 9, 2013, 1:28:39 PM (13 years ago)

Author:

rniwa@webkit.org

Message:

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:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac/editing/input/caret-primary-bidi-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/editing/TextIterator.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/platform/mac/editing/input/caret-primary-bidi-expected.txt

@@ -1 @@
-0: 124,508,0,28
+0: 8,564,0,28
 1: 21,564,0,28
 2: 36,564,0,28

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/editing/TextIterator.cpp

@@ -293 +293 @@
     if (!r)
         return;
+
+    r->ownerDocument().updateLayoutIgnorePendingStylesheets();
 
     // get and validate the range endpoints
@@ -1118 +1120 @@
     if (!r)
         return;
+
+    r->ownerDocument().updateLayoutIgnorePendingStylesheets();
 
     Node* startNode = r->startContainer();
@@ -2428 +2432 @@
             // position for emitted '\n's.
             if (len == 1 && it.characterAt(0) == '\n') {
-                scope->document().updateLayoutIgnorePendingStylesheets();
                 it.advance();
                 if (!it.atEnd()) {
@@ -2594 +2597 @@
 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;