Do refactor in collectGradientAttributes() and renderStyleForLengthResolve()

Source/core/svg/SVGRadialGradientElement.cpp

Index: Source/core/svg/SVGRadialGradientElement.cpp
diff --git a/Source/core/svg/SVGRadialGradientElement.cpp b/Source/core/svg/SVGRadialGradientElement.cpp
index d8fe2a87533664771aa6a0172e1ccf679ad55592..0c1737f512dec370d0d238936002635f03ef0234 100644
--- a/Source/core/svg/SVGRadialGradientElement.cpp
+++ b/Source/core/svg/SVGRadialGradientElement.cpp
@@ -134,58 +134,18 @@ bool SVGRadialGradientElement::collectGradientAttributes(RadialGradientAttribute
 {
     HashSet<SVGGradientElement*> processedGradients;
 
-    bool isRadial = true;
     SVGGradientElement* current = this;
+    if (!current->renderer())
+        return false;
 
-    while (current) {
-        if (!current->renderer())
-            return false;
-
-        if (!attributes.hasSpreadMethod() && current->hasAttribute(SVGNames::spreadMethodAttr))
-            attributes.setSpreadMethod(current->spreadMethodCurrentValue());
-
-        if (!attributes.hasGradientUnits() && current->hasAttribute(SVGNames::gradientUnitsAttr))
-            attributes.setGradientUnits(current->gradientUnitsCurrentValue());
-
-        if (!attributes.hasGradientTransform() && current->hasAttribute(SVGNames::gradientTransformAttr)) {
-            AffineTransform transform;
-            current->gradientTransformCurrentValue().concatenate(transform);
-            attributes.setGradientTransform(transform);
-        }
-
-        if (!attributes.hasStops()) {
-            const Vector<Gradient::ColorStop>& stops(current->buildStops());
-            if (!stops.isEmpty())
-                attributes.setStops(stops);
-        }
-
-        if (isRadial) {
-            SVGRadialGradientElement* radial = toSVGRadialGradientElement(current);
-
-            if (!attributes.hasCx() && current->hasAttribute(SVGNames::cxAttr))
-                attributes.setCx(radial->cx()->currentValue());
-
-            if (!attributes.hasCy() && current->hasAttribute(SVGNames::cyAttr))
-                attributes.setCy(radial->cy()->currentValue());
-
-            if (!attributes.hasR() && current->hasAttribute(SVGNames::rAttr))
-                attributes.setR(radial->r()->currentValue());
-
-            if (!attributes.hasFx() && current->hasAttribute(SVGNames::fxAttr))
-                attributes.setFx(radial->fx()->currentValue());
-
-            if (!attributes.hasFy() && current->hasAttribute(SVGNames::fyAttr))
-                attributes.setFy(radial->fy()->currentValue());
-
-            if (!attributes.hasFr() && current->hasAttribute(SVGNames::frAttr))
-                attributes.setFr(radial->fr()->currentValue());
-        }
+    setAttributeAnimatedLength(current, attributes);
 
+    do {
         processedGradients.add(current);

[gyuyoung-inactive, 2014/01/16 03:34:11]

I think this line doesn't need to call out of do-while loop. Do you think this
line should be called before entering do-while loop ?

[Stephen Chennney, 2014/01/16 15:39:34]

I think it needs to appear before entering the loop (for the initial element)
and then it only needs to be called inside the if statement, at the end of the
block.

[gyuyoung-inactive, 2014/01/17 05:38:26]

Done.

 
         // Respect xlink:href, take attributes from referenced element
         Node* refNode = SVGURIReference::targetElementFromIRIString(current->hrefCurrentValue(), document());
-        if (refNode && (refNode->hasTagName(SVGNames::radialGradientTag) || refNode->hasTagName(SVGNames::linearGradientTag))) {
+        if (refNode && isSVGGradientElement(*refNode)) {
             current = toSVGGradientElement(refNode);
 
             // Cycle detection
@@ -194,10 +154,14 @@ bool SVGRadialGradientElement::collectGradientAttributes(RadialGradientAttribute
                 break;
             }
 
-            isRadial = current->hasTagName(SVGNames::radialGradientTag);
-        } else
+            if (!current->renderer())
+                return false;
+
+            setAttributeAnimatedLength(current, attributes, current->hasTagName(SVGNames::radialGradientTag));
+        } else {
             current = 0;
-    }
+        }
+    } while (current);
 
     // Handle default values for fx/fy
     if (!attributes.hasFx())
@@ -208,6 +172,49 @@ bool SVGRadialGradientElement::collectGradientAttributes(RadialGradientAttribute
     return true;
 }
 
+void SVGRadialGradientElement::setAttributeAnimatedLength(SVGGradientElement* element, RadialGradientAttributes& attributes, bool isRadial)
+{
+    if (!attributes.hasSpreadMethod() && element->hasAttribute(SVGNames::spreadMethodAttr))
+        attributes.setSpreadMethod(element->spreadMethodCurrentValue());
+
+    if (!attributes.hasGradientUnits() && element->hasAttribute(SVGNames::gradientUnitsAttr))
+        attributes.setGradientUnits(element->gradientUnitsCurrentValue());
+
+    if (!attributes.hasGradientTransform() && element->hasAttribute(SVGNames::gradientTransformAttr)) {
+        AffineTransform transform;
+        element->gradientTransformCurrentValue().concatenate(transform);
+        attributes.setGradientTransform(transform);
+    }
+
+    if (!attributes.hasStops()) {
+        const Vector<Gradient::ColorStop>& stops(element->buildStops());
+        if (!stops.isEmpty())
+            attributes.setStops(stops);
+    }
+
+    if (isRadial) {
+        SVGRadialGradientElement* radial = toSVGRadialGradientElement(element);
+
+        if (!attributes.hasCx() && element->hasAttribute(SVGNames::cxAttr))
+            attributes.setCx(radial->cx()->currentValue());
+
+        if (!attributes.hasCy() && element->hasAttribute(SVGNames::cyAttr))
+            attributes.setCy(radial->cy()->currentValue());
+
+        if (!attributes.hasR() && element->hasAttribute(SVGNames::rAttr))
+            attributes.setR(radial->r()->currentValue());
+
+        if (!attributes.hasFx() && element->hasAttribute(SVGNames::fxAttr))
+            attributes.setFx(radial->fx()->currentValue());
+
+        if (!attributes.hasFy() && element->hasAttribute(SVGNames::fyAttr))
+            attributes.setFy(radial->fy()->currentValue());
+
+        if (!attributes.hasFr() && element->hasAttribute(SVGNames::frAttr))
+            attributes.setFr(radial->fr()->currentValue());
+    }
+}
+
 bool SVGRadialGradientElement::selfHasRelativeLengths() const
 {
     return m_cx->currentValue()->isRelative()