[layoutng] Set the intrinsic content logical height on the old layout tree

[layoutng] Set the intrinsic content logical height on the old layout tree

The overflow size is the content size without being affected by width/height
and max/min properties, so it's exactly what we need here.

This is important because without this change, there are DCHECK failures
if we're a child of a flexbox as the size remains at -1.

R=eae@chromium.org,ikilpatrick@chromium.org,glebl@chromium.org
BUG=635619
Committed: https://crrev.com/982432d01a43f9bede4b7b442f9b7d5ffa8a20c2
Cr-Commit-Position: refs/heads/master@{#418695}

[cbiesinger, 2016-09-14 20:33:03]

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

[commit-bot: I haz the power, 2016-09-14 20:34:04]

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

[cbiesinger, 2016-09-14 20:52:06]

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

[commit-bot: I haz the power, 2016-09-14 20:52:21]

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

[Gleb Lanbin, 2016-09-14 20:54:17]

lgtm

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

https://codereview.chromium.org/2337763005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/ng/ng_box.cc:46: NGBoxStrut
border_and_padding =
.nit should we use plural nouns here?
const auto borders_paddings = computeBorders(*style_) +
computePaddings(*constraint_space, *style_);

[cbiesinger, 2016-09-14 21:03:59]

On 2016/09/14 20:54:17, glebl wrote:
> lgtm
> 
>
https://codereview.chromium.org/2337763005/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/layout/ng/ng_box.cc (right):
> 
>
https://codereview.chromium.org/2337763005/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/layout/ng/ng_box.cc:46: NGBoxStrut
> border_and_padding =
> .nit should we use plural nouns here?
> const auto borders_paddings = computeBorders(*style_) +
> computePaddings(*constraint_space, *style_);

Hm, interesting question. To me, padding feels like a singular word (so
"paddings" sounds wrong) but for borders I'd be happy to change it. Mostly I
kept the terminology from the old code, e.g.
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/La...

Emil, Ian, any thoughts?

[ikilpatrick, 2016-09-14 21:21:04]

On 2016/09/14 21:03:59, cbiesinger wrote:
> On 2016/09/14 20:54:17, glebl wrote:
> > lgtm
> > 
> >
>
https://codereview.chromium.org/2337763005/diff/1/third_party/WebKit/Source/c...
> > File third_party/WebKit/Source/core/layout/ng/ng_box.cc (right):
> > 
> >
>
https://codereview.chromium.org/2337763005/diff/1/third_party/WebKit/Source/c...
> > third_party/WebKit/Source/core/layout/ng/ng_box.cc:46: NGBoxStrut
> > border_and_padding =
> > .nit should we use plural nouns here?
> > const auto borders_paddings = computeBorders(*style_) +
> > computePaddings(*constraint_space, *style_);
> 
> Hm, interesting question. To me, padding feels like a singular word (so
> "paddings" sounds wrong) but for borders I'd be happy to change it. Mostly I
> kept the terminology from the old code, e.g.
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/La...
> 
> Emil, Ian, any thoughts?

CSS specs typically just refer these as singular (most of the time at least), so
I'd be happy to leave as is at the moment. :)

(also lgtm)

[cbiesinger, 2016-09-14 21:42:50]

The CQ bit was unchecked by cbiesinger@chromium.org

[cbiesinger, 2016-09-14 21:42:54]

The CQ bit was checked by cbiesinger@chromium.org

[cbiesinger, 2016-09-14 21:43:00]

Thanks!

[commit-bot: I haz the power, 2016-09-14 21:43:20]

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-14 22:23:12]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-14 22:25:09]

Description was changed from

==========
[layoutng] Set the intrinsic content logical height on the old layout tree

The overflow size is the content size without being affected by width/height
and max/min properties, so it's exactly what we need here.

This is important because without this change, there are DCHECK failures
if we're a child of a flexbox as the size remains at -1.

R=eae@chromium.org,ikilpatrick@chromium.org,glebl@chromium.org
BUG=635619
==========

to

==========
[layoutng] Set the intrinsic content logical height on the old layout tree

The overflow size is the content size without being affected by width/height
and max/min properties, so it's exactly what we need here.

This is important because without this change, there are DCHECK failures
if we're a child of a flexbox as the size remains at -1.

R=eae@chromium.org,ikilpatrick@chromium.org,glebl@chromium.org
BUG=635619

Committed: https://crrev.com/982432d01a43f9bede4b7b442f9b7d5ffa8a20c2
Cr-Commit-Position: refs/heads/master@{#418695}
==========

[commit-bot: I haz the power, 2016-09-14 22:25:11]

Patchset 2 (id:??) landed as
https://crrev.com/982432d01a43f9bede4b7b442f9b7d5ffa8a20c2
Cr-Commit-Position: refs/heads/master@{#418695}