Implement 'grid-template' parsing

Implement 'grid-template' parsing

This change makes us recognize grid-template, parse it, store it and
return properly to JavaScript.

The specification mandates to validate to check that the areas defined
by 'grid-template' are rectangular which involves a lot of the
CSSParser code.

Because validating involves building the grid areas, a new CSSValue
was introduced to hold the computed value. Note that we have to track
the explicit size of the named grid areas as the named grid areas (".")
are not tracked in our HashMap of grid areas.

BUG=258092
TEST=fast/css-grid-layout/grid-template-get-set.html

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

[Julien - ping for review, 2013-07-29 20:49:20]

Any taker for this CSS parsing change. Maybe ojan?

[ojan, 2013-07-29 23:03:28]

lgtm

Sorry it took so long to get to this. The size of the patch was a bit
intimidating. :)

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSGridTem...
File Source/core/css/CSSGridTemplateValue.cpp (right):

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSGridTem...
Source/core/css/CSSGridTemplateValue.cpp:76: builder.append('\'');
Does the spec cover whether to serialize with single or double quotes?

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSParser-...
File Source/core/css/CSSParser-in.cpp (right):

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSParser-...
Source/core/css/CSSParser-in.cpp:4752: gridRowNames.split(' ', columnNames);
Does the spec say what to do if there are multiple spaces in a row?

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/resolver/S...
File Source/core/css/resolver/StyleResolver.cpp (right):

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/resolver/S...
Source/core/css/resolver/StyleResolver.cpp:2337: static void
createGridTemplate(CSSValue* value, NamedGridAreaMap& namedGridArea, size_t&
rowCount, size_t& columnCount)
I don't think this function adds value.

https://codereview.chromium.org/18532004/diff/3001/Source/core/rendering/styl...
File Source/core/rendering/style/StyleGridData.h (right):

https://codereview.chromium.org/18532004/diff/3001/Source/core/rendering/styl...
Source/core/rendering/style/StyleGridData.h:67: // This HashMap stored the named
grid areas.
I don't think this comment adds value.

[esprehn, 2013-07-29 23:27:36]

There's an operator for comparing strings with chars so you don't need that
extra static.

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSGridTem...
File Source/core/css/CSSGridTemplateValue.h (right):

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSGridTem...
Source/core/css/CSSGridTemplateValue.h:47: void
reportDescendantMemoryUsage(MemoryObjectInfo*) const;
Memory usage stuff is all dead now.

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSParser-...
File Source/core/css/CSSParser-in.cpp (right):

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSParser-...
Source/core/css/CSSParser-in.cpp:4767: if (gridAreaName ==
unamedAreaPlaceholder)
This is overly complex, just check if gridAreaName == '.' there's a magical
operator for that one.

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/resolver/S...
File Source/core/css/resolver/StyleResolver.cpp (right):

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/resolver/S...
Source/core/css/resolver/StyleResolver.cpp:2347: columnCount =
gridTemplateValue->columnCount();
This makes the code really confusing, can you just put the thing inline? :)

[Julien - ping for review, 2013-07-30 18:43:33]

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSGridTem...
File Source/core/css/CSSGridTemplateValue.cpp (right):

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSGridTem...
Source/core/css/CSSGridTemplateValue.cpp:76: builder.append('\'');
On 2013/07/29 23:03:28, ojan wrote:
> Does the spec cover whether to serialize with single or double quotes?

The spec doesn't say anything about that. We accept <string> so either is fine.

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSGridTem...
File Source/core/css/CSSGridTemplateValue.h (right):

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSGridTem...
Source/core/css/CSSGridTemplateValue.h:47: void
reportDescendantMemoryUsage(MemoryObjectInfo*) const;
On 2013/07/29 23:27:36, esprehn wrote:
> Memory usage stuff is all dead now.

Yay, that's an old patch. Removed.

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSParser-...
File Source/core/css/CSSParser-in.cpp (right):

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSParser-...
Source/core/css/CSSParser-in.cpp:4752: gridRowNames.split(' ', columnNames);
On 2013/07/29 23:03:28, ojan wrote:
> Does the spec say what to do if there are multiple spaces in a row?

Not explicitly (the grammar is <string>+). It would be dumb not to squash /
ignore multiple spaces in a row though as it would prevent doing a nice ASCII
art representation of the grid.

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSParser-...
Source/core/css/CSSParser-in.cpp:4767: if (gridAreaName ==
unamedAreaPlaceholder)
On 2013/07/29 23:27:36, esprehn wrote:
> This is overly complex, just check if gridAreaName == '.' there's a magical
> operator for that one.

