Move grid-auto-column/grid-auto-row into CSSPropertyParser

Move grid-auto-column/grid-auto-row into CSSPropertyParser

Move the grid-auto-column/grid-auto-row longhands from
LegacyCSSPropertyParser into CSSPropertyParser.

Introduce the consumeGridTrackSize helper function that
we can use in upcoming patches.

BUG=499780
Committed: https://crrev.com/d255da18fe5a85dd249ed8f99e84a2a1c66806bf
Cr-Commit-Position: refs/heads/master@{#382749}

[rwlbuis, 2016-03-18 17:45:44]

Description was changed from

==========
foo


1813763003 WIP

BUG=
==========

to

==========
auto-rows WIP
==========

[rwlbuis, 2016-03-21 20:30:48]

Description was changed from

==========
auto-rows WIP
==========

to

==========
Move grid-auto-column/grid-auto-row into CSSPropertyParser

Move the grid-auto-column/grid-auto-row longhands from LegacyCSSPropertyParser
into CSSPropertyParser.

Introduce the consumeGridTrackSize helper function that
we can use in upcoming patches.

BUG=499780
==========

[rwlbuis, 2016-03-21 20:30:58]

Description was changed from

==========
Move grid-auto-column/grid-auto-row into CSSPropertyParser

Move the grid-auto-column/grid-auto-row longhands from LegacyCSSPropertyParser
into CSSPropertyParser.

Introduce the consumeGridTrackSize helper function that
we can use in upcoming patches.

BUG=499780
==========

to

==========
Move grid-auto-column/grid-auto-row into CSSPropertyParser

Move the grid-auto-column/grid-auto-row longhands from
LegacyCSSPropertyParser into CSSPropertyParser.

Introduce the consumeGridTrackSize helper function that
we can use in upcoming patches.

BUG=499780
==========

[rwlbuis, 2016-03-21 21:05:52]

rob.buis@samsung.com changed reviewers:
+ jfernandez@igalia.com, timloh@chromium.org

[rwlbuis, 2016-03-21 21:05:52]

PTAL.

Note that we'll have to keep an eye on any code changes in these helper
functions in the legacy parser now that they are duplicated...

[Timothy Loh, 2016-03-22 06:33:59]

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3072: if
(identMatches<CSSValueMinContent, CSSValueMaxContent, CSSValueAuto>(token.id()))
Maybe nicer if we check the restriction first?

if (restriction == AllowAll) {
    ...
}
return consumeLengthOrPercent();

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3078: double
flexValue = range.consumeIncludingWhitespace().numericValue();
Probably should finish validating before consuming so we don't update the range
and then return nullptr

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3090: if
(identMatches<CSSValueAuto>(token.id()))
IMO clearer to not use a ternary and just write

if (restriction == AllowAll && identMatches..)

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3094: // The
spec defines the following grammar: minmax( <track-breadth> , <track-breadth> ).
Comment isn't very useful imo

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3095:
CSSParserTokenRange args = consumeFunction(range);
Should we make a copy of the range? If the arguments are invalid we'd end up
still consuming the minmax() function and the "[ <line-names>? <string>
<track-size>? <line-names>? ]+ [ / <track-list> ]?" case of grid will be neater
if this function doesn't do that.

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.h (right):

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.h:46: enum
TrackSizeRestriction { FixedSizeOnly, AllowAll };
TODO to move to cpp file once legacy parser is removed?

[rwlbuis, 2016-03-22 20:48:12]

All issues should be fixed, PTAL.

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3072: if
(identMatches<CSSValueMinContent, CSSValueMaxContent, CSSValueAuto>(token.id()))
On 2016/03/22 06:33:59, Timothy Loh wrote:
> Maybe nicer if we check the restriction first?
> 
> if (restriction == AllowAll) {
>     ...
> }
> return consumeLengthOrPercent();

Done.

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3078: double
flexValue = range.consumeIncludingWhitespace().numericValue();
On 2016/03/22 06:33:59, Timothy Loh wrote:
> Probably should finish validating before consuming so we don't update the
range
> and then return nullptr

Done.

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3090: if
(identMatches<CSSValueAuto>(token.id()))
On 2016/03/22 06:33:59, Timothy Loh wrote:
> IMO clearer to not use a ternary and just write
> 
> if (restriction == AllowAll && identMatches..)

Done.

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3094: // The
spec defines the following grammar: minmax( <track-breadth> , <track-breadth> ).
On 2016/03/22 06:33:59, Timothy Loh wrote:
> Comment isn't very useful imo

Done.

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:3095:
CSSParserTokenRange args = consumeFunction(range);
On 2016/03/22 06:33:59, Timothy Loh wrote:
> Should we make a copy of the range? If the arguments are invalid we'd end up
> still consuming the minmax() function and the "[ <line-names>? <string>
> <track-size>? <line-names>? ]+ [ / <track-list> ]?" case of grid will be
neater
> if this function doesn't do that.

Done.

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.h (right):

https://codereview.chromium.org/1813053004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.h:46: enum
TrackSizeRestriction { FixedSizeOnly, AllowAll };
On 2016/03/22 06:33:59, Timothy Loh wrote:
> TODO to move to cpp file once legacy parser is removed?

Done.

[Timothy Loh, 2016-03-22 23:22:51]

lgtm

[rwlbuis, 2016-03-22 23:27:57]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2016-03-22 23:28:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1813053004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1813053004/60001

[commit-bot: I haz the power, 2016-03-23 00:49:06]

Description was changed from

==========
Move grid-auto-column/grid-auto-row into CSSPropertyParser

Move the grid-auto-column/grid-auto-row longhands from
LegacyCSSPropertyParser into CSSPropertyParser.

Introduce the consumeGridTrackSize helper function that
we can use in upcoming patches.

BUG=499780
==========

to

==========
Move grid-auto-column/grid-auto-row into CSSPropertyParser

Move the grid-auto-column/grid-auto-row longhands from
LegacyCSSPropertyParser into CSSPropertyParser.

Introduce the consumeGridTrackSize helper function that
we can use in upcoming patches.

BUG=499780
==========

[commit-bot: I haz the power, 2016-03-23 00:49:07]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-23 00:50:22]

Description was changed from

==========
Move grid-auto-column/grid-auto-row into CSSPropertyParser

Move the grid-auto-column/grid-auto-row longhands from
LegacyCSSPropertyParser into CSSPropertyParser.

Introduce the consumeGridTrackSize helper function that
we can use in upcoming patches.

BUG=499780
==========

to

==========
Move grid-auto-column/grid-auto-row into CSSPropertyParser

Move the grid-auto-column/grid-auto-row longhands from
LegacyCSSPropertyParser into CSSPropertyParser.

Introduce the consumeGridTrackSize helper function that
we can use in upcoming patches.

BUG=499780

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

[commit-bot: I haz the power, 2016-03-23 00:50:24]

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