[svg2] Make SVGLength wrap a CSSPrimitiveValue

Source/core/svg/SVGLength.h

Index: Source/core/svg/SVGLength.h
diff --git a/Source/core/svg/SVGLength.h b/Source/core/svg/SVGLength.h
index b228627f63c79aa57386ce2a89cece9349166cda..aff58645fb9d212920b43fb8b26242f83f6ca02d 100644
--- a/Source/core/svg/SVGLength.h
+++ b/Source/core/svg/SVGLength.h
@@ -50,20 +50,46 @@ public:
     PassRefPtrWillBeRawPtr<SVGLength> clone() const;
     virtual PassRefPtrWillBeRawPtr<SVGPropertyBase> cloneForAnimation(const String&) const override;
 
-    SVGLengthType unitType() const { return static_cast<SVGLengthType>(m_unitType); }
-    CSSPrimitiveValue::UnitType cssUnitTypeQuirk() const
+    CSSPrimitiveValue* getCSSPrimitiveValue() { return m_value.get(); }

[pdr., 2015/05/28 21:28:19]

It looks like this is only used to access the primitiveType. Can you just expose
"UnitType primitiveType() { return m_value->primitiveType(); } const"?

[Erik Dahlström, 2015/06/02 11:57:44]

Sure.

+
+    static CSSPrimitiveValue::UnitType toCSSUnit(SVGLengthType type)
     {
-        if (m_unitType == LengthTypeNumber)
+        switch (type) {
+        case LengthTypeUnknown:
+            return CSSPrimitiveValue::UnitType::CSS_UNKNOWN;
+        case LengthTypeNumber:
+            return CSSPrimitiveValue::UnitType::CSS_NUMBER;
+        case LengthTypePercentage:
+            return CSSPrimitiveValue::UnitType::CSS_PERCENTAGE;
+        case LengthTypeEMS:
+            return CSSPrimitiveValue::UnitType::CSS_EMS;
+        case LengthTypeEXS:
+            return CSSPrimitiveValue::UnitType::CSS_EXS;
+        case LengthTypePX:
             return CSSPrimitiveValue::UnitType::CSS_PX;
+        case LengthTypeCM:
+            return CSSPrimitiveValue::UnitType::CSS_CM;
+        case LengthTypeMM:
+            return CSSPrimitiveValue::UnitType::CSS_MM;
+        case LengthTypeIN:
+            return CSSPrimitiveValue::UnitType::CSS_IN;
+        case LengthTypePT:
+            return CSSPrimitiveValue::UnitType::CSS_PT;
+        case LengthTypePC:
+            return CSSPrimitiveValue::UnitType::CSS_PC;
+        default:
+            return CSSPrimitiveValue::UnitType::CSS_UNKNOWN;
+        }
+    }
 
-        if (m_unitType == LengthTypeREMS)
-            return CSSPrimitiveValue::UnitType::CSS_REMS;
-        if (m_unitType == LengthTypeCHS)
-            return CSSPrimitiveValue::UnitType::CSS_CHS;
+    CSSPrimitiveValue::UnitType cssUnitTypeQuirk() const
+    {
+        if (m_value->primitiveType() == CSSPrimitiveValue::UnitType::CSS_NUMBER)
+            return CSSPrimitiveValue::UnitType::CSS_PX;
 
-        return static_cast<CSSPrimitiveValue::UnitType>(m_unitType);
+        return m_value->primitiveType();
     }
-    void setUnitType(SVGLengthType);
+    void setUnitType(CSSPrimitiveValue::UnitType);
     SVGLengthMode unitMode() const { return static_cast<SVGLengthMode>(m_unitMode); }
 
     bool operator==(const SVGLength&) const;
@@ -72,8 +98,11 @@ public:
     float value(const SVGLengthContext&) const;
     void setValue(float, const SVGLengthContext&);
 
-    float valueInSpecifiedUnits() const { return m_valueInSpecifiedUnits; }
-    void setValueInSpecifiedUnits(float value) { m_valueInSpecifiedUnits = value; }
+    float valueInSpecifiedUnits() const { return m_value->getFloatValue(); }
+    void setValueInSpecifiedUnits(float value)
+    {
+        m_value = CSSPrimitiveValue::create(value, m_value->primitiveType());
+    }
 
     // Resolves LengthTypePercentage into a normalized floating point number (full value is 1.0).
     float valueAsPercentage() const;
