[CSS Grid Layout] Use Render methods, rather than style, to determine shrink-to-fit behavior

[CSS Grid Layout] Use Render methods, rather than style, to
 determine shrink-to-fit behavior

In order to determine whether the RemainingSpace is 
undetermined or not (shrink-to-fit) the current algorithm 
is checking out the style for clarifying if the Render object 
is float or absolute positioned.

It's better to use the Render functions instead, since the 
style might be applied or not, depending on several factors, 
like CSS properties inheritance.

BUG=79180, 234204
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168573

[Julien - ping for review, 2014-02-26 22:43:42]

One comment but the direction looks good.

Beware that the description seems too long (above 80 columns but I didn't
measure it though)

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

https://codereview.chromium.org/181333003/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderGrid.cpp:346: return
isFloatingOrOutOfFlowPositioned();
Are fixed elements also shrink-to-fit? (this will return true for fixed elements
so it is worth adding some testing for that).

[jfernandez, 2014-02-28 23:02:09]

On 2014/02/26 22:43:42, Julien Chaffraix - PST wrote:
> One comment but the direction looks good.
> 
> Beware that the description seems too long (above 80 columns but I didn't
> measure it though)
> 
>
https://codereview.chromium.org/181333003/diff/1/Source/core/rendering/Render...
> File Source/core/rendering/RenderGrid.cpp (right):
> 
>
https://codereview.chromium.org/181333003/diff/1/Source/core/rendering/Render...
> Source/core/rendering/RenderGrid.cpp:346: return
> isFloatingOrOutOfFlowPositioned();
> Are fixed elements also shrink-to-fit? (this will return true for fixed
elements
> so it is worth adding some testing for that).

According to the Positioning Specs, as far as I understood it, Fixed elements
width can be resolved as shrink-to-fit. 
  - http://www.w3.org/TR/css3-positioning/#abs-non-replaced-width

Also, I've verified that the RenderBox:computePositionedLogicalWidth is also
called for out-of-flow positioned elements.

I'll provide css-grid-layout tests for verifying the shrink-to-fit behavior of
grid items, but perhaps you meant that some css2.1, or perhaps css3 more
suitable, additional test would be required for the fixed positioned elements.
I've fond several tests targeting both floating and absolute positioned
elements, but not for the fixed positioned case. The fast/block/float and
fast/block/positioning folders also contain some tests that could be extended to
cover the fixed positioned elements.

[Julien - ping for review, 2014-03-04 18:33:27]

lgtm

Don't land without properly wrapping the description (it's the second time I
mention it).

Btw there is a bug for shrink-to-fit (crbug.com/234204 we should probably fold
this change under it)

https://codereview.chromium.org/181333003/diff/20001/LayoutTests/fast/css-gri...
File LayoutTests/fast/css-grid-layout/grid-element-shink-to-fit.html (right):

https://codereview.chromium.org/181333003/diff/20001/LayoutTests/fast/css-gri...
LayoutTests/fast/css-grid-layout/grid-element-shink-to-fit.html:5:
testRunner.overridePreference("WebKitCSSGridLayoutEnabled", 1);
This is unneeded!

https://codereview.chromium.org/181333003/diff/20001/LayoutTests/fast/css-gri...
LayoutTests/fast/css-grid-layout/grid-element-shink-to-fit.html:55: <p>The
following grid should be 400px * 400px.</p>
The following grid*S* no?

Please let's have a description of what we are testing (the shrink-to-fit
behavior).

https://codereview.chromium.org/181333003/diff/20001/Source/platform/RuntimeE...
File Source/platform/RuntimeEnabledFeatures.in (right):

https://codereview.chromium.org/181333003/diff/20001/Source/platform/RuntimeE...
Source/platform/RuntimeEnabledFeatures.in:32: CSSGridLayout status=stable
I think it's too early for that :-)

[jfernandez, 2014-03-05 12:39:50]

Changed description and applied suggested changes.

https://codereview.chromium.org/181333003/diff/20001/LayoutTests/fast/css-gri...
File LayoutTests/fast/css-grid-layout/grid-element-shink-to-fit.html (right):

https://codereview.chromium.org/181333003/diff/20001/LayoutTests/fast/css-gri...
LayoutTests/fast/css-grid-layout/grid-element-shink-to-fit.html:5:
testRunner.overridePreference("WebKitCSSGridLayoutEnabled", 1);
On 2014/03/04 18:33:27, Julien Chaffraix - PST wrote:
> This is unneeded!

Done.

https://codereview.chromium.org/181333003/diff/20001/LayoutTests/fast/css-gri...
LayoutTests/fast/css-grid-layout/grid-element-shink-to-fit.html:55: <p>The
following grid should be 400px * 400px.</p>
On 2014/03/04 18:33:27, Julien Chaffraix - PST wrote:
> The following grid*S* no?
> 
> Please let's have a description of what we are testing (the shrink-to-fit
> behavior).

Done.

[jfernandez, 2014-03-05 12:40:03]

The CQ bit was checked by jfernandez@igalia.com

[commit-bot: I haz the power, 2014-03-05 12:40:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jfernandez@igalia.com/181333003/40001

[jfernandez, 2014-03-05 12:56:05]

The CQ bit was unchecked by jfernandez@igalia.com

[Manuel Rego, 2014-03-05 12:56:11]

https://codereview.chromium.org/181333003/diff/40001/LayoutTests/fast/css-gri...
File LayoutTests/fast/css-grid-layout/grid-element-shink-to-fit.html (right):

https://codereview.chromium.org/181333003/diff/40001/LayoutTests/fast/css-gri...
LayoutTests/fast/css-grid-layout/grid-element-shink-to-fit.html:55: <div
class="gg" id="one"></div>
This "gg" class is undefined here. It doesn't seem needed.

[jfernandez, 2014-03-05 13:33:04]

The CQ bit was checked by jfernandez@igalia.com

[commit-bot: I haz the power, 2014-03-05 13:33:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jfernandez@igalia.com/181333003/60001

[commit-bot: I haz the power, 2014-03-05 20:58:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jfernandez@igalia.com/181333003/60001

[commit-bot: I haz the power, 2014-03-06 04:25:08]

Change committed as 168573