[webkit-reviews] review granted: [Bug 89898] Fast path for simple transform parsing : [Attachment 149337] patch

[bugzilla-daemon at webkit.org]

Anders Carlsson <andersca at apple.com> has granted Antti Koivisto
<koivisto at iki.fi>'s request for review:
Bug 89898: Fast path for simple transform parsing
https://bugs.webkit.org/show_bug.cgi?id=89898

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

------- Additional Comments from Anders Carlsson <andersca at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=149337&action=review


> Source/WebCore/css/CSSParser.cpp:1009
> +static bool parseTransformArguments(PassRefPtr<WebKitCSSTransformValue>
transformValue, CharType* characters, unsigned length, unsigned start, unsigned
expectedCount)
> +{

I don't think this needs to take a PassRefPtr, just a raw pointer would be
fine.

> Source/WebCore/css/CSSParser.cpp:1033
> +    // Very long strings probably have multiple components which we don't
handle here.
> +    if (string.length() < 13 || string.length() > 32)

I'd be nice if you explained the less than check as well.

> Source/WebCore/css/CSSParser.cpp:1038
> +    UChar c9 = toASCIILowerUnchecked(string[9]);
> +    UChar c10 = toASCIILowerUnchecked(string[10]);

Is it OK to use unchecked here?