[CSS Grid Layout] Percentages of indefinite sizes should compute to "auto"

[CSS Grid Layout] Percentages of indefinite sizes should compute to "auto"

According to the spec, http://dev.w3.org/csswg/css-grid/#valuedef-percentage,
that's what we have to do. We were not considering this case as something
special in the current code.

Added several new test cases including some that verify that tracks are sized
as per their children (following "auto" behaviour).

BUG=313215
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=164062

[Julien - ping for review, 2013-11-07 18:18:25]

+ojan: In case we have an API that returns if we have an "indefinite size" in
rendering.

https://codereview.chromium.org/60633008/diff/1/LayoutTests/fast/css-grid-lay...
File LayoutTests/fast/css-grid-layout/resources/grid.css (right):

https://codereview.chromium.org/60633008/diff/1/LayoutTests/fast/css-grid-lay...
LayoutTests/fast/css-grid-layout/resources/grid.css:1: .grid, .noSizeGrid {
I hate this new class name. grid is not sized itself which makes the new name
very confusing. If there is a pattern in the size we use, we should have a
sizedGrid vs an unsizedGrid. If not, I would replace the broad .grid selector
with a different one (like .explicitlySized) and not apply it to the one. To
make sure we get the point though, we would have to add a comment about it being
unsized and we expect grid to ignore the percentage tracks.

https://codereview.chromium.org/60633008/diff/1/Source/core/rendering/RenderG...
File Source/core/rendering/RenderGrid.cpp (right):

https://codereview.chromium.org/60633008/diff/1/Source/core/rendering/RenderG...
Source/core/rendering/RenderGrid.cpp:495: if (!logicalSize.isSpecified()) {
This is not totally right but a good beginning.

isSpecified will return true for <percentage>, when "indefinite size" has a
different meaning (we should ignore a <percentage> that is not resolved against
a definite size). I don't think we have an API that would return that.

[svillar, 2013-11-07 18:44:29]

Thanks for the review! Waiting for Ojan's input

https://codereview.chromium.org/60633008/diff/1/LayoutTests/fast/css-grid-lay...
File LayoutTests/fast/css-grid-layout/resources/grid.css (right):

https://codereview.chromium.org/60633008/diff/1/LayoutTests/fast/css-grid-lay...
LayoutTests/fast/css-grid-layout/resources/grid.css:1: .grid, .noSizeGrid {
On 2013/11/07 18:18:26, Julien Chaffraix - PST wrote:
> I hate this new class name. grid is not sized itself which makes the new name
> very confusing. If there is a pattern in the size we use, we should have a
> sizedGrid vs an unsizedGrid. If not, I would replace the broad .grid selector
> with a different one (like .explicitlySized) and not apply it to the one. To
> make sure we get the point though, we would have to add a comment about it
being
> unsized and we expect grid to ignore the percentage tracks.

OK got the point. The thing is that this is not about defining a sized vs an
unsized grid, because as you can see this selector basically just defines the
display: grid. I added it because several tests extend .grid by adding an
explicit size.

That's why not applying an hypothetic .explicitlySized is not an option because
we still want the display:. I think I'd better define an unsizedGrid.

https://codereview.chromium.org/60633008/diff/1/Source/core/rendering/RenderG...
File Source/core/rendering/RenderGrid.cpp (right):

https://codereview.chromium.org/60633008/diff/1/Source/core/rendering/RenderG...
Source/core/rendering/RenderGrid.cpp:495: if (!logicalSize.isSpecified()) {
On 2013/11/07 18:18:26, Julien Chaffraix - PST wrote:
> This is not totally right but a good beginning.
> 
> isSpecified will return true for <percentage>, when "indefinite size" has a
> different meaning (we should ignore a <percentage> that is not resolved
against
> a definite size). I don't think we have an API that would return that.

I don't really get what you mean with "when indefinite size has a different
meaning". I understand that you think isSpecified() is not equal to "definite
size" right? I agree, the thing is that I am not sure there is such a check in
the current code.

[ojan, 2013-11-08 20:43:02]

> +ojan: In case we have an API that returns if we have an "indefinite size" in
> rendering.

Not that I know of. I think the way we handle this in other parts of the code is
by actually trying to compute the size and seeing if we get -1 back (e.g. by
calling computeLogicalHeight).

[ojan, 2013-12-10 22:53:25]

https://codereview.chromium.org/60633008/diff/90001/Source/core/rendering/Ren...
File Source/core/rendering/RenderGrid.cpp (right):

https://codereview.chromium.org/60633008/diff/90001/Source/core/rendering/Ren...
Source/core/rendering/RenderGrid.cpp:495: if (logicalSize.isIntrinsic()) {
Should this be isInstrinsicOrAuto? Also, what if it's a percentage that resolves
to auto? Should the nested percentage compute to auto as well?

https://codereview.chromium.org/60633008/diff/90001/Source/core/rendering/Ren...
Source/core/rendering/RenderGrid.cpp:496: DEFINE_STATIC_LOCAL(GridTrackSize,
autoTrackSize, (Auto));
I don't expect you gain anything here by making this a static. I think you can
just return Auto directly.

[svillar, 2013-12-16 17:46:50]

https://codereview.chromium.org/60633008/diff/90001/Source/core/rendering/Ren...
File Source/core/rendering/RenderGrid.cpp (right):

https://codereview.chromium.org/60633008/diff/90001/Source/core/rendering/Ren...
Source/core/rendering/RenderGrid.cpp:495: if (logicalSize.isIntrinsic()) {
On 2013/12/10 22:53:26, ojan wrote:
> Should this be isInstrinsicOrAuto? Also, what if it's a percentage that
resolves
> to auto? Should the nested percentage compute to auto as well?

Yeah I guess auto should be considered here as indefinite size as well.

Regarding your other questions I think I don't get exactly what you mean. We're
talking about row/column sizes which are specified once per grid in
grid-template-columns/rows properties. If we insert an item sized using
percentages into the grid it should work as if it weren't inside a grid,
whatever the expected behavior is. This is just an exception to the general
rule.

https://codereview.chromium.org/60633008/diff/90001/Source/core/rendering/Ren...
Source/core/rendering/RenderGrid.cpp:496: DEFINE_STATIC_LOCAL(GridTrackSize,
autoTrackSize, (Auto));
On 2013/12/10 22:53:26, ojan wrote:
> I don't expect you gain anything here by making this a static. I think you can
> just return Auto directly.

Well, we need to return a reference to a GridTrackSize object with Auto sizing,
why would we want to create a new object everytime?

[ojan, 2013-12-18 01:01:09]

lgtm

[commit-bot: I haz the power, 2013-12-18 01:01:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/svillar@igalia.com/60633008/120001

[commit-bot: I haz the power, 2013-12-18 05:10:00]

Change committed as 164062