[SVG] SVGAnimatedTransform{,List} migration to new SVG property impl.

[SVG] SVGAnimatedTransform{,List} migration to new SVG property impl.

This CL replaces SVGAnimatedTransform{,List} with NewSVGProperty based implementation, and replaces SVGMatrixTearOff implementation.

The class hierarchy is quite complicated, so please refer to diagram: https://docs.google.com/a/chromium.org/document/d/1bg7CUyUszqdwmENY3JX6_PoQD6uHRCNcRPJMlC4qlkw/edit#heading=h.c8zvu36koui6

This patch slightly modifies how "SVG fragment identifiers" are parsed. When multiple transform()s are specified, only the last transform() will be used. This behavior is spec conformant: http://www.w3.org/TR/SVG/single-page.html#linking-SVGFragmentIdentifiers

For details, see design doc: https://docs.google.com/document/d/1bg7CUyUszqdwmENY3JX6_PoQD6uHRCNcRPJMlC4qlkw/edit?usp=sharing

NOTRY=true
BUG=308818
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=167565

[kouhei (in TOK), 2014-02-10 12:30:36]

haraken, pdr: Would you take a look?

[kouhei (in TOK), 2014-02-10 12:33:07]

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/properties/S...
File Source/core/svg/properties/SVGPropertyInfo.h (right):

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/properties/S...
Source/core/svg/properties/SVGPropertyInfo.h:45: AnimatedMatrix,
This seems not needed. Will remove this tomorrow.

[kouhei (in TOK), 2014-02-10 12:37:20]

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/properties/N...
File Source/core/svg/properties/NewSVGAnimatedProperty.h (right):

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/properties/N...
Source/core/svg/properties/NewSVGAnimatedProperty.h:56: virtual
PassRefPtr<NewSVGPropertyBase> createAnimatedValue(SVGAnimationElement*) = 0;
I will revert this interface change. This approach was not used.

[haraken, 2014-02-11 17:00:20]

I scanned half of the CL, but the CL is huge and a bit hard to understand. In
particular, I don't quite understand the relationship between SVGMatrix and
SVGTransform(TearOff) in the new code. Let's discuss offline tomorrow.

