Parse outline shorthand in CSSPropertyParser with CSSParserTokens

Parse outline shorthand in CSSPropertyParser with CSSParserTokens

Move outline shorthand property handling from
LegacyCSSPropertyParser into CSSPropertyParser. Also
move its related longhands.

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

[rwlbuis, 2015-11-04 03:38:13]

Description was changed from

==========
outline WIP


Foo

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

to

==========
Parse outline shorthand in CSSPropertyParser with CSSParserTokens

Move outline shorthand property handling from
LegacyCSSPropertyParser into CSSPropertyParser. Also
move its related longhands.

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

[rwlbuis, 2015-11-04 18:53:29]

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

[rwlbuis, 2015-11-04 18:53:30]

PTAL, should be pretty straightforward this time.

[Timothy Loh, 2015-11-05 05:08:26]

lgtm

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

https://codereview.chromium.org/1406343008/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1517: static
PassRefPtrWillBeRawPtr<CSSPrimitiveValue>
consumeBorderWidth(CSSParserTokenRange& range, CSSParserMode cssParserMode)
The spec calls this a <line-width> for the border-width properties, so maybe
consumeLineWidth is better. Otherwise it's slightly confusing because calling
this isn't sufficient for parsing the border-width property :-)

[rwlbuis, 2015-11-05 15:15:13]

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

[rwlbuis, 2015-11-05 15:15:14]

The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org
Link to the patchset: https://codereview.chromium.org/1406343008/#ps80001
(title: "Patch for landing")

[commit-bot: I haz the power, 2015-11-05 15:15:47]

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

[commit-bot: I haz the power, 2015-11-05 17:07:08]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-11-05 17:08:15]

Patchset 5 (id:??) landed as
https://crrev.com/3447a99f8dce411d4e07c55e6b81dd2068a50390
Cr-Commit-Position: refs/heads/master@{#358065}

[michaeln, 2015-11-05 20:34:00]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/1419143008/ by michaeln@chromium.org.

The reason for reverting is: Reverting in order to revert a suspected top
crasher. See the bug for details.

BUG=551638.

[rwlbuis, 2015-11-06 00:25:19]

On 2015/11/05 05:08:26, Timothy Loh wrote:
> lgtm
> 
>
https://codereview.chromium.org/1406343008/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):
> 
>
https://codereview.chromium.org/1406343008/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1517: static
> PassRefPtrWillBeRawPtr<CSSPrimitiveValue>
> consumeBorderWidth(CSSParserTokenRange& range, CSSParserMode cssParserMode)
> The spec calls this a <line-width> for the border-width properties, so maybe
> consumeLineWidth is better. Otherwise it's slightly confusing because calling
> this isn't sufficient for parsing the border-width property :-)

It is a better name, sure. I had in mind that several properties share this same
functionality and border-width was the most common. However maybe a solution
where we have one actual implementation and various "aliasing" functions calling
it, for the various properties, is good to enough. I guess we'll see when we add
more properties needing this functionality.

[Timothy Loh, 2015-11-06 00:31:19]

On 2015/11/06 00:25:19, rwlbuis wrote:
> On 2015/11/05 05:08:26, Timothy Loh wrote:
> > lgtm
> > 
> >
>
https://codereview.chromium.org/1406343008/diff/60001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
(right):
> > 
> >
>
https://codereview.chromium.org/1406343008/diff/60001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1517: static
> > PassRefPtrWillBeRawPtr<CSSPrimitiveValue>
> > consumeBorderWidth(CSSParserTokenRange& range, CSSParserMode cssParserMode)
> > The spec calls this a <line-width> for the border-width properties, so maybe
> > consumeLineWidth is better. Otherwise it's slightly confusing because
calling
> > this isn't sufficient for parsing the border-width property :-)
> 
> It is a better name, sure. I had in mind that several properties share this
same
> functionality and border-width was the most common. However maybe a solution
> where we have one actual implementation and various "aliasing" functions
calling
> it, for the various properties, is good to enough. I guess we'll see when we
add
> more properties needing this functionality.

I'm not sure if we'll need this. I guess if there are properties which are
defined to parse exactly as other properties we can just call their relevant
consume functions (or put the case statements next to each other) for now, it
seems pretty simple to me.

BTW, border-width and its longhands are actually a tiny bit different from
outline-width, they allow quirky lengths.

[rwlbuis, 2015-11-06 04:09:01]

On 2015/11/06 00:31:19, Timothy Loh wrote:
> I'm not sure if we'll need this. I guess if there are properties which are
> defined to parse exactly as other properties we can just call their relevant
> consume functions (or put the case statements next to each other) for now, it
> seems pretty simple to me.

Ah right, per-css-type sounds good too, so we just follow the css definitions.

> BTW, border-width and its longhands are actually a tiny bit different from
> outline-width, they allow quirky lengths.

Good to know.