[CSS Grid Layout] Implementation of the grid shorthand.

[CSS Grid Layout] Implementation of the grid shorthand.

The grid property is a shorthand that sets all of the explicit
grid properties (grid-template-rows, grid-template-columns, and 
grid-template-areas) as well as all the implicit grid properties
(grid-auto-rows, grid-auto-columns, and grid-auto-flow) in a
single declaration

Notice that either explicit or implicit grid can be specified,
assigning the initial values to the omitted properties.

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

[jfernandez, 2014-04-03 10:15:39]

Initial implementation of the "grid" shorthand.

[Julien - ping for review, 2014-04-03 21:14:30]

lgtm.

From the description:

> It basically defines the implicit and explicit grid properties using just 
one command. 

'grid' defines either the explicit or the implicit grid (but not the 2 at the
same time as one is reset to its initial values) so shouldn't this sentence be
refined.

https://codereview.chromium.org/176853007/diff/120001/Source/core/css/parser/...
File Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/176853007/diff/120001/Source/core/css/parser/...
Source/core/css/parser/CSSPropertyParser.cpp:3611: ASSERT_UNUSED(shorthand,
shorthand.length() == 4);
Can't we just do:

ASSERT(shorthandForProperty(CSSPropertyGrid).lenght() == 4);

https://codereview.chromium.org/176853007/diff/120001/Source/core/css/parser/...
Source/core/css/parser/CSSPropertyParser.cpp:3631: RefPtrWillBeRawPtr<CSSValue>
autoColumnsValue = cssValuePool().createIdentifierValue(CSSValueAuto);
I would rather leave both auto*Value to nullptr and initialize them in the
'else' branches below (if (m_valueList->next())).

Also the specification prose mentions to set them up to the *initial* value, for
which we have a keyword: CSSValueInitial.

https://codereview.chromium.org/176853007/diff/120001/Source/core/css/parser/...
Source/core/css/parser/CSSPropertyParser.cpp:3639: if
(!(isForwardSlashOperator(m_valueList->current()) && m_valueList->next()))
Let's avoid !(... && ...) by expanding it.

https://codereview.chromium.org/176853007/diff/120001/Source/core/css/parser/...
Source/core/css/parser/CSSPropertyParser.cpp:3655: // Default values for the
explicit grid properties.
You should quote the specification here or give a more precise comment as this
looks really suspicious until you read the specification prose.

[jfernandez, 2014-04-04 15:09:34]

Applied suggested changes.

https://codereview.chromium.org/176853007/diff/120001/Source/core/css/parser/...
File Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/176853007/diff/120001/Source/core/css/parser/...
Source/core/css/parser/CSSPropertyParser.cpp:3611: ASSERT_UNUSED(shorthand,
shorthand.length() == 4);
On 2014/04/03 21:14:30, Julien Chaffraix - PST wrote:
> Can't we just do:
> 
> ASSERT(shorthandForProperty(CSSPropertyGrid).lenght() == 4);

Done.

https://codereview.chromium.org/176853007/diff/120001/Source/core/css/parser/...
Source/core/css/parser/CSSPropertyParser.cpp:3631: RefPtrWillBeRawPtr<CSSValue>
autoColumnsValue = cssValuePool().createIdentifierValue(CSSValueAuto);
On 2014/04/03 21:14:30, Julien Chaffraix - PST wrote:
> I would rather leave both auto*Value to nullptr and initialize them in the
> 'else' branches below (if (m_valueList->next())).
> 

Done.

> Also the specification prose mentions to set them up to the *initial* value,
for
> which we have a keyword: CSSValueInitial.

Well, I think what is has to be done is to use the
"createExplicitInitialValue()" method to set the corresponding "initial" values
for each property; right ?

https://codereview.chromium.org/176853007/diff/120001/Source/core/css/parser/...
Source/core/css/parser/CSSPropertyParser.cpp:3639: if
(!(isForwardSlashOperator(m_valueList->current()) && m_valueList->next()))
On 2014/04/03 21:14:30, Julien Chaffraix - PST wrote:
> Let's avoid !(... && ...) by expanding it.

Done.

https://codereview.chromium.org/176853007/diff/120001/Source/core/css/parser/...
Source/core/css/parser/CSSPropertyParser.cpp:3655: // Default values for the
explicit grid properties.
On 2014/04/03 21:14:30, Julien Chaffraix - PST wrote:
> You should quote the specification here or give a more precise comment as this
> looks really suspicious until you read the specification prose.

Done.

[jfernandez, 2014-04-04 15:17:30]

I'll wait for feedback on the "initial" values before trying the CQ, since I'm
not totally sure my changes what it was expected after the review. 

It's also interesting that we didn't follow that approach for the grid-template
shorthand, where the "none" values were set when required to set the "initial·
values, according tot he specs.

I think other properties might have to use the createExplicitInitialValue, or
perhaps the createImplicitInitialValue; still not totally sure in which cases we
should apply them.

[apavlov, 2014-04-04 15:40:36]

On 2014/04/04 15:17:30, jfernandez wrote:
> I think other properties might have to use the createExplicitInitialValue, or
> perhaps the createImplicitInitialValue; still not totally sure in which cases
we
> should apply them.

These two are easy once you know what explicit and implicit values in Blink are:
- explicit values: explicitly specified in a [shorthand] property value
- implicit values: not specified in a shorthand and hence inferred (e.g. from
other items of the same shorthand or assigned the value of "initial" or
something else).
Examples:
- "margin: 1px":
  * margin-top - explicit 1px (the first position)
  * margin-right - implicit 1px
  * margin-bottom - implicit 1px
  * margin-left - implicit 1px

- "border: 1px":
  * border-top-color - implicit initial
  * border-top-style - implicit initial
  * border-top-width - explicit 1px
  * border-right-color - implicit initial
  * border-right-style - implicit initial
  * border-right-width - implicit 1px
  * border-bottom-color - implicit initial
  * border-bottom-style - implicit initial
  * border-bottom-width - implicit 1px
  * border-left-color - implicit initial
  * border-left-style - implicit initial
  * border-left-width - implicit 1px

You can use DevTools's Styles sidebar to inspect the longhand values originating
from a shorthand. The implicit values are dimmed.

[jfernandez, 2014-04-07 19:51:28]

The CQ bit was checked by jfernandez@igalia.com

[commit-bot: I haz the power, 2014-04-07 19:51:36]

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

[commit-bot: I haz the power, 2014-04-07 20:02:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-07 20:02:20]

Try jobs failed on following builders:
  tryserver.blink on win_blink_rel

[jfernandez, 2014-04-07 21:00:27]

The CQ bit was checked by jfernandez@igalia.com

[commit-bot: I haz the power, 2014-04-07 21:00:32]

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

[commit-bot: I haz the power, 2014-04-07 21:28:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-07 21:28:14]

Try jobs failed on following builders:
  tryserver.blink on win_blink_compile_dbg

[jfernandez, 2014-04-09 08:23:58]

The CQ bit was checked by jfernandez@igalia.com

[commit-bot: I haz the power, 2014-04-09 08:24:01]

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

[commit-bot: I haz the power, 2014-04-09 08:56:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-09 08:56:59]

Try jobs failed on following builders:
  tryserver.blink on mac_blink_compile_dbg

[jfernandez, 2014-04-09 12:27:30]

The CQ bit was checked by jfernandez@igalia.com

[commit-bot: I haz the power, 2014-04-09 12:27:40]

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

[commit-bot: I haz the power, 2014-04-09 13:59:59]

Change committed as 171143