[webkit-reviews] review granted: [Bug 93295] CSSParser::parseTransform() refactor to accept valueList as argument : [Attachment 156987] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Michelangelo De Simone
<michelangelo at webkit.org>'s request for review:
Bug 93295: CSSParser::parseTransform() refactor to accept valueList as argument
https://bugs.webkit.org/show_bug.cgi?id=93295

Attachment 156987: Patch
https://bugs.webkit.org/attachment.cgi?id=156987&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=156987&action=review


Seems OK.

> Source/WebCore/ChangeLog:3
> +	   CSSParser::parseTransform() refactor to accept valueList as argument


This says “what”, but should say “why”.

> Source/WebCore/ChangeLog:8
> +	   parseTransform now accepts a CSSParserValueList as argument instead
of relying on m_valueList.

This repeats the bug title without really adding anything.

> Source/WebCore/css/CSSParser.cpp:2336
> -	       PassRefPtr<CSSValue> val = parseTransform();
> +	       RefPtr<CSSValue> val = parseTransform(m_valueList.get());

If touching this code would be better to replace “val” with a word, either
“value” or “transform”.

> Source/WebCore/css/CSSParser.cpp:2338
>		   addProperty(propId, val, important);

Should say val.release() instead of just val since this is now a RefPtr rather
than a PassRefPtr.