Avoid unnecessary relayout of floats when not paginated.

Avoid unnecessary relayout of floats when not paginated.

Made a mistake when excluding floats from being considered for pagination
relayout skipping, by ALWAYS marking them for layout, EVEN WHEN NOT PAGINATED.
Make sure that we check that we're paginated first. No need to slow down layout
when not paginated.

Broke the logic for determining whether we need layout or not into a separate
method, so that we don't need a quarter of a dozen calls to
setChildNeedsLayout(). The logic is now reversed; rather than checking if we
don't need layout, we check if we DO need layout. Tried to make the code a bit
clearer, and document what goes on at each step.

Committed: https://crrev.com/fbebed78158693067e5bc3eeb90934f33590ef3b
Cr-Commit-Position: refs/heads/master@{#429051}

[mstensho (USE GERRIT), 2016-10-31 21:32:24]

The CQ bit was checked by mstensho@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-31 21:33:06]

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-31 23:00:43]

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

[commit-bot: I haz the power, 2016-10-31 23:00:44]

Dry run: This issue passed the CQ dry run.

[mstensho (USE GERRIT), 2016-10-31 23:11:30]

Description was changed from

==========
Avoid unnecessary relayout of floats when not paginated.

Made a mistake when excluding floats from being considered for pagination
relayout skipping, by ALWAYS marking them for layout, EVEN WHEN NOT PAGINATED.
Make sure that we check that we're paginated first. No need to slow down layout
when not paginated.

Broke the logic for determining whether we need layout or not into a separate
method, so that we don't need a quarter of a dozen calls to
setChildNeedsLayout().
==========

to

==========
Avoid unnecessary relayout of floats when not paginated.

Made a mistake when excluding floats from being considered for pagination
relayout skipping, by ALWAYS marking them for layout, EVEN WHEN NOT PAGINATED.
Make sure that we check that we're paginated first. No need to slow down layout
when not paginated.

Broke the logic for determining whether we need layout or not into a separate
method, so that we don't need a quarter of a dozen calls to
setChildNeedsLayout(). The logic is now reversed; rather than checking if we
don't need layout, we check if we DO need layout. Tried to make the code a bit
clearer, and document what goes on at each step.
==========

[mstensho (USE GERRIT), 2016-10-31 23:11:53]

mstensho@opera.com changed reviewers:
+ eae@chromium.org

[mstensho (USE GERRIT), 2016-10-31 23:11:53]

Follow-up to https://codereview.chromium.org/2462643002/

[eae, 2016-11-01 16:55:10]

https://codereview.chromium.org/2459293004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/2459293004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:4738: if
(child.offsetToNextPage() != remainingSpace)
Is checking the break offset sufficient here? Don't you need to compare the
start offset as well?

[mstensho (USE GERRIT), 2016-11-01 17:54:26]

https://codereview.chromium.org/2459293004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/2459293004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:4738: if
(child.offsetToNextPage() != remainingSpace)
On 2016/11/01 16:55:10, eae wrote:
> Is checking the break offset sufficient here? Don't you need to compare the
> start offset as well?

We no longer store the start offset, as of
https://codereview.chromium.org/2462643002/ . The break offset is all we need.
How far we are from the top of the page isn't really that interesting. What
matters is where we break.

[eae, 2016-11-01 17:57:39]

Ah, that makes sense. Thanks for the explanation.

LGTM

[mstensho (USE GERRIT), 2016-11-01 18:01:06]

The CQ bit was checked by mstensho@opera.com

[commit-bot: I haz the power, 2016-11-01 18:01:43]

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-11-01 18:07:09]

Description was changed from

==========
Avoid unnecessary relayout of floats when not paginated.

Made a mistake when excluding floats from being considered for pagination
relayout skipping, by ALWAYS marking them for layout, EVEN WHEN NOT PAGINATED.
Make sure that we check that we're paginated first. No need to slow down layout
when not paginated.

Broke the logic for determining whether we need layout or not into a separate
method, so that we don't need a quarter of a dozen calls to
setChildNeedsLayout(). The logic is now reversed; rather than checking if we
don't need layout, we check if we DO need layout. Tried to make the code a bit
clearer, and document what goes on at each step.
==========

to

==========
Avoid unnecessary relayout of floats when not paginated.

Made a mistake when excluding floats from being considered for pagination
relayout skipping, by ALWAYS marking them for layout, EVEN WHEN NOT PAGINATED.
Make sure that we check that we're paginated first. No need to slow down layout
when not paginated.

Broke the logic for determining whether we need layout or not into a separate
method, so that we don't need a quarter of a dozen calls to
setChildNeedsLayout(). The logic is now reversed; rather than checking if we
don't need layout, we check if we DO need layout. Tried to make the code a bit
clearer, and document what goes on at each step.
==========

[commit-bot: I haz the power, 2016-11-01 18:07:10]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-01 18:24:56]

Description was changed from

==========
Avoid unnecessary relayout of floats when not paginated.

Made a mistake when excluding floats from being considered for pagination
relayout skipping, by ALWAYS marking them for layout, EVEN WHEN NOT PAGINATED.
Make sure that we check that we're paginated first. No need to slow down layout
when not paginated.

Broke the logic for determining whether we need layout or not into a separate
method, so that we don't need a quarter of a dozen calls to
setChildNeedsLayout(). The logic is now reversed; rather than checking if we
don't need layout, we check if we DO need layout. Tried to make the code a bit
clearer, and document what goes on at each step.
==========

to

==========
Avoid unnecessary relayout of floats when not paginated.

Made a mistake when excluding floats from being considered for pagination
relayout skipping, by ALWAYS marking them for layout, EVEN WHEN NOT PAGINATED.
Make sure that we check that we're paginated first. No need to slow down layout
when not paginated.

Broke the logic for determining whether we need layout or not into a separate
method, so that we don't need a quarter of a dozen calls to
setChildNeedsLayout(). The logic is now reversed; rather than checking if we
don't need layout, we check if we DO need layout. Tried to make the code a bit
clearer, and document what goes on at each step.

Committed: https://crrev.com/fbebed78158693067e5bc3eeb90934f33590ef3b
Cr-Commit-Position: refs/heads/master@{#429051}
==========

[commit-bot: I haz the power, 2016-11-01 18:24:57]

Patchset 1 (id:??) landed as
https://crrev.com/fbebed78158693067e5bc3eeb90934f33590ef3b
Cr-Commit-Position: refs/heads/master@{#429051}