[layoutng] Create correct constraint spaces for children

[layoutng] Create correct constraint spaces for children

This does a couple of things:
- Rationalizes the constructors for the NGConstraintSpace. We need the
  following:
  - A constructor for use by the layout opportunity iterator. The only thing
    this constructor changes is the size and the offset:
    NGConstraintSpace(const NGConstraintSpace&, NGLogicalOffset, NGLogicalSize);
  - A constructor for use by layout algorithms, as a basis for finding
    layout opportunities. This should reset the offset, set the container size,
    writing mode and direction and reset the fixed size properties:
    NGConstraintSpace(NGWritingMode, NGDirection, const NGConstraintSpace&, NGLogicalSize);
  - A constructor for tests and the root constraint space:
    NGConstraintSpace(NGWritingMode, NGDirection, NGLogicalSize);
- Makes sure to clamp inline and block sizes to zero after we subtract border
  and padding
- Make sure not to subtract border and padding from an indefinite size

R=eae@chromium.org, ikilpatrick@chromium.org, mstensho@opera.com
BUG=635619
Committed: https://chromium.googlesource.com/chromium/src/+/761988cdc921282f5a7baa7e56b511456c4e1843

[cbiesinger, 2016-09-19 22:21:56]

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

[commit-bot: I haz the power, 2016-09-19 22:22: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, 2016-09-20 00:24:01]

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

[commit-bot: I haz the power, 2016-09-20 00:24:02]

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

[cbiesinger1, 2016-09-20 01:34:59]

The CQ bit was checked by cbiesinger@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-20 01:35:29]

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-20 02:25:03]

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

[commit-bot: I haz the power, 2016-09-20 02:25:05]

Dry run: This issue passed the CQ dry run.

[eae, 2016-09-20 08:09:38]

LGTM w/nit

https://codereview.chromium.org/2346403002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc
(right):

https://codereview.chromium.org/2346403002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc:426:
EXPECT_EQ(child->Width(), LayoutUnit(12));
You might want an EXPECT_EQ(frag->Children().size(), 1) before trying to access
the first one. More helpful error than a out of bound crash.

[ikilpatrick, 2016-09-20 10:47:10]

lgtm

https://codereview.chromium.org/2346403002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc
(right):

