CSSParser: Avoid creating dummy declaration in parseColor() slow path.

• Component: CSS
• Status: RESOLVED FIXED

[Andreas Kling]

The slow-path in CSSParser::parseColor() currently creates a CSSMutableStyleDeclaration in order to trick the CSSParser into instantiating a local CSSValuePool. We shouldn't have to do that.

[Andreas Kling]

Created attachment 120354 [details]
Proposed patch

[WebKit Review Bot]

Attachment 120354 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/css/CSSParser.h:154:  The parameter name "rgb" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 120354 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=120354&action=review

> Source/WebCore/css/CSSParser.cpp:520
> +    ensureCSSValuePool();

Since it’s so non-obvious that this call is needed, I think we should add a comment to explain why it is.

> Source/WebCore/css/CSSParser.cpp:663
> +void CSSParser::ensureCSSValuePool()
> +{
> +    if (!m_cssValuePool)
> +        m_cssValuePool = CSSValuePool::create();
> +}

Seems a little strange to add a function like this that is only called in one place. I probably would have put it at the top of the source file and marked it inline.

>> Source/WebCore/css/CSSParser.h:154
>> +    static bool fastParseColor(const String&, RGBA32& rgb, bool strict);
> 
> The parameter name "rgb" adds no information, so it should be removed.  [readability/parameter_name] [5]

I agree with Mr. Style Queue. We should either use no name or the name color.

I also probably would have put the out parameter either last or first, rather than between two other arguments. It seems crazy, but this used to be distinguished from the real full parseColor only because it had arguments in a different order. We should put the arguments in the same order.

[Andreas Kling]

Committed r103639: <http://trac.webkit.org/changeset/103639>