@@ -87,27 +116,27 @@ public:
     virtual String valueAsString() const override;
     void setValueAsString(const String&, ExceptionState&);
 
-    void newValueSpecifiedUnits(SVGLengthType, float valueInSpecifiedUnits);
-    void convertToSpecifiedUnits(SVGLengthType, const SVGLengthContext&);
+    void newValueSpecifiedUnits(CSSPrimitiveValue::UnitType, float valueInSpecifiedUnits);
+    void convertToSpecifiedUnits(CSSPrimitiveValue::UnitType, const SVGLengthContext&);
 
     // Helper functions
-    static inline bool isRelativeUnit(SVGLengthType unitType)
+    static inline bool isRelativeUnit(CSSPrimitiveValue::UnitType unitType)
     {
-        return unitType == LengthTypePercentage
-            || unitType == LengthTypeEMS
-            || unitType == LengthTypeEXS
-            || unitType == LengthTypeREMS
-            || unitType == LengthTypeCHS;
+        return unitType == CSSPrimitiveValue::CSS_PERCENTAGE
+            || unitType == CSSPrimitiveValue::CSS_EMS
+            || unitType == CSSPrimitiveValue::CSS_EXS
+            || unitType == CSSPrimitiveValue::CSS_REMS
+            || unitType == CSSPrimitiveValue::CSS_CHS;
     }
-    inline bool isRelative() const { return isRelativeUnit(unitType()); }
+    inline bool isRelative() const { return isRelativeUnit(m_value->primitiveType()); }
 
     bool isZero() const
     {
-        return !m_valueInSpecifiedUnits;
+        return m_value->getFloatValue() == 0;
     }
 
     static PassRefPtrWillBeRawPtr<SVGLength> fromCSSPrimitiveValue(CSSPrimitiveValue*);
-    static PassRefPtrWillBeRawPtr<CSSPrimitiveValue> toCSSPrimitiveValue(PassRefPtrWillBeRawPtr<SVGLength>);
+
     static SVGLengthMode lengthModeForAnimatedLengthAttribute(const QualifiedName&);
 
     PassRefPtrWillBeRawPtr<SVGLength> blend(PassRefPtrWillBeRawPtr<SVGLength> from, float progress) const;
@@ -122,9 +151,8 @@ private:
     SVGLength(SVGLengthMode);
     SVGLength(const SVGLength&);
 
-    float m_valueInSpecifiedUnits;
+    RefPtrWillBeMember<CSSPrimitiveValue> m_value;

[pdr., 2015/05/28 21:28:18]

Does this need to be a pointer? I wonder if it can just be a member like
"CSSPrimitiveValue m_value";

Otherwise, can it just be an OwnPtr to avoid some overhead of ref counting?

[fs, 2015/05/29 08:09:42]

Then it'd need to be copied when it's used as a presentation attribute. (I
notice that that simplification/optimization is not in this CL though, but I'd
expect that to be a close follower.)

[fs, 2015/05/29 13:49:06]

Oh, yeah, almost forgot, Oilpan. That also means that a trace() definition is
missing.

[Erik Dahlström, 2015/06/02 11:57:44]

Ok, will add that.

[Erik Dahlström, 2015/06/02 11:57:44]

It can't be a simple member, since CSSPrimitiveValue is immutable.

     unsigned m_unitMode : 2;

[pdr., 2015/05/28 21:28:18]

Can we use CSSPrimitiveValue's LengthUnitType and get rid of this entirely?

[fs, 2015/05/29 08:09:42]

That's a different thing. In most cases it should be possible to trivially get
rid of this by specifying it at the callsite. The exception I can think of is
SVGLengthTearOff (the DOM interface for .value etc)

[Erik Dahlström, 2015/06/02 11:57:44]

Yes, it's not quite the same as LengthUnitType I think. Attempting to get rid of
this should be done separately.

-    unsigned m_unitType : 4;
 };
 
 inline PassRefPtrWillBeRawPtr<SVGLength> toSVGLength(PassRefPtrWillBeRawPtr<SVGPropertyBase> passBase)