Use the sibling limit to decide whether to create layout for whitespace

Use the sibling limit to decide whether to create layout for whitespace

Text::textLayoutObjectIsNeeded is an optimization which avoids
creating layout objects for whitespace text nodes. However
LayoutTreeBuilderTraversal::nextSiblingLayoutObject traverses all
siblings. For long lists of N space-separated siblings, this
degenerates into an N^2 DOM walk. Oops.

Text::textLayoutObjectIsNeeded already has a sibling limit; this
passes the limit through to
prevSiblingLayoutObject/nextSiblingLayoutObject.

BUG=650938
Committed: https://crrev.com/99c2073218960208c0f54de95b9c31ff9521ba46
Cr-Commit-Position: refs/heads/master@{#422631}

[dominicc (has gone to gerrit), 2016-09-28 05:24:07]

dominicc@chromium.org changed reviewers:
+ kojii@chromium.org

[dominicc (has gone to gerrit), 2016-09-28 05:24:07]

PTAL

[dominicc (has gone to gerrit), 2016-09-28 06:33:17]

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

[commit-bot: I haz the power, 2016-09-28 06:33:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

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

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

[commit-bot: I haz the power, 2016-09-28 07:02:21]

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

[dominicc (has gone to gerrit), 2016-09-29 01:25:48]

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

[commit-bot: I haz the power, 2016-09-29 01:26:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-29 02:49:23]

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

[commit-bot: I haz the power, 2016-09-29 02:49:23]

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

[kojii, 2016-09-29 04:52:04]

The code looks good to me but there are a few cases where limit == -1 from the
test failures?

[dominicc (has gone to gerrit), 2016-09-29 05:35:59]

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

[commit-bot: I haz the power, 2016-09-29 05:36:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-29 06:47:32]

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

[commit-bot: I haz the power, 2016-09-29 06:47:33]

Dry run: This issue passed the CQ dry run.

[dominicc (has gone to gerrit), 2016-09-29 07:31:30]

Good point. The caller underflows the unsigned limit. PTAL.

[kojii, 2016-09-29 08:23:19]

lgtm

[dominicc (has gone to gerrit), 2016-10-03 05:51:16]

The CQ bit was checked by dominicc@chromium.org

[commit-bot: I haz the power, 2016-10-03 05:51:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-03 05:53:46]

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

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

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[dominicc (has gone to gerrit), 2016-10-03 08:14:58]

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

[commit-bot: I haz the power, 2016-10-03 08:15:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-03 09:35:47]

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

[commit-bot: I haz the power, 2016-10-03 09:35:48]

Dry run: This issue passed the CQ dry run.

[dominicc (has gone to gerrit), 2016-10-04 00:24:16]

The CQ bit was checked by dominicc@chromium.org

[dominicc (has gone to gerrit), 2016-10-04 00:24:17]

The patchset sent to the CQ was uploaded after l-g-t-m from kojii@chromium.org
Link to the patchset: https://codereview.chromium.org/2379483002/#ps60001
(title: "Bring patch to head.
")

[commit-bot: I haz the power, 2016-10-04 00:25:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-04 00:35:41]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-04 00:39:35]

Description was changed from

==========
Use the sibling limit to decide whether to create layout for whitespace

Text::textLayoutObjectIsNeeded is an optimization which avoids
creating layout objects for whitespace text nodes. However
LayoutTreeBuilderTraversal::nextSiblingLayoutObject traverses all
siblings. For long lists of N space-separated siblings, this
degenerates into an N^2 DOM walk. Oops.

Text::textLayoutObjectIsNeeded already has a sibling limit; this
passes the limit through to
prevSiblingLayoutObject/nextSiblingLayoutObject.

BUG=650938
==========

to

==========
Use the sibling limit to decide whether to create layout for whitespace

Text::textLayoutObjectIsNeeded is an optimization which avoids
creating layout objects for whitespace text nodes. However
LayoutTreeBuilderTraversal::nextSiblingLayoutObject traverses all
siblings. For long lists of N space-separated siblings, this
degenerates into an N^2 DOM walk. Oops.

Text::textLayoutObjectIsNeeded already has a sibling limit; this
passes the limit through to
prevSiblingLayoutObject/nextSiblingLayoutObject.

BUG=650938

Committed: https://crrev.com/99c2073218960208c0f54de95b9c31ff9521ba46
Cr-Commit-Position: refs/heads/master@{#422631}
==========

[commit-bot: I haz the power, 2016-10-04 00:39:36]

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

[Nico, 2016-10-06 15:13:44]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2016-10-06 15:13:45]

https://codereview.chromium.org/2379483002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Text.cpp (right):

https://codereview.chromium.org/2379483002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Text.cpp:311: unsigned maxSiblingsToVisit =
50;
did you mean to delete this here? (i'll send a cl to do so, unless you tell me
it's intentionally in two places now)

[kojii, 2016-10-07 03:22:54]

https://codereview.chromium.org/2379483002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Text.cpp (right):

https://codereview.chromium.org/2379483002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Text.cpp:311: unsigned maxSiblingsToVisit =
50;
On 2016/10/06 at 15:13:45, Nico wrote:
> did you mean to delete this here? (i'll send a cl to do so, unless you tell me
it's intentionally in two places now)

Looks like a merge failure, PS3 removed this but not in PS4.

[dominicc (has gone to gerrit), 2016-10-14 07:51:33]

On 2016/10/07 at 03:22:54, kojii wrote:
>
https://codereview.chromium.org/2379483002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/dom/Text.cpp (right):
> 
>
https://codereview.chromium.org/2379483002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/dom/Text.cpp:311: unsigned maxSiblingsToVisit =
50;
> On 2016/10/06 at 15:13:45, Nico wrote:
> > did you mean to delete this here? (i'll send a cl to do so, unless you tell
me it's intentionally in two places now)
> 
> Looks like a merge failure, PS3 removed this but not in PS4.

Yes, merge failure. Thank you for cleaning this up jochen.