Rewrite LayoutNG margin collapsing/floats unit tests

Rewrite LayoutNG margin collapsing/floats unit tests

This patch rewrites PositionFloatFragments, CollapsingMarginsCase2WithFloats,
CollapsingMarginsCase1WithFloats and PositionFloatInsideEmptyBlocks(new).

Also it makes NGBlockLayoutAlgorithmTest to be based on RenderingTest. That
allows to use setBodyInnerHTML to set HTML representation of tests and enables
unittests to access old layout objects like FloatingObjects etc.

The tests that are being changed here are temporarily marked as DISABLED.
That's because the old tests are not fully correct. For the new float/margins
collapsing algorithm these new tests will set correct expectations and will
be enabled once the new code checked in.

BUG=635619
Review-Url: https://codereview.chromium.org/2641253002
Cr-Commit-Position: refs/heads/master@{#445900}
Committed: https://chromium.googlesource.com/chromium/src/+/59c1ac58a65bb815893621feb68fe228e051a2b4

[Gleb Lanbin, 2017-01-19 22:21:24]

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

[Gleb Lanbin, 2017-01-19 22:21:44]

On 2017/01/19 22:21:25, Gleb Lanbin wrote:

this patch is based on http://crrev.com/2642823008(on review).

[Gleb Lanbin, 2017-01-19 22:31:05]

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

[commit-bot: I haz the power, 2017-01-19 22:31:42]

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-01-19 22:39:27]

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

[commit-bot: I haz the power, 2017-01-19 22:39:28]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[ikilpatrick, 2017-01-23 23:26:51]

tests look good, can i have one last look after you rebase?

https://codereview.chromium.org/2641253002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc
(right):

https://codereview.chromium.org/2641253002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc:201:
"    height: 200px;"
remove.

https://codereview.chromium.org/2641253002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc:243:
int float_child_left_block_offset = first_child_block_offset + 10;
is this wrong? it's position is based for containers padding, and its margins.

[Gleb Lanbin, 2017-01-24 19:31:16]

PTAL

https://codereview.chromium.org/2641253002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc
(right):

https://codereview.chromium.org/2641253002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc:201:
"    height: 200px;"
On 2017/01/23 23:26:50, ikilpatrick wrote:
> remove.

Done.

https://codereview.chromium.org/2641253002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc:243:
int float_child_left_block_offset = first_child_block_offset + 10;
On 2017/01/23 23:26:50, ikilpatrick wrote:
> is this wrong? it's position is based for containers padding, and its margins.

