[webkit-reviews] review granted: [Bug 124618] [css shapes] Parse new circle shape syntax : [Attachment 217454] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Bem Jones-Bey
<bjonesbe at adobe.com>'s request for review:
Bug 124618: [css shapes] Parse new circle shape syntax
https://bugs.webkit.org/show_bug.cgi?id=124618

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

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=217454&action=review


ok though i don't really see how the new syntax is better

> Source/WebCore/css/BasicShapeFunctions.cpp:85
> +	   return pool.createValue(center.length(), style);

Not really in this patch but we need to get rid of CSSValues that take style.
The factoring is just totally wrong. CSSValues are the input for constructing
RenderStyle!

> Source/WebCore/css/CSSBasicShapes.cpp:130
> +    char opening[] = "circle(";
> +    char at[] = "at";
> +    char separator[] = " ";
> +    StringBuilder result;
> +    // Compute the required capacity in advance to reduce allocations.
> +    result.reserveCapacity((sizeof(opening) - 1) + (3 * (sizeof(separator) -
1)) + 1 + radius.length() + sizeof(at) + centerX.length() + centerY.length());

There are probably some more modern ways to do this though I don't know what
the recommended way is currently.