SVGElement::parseAttribute resolves property via property map (Step 2)

SVGElement::parseAttribute resolves property via property map (Step 2)

This is to replace the existing |SVGElement::parseAttribute| implementation,
but as there are many callers, we would like to proceed this incrementally.
a) Introduce |SVGElement::parseAttributeNew| *LANDED*
b) Convert all overrides |SVGElement::parseAttribute| to
   a |parseAttributeNew| call. <-- This patch
c) Replace existing |parseAttribute| with |parseAttributeNew|, and remove all
   overrides simply calling |parseAttributeNew|

This patch is (b).

Bonus: Remove {parseAttribute,isSupportedAttribute,svgAttributeChanged} on SVGGElement which was a noop.

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

[kouhei (in TOK), 2014-07-29 04:27:17]

Would you take a look?

[pdr., 2014-07-29 05:07:10]

On 2014/07/29 04:27:17, kouhei wrote:
> Would you take a look?

This patch: LGTM.

This is the first time I've seen this work, and I like it, but I have a few
questions:
1) Will this model work well for nontrivial parseAttributes such as
SVGSVGElement's or SVGFontFaceUriElement's?
2) In SVGClipPathElement::svgAttributeChanged you are checking for all of the
supported attributes. How will that work for more complex cases such as
SVGFELightElement's 10 supported attributes?

[kouhei (in TOK), 2014-07-29 05:20:19]

Thanks for your comments!

> This is the first time I've seen this work, and I like it, but I have a few
> questions:
> 1) Will this model work well for nontrivial parseAttributes such as
> SVGSVGElement's or SVGFontFaceUriElement's?
Moving to a single |parseAttributeNew| call is only possible iff
all attributes for the element is backed by SVGAnimatedProperty.
For elements with attributes not backed with SVGAnimatedProperty,
I plan to override parseAttribute() on those classes as it is currently done.
Those parseAttribute() will still be simpler as SVGAnimatedProperty part can be
delegate to |parseAttributeNew|.

> 2) In SVGClipPathElement::svgAttributeChanged you are checking for all of the
> supported attributes. How will that work for more complex cases such as
> SVGFELightElement's 10 supported attributes?
I plan to keep isSupportedAttribute for SVGFELightElement.
For now I'm only removing isSupportedAttribute which:
only has one attribute to compare with &&
is only used from svgAttributeChanged.

In future, I think it is also possible to make
{isSupportedAttribute,selfHasRelativeLengths}
simpler and faster by using property map info.

[fs, 2014-07-29 08:12:16]

LGTM

[kouhei (in TOK), 2014-07-29 08:13:31]

The CQ bit was checked by kouhei@chromium.org

[kouhei (in TOK), 2014-07-29 08:13:49]

Thanks for review!

[commit-bot: I haz the power, 2014-07-29 08:14:56]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kouhei@chromium.org/420193011/1

[commit-bot: I haz the power, 2014-07-29 08:18:45]

Change committed as 179105