Also it would be great if you could explain what this CL is doing in the CL
description.

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGAnimatedN...
File Source/core/svg/SVGAnimatedNewPropertyAnimator.cpp (right):

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGAnimatedN...
Source/core/svg/SVGAnimatedNewPropertyAnimator.cpp:69: if (m_type ==
AnimatedTransformList) {

Would you add a comment about why we need to treat AnimatedTransformList
specially?

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGAnimatedT...
File Source/core/svg/SVGAnimatedType.cpp (right):

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGAnimatedT...
Source/core/svg/SVGAnimatedType.cpp:71: case AnimatedMatrix:

Alphabetical order please.

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGAnimatedT...
Source/core/svg/SVGAnimatedType.cpp:155: case AnimatedMatrix:

Ditto.

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGAnimatedT...
Source/core/svg/SVGAnimatedType.cpp:193: case AnimatedMatrix:

Ditto.

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGAnimatorF...
File Source/core/svg/SVGAnimatorFactory.h (right):

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGAnimatorF...
Source/core/svg/SVGAnimatorFactory.h:72: //
SVGAnimated{Matrix,Point,StringListTransform,} does not exist.

StringList,Transform

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGGraphicsE...
File Source/core/svg/SVGGraphicsElement.idl (left):

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGGraphicsE...
Source/core/svg/SVGGraphicsElement.idl:40: [RaisesException] SVGMatrix
getTransformToElement([Default=Undefined] optional SVGElement element);

This will change web-exposed behavior. Is this intentional?

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGPointList...
File Source/core/svg/SVGPointList.cpp (left):

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGPointList...
Source/core/svg/SVGPointList.cpp:108: skipOptionalSVGSpaces(ptr, end);

Why did you remove this?

[kouhei (in TOK), 2014-02-12 01:57:48]

haraken: PTAL.

I've documented the class hierarchy in
https://docs.google.com/a/chromium.org/document/d/1bg7CUyUszqdwmENY3JX6_PoQD6...

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGAnimatedN...
File Source/core/svg/SVGAnimatedNewPropertyAnimator.cpp (right):

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGAnimatedN...
Source/core/svg/SVGAnimatedNewPropertyAnimator.cpp:69: if (m_type ==
AnimatedTransformList) {
On 2014/02/11 17:00:21, haraken wrote:
> 
> Would you add a comment about why we need to treat AnimatedTransformList
> specially?

Done.

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGAnimatedT...
File Source/core/svg/SVGAnimatedType.cpp (right):

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGAnimatedT...
Source/core/svg/SVGAnimatedType.cpp:71: case AnimatedMatrix:
On 2014/02/11 17:00:21, haraken wrote:
> 
> Alphabetical order please.
Removed matrix.

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGAnimatorF...
File Source/core/svg/SVGAnimatorFactory.h (right):

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGAnimatorF...
Source/core/svg/SVGAnimatorFactory.h:72: //
SVGAnimated{Matrix,Point,StringListTransform,} does not exist.
On 2014/02/11 17:00:21, haraken wrote:
> 
> StringList,Transform

Done.

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGGraphicsE...
File Source/core/svg/SVGGraphicsElement.idl (left):

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGGraphicsE...
Source/core/svg/SVGGraphicsElement.idl:40: [RaisesException] SVGMatrix
getTransformToElement([Default=Undefined] optional SVGElement element);
Yes. Previous def was wrong.

http://www.w3.org/TR/SVG/single-page.html#types-__svg__SVGLocatable__getTrans...

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGPointList...
File Source/core/svg/SVGPointList.cpp (left):

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/SVGPointList...
Source/core/svg/SVGPointList.cpp:108: skipOptionalSVGSpaces(ptr, end);
On 2014/02/11 17:00:21, haraken wrote:
> 
> Why did you remove this?

This was redundant. There is a call already in line 98.

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/properties/N...
File Source/core/svg/properties/NewSVGAnimatedProperty.h (right):

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/properties/N...
Source/core/svg/properties/NewSVGAnimatedProperty.h:56: virtual
PassRefPtr<NewSVGPropertyBase> createAnimatedValue(SVGAnimationElement*) = 0;
On 2014/02/10 12:37:21, kouhei wrote:
> I will revert this interface change. This approach was not used.

Done.

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/properties/S...
File Source/core/svg/properties/SVGPropertyInfo.h (right):

https://codereview.chromium.org/153883003/diff/1/Source/core/svg/properties/S...
Source/core/svg/properties/SVGPropertyInfo.h:45: AnimatedMatrix,
On 2014/02/10 12:33:08, kouhei wrote:
> This seems not needed. Will remove this tomorrow.

Done.

[fs, 2014-02-17 14:14:31]

Looks-good-to-me (non-owner, owners will probably RS rather than read the whole
thing through though ;-)) with some minors nits and some questions (to "cure the
curious").

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGMPat...
File Source/core/svg/SVGMPathElement.cpp (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGMPat...
Source/core/svg/SVGMPathElement.cpp:28: #include "core/svg/SVGElementInstance.h"
Why was this suddenly needed here? I'm guessing due to it being included
indirectly previously, but haven't seen where (or what).

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGMatr...
File Source/core/svg/SVGMatrix.idl (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGMatr...
Source/core/svg/SVGMatrix.idl:30: [RaisesException=Setter] attribute double a;
It might make sense to add a comment here about why we add the [RaisesException]
- it took me more than 2s to figure that out...

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGSVGE...
File Source/core/svg/SVGSVGElement.cpp (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGSVGE...
Source/core/svg/SVGSVGElement.cpp:448: // FIXME: remove after nbarth@'s
[StrictTypeChecking] patch
This should be fixed on trunk/master now. Just need to rebase.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
File Source/core/svg/SVGTransform.h (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
Source/core/svg/SVGTransform.h:24: #include "core/svg/SVGMatrixTearOff.h"
What (in this file) uses this?

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
File Source/core/svg/SVGTransform.idl (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
Source/core/svg/SVGTransform.idl:40: [RaisesException] void setMatrix(SVGMatrix
matrix);
Same here as for SVGMatrix.{a,b,c,d,e,f} I suppose,

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
File Source/core/svg/SVGTransformList.cpp (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
Source/core/svg/SVGTransformList.cpp:129: String aaa(ptr, end-ptr);
Nit: Not used/needed.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
Source/core/svg/SVGTransformList.cpp:174: String(ptr, end-ptr).show();
Nit: Drop.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
Source/core/svg/SVGTransformList.cpp:206: skipOptionalSVGSpaces(ptr, end);
Could also be moved into the if-block, since it's redundant with the preceding
call otherwise (similarly I suspect the first skip* call in the while should be
hoisted. Like in parseTransformParamList.) Not a nit/issue though - just a
"would be nice if...", so feel free to ignore.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
Source/core/svg/SVGTransformList.cpp:228: if (it != itEnd) {
Alternatively:

while (it != itEnd) {
  builder.append(...);
  ++it;
  if (it != itEnd)
    builder.append(' ');
}

(which hopefully results in smaller code.)

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGView...
File Source/core/svg/SVGViewSpec.cpp (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGView...
Source/core/svg/SVGViewSpec.cpp:88: m_transform = 0;
Any reason we can't hold on to these when the context element is detached?
(Looking at the *String() methods above, it look like they don't really handle
this, so arguably that'd go for the other two as well...)

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGView...
Source/core/svg/SVGViewSpec.cpp:179: m_transform->baseValue()->parse(ptr, end);
This appears to drop the "DoNotClearList"-semantic previously. I don't see what
that would've been (good) for though.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/animati...
File Source/core/svg/animation/SVGSMILElement.cpp (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/animati...
Source/core/svg/animation/SVGSMILElement.cpp:37: #include
"core/svg/SVGElementInstance.h"
Suddenly required because...?

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/propert...
File Source/core/svg/properties/NewSVGAnimatedProperty.h (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/propert...
Source/core/svg/properties/NewSVGAnimatedProperty.h:47: class
SVGAnimationElement;
Needed because...?

[pdr., 2014-02-18 00:53:56]

What a patch!

I think the new hierarchy looks good. LGTM with fs@opera's concerns addressed.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
File Source/core/svg/SVGTransformList.cpp (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
Source/core/svg/SVGTransformList.cpp:2: * Copyright (C) 2004, 2005 Nikolas
Zimmermann <zimmermann@kde.org>
We lost a 2008 for Niko

[kouhei (in TOK), 2014-02-18 02:09:12]

Thanks for your comments!

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGMPat...
File Source/core/svg/SVGMPathElement.cpp (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGMPat...
Source/core/svg/SVGMPathElement.cpp:28: #include "core/svg/SVGElementInstance.h"
On 2014/02/17 14:14:31, fs wrote:
> Why was this suddenly needed here? I'm guessing due to it being included
> indirectly previously, but haven't seen where (or what).

Yes. I think this was included via SVGAnimatedTransformList.h ->
SVGAnimatedTypeAnimator.h -> SVGElementInstance.h, but
SVGAnimatedTransformList.h doesn't require SVGAnimatedTypeAnimator anymore.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGMatr...
File Source/core/svg/SVGMatrix.idl (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGMatr...
Source/core/svg/SVGMatrix.idl:30: [RaisesException=Setter] attribute double a;
On 2014/02/17 14:14:31, fs wrote:
> It might make sense to add a comment here about why we add the
[RaisesException]
> - it took me more than 2s to figure that out...

SVGMatrix.[a-f] setter may throw exception when its readonly.
This is a direct translation of the spec:
http://www.w3.org/TR/SVG/single-page.html#coords-InterfaceSVGMatrix

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGSVGE...
File Source/core/svg/SVGSVGElement.cpp (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGSVGE...
Source/core/svg/SVGSVGElement.cpp:448: // FIXME: remove after nbarth@'s
[StrictTypeChecking] patch
On 2014/02/17 14:14:31, fs wrote:
> This should be fixed on trunk/master now. Just need to rebase.

Right! Thanks for pointing out.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
File Source/core/svg/SVGTransform.h (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
Source/core/svg/SVGTransform.h:24: #include "core/svg/SVGMatrixTearOff.h"
On 2014/02/17 14:14:31, fs wrote:
> What (in this file) uses this?

Removed

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
File Source/core/svg/SVGTransform.idl (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
Source/core/svg/SVGTransform.idl:40: [RaisesException] void setMatrix(SVGMatrix
matrix);
On 2014/02/17 14:14:31, fs wrote:
> Same here as for SVGMatrix.{a,b,c,d,e,f} I suppose,
Direct translation of the spec:
http://www.w3.org/TR/SVG/single-page.html#coords-InterfaceSVGTransform

Setter exception raised on an attempt to change the value of a read only
attribute.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
File Source/core/svg/SVGTransformList.cpp (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
Source/core/svg/SVGTransformList.cpp:2: * Copyright (C) 2004, 2005 Nikolas
Zimmermann <zimmermann@kde.org>
On 2014/02/18 00:53:57, pdr wrote:
> We lost a 2008 for Niko

Done.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
Source/core/svg/SVGTransformList.cpp:129: String aaa(ptr, end-ptr);
On 2014/02/17 14:14:31, fs wrote:
> Nit: Not used/needed.

Done.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
Source/core/svg/SVGTransformList.cpp:174: String(ptr, end-ptr).show();
On 2014/02/17 14:14:31, fs wrote:
> Nit: Drop.

Done.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
Source/core/svg/SVGTransformList.cpp:206: skipOptionalSVGSpaces(ptr, end);
On 2014/02/17 14:14:31, fs wrote:
> Could also be moved into the if-block, since it's redundant with the preceding
> call otherwise (similarly I suspect the first skip* call in the while should
be
> hoisted. Like in parseTransformParamList.) Not a nit/issue though - just a
> "would be nice if...", so feel free to ignore.

Done.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGTran...
Source/core/svg/SVGTransformList.cpp:228: if (it != itEnd) {
On 2014/02/17 14:14:31, fs wrote:
> Alternatively:
> 
> while (it != itEnd) {
>   builder.append(...);
>   ++it;
>   if (it != itEnd)
>     builder.append(' ');
> }
> 
> (which hopefully results in smaller code.)

Done.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGView...
File Source/core/svg/SVGViewSpec.cpp (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGView...
Source/core/svg/SVGViewSpec.cpp:88: m_transform = 0;
On 2014/02/17 14:14:31, fs wrote:
> Any reason we can't hold on to these when the context element is detached?
> (Looking at the *String() methods above, it look like they don't really handle
> this, so arguably that'd go for the other two as well...)

These tear-offs' wrappers keep context element alive. They need to be detached
if contextElement dies.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/SVGView...
Source/core/svg/SVGViewSpec.cpp:179: m_transform->baseValue()->parse(ptr, end);
On 2014/02/17 14:14:31, fs wrote:
> This appears to drop the "DoNotClearList"-semantic previously. I don't see
what
> that would've been (good) for though.
True, added to description.
I think this is a correct behavior according to the spec:
http://www.w3.org/TR/SVG/single-page.html#linking-SVGFragmentIdentifiers

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/animati...
File Source/core/svg/animation/SVGSMILElement.cpp (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/animati...
Source/core/svg/animation/SVGSMILElement.cpp:37: #include
"core/svg/SVGElementInstance.h"
On 2014/02/17 14:14:31, fs wrote:
> Suddenly required because...?
Ditto.

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/propert...
File Source/core/svg/properties/NewSVGAnimatedProperty.h (right):

https://codereview.chromium.org/153883003/diff/190001/Source/core/svg/propert...
Source/core/svg/properties/NewSVGAnimatedProperty.h:47: class
SVGAnimationElement;
On 2014/02/17 14:14:31, fs wrote:
> Needed because...?
Dropped.

[kouhei (in TOK), 2014-02-18 02:09:32]

The CQ bit was checked by kouhei@chromium.org

[commit-bot: I haz the power, 2014-02-18 02:09:40]

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

[commit-bot: I haz the power, 2014-02-18 02:10:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-18 02:10:01]

Failed to apply patch for Source/core/svg/SVGGradientElement.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/svg/SVGGradientElement.cpp
  Hunk #1 FAILED at 38.
  Hunk #2 succeeded at 83 (offset -1 lines).
  Hunk #3 FAILED at 95.
  2 out of 3 hunks FAILED -- saving rejects to file
Source/core/svg/SVGGradientElement.cpp.rej

Patch:       Source/core/svg/SVGGradientElement.cpp
Index: Source/core/svg/SVGGradientElement.cpp
diff --git a/Source/core/svg/SVGGradientElement.cpp
b/Source/core/svg/SVGGradientElement.cpp
index
bfee1004ddeb1af3f0eb9de13a785f2d3be2b830..d65bbb50ce8fcdf7770f0358d8c626e6fdfc2c35
100644
--- a/Source/core/svg/SVGGradientElement.cpp
+++ b/Source/core/svg/SVGGradientElement.cpp
@@ -38,23 +38,23 @@ namespace WebCore {
 // Animated property definitions
 DEFINE_ANIMATED_ENUMERATION(SVGGradientElement, SVGNames::spreadMethodAttr,
SpreadMethod, spreadMethod, SVGSpreadMethodType)
 DEFINE_ANIMATED_ENUMERATION(SVGGradientElement, SVGNames::gradientUnitsAttr,
GradientUnits, gradientUnits, SVGUnitTypes::SVGUnitType)
-DEFINE_ANIMATED_TRANSFORM_LIST(SVGGradientElement,
SVGNames::gradientTransformAttr, GradientTransform, gradientTransform)
 
 BEGIN_REGISTER_ANIMATED_PROPERTIES(SVGGradientElement)
     REGISTER_LOCAL_ANIMATED_PROPERTY(spreadMethod)
     REGISTER_LOCAL_ANIMATED_PROPERTY(gradientUnits)
-    REGISTER_LOCAL_ANIMATED_PROPERTY(gradientTransform)
     REGISTER_PARENT_ANIMATED_PROPERTIES(SVGElement)
 END_REGISTER_ANIMATED_PROPERTIES
 
 SVGGradientElement::SVGGradientElement(const QualifiedName& tagName, Document&
document)
     : SVGElement(tagName, document)
     , m_href(SVGAnimatedString::create(this, XLinkNames::hrefAttr,
SVGString::create()))
+    , m_gradientTransform(SVGAnimatedTransformList::create(this,
SVGNames::gradientTransformAttr, SVGTransformList::create()))
     , m_spreadMethod(SVGSpreadMethodPad)
     , m_gradientUnits(SVGUnitTypes::SVG_UNIT_TYPE_OBJECTBOUNDINGBOX)
 {
     ScriptWrappable::init(this);
     addToPropertyMap(m_href);
+    addToPropertyMap(m_gradientTransform);
     registerAnimatedPropertiesForSVGGradientElement();
 }
 
@@ -84,14 +84,6 @@ void SVGGradientElement::parseAttribute(const QualifiedName&
name, const AtomicS
         return;
     }
 
-    if (name == SVGNames::gradientTransformAttr) {
-        SVGTransformList newList;
-        newList.parse(value);
-        detachAnimatedGradientTransformListWrappers(newList.size());
-        setGradientTransformBaseValue(newList);
-        return;
-    }
-
     if (name == SVGNames::spreadMethodAttr) {
         SVGSpreadMethodType propertyValue =
SVGPropertyTraits<SVGSpreadMethodType>::fromString(value);
         if (propertyValue > 0)
@@ -103,6 +95,8 @@ void SVGGradientElement::parseAttribute(const QualifiedName&
name, const AtomicS
 
     if (name.matches(XLinkNames::hrefAttr))
         m_href->setBaseValueAsString(value, parseError);
+    else if (name == SVGNames::gradientTransformAttr)
+        m_gradientTransform->setBaseValueAsString(value, parseError);
     else
         ASSERT_NOT_REACHED();

[kouhei (in TOK), 2014-02-18 02:14:49]

LKGR seems far behind. I'll try after rolls.

[kouhei (in TOK), 2014-02-20 03:13:11]

The CQ bit was checked by kouhei@chromium.org

[commit-bot: I haz the power, 2014-02-20 03:16:03]

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

[haraken, 2014-02-20 03:25:13]

LGTM for bindings/

[commit-bot: I haz the power, 2014-02-20 03:42:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-20 03:42:20]

Retried try job too often on blink_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_pres...

[kouhei (in TOK), 2014-02-20 03:46:00]

The CQ bit was checked by kouhei@chromium.org

[commit-bot: I haz the power, 2014-02-20 03:47:03]

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

[kouhei (in TOK), 2014-02-20 04:11:19]

The CQ bit was checked by kouhei@chromium.org

[kouhei (in TOK), 2014-02-20 04:25:55]

The CQ bit was unchecked by kouhei@chromium.org

[kouhei (in TOK), 2014-02-20 04:26:11]

The CQ bit was checked by kouhei@chromium.org

[kouhei (in TOK), 2014-02-20 07:00:54]

The CQ bit was unchecked by kouhei@chromium.org

[kouhei (in TOK), 2014-02-20 07:01:08]

The CQ bit was checked by kouhei@chromium.org

[commit-bot: I haz the power, 2014-02-20 07:01:20]

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

[commit-bot: I haz the power, 2014-02-20 07:42:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-20 07:42:46]

Retried try job too often on mac_blink_rel for step(s) blink_heap_unittests,
blink_platform_unittests, webkit_lint, webkit_python_tests, webkit_tests,
webkit_unit_tests, wtf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_blink_...

[kouhei (in TOK), 2014-02-20 07:44:14]

The CQ bit was checked by kouhei@chromium.org

[commit-bot: I haz the power, 2014-02-20 07:45:31]

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

[commit-bot: I haz the power, 2014-02-20 08:42:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-20 08:42:33]

Retried try job too often on mac_blink_rel for step(s) blink_heap_unittests,
blink_platform_unittests, webkit_lint, webkit_python_tests, webkit_tests,
webkit_unit_tests, wtf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_blink_...

[kouhei (in TOK), 2014-02-21 01:18:20]

The CQ bit was checked by kouhei@chromium.org

[commit-bot: I haz the power, 2014-02-21 01:18:48]

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

[kouhei (in TOK), 2014-02-21 01:42:13]

The CQ bit was checked by kouhei@chromium.org

[commit-bot: I haz the power, 2014-02-21 01:42:32]

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

[commit-bot: I haz the power, 2014-02-21 03:25:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-21 03:25:04]

Retried try job too often on win_blink_rel for step(s) webkit_lint,
webkit_python_tests, webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_...

[kouhei (in TOK), 2014-02-21 03:49:44]

Failing on unrelated tests. NOTRYing

[kouhei (in TOK), 2014-02-21 03:51:09]

The CQ bit was checked by kouhei@chromium.org

[commit-bot: I haz the power, 2014-02-21 03:51:33]

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

[commit-bot: I haz the power, 2014-02-21 03:52:39]

Change committed as 167565