Parse new circle shape syntax

Parse new circle shape syntax

Implement parsing of the new circle shape syntax. The implementation of
the old syntax has been move aside as deprecated, and will be removed
once the new syntax is stable.

Updated existing parsing tests to cover this.

Patch by Bem Jones-Bey.

BUG=322165
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=164131

[Bem Jones-Bey (adobe), 2013-12-18 21:57:57]

Generally looks good, but there were a couple of minor bug fixes that went in
after the initial WebKit patch that you should check to make sure you've
included. I know that one was missed (the || instead of the && that I mention in
my comments on this patch). I don't see much value in having those be individual
Blink patches.

Also, on your issue description, my last name is Jones-Bey, not Bey-Jones. :-)

https://codereview.chromium.org/98723006/diff/40001/Source/core/css/CSSBasicS...
File Source/core/css/CSSBasicShapes.h (right):

https://codereview.chromium.org/98723006/diff/40001/Source/core/css/CSSBasicS...
Source/core/css/CSSBasicShapes.h:49: CSSBasicShapeCircleType = 6
The explicit numbers were removed in a subsequent patch in WebKit. Any reason
not to just blend that into this patch?

https://codereview.chromium.org/98723006/diff/40001/Source/core/css/CSSParser...
File Source/core/css/CSSParser-in.cpp (right):

https://codereview.chromium.org/98723006/diff/40001/Source/core/css/CSSParser...
Source/core/css/CSSParser-in.cpp:5162: if (value->id == CSSValueClosestSide &&
value->id == CSSValueFarthestSide)
This should be ||, not &&.

https://codereview.chromium.org/98723006/diff/40001/Source/core/rendering/sty...
File Source/core/rendering/style/BasicShapes.h (right):

https://codereview.chromium.org/98723006/diff/40001/Source/core/rendering/sty...
Source/core/rendering/style/BasicShapes.h:55: BasicShapeCircleType = 6
The explicit numbers got removed in a subsequent patch in WebKit. Any reason not
to just do that here in this patch?

[rwlbuis, 2013-12-18 22:21:16]

On 2013/12/18 21:57:57, Bem Jones-Bey (adobe) wrote:
> Generally looks good, but there were a couple of minor bug fixes that went in
> after the initial WebKit patch that you should check to make sure you've
> included. I know that one was missed (the || instead of the && that I mention
in
> my comments on this patch). I don't see much value in having those be
individual
> Blink patches.
> 
> Also, on your issue description, my last name is Jones-Bey, not Bey-Jones. :-)

Sorry! Fixed.

>
https://codereview.chromium.org/98723006/diff/40001/Source/core/css/CSSBasicS...
> File Source/core/css/CSSBasicShapes.h (right):
> 
>
https://codereview.chromium.org/98723006/diff/40001/Source/core/css/CSSBasicS...
> Source/core/css/CSSBasicShapes.h:49: CSSBasicShapeCircleType = 6
> The explicit numbers were removed in a subsequent patch in WebKit. Any reason
> not to just blend that into this patch?

Not really, I thought maybe rebasing the next patch (new ellipse syntax) may be
harder, but I guess it won't be. Fixed.

>
https://codereview.chromium.org/98723006/diff/40001/Source/core/css/CSSParser...
> File Source/core/css/CSSParser-in.cpp (right):
> 
>
https://codereview.chromium.org/98723006/diff/40001/Source/core/css/CSSParser...
> Source/core/css/CSSParser-in.cpp:5162: if (value->id == CSSValueClosestSide &&
> value->id == CSSValueFarthestSide)
> This should be ||, not &&.

Well spotted, fixed.

>
https://codereview.chromium.org/98723006/diff/40001/Source/core/rendering/sty...
> File Source/core/rendering/style/BasicShapes.h (right):
> 
>
https://codereview.chromium.org/98723006/diff/40001/Source/core/rendering/sty...
> Source/core/rendering/style/BasicShapes.h:55: BasicShapeCircleType = 6
> The explicit numbers got removed in a subsequent patch in WebKit. Any reason
not
> to just do that here in this patch?

See above :) Fixed.

[Bem Jones-Bey (adobe), 2013-12-18 22:22:22]

lgtm

[rwlbuis, 2013-12-18 22:28:35]

eseidel@chromium.org: This is a patch ported from WebKit, originally done by
Bem.

[eseidel, 2013-12-18 22:33:12]

lgtm

OK.  When do we get to remove the deprecated one?  I presume this is not in the
shipping build?

[eseidel, 2013-12-18 22:33:28]

Rather, I mean, this is not reachable from the drive-by-web, of course it's in
the build. :)

[Bem Jones-Bey (adobe), 2013-12-18 22:36:46]

On 2013/12/18 22:33:28, eseidel wrote:
> Rather, I mean, this is not reachable from the drive-by-web, of course it's in
> the build. :)

The deprecated ones will go away once we've gotten the fully working
implementation of the new syntax landed and updated demos, documentation, etc.
We intend to remove the old syntax much before making the feature available to
the web at large. So we will be removing it as soon as we possibly can. :-)

[commit-bot: I haz the power, 2013-12-18 22:40:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rob.buis@samsung.com/98723006/60001

[commit-bot: I haz the power, 2013-12-19 02:25:13]

Change committed as 164131