Move NG floats utility methods to the separate file

Move NG floats utility methods to the separate file

These methods will be used by text/block layout algorithms.

BUG=635619
Review-Url: https://codereview.chromium.org/2757603007
Cr-Commit-Position: refs/heads/master@{#458145}
Committed: https://chromium.googlesource.com/chromium/src/+/8b17da783915297bb8a3a46a6484b8787c7db28d

[Gleb Lanbin, 2017-03-17 21:01:06]

glebl@chromium.org changed reviewers:
+ ikilpatrick@chromium.org

[Gleb Lanbin, 2017-03-17 21:01:11]

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

[commit-bot: I haz the power, 2017-03-17 21:01:48]

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

[ikilpatrick, 2017-03-17 21:25:24]

lgtm

It also may pay to get fragmentation to work before inline, as this will change
this PositionFloat and NGFloatingObjects a bit.

I.e.  NGFloatingObjects wont be able to have a fragment associated with it, as
it'll need to take into account the fragmentation line.... we'll need to
position the float before we are able to perform layout.

The NGSizingSpace will be useful here as NGFloatingObject should just become {
  const RefPtr<const NGSizingSpace> sizing_space; // instead of space.
  NGBoxStrut margins;
  LayoutUnit static_bfc_inline_offset; // instead of space.
  Layoutunit static_parent_bfc_inline_offset; // instead of
original_parent_space.   
}

... and instead of storing the fragment, and left_offset here, it should be
enough to have a special NGPhysicalFloatBoxFragment which has the additional
left_offset field?

(this should also remove side-effects from the float code).

https://codereview.chromium.org/2757603007/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/ng/ng_floating_object.h (right):

https://codereview.chromium.org/2757603007/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/ng/ng_floating_object.h:53: String
ToString() const {
I know we haven't done this previously but we should probably surround all of
out ToString methods with
#ifndef NDEBUG

[Gleb Lanbin, 2017-03-17 21:37:12]

On 2017/03/17 21:25:24, ikilpatrick wrote:
> lgtm
> 
> It also may pay to get fragmentation to work before inline, as this will
change
> this PositionFloat and NGFloatingObjects a bit.
> 
> I.e.  NGFloatingObjects wont be able to have a fragment associated with it, as
> it'll need to take into account the fragmentation line.... we'll need to
> position the float before we are able to perform layout.
> 
> The NGSizingSpace will be useful here as NGFloatingObject should just become {
>   const RefPtr<const NGSizingSpace> sizing_space; // instead of space.
>   NGBoxStrut margins;
>   LayoutUnit static_bfc_inline_offset; // instead of space.
>   Layoutunit static_parent_bfc_inline_offset; // instead of
> original_parent_space.   
> }
> 
> ... and instead of storing the fragment, and left_offset here, it should be
> enough to have a special NGPhysicalFloatBoxFragment which has the additional
> left_offset field?
> 
> (this should also remove side-effects from the float code).

ack, I can work on this next.

> 
>
https://codereview.chromium.org/2757603007/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/layout/ng/ng_floating_object.h (right):
> 
>
https://codereview.chromium.org/2757603007/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/layout/ng/ng_floating_object.h:53: String
> ToString() const {
> I know we haven't done this previously but we should probably surround all of
> out ToString methods with
> #ifndef NDEBUG

ack, good point. let me prepare a follow up patch for this.

[commit-bot: I haz the power, 2017-03-17 22:30:44]

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

[commit-bot: I haz the power, 2017-03-17 22:30:45]

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

[Gleb Lanbin, 2017-03-17 22:31:06]

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

[commit-bot: I haz the power, 2017-03-17 22:31:48]

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, 2017-03-17 23:13:47]

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

[commit-bot: I haz the power, 2017-03-17 23:13:48]

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

[Gleb Lanbin, 2017-03-17 23:14:34]

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

[commit-bot: I haz the power, 2017-03-17 23:15:43]

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, 2017-03-18 01:13:42]

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

[commit-bot: I haz the power, 2017-03-18 01:13:43]

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_...)

[Gleb Lanbin, 2017-03-18 02:16:20]

The CQ bit was checked by glebl@chromium.org

[commit-bot: I haz the power, 2017-03-18 02:16:47]

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, 2017-03-18 02:25:58]

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

[commit-bot: I haz the power, 2017-03-18 02:25:59]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Gleb Lanbin, 2017-03-18 05:21:34]

glebl@chromium.org changed reviewers:
+ cbiesinger@chromium.org

[Gleb Lanbin, 2017-03-18 05:21:35]

+cbiesinger for BUILD.gn

[cbiesinger, 2017-03-20 16:10:10]

lgtm

[Gleb Lanbin, 2017-03-20 16:10:24]

The CQ bit was checked by glebl@chromium.org

[commit-bot: I haz the power, 2017-03-20 16:10:40]

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, 2017-03-20 19:14:08]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1490026224208140, "parent_rev":
"0df6b55feadf2848cc418470196ced35515b19d1", "commit_rev":
"8b17da783915297bb8a3a46a6484b8787c7db28d"}

[commit-bot: I haz the power, 2017-03-20 19:14:48]

Description was changed from

==========
Move NG floats utility methods to the separate file

These methods will be used by text/block layout algorithms.

BUG=635619
==========

to

==========
Move NG floats utility methods to the separate file

These methods will be used by text/block layout algorithms.

BUG=635619

Review-Url: https://codereview.chromium.org/2757603007
Cr-Commit-Position: refs/heads/master@{#458145}
Committed:
https://chromium.googlesource.com/chromium/src/+/8b17da783915297bb8a3a46a6484...
==========

[commit-bot: I haz the power, 2017-03-20 19:14:49]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/8b17da783915297bb8a3a46a6484...