[css-grid] Fix intrinsic size computation with flexible sized tracks

[css-grid] Fix intrinsic size computation with flexible sized tracks

This is fixing a regression added in crrev.com/1317643005. When computing
the min content size of a grid container (min preferred logical width) we
should not take into account the fr tracks. As stated in the early versions
of the spec that size is the sum of the tracks' base sizes before running
the maximize tracks step.

That regression was causing sizing issues in grids with fr tracks both when
under min|max-content constrains and also when used as grid items (in nested
grids).

BUG=578787, 619567
Committed: https://crrev.com/ac2a89c4ede377100fd06d965234420217875197
Cr-Commit-Position: refs/heads/master@{#418212}

[svillar, 2016-09-12 20:28:11]

svillar@igalia.com changed reviewers:
+ cbiesinger@chromium.org, eae@chromium.org, jfernandez@igalia.com,
rego@igalia.com

[svillar, 2016-09-12 20:28:11]

Sending out for review

[Manuel Rego, 2016-09-12 21:16:48]

I guess the related text in the current spec is in the following sections:

12.6) Maximize tracks (https://drafts.csswg.org/css-grid/#algo-grow-tracks)
"For the purpose of this step: if sizing the grid container
 under a max-content constraint, the free space is infinite;
 if sizing under a min-content constraint, the free space is zero."

5.2) Sizing Grid Containers (https://drafts.csswg.org/css-grid/#intrinsic-sizes)
"The min-content size of a grid container is the sum
 of the grid container’s track sizes (including gutters) in the appropriate
axis,
 when the grid is sized under a min-content constraint."

[Manuel Rego, 2016-09-12 21:18:34]

The patch LGTM. Nice catch!

I'm wondering if it'd be nice to add some more tests
based on the cases from the bug reports.
For example, a main grid with some flexible tracks
that has inside another grid with flexible tracks.
And verify that the sizes of the tracks are right.

[svillar, 2016-09-13 07:37:25]

On 2016/09/12 21:18:34, Manuel Rego wrote:
> The patch LGTM. Nice catch!
> 
> I'm wondering if it'd be nice to add some more tests
> based on the cases from the bug reports.
> For example, a main grid with some flexible tracks
> that has inside another grid with flexible tracks.
> And verify that the sizes of the tracks are right.

Actually one of the new tests is precisely that, a grid with a flex track inside
of a grid with a flex track.

[Manuel Rego, 2016-09-13 07:41:58]

On 2016/09/13 07:37:25, svillar wrote:
> On 2016/09/12 21:18:34, Manuel Rego wrote:
> > I'm wondering if it'd be nice to add some more tests
> > based on the cases from the bug reports.
> > For example, a main grid with some flexible tracks
> > that has inside another grid with flexible tracks.
> > And verify that the sizes of the tracks are right.
> 
> Actually one of the new tests is precisely that, a grid with a flex track
inside
> of a grid with a flex track.

Ups, yeah didn't realize that the main grid was "1fr / 1fr" too.
Anyway, maybe combining two flexible tracks and things like that could be
useful.
Just a suggestion though.

[svillar, 2016-09-13 08:50:29]

The CQ bit was checked by svillar@igalia.com

[svillar, 2016-09-13 08:50:29]

The patchset sent to the CQ was uploaded after l-g-t-m from rego@igalia.com
Link to the patchset: https://codereview.chromium.org/2337573003/#ps20001
(title: "Patch for landing")

[commit-bot: I haz the power, 2016-09-13 08:50: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, 2016-09-13 10:35:52]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-13 10:37:25]

Description was changed from

==========
[css-grid] Fix intrinsic size computation with flexible sized tracks

This is fixing a regression added in crrev.com/1317643005. When computing
the min content size of a grid container (min preferred logical width) we
should not take into account the fr tracks. As stated in the early versions
of the spec that size is the sum of the tracks' base sizes before running
the maximize tracks step.

That regression was causing sizing issues in grids with fr tracks both when
under min|max-content constrains and also when used as grid items (in nested
grids).

BUG=578787,619567
==========

to

==========
[css-grid] Fix intrinsic size computation with flexible sized tracks

This is fixing a regression added in crrev.com/1317643005. When computing
the min content size of a grid container (min preferred logical width) we
should not take into account the fr tracks. As stated in the early versions
of the spec that size is the sum of the tracks' base sizes before running
the maximize tracks step.

That regression was causing sizing issues in grids with fr tracks both when
under min|max-content constrains and also when used as grid items (in nested
grids).

BUG=578787,619567

Committed: https://crrev.com/ac2a89c4ede377100fd06d965234420217875197
Cr-Commit-Position: refs/heads/master@{#418212}
==========

[commit-bot: I haz the power, 2016-09-13 10:37:26]

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

[eae, 2016-09-13 11:02:37]

LGTM