Do refactor in collectGradientAttributes() and renderStyleForLengthResolve()

Source/core/svg/SVGRadialGradientElement.cpp

 /*
  * Copyright (C) 2004, 2005, 2006, 2008 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2004, 2005, 2006, 2007 Rob Buis <buis@kde.org>
  * Copyright (C) 2008 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2008 Dirk Schulze <krit@webkit.org>
  * Copyright (C) Research In Motion Limited 2010. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
@@ skipping 116 matching lines @@
 
 RenderObject* SVGRadialGradientElement::createRenderer(RenderStyle*)
 {
     return new RenderSVGResourceRadialGradient(this);
 }
 
 bool SVGRadialGradientElement::collectGradientAttributes(RadialGradientAttributes& attributes)
 {
     HashSet<SVGGradientElement*> processedGradients;
 
-    bool isRadial = true;
     SVGGradientElement* current = this;
+    if (!current->renderer())
+        return false;
 
-    while (current) {
-        if (!current->renderer())
-            return false;
+    setAttributeAnimatedLength(current, attributes);
 
-        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());
-        }
-
+    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
             if (processedGradients.contains(current)) {
                 current = 0;
                 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())
         attributes.setFx(attributes.cx());
 
     if (!attributes.hasFy())
         attributes.setFy(attributes.cy());
     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()
         || m_cy->currentValue()->isRelative()
         || m_r->currentValue()->isRelative()
         || m_fx->currentValue()->isRelative()
         || m_fy->currentValue()->isRelative()
         || m_fr->currentValue()->isRelative();
 }
 
 }