I think, it's correct. Here is the layout tree representation produced by legacy
code
layer at (0,0) size 800x228
  LayoutBlockFlow {HTML} at (0,0) size 800x228
    LayoutBlockFlow {BODY} at (8,20) size 784x200
      LayoutBlockFlow {DIV} at (0,0) size 214x200 [bgcolor=#FF0000]
        LayoutBlockFlow (floating) {DIV} at (17,10) size 30x30 [bgcolor=#008000]
        LayoutBlockFlow (floating) {DIV} at (177,0) size 30x30 [bgcolor=#FFC0CB]
        LayoutBlockFlow {DIV} at (7,0) size 200x10 [bgcolor=#0000FF]

you can see that the first floating(#float-child-left) is positioned at 
body's offset 20 + float-child-left's margin 10.

[Gleb Lanbin, 2017-01-24 21:41:55]

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

[commit-bot: I haz the power, 2017-01-24 21:42: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, 2017-01-24 21:46:54]

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

[commit-bot: I haz the power, 2017-01-24 21:46:54]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Gleb Lanbin, 2017-01-24 21:50:05]

Patchset #3 (id:40001) has been deleted

[Gleb Lanbin, 2017-01-24 21:50:26]

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

[commit-bot: I haz the power, 2017-01-24 21:50:51]

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

[ikilpatrick, 2017-01-24 22:05:37]

lgtm

https://codereview.chromium.org/2641253002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc
(right):

https://codereview.chromium.org/2641253002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc:257:
// The left float to be positioned at first-child's top.
So I was more pushing back on the wording here, it's more positioned at the
containers padding box. If that makes sense?

(e.g. the position of the float doesn't depend on the first child).

https://codereview.chromium.org/2641253002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc:263:
// The right float to be positioned at first-child's top.
and here.

[Gleb Lanbin, 2017-01-24 22:30:26]

Patchset #4 (id:80001) has been deleted

[Gleb Lanbin, 2017-01-24 22:30:32]

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

[Gleb Lanbin, 2017-01-24 22:30:43]

https://codereview.chromium.org/2641253002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc
(right):

https://codereview.chromium.org/2641253002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc:257:
// The left float to be positioned at first-child's top.
On 2017/01/24 22:05:37, ikilpatrick wrote:
> So I was more pushing back on the wording here, it's more positioned at the
> containers padding box. If that makes sense?
> 
> (e.g. the position of the float doesn't depend on the first child).

Done.

https://codereview.chromium.org/2641253002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc:263:
// The right float to be positioned at first-child's top.
On 2017/01/24 22:05:37, ikilpatrick wrote:
> and here.

Done.

[commit-bot: I haz the power, 2017-01-24 22:31:25]

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-01-25 00:12:30]

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

[commit-bot: I haz the power, 2017-01-25 00:12:31]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Gleb Lanbin, 2017-01-25 00:17:18]

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

[commit-bot: I haz the power, 2017-01-25 00:18:01]

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-01-25 01:04:22]

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

[commit-bot: I haz the power, 2017-01-25 01:04:23]

Dry run: This issue passed the CQ dry run.

[Gleb Lanbin, 2017-01-25 01:05:14]

The CQ bit was checked by glebl@chromium.org

[Gleb Lanbin, 2017-01-25 01:05:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
ikilpatrick@chromium.org
Link to the patchset: https://codereview.chromium.org/2641253002/#ps100001
(title: "fix comments")

[commit-bot: I haz the power, 2017-01-25 01:06:23]

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-01-25 01:11:30]

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

[commit-bot: I haz the power, 2017-01-25 01:11:32]

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-01-25 01:22:04]

The CQ bit was checked by glebl@chromium.org

[commit-bot: I haz the power, 2017-01-25 01:22:32]

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-01-25 01:28:24]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1485307324905650,
"parent_rev": "9c67bca9e430b4f5b4f9810d9fa414dbe90dab3e", "commit_rev":
"59c1ac58a65bb815893621feb68fe228e051a2b4"}

[commit-bot: I haz the power, 2017-01-25 01:29:20]

Description was changed from

==========
Rewrite LayoutNG margin collapsing/floats unit tests

This patch rewrites PositionFloatFragments, CollapsingMarginsCase2WithFloats,
CollapsingMarginsCase1WithFloats and PositionFloatInsideEmptyBlocks(new).

Also it makes NGBlockLayoutAlgorithmTest to be based on RenderingTest. That
allows to use setBodyInnerHTML to set HTML representation of tests and enables
unittests to access old layout objects like FloatingObjects etc.

The tests that are being changed here are temporarily marked as DISABLED.
That's because the old tests are not fully correct. For the new float/margins
collapsing algorithm these new tests will set correct expectations and will
be enabled once the new code checked in.

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

to

==========
Rewrite LayoutNG margin collapsing/floats unit tests

This patch rewrites PositionFloatFragments, CollapsingMarginsCase2WithFloats,
CollapsingMarginsCase1WithFloats and PositionFloatInsideEmptyBlocks(new).

Also it makes NGBlockLayoutAlgorithmTest to be based on RenderingTest. That
allows to use setBodyInnerHTML to set HTML representation of tests and enables
unittests to access old layout objects like FloatingObjects etc.

The tests that are being changed here are temporarily marked as DISABLED.
That's because the old tests are not fully correct. For the new float/margins
collapsing algorithm these new tests will set correct expectations and will
be enabled once the new code checked in.

BUG=635619

Review-Url: https://codereview.chromium.org/2641253002
Cr-Commit-Position: refs/heads/master@{#445900}
Committed:
https://chromium.googlesource.com/chromium/src/+/59c1ac58a65bb815893621feb68f...
==========

[commit-bot: I haz the power, 2017-01-25 01:29:21]

Committed patchset #4 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/59c1ac58a65bb815893621feb68f...