Bug 629200 part 7 - Remove unnecessary serialisation from setting SVGEnum; r=jwatt

content/base/src/nsAttrValue.cpp

@@ -76,6 +76,12 @@ nsAttrValue::nsAttrValue(const nsAString& aValue)
   SetTo(aValue);
 }
 
+nsAttrValue::nsAttrValue(nsIAtom* aValue)
+    : mBits(0)
+{
+  SetTo(aValue);
+}
+
 nsAttrValue::nsAttrValue(css::StyleRule* aValue, const nsAString* aSerialized)
     : mBits(0)
 {
@@ -290,6 +296,16 @@ nsAttrValue::SetTo(const nsAString& aValue)
   }
 }
 
+void
+nsAttrValue::SetTo(nsIAtom* aValue)
+{
+  ResetIfSet();
+  if (aValue) {
+    NS_ADDREF(aValue);
+    SetPtrValueAndType(aValue, eAtomBase);
+  }
+}
+
 void
 nsAttrValue::SetTo(PRInt16 aInt)
 {

content/base/src/nsAttrValue.h

@@ -102,6 +102,7 @@ public:
   nsAttrValue();
   nsAttrValue(const nsAttrValue& aOther);
   explicit nsAttrValue(const nsAString& aValue);
+  explicit nsAttrValue(nsIAtom* aValue);
   nsAttrValue(mozilla::css::StyleRule* aValue, const nsAString* aSerialized);
   explicit nsAttrValue(const nsIntMargin& aValue);
   ~nsAttrValue();
@@ -134,6 +135,7 @@ public:
 
   void SetTo(const nsAttrValue& aOther);
   void SetTo(const nsAString& aValue);
+  void SetTo(nsIAtom* aValue);
   void SetTo(PRInt16 aInt);
   void SetTo(mozilla::css::StyleRule* aValue, const nsAString* aSerialized);
   void SetTo(const nsIntMargin& aValue);

content/svg/content/src/nsSVGElement.cpp

@@ -290,6 +290,8 @@ nsSVGElement::ParseAttribute(PRInt32 aNamespaceID,
 {
   nsresult rv = NS_OK;
   bool foundMatch = false;
+  bool didSetResult = false;
+
   if (aNamespaceID == kNameSpaceID_None) {
 
     // Check for nsSVGLength2 attribute
@@ -462,9 +464,13 @@ nsSVGElement::ParseAttribute(PRInt32 aNamespaceID,
       EnumAttributesInfo enumInfo = GetEnumInfo();
       for (i = 0; i < enumInfo.mEnumCount; i++) {
         if (aAttribute == *enumInfo.mEnumInfo[i].mName) {
-          rv = enumInfo.mEnums[i].SetBaseValueString(aValue, this);
+          nsCOMPtr<nsIAtom> valAtom = do_GetAtom(aValue);
+          rv = enumInfo.mEnums[i].SetBaseValueAtom(valAtom, this);
           if (NS_FAILED(rv)) {
             enumInfo.Reset(i);
+          } else {
+            aResult.SetTo(valAtom);
+            didSetResult = true;
           }
           foundMatch = true;
           break;
@@ -550,7 +556,9 @@ nsSVGElement::ParseAttribute(PRInt32 aNamespaceID,
       ReportAttributeParseFailure(OwnerDoc(), aAttribute, aValue);
       return false;
     }
-    aResult.SetTo(aValue);
+    if (!didSetResult) {
+      aResult.SetTo(aValue);
+    }
     return true;
   }
 
@@ -703,7 +711,6 @@ nsSVGElement::UnsetAttrInternal(PRInt32 aNamespaceID, nsIAtom* aName,
     for (PRUint32 i = 0; i < enumInfo.mEnumCount; i++) {
       if (aName == *enumInfo.mEnumInfo[i].mName) {
         enumInfo.Reset(i);
-        DidChangeEnum(i, false);
         return;
       }
     }
@@ -1959,22 +1966,15 @@ void nsSVGElement::EnumAttributesInfo::Reset(PRUint8 aAttrEnum)
 }
 
 void
-nsSVGElement::DidChangeEnum(PRUint8 aAttrEnum, bool aDoSetAttr)
+nsSVGElement::DidChangeEnum(PRUint8 aAttrEnum)
 {
-  if (!aDoSetAttr)
-    return;
-
   EnumAttributesInfo info = GetEnumInfo();
 
   NS_ASSERTION(info.mEnumCount > 0,
                "DidChangeEnum on element with no enum attribs");
-
   NS_ASSERTION(aAttrEnum < info.mEnumCount, "aAttrEnum out of range");
 
-  nsAutoString serializedValue;
-  info.mEnums[aAttrEnum].GetBaseValueString(serializedValue, this);
-
-  nsAttrValue attrValue(serializedValue);
+  nsAttrValue attrValue(info.mEnums[aAttrEnum].GetBaseValueAtom(this));
   SetParsedAttr(kNameSpaceID_None, *info.mEnumInfo[aAttrEnum].mName, nsnull,
                 attrValue, true);
 }

content/svg/content/src/nsSVGElement.h

@@ -176,7 +176,7 @@ public:
   virtual void DidChangeIntegerPair(PRUint8 aAttrEnum, bool aDoSetAttr);
   virtual void DidChangeAngle(PRUint8 aAttrEnum, bool aDoSetAttr);
   virtual void DidChangeBoolean(PRUint8 aAttrEnum, bool aDoSetAttr);
-  virtual void DidChangeEnum(PRUint8 aAttrEnum, bool aDoSetAttr);
+  void DidChangeEnum(PRUint8 aAttrEnum);
   virtual void DidChangeViewBox(bool aDoSetAttr);
   virtual void DidChangePreserveAspectRatio(bool aDoSetAttr);
   virtual void DidChangeNumberList(PRUint8 aAttrEnum, bool aDoSetAttr);

content/svg/content/src/nsSVGEnum.cpp

@@ -67,15 +67,12 @@ nsSVGEnum::GetMapping(nsSVGElement *aSVGElement)
 }
 
 nsresult
-nsSVGEnum::SetBaseValueString(const nsAString& aValue,
-                              nsSVGElement *aSVGElement)
+nsSVGEnum::SetBaseValueAtom(const nsIAtom* aValue, nsSVGElement *aSVGElement)
 {
-  nsCOMPtr<nsIAtom> valAtom = do_GetAtom(aValue);
-
   nsSVGEnumMapping *mapping = GetMapping(aSVGElement);
 
   while (mapping && mapping->mKey) {
-    if (valAtom == *(mapping->mKey)) {
+    if (aValue == *(mapping->mKey)) {
       mIsBaseSet = true;
       if (mBaseVal != mapping->mVal) {
         mBaseVal = mapping->mVal;
@@ -99,19 +96,19 @@ nsSVGEnum::SetBaseValueString(const nsAString& aValue,
   return NS_ERROR_DOM_SYNTAX_ERR;
 }
 
-void
-nsSVGEnum::GetBaseValueString(nsAString& aValue, nsSVGElement *aSVGElement)
+nsIAtom*
+nsSVGEnum::GetBaseValueAtom(nsSVGElement *aSVGElement)
 {
   nsSVGEnumMapping *mapping = GetMapping(aSVGElement);
 
   while (mapping && mapping->mKey) {
     if (mBaseVal == mapping->mVal) {
-      (*mapping->mKey)->ToString(aValue);
-      return;
+      return *mapping->mKey;
     }
     mapping++;
   }
   NS_ERROR("unknown enumeration value");
+  return nsGkAtoms::_empty;
 }
 
 nsresult
@@ -131,7 +128,7 @@ nsSVGEnum::SetBaseValue(PRUint16 aValue,
         else {
           aSVGElement->AnimationNeedsResample();
         }
-        aSVGElement->DidChangeEnum(mAttrEnum, true);
+        aSVGElement->DidChangeEnum(mAttrEnum);
       }
       return NS_OK;
     }

content/svg/content/src/nsSVGEnum.h

@@ -65,11 +65,8 @@ public:
     mIsBaseSet = false;
   }
 
-  nsresult SetBaseValueString(const nsAString& aValue,
-                              nsSVGElement *aSVGElement);
-  void GetBaseValueString(nsAString& aValue,
-                          nsSVGElement *aSVGElement);
-
+  nsresult SetBaseValueAtom(const nsIAtom* aValue, nsSVGElement *aSVGElement);
+  nsIAtom* GetBaseValueAtom(nsSVGElement *aSVGElement);
   nsresult SetBaseValue(PRUint16 aValue,
                         nsSVGElement *aSVGElement);
   PRUint16 GetBaseValue() const

content/svg/content/test/test_dataTypes.html

@@ -153,6 +153,10 @@ function runTests()
   convolve.edgeMode.baseVal = 1;
   is(convolve.edgeMode.animVal, 1, "enum animVal");
   is(convolve.getAttribute("edgeMode"), "duplicate", "enum attribute");
+  convolve.setAttribute("edgeMode", "");
+  ok(convolve.getAttribute("edgeMode") === "", "empty enum attribute");
+  convolve.removeAttribute("edgeMode");
+  ok(convolve.getAttribute("edgeMode") === null, "removed enum attribute");
 
   // string attribute
 

content/svg/content/test/test_dataTypesModEvents.html

@@ -31,7 +31,21 @@ function runTests()
   var marker = doc.getElementById("marker");
   var eventChecker = new MutationEventChecker;
 
-  // Tests go here
+  // enum attribute
+
+  eventChecker.watchAttr(convolve, "edgeMode");
+  eventChecker.expect("add modify remove add");
+  convolve.setAttribute("edgeMode", "none");
+  convolve.edgeMode.baseVal = SVGFEConvolveMatrixElement.SVG_EDGEMODE_WRAP;
+  convolve.removeAttribute("edgeMode");
+  convolve.edgeMode.baseVal = SVGFEConvolveMatrixElement.SVG_EDGEMODE_NONE;
+
+  eventChecker.expect("");
+  convolve.edgeMode.baseVal = SVGFEConvolveMatrixElement.SVG_EDGEMODE_NONE;
+  convolve.setAttribute("edgeMode", "none");
+  convolve.edgeMode.baseVal = SVGFEConvolveMatrixElement.SVG_EDGEMODE_NONE;
+
+  eventChecker.finish();
 
   SimpleTest.finish();
 }