I thought it would be more readable but apparently YMMV. Changed.

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/resolver/S...
File Source/core/css/resolver/StyleResolver.cpp (right):

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/resolver/S...
Source/core/css/resolver/StyleResolver.cpp:2337: static void
createGridTemplate(CSSValue* value, NamedGridAreaMap& namedGridArea, size_t&
rowCount, size_t& columnCount)
On 2013/07/29 23:03:28, ojan wrote:
> I don't think this function adds value.

ACK!

https://codereview.chromium.org/18532004/diff/3001/Source/core/rendering/styl...
File Source/core/rendering/style/StyleGridData.h (right):

https://codereview.chromium.org/18532004/diff/3001/Source/core/rendering/styl...
Source/core/rendering/style/StyleGridData.h:67: // This HashMap stored the named
grid areas.
On 2013/07/29 23:03:28, ojan wrote:
> I don't think this comment adds value.

Fair enough, removed.

[ojan, 2013-07-30 18:53:48]

Tab, a couple spec things for your consideration...

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSGridTem...
File Source/core/css/CSSGridTemplateValue.cpp (right):

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSGridTem...
Source/core/css/CSSGridTemplateValue.cpp:76: builder.append('\'');
On 2013/07/30 18:43:33, Julien Chaffraix wrote:
> On 2013/07/29 23:03:28, ojan wrote:
> > Does the spec cover whether to serialize with single or double quotes?
> 
> The spec doesn't say anything about that. We accept <string> so either is
fine.

I'd like the spec to be explicit. Don't need to block this patch on it, but a
FIXME would be nice. Incompatibilities between browsers on whether they
serialize using single or double quotes with fonts has been a huge pain for web
developers.

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSParser-...
File Source/core/css/CSSParser-in.cpp (right):

https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSParser-...
Source/core/css/CSSParser-in.cpp:4752: gridRowNames.split(' ', columnNames);
On 2013/07/30 18:43:33, Julien Chaffraix wrote:
> On 2013/07/29 23:03:28, ojan wrote:
> > Does the spec say what to do if there are multiple spaces in a row?
> 
> Not explicitly (the grammar is <string>+). It would be dumb not to squash /
> ignore multiple spaces in a row though as it would prevent doing a nice ASCII
> art representation of the grid.

Makes sense to me. We should make the spec explicit if possible.

[commit-bot: I haz the power, 2013-07-30 21:37:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jchaffraix@chromium.org/18532004/15001

[commit-bot: I haz the power, 2013-07-31 01:38:41]

Change committed as 155199

[TabAtkins, 2013-07-31 16:56:22]

On 2013/07/30 18:53:48, ojan wrote:
> Tab, a couple spec things for your consideration...
> 
>
https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSGridTem...
> File Source/core/css/CSSGridTemplateValue.cpp (right):
> 
>
https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSGridTem...
> Source/core/css/CSSGridTemplateValue.cpp:76: builder.append('\'');
> On 2013/07/30 18:43:33, Julien Chaffraix wrote:
> > On 2013/07/29 23:03:28, ojan wrote:
> > > Does the spec cover whether to serialize with single or double quotes?
> > 
> > The spec doesn't say anything about that. We accept <string> so either is
> fine.
> 
> I'd like the spec to be explicit. Don't need to block this patch on it, but a
> FIXME would be nice. Incompatibilities between browsers on whether they
> serialize using single or double quotes with fonts has been a huge pain for
web
> developers.

CSS in general doesn't specify this yet.  It's a bad problem.

We should already have code that serializes these basic data types, right?  That
said, use double quotes.

>
https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSParser-...
> File Source/core/css/CSSParser-in.cpp (right):
> 
>
https://codereview.chromium.org/18532004/diff/3001/Source/core/css/CSSParser-...
> Source/core/css/CSSParser-in.cpp:4752: gridRowNames.split(' ', columnNames);
> On 2013/07/30 18:43:33, Julien Chaffraix wrote:
> > On 2013/07/29 23:03:28, ojan wrote:
> > > Does the spec say what to do if there are multiple spaces in a row?
> > 
> > Not explicitly (the grammar is <string>+). It would be dumb not to squash /
> > ignore multiple spaces in a row though as it would prevent doing a nice
ASCII
> > art representation of the grid.
> 
> Makes sense to me. We should make the spec explicit if possible.

The CSS parsing spec <http://dev.w3.org/csswg/css-syntax/> squashes whitespace.