[css-grid] Use min|max-sizes to compute flex fraction

[css-grid] Use min|max-sizes to compute flex fraction

The min|max-sizes must be used to compute the flex fraction for indefinite
free spaces. According to the spec "If using this flex fraction would cause
the grid to be smaller than the grid container’s min-width/height (or larger
than the grid container’s max-width/height), then redo this step, treating
the free space as definite and the available grid space as equal to the grid
container’s content box size when it’s sized to its min-width/height
(max-width/height)."

This only affects heights because during layout widths are properly
constrained by min|max-width restrictions.

BUG=660690
Committed: https://crrev.com/ee54fbc9b393d7d7b7e49f3f17f3178e817f2e94
Cr-Commit-Position: refs/heads/master@{#430654}

[svillar, 2016-11-03 18:18:56]

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

[jfernandez, 2016-11-04 11:30:43]

I think we could do this in a different way, trying to reuse the flex tracks
base size computation logic we already have.

https://codereview.chromium.org/2480493002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutGrid.cpp (right):

https://codereview.chromium.org/2480493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutGrid.cpp:880: if (direction ==
ForRows) {
We will compute the flex tracks' base size twice, even in the case that rowsSize
end up between the min/max constraints.

https://codereview.chromium.org/2480493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutGrid.cpp:902: LayoutUnit
freeSpace(-1);
freeSpace = checkMaxSize ? maxSize : -1;

https://codereview.chromium.org/2480493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutGrid.cpp:906: std::max(freeSpace,
minSize) -
What we should do if maxSize < minSize ?

https://codereview.chromium.org/2480493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutGrid.cpp:911: direction, freeSpace);
Perhaps using here ForRows, instead of direction, to be more explicit.

[svillar, 2016-11-04 11:42:06]

Thanks for the review

https://codereview.chromium.org/2480493002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutGrid.cpp (right):

https://codereview.chromium.org/2480493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutGrid.cpp:880: if (direction ==
ForRows) {
On 2016/11/04 11:30:43, jfernandez wrote:
> We will compute the flex tracks' base size twice, even in the case that
rowsSize
> end up between the min/max constraints. 

Right. I thought about caching the results but I don't think it will cause
performance penalties unless we have tons of fr tracks in a huge grid, something
I consider unlikely.

https://codereview.chromium.org/2480493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutGrid.cpp:902: LayoutUnit
freeSpace(-1);
On 2016/11/04 11:30:43, jfernandez wrote:
> freeSpace = checkMaxSize ? maxSize : -1;

Acknowledged.

https://codereview.chromium.org/2480493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutGrid.cpp:906: std::max(freeSpace,
minSize) -
On 2016/11/04 11:30:43, jfernandez wrote:
> What we should do if maxSize < minSize ? 

min-size always trumps max-size.

https://codereview.chromium.org/2480493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutGrid.cpp:911: direction, freeSpace);
On 2016/11/04 11:30:43, jfernandez wrote:
> Perhaps using here ForRows, instead of direction, to be more explicit.

Acknowledged.

[jfernandez, 2016-11-08 11:38:53]

lgtm

[svillar, 2016-11-08 16:39:47]

The CQ bit was checked by svillar@igalia.com

[svillar, 2016-11-08 16:39:47]

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

[commit-bot: I haz the power, 2016-11-08 16:40:14]

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-11-08 18:17:51]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-08 18:38:34]

Description was changed from

==========
[css-grid] Use min|max-sizes to compute flex fraction

The min|max-sizes must be used to compute the flex fraction for indefinite
free spaces. According to the spec "If using this flex fraction would cause
the grid to be smaller than the grid container’s min-width/height (or larger
than the grid container’s max-width/height), then redo this step, treating
the free space as definite and the available grid space as equal to the grid
container’s content box size when it’s sized to its min-width/height
(max-width/height)."

This only affects heights because during layout widths are properly
constrained by min|max-width restrictions.

BUG=660690
==========

to

==========
[css-grid] Use min|max-sizes to compute flex fraction

The min|max-sizes must be used to compute the flex fraction for indefinite
free spaces. According to the spec "If using this flex fraction would cause
the grid to be smaller than the grid container’s min-width/height (or larger
than the grid container’s max-width/height), then redo this step, treating
the free space as definite and the available grid space as equal to the grid
container’s content box size when it’s sized to its min-width/height
(max-width/height)."

This only affects heights because during layout widths are properly
constrained by min|max-width restrictions.

BUG=660690

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

[commit-bot: I haz the power, 2016-11-08 18:38:35]

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