[webkit-reviews] review granted: [Bug 42965] CSS: Add fast-path for rgba() color parsing : [Attachment 63446] Proposed patch v4

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 16 16:30:03 PDT 2010


Darin Adler <darin at apple.com> has granted Andreas Kling
<andreas.kling at nokia.com>'s request for review:
Bug 42965: CSS: Add fast-path for rgba() color parsing
https://bugs.webkit.org/show_bug.cgi?id=42965

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

------- Additional Comments from Darin Adler <darin at apple.com>
> +    int length = end - string;
> +    if (length < 2)
> +	   return false;

So just plain "0" and "1" are not in the fast path?

> +    if ((length == 4 && string[0] == '0' && string[1] == '.' &&
isASCIIDigit(string[2])
> +	   || (length == 3 && string[0] == '.' && isASCIIDigit(string[1])))) {

Might be nice to use a helper function just to make this if statement easier to
read.

> +	   switch (string[length - 2]) {
> +	   case '0': value = 0; break;
> +	   case '1': value = 25; break;
> +	   case '2': value = 51; break;
> +	   case '3': value = 76; break;
> +	   case '4': value = 102; break;
> +	   case '5': value = 127; break;
> +	   case '6': value = 153; break;
> +	   case '7': value = 179; break;
> +	   case '8': value = 204; break;
> +	   case '9': value = 230; break;
> +	   }

Seems like a small array would work better here than a switch.

> +    Vector<char, 256> bytes(length + 1);

The number 256 seems a little high here.

r=me


More information about the webkit-reviews mailing list