Move -webkit-filter/background-filter properties into CSSPropertyParser

Move -webkit-filter/background-filter properties into CSSPropertyParser

Move -webkit-filter/background-filter property handling from
LegacyCSSPropertyParser into CSSPropertyParser.

This patch also makes consumeAngle handle unitless
values (like consumeLength already does).

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

[rwlbuis, 2015-10-29 14:20:53]

Description was changed from

==========
filter WIP


Move two color related properties into CSSPropertyParser

Move two color related properties handling from
LegacyCSSPropertyParser into CSSPropertyParser.

BUG=499780

patch from issue 1415953002 at patchset 100001
(http://crrev.com/1415953002#ps100001)
==========

to

==========
Move filter property into CSSPropertyParser
    
Move filter property handling from
LegacyCSSPropertyParser into CSSPropertyParser.
    
BUG=499780
==========

[rwlbuis, 2015-10-29 20:38:48]

Description was changed from

==========
Move filter property into CSSPropertyParser
    
Move filter property handling from
LegacyCSSPropertyParser into CSSPropertyParser.
    
BUG=499780
==========

to

==========
Move filter property into CSSPropertyParser
    
Move filter property handling from
LegacyCSSPropertyParser into CSSPropertyParser.

This patch also teaches consumeAngle to handle
unitless values (like consumeLength already does).

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

[rwlbuis, 2015-10-29 20:40:35]

Description was changed from

==========
Move filter property into CSSPropertyParser
    
Move filter property handling from
LegacyCSSPropertyParser into CSSPropertyParser.

This patch also teaches consumeAngle to handle
unitless values (like consumeLength already does).

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

to

==========
Move filter property into CSSPropertyParser
    
Move filter property handling from
LegacyCSSPropertyParser into CSSPropertyParser.

This patch also teaches consumeAngle to handle
unitless values (like consumeLength already does).

The test box-shadow-parsing-invalid.html is
adjusted to test for extraneous comma's.

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

[rwlbuis, 2015-10-29 20:41:35]

Description was changed from

==========
Move filter property into CSSPropertyParser
    
Move filter property handling from
LegacyCSSPropertyParser into CSSPropertyParser.

This patch also teaches consumeAngle to handle
unitless values (like consumeLength already does).

The test box-shadow-parsing-invalid.html is
adjusted to test for extraneous comma's.

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

to

==========
Move filter property into CSSPropertyParser
    
Move filter property handling from LegacyCSSPropertyParser
into CSSPropertyParser.

This patch also teaches consumeAngle to handle unitless values (like
consumeLength already does).

The test box-shadow-parsing-invalid.html is adjusted
to test for extraneous comma's.

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

[rwlbuis, 2015-10-29 20:41:48]

Description was changed from

==========
Move filter property into CSSPropertyParser
    
Move filter property handling from LegacyCSSPropertyParser
into CSSPropertyParser.

This patch also teaches consumeAngle to handle unitless values (like
consumeLength already does).

The test box-shadow-parsing-invalid.html is adjusted
to test for extraneous comma's.

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

to

==========
Move filter property into CSSPropertyParser
    
Move filter property handling from LegacyCSSPropertyParser
into CSSPropertyParser.

This patch also teaches consumeAngle to handle unitless
values (like consumeLength already does).

The test box-shadow-parsing-invalid.html is adjusted
to test for extraneous comma's.

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

[rwlbuis, 2015-10-29 20:42:49]

Description was changed from

==========
Move filter property into CSSPropertyParser
    
Move filter property handling from LegacyCSSPropertyParser
into CSSPropertyParser.

This patch also teaches consumeAngle to handle unitless
values (like consumeLength already does).

The test box-shadow-parsing-invalid.html is adjusted
to test for extraneous comma's.

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

to

==========
Move filter property into CSSPropertyParser
    
Move filter property handling from LegacyCSSPropertyParser
into CSSPropertyParser.

This patch also makes consumeAngle handle unitless
values (like consumeLength already does).

The test box-shadow-parsing-invalid.html is adjusted
to test for extraneous comma's.

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

[rwlbuis, 2015-10-30 03:15:52]

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

[rwlbuis, 2015-10-30 03:15:53]

PTAL. this one actually removes a lot of code :)

[rwlbuis, 2015-10-30 03:16:30]

Description was changed from

==========
Move filter property into CSSPropertyParser
    
Move filter property handling from LegacyCSSPropertyParser
into CSSPropertyParser.

This patch also makes consumeAngle handle unitless
values (like consumeLength already does).

The test box-shadow-parsing-invalid.html is adjusted
to test for extraneous comma's.

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

to

==========
Move filter property into CSSPropertyParser
    
Move filter property handling from LegacyCSSPropertyParser
into CSSPropertyParser.

This patch also makes consumeAngle handle unitless
values (like consumeLength already does).

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

