Fix small bugs in new CSS Property parser

Fix small bugs in new CSS Property parser

Fix small bugs in new CSS Property parser:

- for flex and -webkit-columns, reject whitespace only values.
- for will-change, fix roundtripping by generating a CSSPrimitiveValue.
- for counter-increment, serialize using space separated lists and integer type [1].
- for cursor, require a keyword after the comma.

Add various test for these fixes.

BUG=568435
[1] https://drafts.csswg.org/css-lists-3/#counter-properties

Committed: https://crrev.com/a20f0a89a92c3e94cd35a4026fafd5653bd6b43b
Cr-Commit-Position: refs/heads/master@{#368028}

[rwlbuis, 2015-12-21 22:54:30]

Description was changed from

==========
invalid decls

BUG=568435
==========

to

==========
Fix small bugs in new CSS Property parser

Fix small bugs in new CSS Property parser:

- for flex and -webkit-columns, reject whitespace only values.
- for will-change, fix roundtripping by generating a CSSPrimitiveValue.
- for counter-increment, serialize using space separated lists and integer type
[1].
- for cursor, require a keyword after the comma.

Add various test for these fixes.

BUG=568435

[1] https://drafts.csswg.org/css-lists-3/#counter-properties
==========

[rwlbuis, 2016-01-05 04:51:56]

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

[rwlbuis, 2016-01-05 04:51:57]

PTAL.

[Timothy Loh, 2016-01-06 00:14:52]

https://codereview.chromium.org/1541443002/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/css-parser/whitespace-declaration.html
(right):

https://codereview.chromium.org/1541443002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/css-parser/whitespace-declaration.html:5:
<script>
Can we use testharness instead of js-test? Otherwise people will need to touch
one more file when adding new properties.

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

https://codereview.chromium.org/1541443002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:2387: if
(!consumeCommaIncludingWhitespace(range) || range.atEnd())
I think there's a better way to fix this (see below).

https://codereview.chromium.org/1541443002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:2406: }
I think } else { return nullptr; } here is better (this makes the if
(cursorType) below is always true). In particular, this bit isn't optional and I
think it's clearer to enforce that here instead of the loop above.

We can also change the while loop to while (true).

[rwlbuis, 2016-01-06 08:11:03]

I addressed the review comments, PTAL.

https://codereview.chromium.org/1541443002/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/css-parser/whitespace-declaration.html
(right):

https://codereview.chromium.org/1541443002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/css-parser/whitespace-declaration.html:5:
<script>
On 2016/01/06 00:14:52, Timothy Loh wrote:
> Can we use testharness instead of js-test? Otherwise people will need to touch
> one more file when adding new properties.

Done.

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

https://codereview.chromium.org/1541443002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:2387: if
(!consumeCommaIncludingWhitespace(range) || range.atEnd())
On 2016/01/06 00:14:52, Timothy Loh wrote:
> I think there's a better way to fix this (see below).

Acknowledged.

https://codereview.chromium.org/1541443002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:2406: }
On 2016/01/06 00:14:52, Timothy Loh wrote:
> I think } else { return nullptr; } here is better (this makes the if
> (cursorType) below is always true). In particular, this bit isn't optional and
I
> think it's clearer to enforce that here instead of the loop above.
> 
> We can also change the while loop to while (true).
> 

Done.

[Timothy Loh, 2016-01-07 03:15:04]

lgtm, sorry for the delay (and I know you still have a patch up I need to look
at)

[rwlbuis, 2016-01-07 06:10:27]

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

[commit-bot: I haz the power, 2016-01-07 06:10:53]

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

[commit-bot: I haz the power, 2016-01-07 07:42:40]

Description was changed from

==========
Fix small bugs in new CSS Property parser

Fix small bugs in new CSS Property parser:

- for flex and -webkit-columns, reject whitespace only values.
- for will-change, fix roundtripping by generating a CSSPrimitiveValue.
- for counter-increment, serialize using space separated lists and integer type
[1].
- for cursor, require a keyword after the comma.

Add various test for these fixes.

BUG=568435

[1] https://drafts.csswg.org/css-lists-3/#counter-properties
==========

to

==========
Fix small bugs in new CSS Property parser

Fix small bugs in new CSS Property parser:

- for flex and -webkit-columns, reject whitespace only values.
- for will-change, fix roundtripping by generating a CSSPrimitiveValue.
- for counter-increment, serialize using space separated lists and integer type
[1].
- for cursor, require a keyword after the comma.

Add various test for these fixes.

BUG=568435

[1] https://drafts.csswg.org/css-lists-3/#counter-properties
==========

[commit-bot: I haz the power, 2016-01-07 07:42:40]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-01-07 07:43:57]

Description was changed from

==========
Fix small bugs in new CSS Property parser

Fix small bugs in new CSS Property parser:

- for flex and -webkit-columns, reject whitespace only values.
- for will-change, fix roundtripping by generating a CSSPrimitiveValue.
- for counter-increment, serialize using space separated lists and integer type
[1].
- for cursor, require a keyword after the comma.

Add various test for these fixes.

BUG=568435

[1] https://drafts.csswg.org/css-lists-3/#counter-properties
==========

to

==========
Fix small bugs in new CSS Property parser

Fix small bugs in new CSS Property parser:

- for flex and -webkit-columns, reject whitespace only values.
- for will-change, fix roundtripping by generating a CSSPrimitiveValue.
- for counter-increment, serialize using space separated lists and integer type
[1].
- for cursor, require a keyword after the comma.

Add various test for these fixes.

BUG=568435

[1] https://drafts.csswg.org/css-lists-3/#counter-properties

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

[commit-bot: I haz the power, 2016-01-07 07:43:58]

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