TextIteratorAlgorithm should not force layout update in constructor.

TextIteratorAlgorithm should not force layout update in constructor.

Asking to update layout should not be done at this level -- updating layout is part of the document/view lifecycle and should be coordinated at a higher level than an iterator constructor.

Thus, move calling Document::updateLayoutIgnorePendingStylesheets up to the callsites of the TextIterator constructor. These callsites should be audited and moved up even farther up the call stack.

BUG=590295
Committed: https://crrev.com/4ed5e3b54598950731022926b09fdc8742abea74
Cr-Commit-Position: refs/heads/master@{#379205}

[dglazkov, 2016-02-26 22:22:39]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-02-26 22:23:15]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1740173003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1740173003/1

[commit-bot: I haz the power, 2016-02-26 22:49:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-26 22:49:32]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[dglazkov, 2016-02-27 00:50:30]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[dglazkov, 2016-02-27 00:50:35]

Added some explicit layout calls.

[commit-bot: I haz the power, 2016-02-27 00:53:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1740173003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1740173003/20001

[commit-bot: I haz the power, 2016-02-27 01:21:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-27 01:21:01]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[dglazkov, 2016-02-27 04:52:44]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[dglazkov, 2016-02-27 04:52:45]

Fixed content_browsertests compile.

[commit-bot: I haz the power, 2016-02-27 04:53:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1740173003/30001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1740173003/30001

[commit-bot: I haz the power, 2016-02-27 05:38:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-27 05:38:36]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dglazkov, 2016-02-28 05:29:11]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[dglazkov, 2016-02-28 05:29:13]

Oops, missed PlainTextRange::create.

[commit-bot: I haz the power, 2016-02-28 05:29:23]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1740173003/50001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1740173003/50001

[commit-bot: I haz the power, 2016-02-28 06:57:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-28 06:57:13]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[dglazkov, 2016-02-28 16:48:42]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[dglazkov, 2016-02-28 16:48:44]

Ah, needed to go deeper.

[commit-bot: I haz the power, 2016-02-28 16:48:55]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1740173003/70001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1740173003/70001

[commit-bot: I haz the power, 2016-02-28 18:07:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-28 18:07:20]

Dry run: This issue passed the CQ dry run.

[dglazkov, 2016-02-29 00:23:33]

Description was changed from

==========
TextIteratorAlgorithm should not force layout update in constructor.

WIP

BUG=590295
==========

to

==========
TextIteratorAlgorithm should not force layout update in constructor.

Asking to update layout should not be done at this level -- updating layout is
part of the document/view lifecycle and should be coordinated at a higher level
than an iterator constructor.

Thus, move calling Document::updateLayoutIgnorePendingStylesheets up to the
callsites of the TextIterator constructor. These callsites should be audited and
moved up even farther up the call stack.

BUG=590295
==========

[dglazkov, 2016-03-01 05:32:53]

dglazkov@chromium.org changed reviewers:
+ esprehn@chromium.org, yosin@chromium.org

[dglazkov, 2016-03-01 05:33:03]

PTAL

[yosin_UTC9, 2016-03-01 06:17:38]

lgtm

Sorry, editing is the biggest source of updatingLayoutTree().

[esprehn, 2016-03-01 06:34:46]

https://codereview.chromium.org/1740173003/diff/70001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp (right):

https://codereview.chromium.org/1740173003/diff/70001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp:149:
ASSERT(!start.document()->needsLayoutTreeUpdate());
Sadly due to bad naming checking this isn't enough, you need to also check
needsLayout()

LayoutTreeUpdate is style.

This a mess from renaming rendering to layout.

I'm not sure if there's a single bit to check if anything is dirty? This code
needs love.

[dglazkov, 2016-03-03 04:26:11]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[dglazkov, 2016-03-03 04:26:13]

Added FrameView ASSERT.

[commit-bot: I haz the power, 2016-03-03 04:26:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1740173003/90001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1740173003/90001

[commit-bot: I haz the power, 2016-03-03 05:56:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-03 05:56:47]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dglazkov, 2016-03-04 01:05:53]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[dglazkov, 2016-03-04 01:05:55]

Tweaked ASSERT, filed a bug.

[commit-bot: I haz the power, 2016-03-04 01:06:24]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1740173003/110001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1740173003/110001

[dglazkov, 2016-03-04 01:28:23]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[dglazkov, 2016-03-04 01:28:24]

Removed my WIP comment.

[commit-bot: I haz the power, 2016-03-04 01:28:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1740173003/130001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1740173003/130001

[commit-bot: I haz the power, 2016-03-04 03:03:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-04 03:03:32]

Dry run: This issue passed the CQ dry run.

[esprehn, 2016-03-04 03:07:56]

lgtm

[dglazkov, 2016-03-04 04:08:12]

The CQ bit was checked by dglazkov@chromium.org

[dglazkov, 2016-03-04 04:08:12]

The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/1740173003/#ps130001
(title: "Removed my WIP comment.")

[commit-bot: I haz the power, 2016-03-04 04:08:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1740173003/130001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1740173003/130001

[commit-bot: I haz the power, 2016-03-04 04:14:36]

Committed patchset #8 (id:130001)

[commit-bot: I haz the power, 2016-03-04 04:16:10]

Description was changed from

==========
TextIteratorAlgorithm should not force layout update in constructor.

Asking to update layout should not be done at this level -- updating layout is
part of the document/view lifecycle and should be coordinated at a higher level
than an iterator constructor.

Thus, move calling Document::updateLayoutIgnorePendingStylesheets up to the
callsites of the TextIterator constructor. These callsites should be audited and
moved up even farther up the call stack.

BUG=590295
==========

to

==========
TextIteratorAlgorithm should not force layout update in constructor.

Asking to update layout should not be done at this level -- updating layout is
part of the document/view lifecycle and should be coordinated at a higher level
than an iterator constructor.

Thus, move calling Document::updateLayoutIgnorePendingStylesheets up to the
callsites of the TextIterator constructor. These callsites should be audited and
moved up even farther up the call stack.

BUG=590295

Committed: https://crrev.com/4ed5e3b54598950731022926b09fdc8742abea74
Cr-Commit-Position: refs/heads/master@{#379205}
==========

[commit-bot: I haz the power, 2016-03-04 04:16:11]

Patchset 8 (id:??) landed as
https://crrev.com/4ed5e3b54598950731022926b09fdc8742abea74
Cr-Commit-Position: refs/heads/master@{#379205}

[Marc Treib, 2016-03-04 10:49:04]

A revert of this CL (patchset #8 id:130001) has been created in
https://codereview.chromium.org/1762993002/ by treib@chromium.org.

The reason for reverting is: Suspected of breaking
RenderViewImplTest.OnHandleKeyboardEvent on
https://build.chromium.org/p/chromium.win/builders/Win%20x64%20GN%20(dbg)

First failing build:
https://build.chromium.org/p/chromium.win/builders/Win%20x64%20GN%20%28dbg%29....

[dglazkov, 2016-03-04 19:48:43]

On 2016/03/04 at 10:49:04, treib wrote:
> A revert of this CL (patchset #8 id:130001) has been created in
https://codereview.chromium.org/1762993002/ by treib@chromium.org.
> 
> The reason for reverting is: Suspected of breaking
RenderViewImplTest.OnHandleKeyboardEvent on
https://build.chromium.org/p/chromium.win/builders/Win%20x64%20GN%20(dbg)
> 
> First failing build:
>
https://build.chromium.org/p/chromium.win/builders/Win%20x64%20GN%20%28dbg%29....

Thank you for reverting, treib!