Remove one SVG animated type special case from the bindings generator

Remove one SVG animated type special case from the bindings generator

Remove one SVG animated type special case from the bindings generator that
would append 'Animated' to the implementation getter method.
Instead, the implementation methods are renamed to match the IDL attribute
name.

To avoid naming clashes, existing getters were renamed *CurrentValue() since they
return the current value of the animated property. This naming is consistent
with the existing *BaseValue() methods.

IsSVGAnimatedType() subroutine is not removed from the generator yet as it
is still needed. It will be removed in a later commit.

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

[haraken, 2013-07-12 12:11:07]

You're prince!

At a first glance, the change looks reasonable to me. SVG experts wants to take
a look.

> To avoid clashes, existing getters were renamed *CurrentValue() since they
> return the current value of the animated property.

Just to confirm: "To avoid crashes" means "To keep the current behavior as is".
Right?

[do-not-use, 2013-07-12 12:15:58]

On 2013/07/12 12:11:07, haraken wrote:
> You're prince!
> 
> At a first glance, the change looks reasonable to me. SVG experts wants to
take
> a look.
> 
> > To avoid clashes, existing getters were renamed *CurrentValue() since they
> > return the current value of the animated property.
> 
> Just to confirm: "To avoid crashes" means "To keep the current behavior as
is".
> Right?

not *crashes*, naming *clashes* :)

Existing getters were renamed to {attrName}CurrentValue() since they return the
current value of the animated property. {attrName}() now returns the animated
type, which is what the bindings expect.

Of course, there is no exposed behavior change.

[haraken, 2013-07-12 12:25:02]

> > Just to confirm: "To avoid crashes" means "To keep the current behavior as
> is".
> > Right?
> 
> not *crashes*, naming *clashes* :)

Got it :)

[do-not-use, 2013-07-12 12:25:56]

Haraken, the reason I had to keep the IsSVGAnimatedType() subroutine is because
it is still used by CanUseFastAttribute(). We apparently cannot use
fastHasAttribute / fastGetAttribute for attributes returning svg-animated types.

One possible solution would be to introduce a [DoNotUseFastAttribute] IDL
extended attribute and use it for all attributes returning a SVG Animated type.
In theory, it would be reusable for non-SVG types but it does not seems there is
a need for that currently.

What do you think about this proposal? Please let me know if you have other
ideas.

[do-not-use, 2013-07-12 12:27:23]

Hmm, the try bots aren't happy. I must have changed the behavior somehow. I'll
investigate and fix. Sorry about that.

[haraken, 2013-07-12 12:31:13]

On 2013/07/12 12:25:56, Christophe Dumez wrote:
> Haraken, the reason I had to keep the IsSVGAnimatedType() subroutine is
because
> it is still used by CanUseFastAttribute(). We apparently cannot use
> fastHasAttribute / fastGetAttribute for attributes returning svg-animated
types.
> 
> One possible solution would be to introduce a [DoNotUseFastAttribute] IDL
> extended attribute and use it for all attributes returning a SVG Animated
type.
> In theory, it would be reusable for non-SVG types but it does not seems there
is
> a need for that currently.
> 
> What do you think about this proposal? Please let me know if you have other
> ideas.

Introducing some IDL attribute sounds reasonable to me. Maybe we want to remove
hard-coded "getXXXAttribute" in GetterExpression() as well.

[Stephen Chennney, 2013-07-12 14:28:21]

The try bots look like issues not related to your changes.

LGTM

https://codereview.chromium.org/19097005/diff/10001/Source/core/svg/SVGTextCo...
File Source/core/svg/SVGTextContentElement.h (right):

https://codereview.chromium.org/19097005/diff/10001/Source/core/svg/SVGTextCo...
Source/core/svg/SVGTextContentElement.h:92: PassRefPtr<SVGAnimatedLength>
textLength();
This is unused. Apparently we do not support it. I suppose we should leave it
in, but please file a bug so we can figure out if we indeed do not support or if
we are elsewhere calling the wrong method.

[commit-bot: I haz the power, 2013-07-12 14:28:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/19097005/10001

[commit-bot: I haz the power, 2013-07-12 15:21:37]

Change committed as 154121