Dissect and clean up instances of color parsing in CSSPropertyParser

Dissect and clean up instances of color parsing in CSSPropertyParser

This moves all instances of color parsing (mostly <color> parsing
even) into their separate methods. The new methods (except parseSVGColor)
are formulated using parseColor(). parseColor() itself is combined with
the "most common" color parsing case by folding the keyword case into it.
The new methods are structured as first checking for keywords that they
handle differently from the generic <color> case.
The <quirky-color> determination is moved into a function of its own.
The parsing of <color> for shadow is cleaned up.

Behavioral changes:
 * Accept '-internal-*' in all cases (for UA-sheets).
 * Keep 'grey' as a keyword in all cases.
 * Accept '-webkit-focus-ring-color' in quirks mode and UA-sheets in all
   cases.
 * Keep keywords for fallback colors to 'fill' and 'stroke.

The intention of the change is to make the differences visible, and try
to eliminate the remaining differences in follow-up CLs.

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

[fs, 2015-06-29 11:43:05]

fs@opera.com changed reviewers:
+ timloh@chromium.org

[fs, 2015-06-29 11:43:05]

Initially I tried to keep this free from behavioral changes, but that turned
into quite a mess, so I chose to just accept some additional keywords where it
didn't seem too controversial (like the -internal-* bits, grey and focus-ring).

The follow-ups I'm thinking of ATM are:

 * Remove parseSVGColor in favor of plain parseColor. (Related:
crbug.com/370830)
 * Drop the (strange mix of) exceptions for -webkit-text.
 * Don't know the story of tap-highlight vs. currentcolor, but just allowing it
in non-quirks mode doesn't feel like too much of a stretch goal. (I noticed that
it'll resolve to 'transparent' regardless...)

Somewhat related but future goal:

 * Keep the "extended color keywords" as keywords too. (Will require some
additional massaging to the resolving machinery it seems.)

If you can think of anything else related, let me know and I'll try to look into
it.

[Timothy Loh, 2015-06-30 01:17:59]

lgtm

On 2015/06/29 11:43:05, fs wrote:
> Somewhat related but future goal:
> 
>  * Keep the "extended color keywords" as keywords too. (Will require some
> additional massaging to the resolving machinery it seems.)

If you haven't seen it already, CSSParserFastPaths.cpp has some 300+ lines
(could probably do with some cleaning up..) of a separate color parsing
code-path (from a string, e.g. foo.stye.color = 'red'), so you'll probably want
to make sure that function is consistent with any changes to the property
parser.

[fs, 2015-06-30 13:22:57]

On 2015/06/30 01:17:59, Timothy Loh wrote:
> lgtm
> 
> On 2015/06/29 11:43:05, fs wrote:
> > Somewhat related but future goal:
> > 
> >  * Keep the "extended color keywords" as keywords too. (Will require some
> > additional massaging to the resolving machinery it seems.)
> 
> If you haven't seen it already, CSSParserFastPaths.cpp has some 300+ lines
> (could probably do with some cleaning up..) of a separate color parsing
> code-path (from a string, e.g. foo.stye.color = 'red'), so you'll probably
want
> to make sure that function is consistent with any changes to the property
> parser.

Yepp, knew about it. Thanks for the reminder though - I needed to update the
'grey' keyword handling. I think it should be possible to use isColorKeyword
there eventually.

[fs, 2015-06-30 14:30:53]

The CQ bit was checked by fs@opera.com

[fs, 2015-06-30 14:30:54]

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/1219463003/#ps40001
(title: "Update handling of 'grey' in fast-path color parser.")

[commit-bot: I haz the power, 2015-06-30 14:31:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1219463003/40001

[commit-bot: I haz the power, 2015-06-30 14:34:58]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=198062