Do refactor in collectGradientAttributes() and renderStyleForLengthResolve()

Source/core/svg/SVGLinearGradientElement.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 101 matching lines @@
 
 RenderObject* SVGLinearGradientElement::createRenderer(RenderStyle*)
 {
     return new RenderSVGResourceLinearGradient(this);
 }
 
 bool SVGLinearGradientElement::collectGradientAttributes(LinearGradientAttributes& attributes)
 {
     HashSet<SVGGradientElement*> processedGradients;
 
-    bool isLinear = true;
     SVGGradientElement* current = this;
+    if (!current->renderer())
+        return false;

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

Should just be "if (!renderer()) return false;" at the very beginning of the
method.

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

Done.

 
-    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 (isLinear) {
-            SVGLinearGradientElement* linear = toSVGLinearGradientElement(current);
-
-            if (!attributes.hasX1() && current->hasAttribute(SVGNames::x1Attr))
-                attributes.setX1(linear->x1()->currentValue());
-
-            if (!attributes.hasY1() && current->hasAttribute(SVGNames::y1Attr))
-                attributes.setY1(linear->y1()->currentValue());
-
-            if (!attributes.hasX2() && current->hasAttribute(SVGNames::x2Attr))
-                attributes.setX2(linear->x2()->currentValue());
-
-            if (!attributes.hasY2() && current->hasAttribute(SVGNames::y2Attr))
-                attributes.setY2(linear->y2()->currentValue());
-        }
-
+    do {

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

while (true)

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

Done.

         processedGradients.add(current);
 
         // Respect xlink:href, take attributes from referenced element
         Node* refNode = SVGURIReference::targetElementFromIRIString(current->hrefCurrentValue(), document());
-        if (refNode && (refNode->hasTagName(SVGNames::linearGradientTag) || refNode->hasTagName(SVGNames::radialGradientTag))) {
+        if (refNode && isSVGGradientElement(*refNode)) {
             current = toSVGGradientElement(refNode);
 
             // Cycle detection
             if (processedGradients.contains(current)) {
                 current = 0;
                 break;

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

return true;

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

In SVGRadialGradientElement, we can't return true because there is a work out of
while() loop as below,

    else {
            current = 0;
            break;
        }
    }

    // Handle default values for fx/fy
    if (!attributes.hasFx())
        attributes.setFx(attributes.cx());

    if (!attributes.hasFy())
        attributes.setFy(attributes.cy());

So, I use "break;" instead of returning "true". How do you think ?

             }
 
-            isLinear = current->hasTagName(SVGNames::linearGradientTag);
-        } else
+            if (!current->renderer())
+                return false;
+
+            setAttributeAnimatedLength(current, attributes, current->hasTagName(SVGNames::linearGradientTag));
+        } else {
             current = 0;

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

return true;

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

Done.

+        }
+    } while (current);

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

}

ASSERT_NOT_REACHED();

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

Done.

+
+    return true;
+}
+
+void SVGLinearGradientElement::setAttributeAnimatedLength(SVGGradientElement* element, LinearGradientAttributes& attributes, bool isLinear)
+{
+    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);
     }
 
-    return true;
+    if (!attributes.hasStops()) {
+        const Vector<Gradient::ColorStop>& stops(element->buildStops());
+        if (!stops.isEmpty())
+            attributes.setStops(stops);
+    }
+
+    if (isLinear) {
+        SVGLinearGradientElement* linear = toSVGLinearGradientElement(element);
+
+        if (!attributes.hasX1() && element->hasAttribute(SVGNames::x1Attr))
+            attributes.setX1(linear->x1()->currentValue());
+
+        if (!attributes.hasY1() && element->hasAttribute(SVGNames::y1Attr))
+            attributes.setY1(linear->y1()->currentValue());
+
+        if (!attributes.hasX2() && element->hasAttribute(SVGNames::x2Attr))
+            attributes.setX2(linear->x2()->currentValue());
+
+        if (!attributes.hasY2() && element->hasAttribute(SVGNames::y2Attr))
+            attributes.setY2(linear->y2()->currentValue());
+    }
 }
 
 bool SVGLinearGradientElement::selfHasRelativeLengths() const
 {
     return m_x1->currentValue()->isRelative()
         || m_y1->currentValue()->isRelative()
         || m_x2->currentValue()->isRelative()
         || m_y2->currentValue()->isRelative();
 }
 
 }