[Timothy Loh, 2015-11-02 00:14:07]

Patch title/description should probably say that this is the -webkit-filter and
background-filter properties, not filter.

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1425: static
PassRefPtrWillBeRawPtr<CSSFunctionValue>
parseBuiltinFilterArguments(CSSParserTokenRange& range,  const CSSParserContext&
context)
consumeFilterFunction?

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1434: if
(filterType == CSSValueDropShadow) { // drop-shadow() takes a single shadow.
I don't really like any of the comments here. This is pretty obvious from the
code, and the comments about the default values are a bit confusing since the
defaulting logic is not here but in the StyleBuilder. I guess we can keep the
fixmes for calcs though :/

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1437: if
(args.atEnd())
This is non-standard btw, I guess I can take a TODO here.

// TODO(timloh): Add UseCounters for empty filter arguments

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1475: while
(!range.atEnd()) {
do while, to guarantee we always have at least one list entry

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1476: String url
= consumeUrl(range);
not sure why urls are special and not part of the other function?

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1477: // See if
the specified primitive is one we understand.
this comment isn't useful either

[rwlbuis, 2015-11-02 00:52:43]

Description was changed from

==========
Move filter property into CSSPropertyParser
    
Move filter property handling from LegacyCSSPropertyParser
into CSSPropertyParser.

This patch also makes consumeAngle handle unitless
values (like consumeLength already does).

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

to

==========
Move -webkit-filter/background-filter properties into CSSPropertyParser
    
Move -webkit-filter/background-filter property handling from
LegacyCSSPropertyParser into CSSPropertyParser.

This patch also makes consumeAngle handle unitless
values (like consumeLength already does).

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

[rwlbuis, 2015-11-02 02:07:51]

PTAL.

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1425: static
PassRefPtrWillBeRawPtr<CSSFunctionValue>
parseBuiltinFilterArguments(CSSParserTokenRange& range,  const CSSParserContext&
context)
On 2015/11/02 00:14:07, Timothy Loh wrote:
> consumeFilterFunction?

Done.

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1434: if
(filterType == CSSValueDropShadow) { // drop-shadow() takes a single shadow.
On 2015/11/02 00:14:07, Timothy Loh wrote:
> I don't really like any of the comments here. This is pretty obvious from the
> code, and the comments about the default values are a bit confusing since the
> defaulting logic is not here but in the StyleBuilder. I guess we can keep the
> fixmes for calcs though :/

Done.

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1437: if
(args.atEnd())
On 2015/11/02 00:14:07, Timothy Loh wrote:
> This is non-standard btw, I guess I can take a TODO here.
> 
> // TODO(timloh): Add UseCounters for empty filter arguments

Done.

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1475: while
(!range.atEnd()) {
On 2015/11/02 00:14:07, Timothy Loh wrote:
> do while, to guarantee we always have at least one list entry

Done.

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1476: String url
= consumeUrl(range);
On 2015/11/02 00:14:07, Timothy Loh wrote:
> not sure why urls are special and not part of the other function?

There is no real reason for it, though they are listed as separate entities:
https://developer.mozilla.org/en-US/docs/Web/CSS/filter

I am fine either way, let me know.

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1477: // See if
the specified primitive is one we understand.
On 2015/11/02 00:14:07, Timothy Loh wrote:
> this comment isn't useful either

Done.

[rwlbuis, 2015-11-02 02:08:45]

Description was changed from

==========
Move -webkit-filter/background-filter properties into CSSPropertyParser
    
Move -webkit-filter/background-filter property handling from
LegacyCSSPropertyParser into CSSPropertyParser.

This patch also makes consumeAngle handle unitless
values (like consumeLength already does).

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

to

==========
Move -webkit-filter/background-filter properties into CSSPropertyParser
    
Move -webkit-filter/background-filter property handling from
LegacyCSSPropertyParser into CSSPropertyParser.

This patch also makes consumeAngle handle unitless
values (like consumeLength already does).

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

[Timothy Loh, 2015-11-02 02:17:01]

lgtm

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1412613005/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1476: String url
= consumeUrl(range);
On 2015/11/02 02:07:50, rwlbuis wrote:
> On 2015/11/02 00:14:07, Timothy Loh wrote:
> > not sure why urls are special and not part of the other function?
> 
> There is no real reason for it, though they are listed as separate entities:
> https://developer.mozilla.org/en-US/docs/Web/CSS/filter
> 
> I am fine either way, let me know.

I guess this is fine, esp. as the spec has <filter-function> which matches the
name consumeFilterFunction :)

[rwlbuis, 2015-11-02 02:39:01]

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

[commit-bot: I haz the power, 2015-11-02 02:39:06]

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

[commit-bot: I haz the power, 2015-11-02 04:05:06]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-11-02 04:05:59]

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