https://codereview.chromium.org/2346403002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc:52: if
(adjusted_block_size != NGSizeIndefinite) {
is a comment here explaining what is going on worth it?

[mstensho (USE GERRIT), 2016-09-20 11:30:17]

https://codereview.chromium.org/2346403002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc
(right):

https://codereview.chromium.org/2346403002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc:46:
(inline_size - border_and_padding_.InlineSum()).clampNegativeToZero();
Shouldn't computeInlineSizeForFragment() make sure that inline_size >=
border_and_padding?

[cbiesinger, 2016-09-20 13:43:34]

The CQ bit was checked by cbiesinger@chromium.org

[cbiesinger, 2016-09-20 13:43:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
ikilpatrick@chromium.org, eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2346403002/#ps20001
(title: "review comments")

[cbiesinger, 2016-09-20 13:43:37]

https://codereview.chromium.org/2346403002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc
(right):

https://codereview.chromium.org/2346403002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc:46:
(inline_size - border_and_padding_.InlineSum()).clampNegativeToZero();
On 2016/09/20 11:30:17, mstensho wrote:
> Shouldn't computeInlineSizeForFragment() make sure that inline_size >=
> border_and_padding?

Good point, done.

https://codereview.chromium.org/2346403002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc:52: if
(adjusted_block_size != NGSizeIndefinite) {
On 2016/09/20 10:47:10, ikilpatrick wrote:
> is a comment here explaining what is going on worth it?

Done.

https://codereview.chromium.org/2346403002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc
(right):

https://codereview.chromium.org/2346403002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc:426:
EXPECT_EQ(child->Width(), LayoutUnit(12));
On 2016/09/20 08:09:38, eae wrote:
> You might want an EXPECT_EQ(frag->Children().size(), 1) before trying to
access
> the first one. More helpful error than a out of bound crash.

There's already an ASSERT_EQ for that above.

[commit-bot: I haz the power, 2016-09-20 13:43:48]

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

[cbiesinger, 2016-09-20 14:54:18]

The CQ bit was checked by cbiesinger@chromium.org

[cbiesinger, 2016-09-20 14:54:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
ikilpatrick@chromium.org, eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2346403002/#ps40001
(title: "fix assert failures")

[commit-bot: I haz the power, 2016-09-20 14:54:46]

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-20 16:30:30]

Description was changed from

==========
[layoutng] Create correct constraint spaces for children

This does a couple of things:
- Rationalizes the constructors for the NGConstraintSpace. We need the
  following:
  - A constructor for use by the layout opportunity iterator. The only thing
    this constructor changes is the size and the offset:
    NGConstraintSpace(const NGConstraintSpace&, NGLogicalOffset, NGLogicalSize);
  - A constructor for use by layout algorithms, as a basis for finding
    layout opportunities. This should reset the offset, set the container size,
    writing mode and direction and reset the fixed size properties:
    NGConstraintSpace(NGWritingMode, NGDirection, const NGConstraintSpace&,
NGLogicalSize);
  - A constructor for tests and the root constraint space:
    NGConstraintSpace(NGWritingMode, NGDirection, NGLogicalSize);
- Makes sure to clamp inline and block sizes to zero after we subtract border
  and padding
- Make sure not to subtract border and padding from an indefinite size

R=mstensho@opera.com,eae@chromium.org,ikilpatrick@chromium.org
BUG=635619
==========

to

==========
[layoutng] Create correct constraint spaces for children

This does a couple of things:
- Rationalizes the constructors for the NGConstraintSpace. We need the
  following:
  - A constructor for use by the layout opportunity iterator. The only thing
    this constructor changes is the size and the offset:
    NGConstraintSpace(const NGConstraintSpace&, NGLogicalOffset, NGLogicalSize);
  - A constructor for use by layout algorithms, as a basis for finding
    layout opportunities. This should reset the offset, set the container size,
    writing mode and direction and reset the fixed size properties:
    NGConstraintSpace(NGWritingMode, NGDirection, const NGConstraintSpace&,
NGLogicalSize);
  - A constructor for tests and the root constraint space:
    NGConstraintSpace(NGWritingMode, NGDirection, NGLogicalSize);
- Makes sure to clamp inline and block sizes to zero after we subtract border
  and padding
- Make sure not to subtract border and padding from an indefinite size

R=eae@chromium.org, ikilpatrick@chromium.org, mstensho@opera.com
BUG=635619

Committed: https://crrev.com/761988cdc921282f5a7baa7e56b511456c4e1843
Cr-Commit-Position: refs/heads/master@{#419787}
==========

[commit-bot: I haz the power, 2016-09-20 16:30:32]

Patchset 3 (id:??) landed as
https://crrev.com/761988cdc921282f5a7baa7e56b511456c4e1843
Cr-Commit-Position: refs/heads/master@{#419787}

[cbiesinger, 2016-09-20 16:31:34]

Description was changed from

==========
[layoutng] Create correct constraint spaces for children

This does a couple of things:
- Rationalizes the constructors for the NGConstraintSpace. We need the
  following:
  - A constructor for use by the layout opportunity iterator. The only thing
    this constructor changes is the size and the offset:
    NGConstraintSpace(const NGConstraintSpace&, NGLogicalOffset, NGLogicalSize);
  - A constructor for use by layout algorithms, as a basis for finding
    layout opportunities. This should reset the offset, set the container size,
    writing mode and direction and reset the fixed size properties:
    NGConstraintSpace(NGWritingMode, NGDirection, const NGConstraintSpace&,
NGLogicalSize);
  - A constructor for tests and the root constraint space:
    NGConstraintSpace(NGWritingMode, NGDirection, NGLogicalSize);
- Makes sure to clamp inline and block sizes to zero after we subtract border
  and padding
- Make sure not to subtract border and padding from an indefinite size

R=eae@chromium.org, ikilpatrick@chromium.org, mstensho@opera.com
BUG=635619

Committed: https://crrev.com/761988cdc921282f5a7baa7e56b511456c4e1843
Cr-Commit-Position: refs/heads/master@{#419787}
==========

to

==========
[layoutng] Create correct constraint spaces for children

This does a couple of things:
- Rationalizes the constructors for the NGConstraintSpace. We need the
  following:
  - A constructor for use by the layout opportunity iterator. The only thing
    this constructor changes is the size and the offset:
    NGConstraintSpace(const NGConstraintSpace&, NGLogicalOffset, NGLogicalSize);
  - A constructor for use by layout algorithms, as a basis for finding
    layout opportunities. This should reset the offset, set the container size,
    writing mode and direction and reset the fixed size properties:
    NGConstraintSpace(NGWritingMode, NGDirection, const NGConstraintSpace&,
NGLogicalSize);
  - A constructor for tests and the root constraint space:
    NGConstraintSpace(NGWritingMode, NGDirection, NGLogicalSize);
- Makes sure to clamp inline and block sizes to zero after we subtract border
  and padding
- Make sure not to subtract border and padding from an indefinite size

R=eae@chromium.org, ikilpatrick@chromium.org, mstensho@opera.com
BUG=635619

Committed:
https://chromium.googlesource.com/chromium/src/+/761988cdc921282f5a7baa7e56b5...
==========

[cbiesinger, 2016-09-20 16:31:36]

Committed patchset #3 (id:40001) manually as
761988cdc921282f5a7baa7e56b511456c4e1843 (presubmit successful).