Fix incorrectly calculated size/position information for writing-modes

Fix incorrectly calculated size/position information for writing-modes

List of changes:
- Calculate margins before we switch writing modes for a child CS
- Collapse MarginStrut and update parent's BFC if we lay out the block
  that establishes a new formatting context.
- Rewrite NGBlockLayoutAlgorithmTest::CollapsingMarginsCase5 that
  verifies that we correctly collapses margins in different writing modes
- Set ShrinkToFit to child's CS if it's orthogonal to its parent.

BUG=635619
Review-Url: https://codereview.chromium.org/2651793013
Cr-Commit-Position: refs/heads/master@{#448626}
Committed: https://chromium.googlesource.com/chromium/src/+/4db6b79aab37bf3c687e067126241ab615d5c645

[Gleb Lanbin, 2017-01-28 00:53:47]

Patchset #1 (id:1) has been deleted

[Gleb Lanbin, 2017-01-28 00:54:12]

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

[Gleb Lanbin, 2017-01-30 20:57:37]

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

[cbiesinger, 2017-01-30 22:05:40]

Can you update the description? This part seems wrong now:
"  max_inline_size if writing_mode is vertical and size is not fixed."

It also does not mention the parent_writing_mode fix.

Otherwise, see comment below. I'll let Ian review the margin collapsing aspects.

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

https://codereview.chromium.org/2651793013/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc:40:
child_style.isFloating() || is_in_parallel_flow;
Shouldn't this be !is_in_parallel_flow?

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

Description was changed from

==========
Fix incorrectly calculated size/position information for writing-modes

List of changes:
- Calculate margins before we switch writing modes for a child CS
- Collapse MarginStrut and update parent's BFC if we lay out the block
  that establishes a new formatting context.
- Rewrite NGBlockLayoutAlgorithmTest::CollapsingMarginsCase5 that
  verifies that we correctly collapses margins in different writing modes
- Fix the size calculation for the case when we need to use
  max_inline_size if writing_mode is vertical and size is not fixed.

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

to

==========
Fix incorrectly calculated size/position information for writing-modes

List of changes:
- Calculate margins before we switch writing modes for a child CS
- Collapse MarginStrut and update parent's BFC if we lay out the block
  that establishes a new formatting context.
- Rewrite NGBlockLayoutAlgorithmTest::CollapsingMarginsCase5 that
  verifies that we correctly collapses margins in different writing modes
- Set ShrinkToFit to child's CS if it's orthogonal to its parent.

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

[Gleb Lanbin, 2017-01-30 22:11:12]

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

https://codereview.chromium.org/2651793013/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc:40:
child_style.isFloating() || is_in_parallel_flow;
On 2017/01/30 22:05:40, cbiesinger wrote:
> Shouldn't this be !is_in_parallel_flow?

Done.

[cbiesinger, 2017-01-30 22:13:38]

lgtm but please have Ian review the margin collapsing parts.

[ikilpatrick, 2017-01-31 21:34:43]

lgtm

https://codereview.chromium.org/2651793013/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc
(right):

https://codereview.chromium.org/2651793013/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc:708:
curr_child_margins_ = CalculateMargins(*space_builder_->ToConstraintSpace(),
This looks wrong as the space_builder_ writing mode could be set to something
orthoganal giving incorrect results. It was already wrong so maybe just a TODO.

[Gleb Lanbin, 2017-02-01 18:31:11]

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

[commit-bot: I haz the power, 2017-02-01 18:31:55]

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-02-01 19:56:14]

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

[commit-bot: I haz the power, 2017-02-01 19:56:14]

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

[Gleb Lanbin, 2017-02-06 20:32:42]

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

[commit-bot: I haz the power, 2017-02-06 20:33:20]

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-02-06 22:08:23]

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

[commit-bot: I haz the power, 2017-02-06 22:08:24]

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-02-07 00:21:21]

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

[commit-bot: I haz the power, 2017-02-07 00:22:27]

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-02-07 02:47:40]

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

[commit-bot: I haz the power, 2017-02-07 02:47:41]

Dry run: This issue passed the CQ dry run.

[Gleb Lanbin, 2017-02-07 05:21:06]

Patchset #7 (id:140001) has been deleted

[Gleb Lanbin, 2017-02-07 05:21:13]

Patchset #6 (id:120001) has been deleted

[Gleb Lanbin, 2017-02-07 05:21:21]

Patchset #5 (id:100001) has been deleted

[Gleb Lanbin, 2017-02-07 06:24:15]

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

[commit-bot: I haz the power, 2017-02-07 06:24:21]

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-02-07 06:26:38]

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

[commit-bot: I haz the power, 2017-02-07 06:26:39]

Dry run: 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...)

[Gleb Lanbin, 2017-02-07 13:42:43]

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

[commit-bot: I haz the power, 2017-02-07 13:42:58]

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-02-07 15:19:42]

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

[commit-bot: I haz the power, 2017-02-07 15:19:43]

Dry run: This issue passed the CQ dry run.

[Gleb Lanbin, 2017-02-07 15:23:14]

The CQ bit was checked by glebl@chromium.org

[Gleb Lanbin, 2017-02-07 15:23:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
cbiesinger@chromium.org, ikilpatrick@chromium.org
Link to the patchset: https://codereview.chromium.org/2651793013/#ps180001
(title: "git rebase-update")

[commit-bot: I haz the power, 2017-02-07 15:23:25]

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-02-07 15:27:27]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1486480994317050,
"parent_rev": "920b15e572ba9de0a3906187d91641882b73fecd", "commit_rev":
"4db6b79aab37bf3c687e067126241ab615d5c645"}

[commit-bot: I haz the power, 2017-02-07 15:28:08]

Description was changed from

==========
Fix incorrectly calculated size/position information for writing-modes

List of changes:
- Calculate margins before we switch writing modes for a child CS
- Collapse MarginStrut and update parent's BFC if we lay out the block
  that establishes a new formatting context.
- Rewrite NGBlockLayoutAlgorithmTest::CollapsingMarginsCase5 that
  verifies that we correctly collapses margins in different writing modes
- Set ShrinkToFit to child's CS if it's orthogonal to its parent.

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

to

==========
Fix incorrectly calculated size/position information for writing-modes

List of changes:
- Calculate margins before we switch writing modes for a child CS
- Collapse MarginStrut and update parent's BFC if we lay out the block
  that establishes a new formatting context.
- Rewrite NGBlockLayoutAlgorithmTest::CollapsingMarginsCase5 that
  verifies that we correctly collapses margins in different writing modes
- Set ShrinkToFit to child's CS if it's orthogonal to its parent.

BUG=635619

Review-Url: https://codereview.chromium.org/2651793013
Cr-Commit-Position: refs/heads/master@{#448626}
Committed:
https://chromium.googlesource.com/chromium/src/+/4db6b79aab37bf3c687e06712624...
==========

[commit-bot: I haz the power, 2017-02-07 15:28:10]

Committed patchset #6 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/4db6b79aab37bf3c687e06712624...