Bug 629200 part 17 - Remove unnecessary serialisation from setting nsSVGPreserveAspectRatio; r=jwatt

content/base/src/nsAttrValue.cpp

@@ -55,6 +55,7 @@
 #include "nsSVGIntegerPair.h"
 #include "nsSVGLength2.h"
 #include "nsSVGNumberPair.h"
+#include "SVGAnimatedPreserveAspectRatio.h"
 #include "SVGLengthList.h"
 
 namespace css = mozilla::css;
@@ -294,6 +295,11 @@ nsAttrValue::SetTo(const nsAttrValue& aOther)
       cont->mSVGNumberPair = otherCont->mSVGNumberPair;
       break;
     }
+    case eSVGPreserveAspectRatio:
+    {
+      cont->mSVGPreserveAspectRatio = otherCont->mSVGPreserveAspectRatio;
+      break;
+    }
     default:
     {
       NS_NOTREACHED("unknown type stored in MiscContainer");
@@ -451,6 +457,18 @@ nsAttrValue::SetTo(const nsSVGNumberPair& aValue, const nsAString* aSerialized)
   }
 }
 
+void
+nsAttrValue::SetTo(const mozilla::SVGAnimatedPreserveAspectRatio& aValue,
+                   const nsAString* aSerialized)
+{
+  if (EnsureEmptyMiscContainer()) {
+    MiscContainer* cont = GetMiscContainer();
+    cont->mSVGPreserveAspectRatio = &aValue;
+    cont->mType = eSVGPreserveAspectRatio;
+    SetMiscAtomOrString(aSerialized);
+  }
+}
+
 void
 nsAttrValue::SwapValueWith(nsAttrValue& aOther)
 {
@@ -573,6 +591,11 @@ nsAttrValue::ToString(nsAString& aResult) const
       GetMiscContainer()->mSVGNumberPair->GetBaseValueString(aResult);
       break;
     }
+    case eSVGPreserveAspectRatio:
+    {
+      GetMiscContainer()->mSVGPreserveAspectRatio->GetBaseValueString(aResult);
+      break;
+    }
     default:
     {
       aResult.Truncate();
@@ -777,6 +800,10 @@ nsAttrValue::HashValue() const
     {
       return NS_PTR_TO_INT32(cont->mSVGNumberPair);
     }
+    case eSVGPreserveAspectRatio:
+    {
+      return NS_PTR_TO_INT32(cont->mSVGPreserveAspectRatio);
+    }
     default:
     {
       NS_NOTREACHED("unknown type stored in MiscContainer");
@@ -889,6 +916,11 @@ nsAttrValue::Equals(const nsAttrValue& aOther) const
     {
       return thisCont->mSVGNumberPair == otherCont->mSVGNumberPair;
     }
+    case eSVGPreserveAspectRatio:
+    {
+      return thisCont->mSVGPreserveAspectRatio ==
+        otherCont->mSVGPreserveAspectRatio;
+    }
     default:
     {
       NS_NOTREACHED("unknown type stored in MiscContainer");

content/base/src/nsAttrValue.h

@@ -67,6 +67,7 @@ namespace mozilla {
 namespace css {
 class StyleRule;
 }
+class SVGAnimatedPreserveAspectRatio;
 class SVGLengthList;
 }
 
@@ -137,6 +138,7 @@ public:
     ,eSVGLength =      0x16
     ,eSVGLengthList =  0x17
     ,eSVGNumberPair =  0x18
+    ,eSVGPreserveAspectRatio = 0x19
   };
 
   ValueType Type() const;
@@ -157,6 +159,8 @@ public:
   void SetTo(const mozilla::SVGLengthList& aValue,
              const nsAString* aSerialized);
   void SetTo(const nsSVGNumberPair& aValue, const nsAString* aSerialized);
+  void SetTo(const mozilla::SVGAnimatedPreserveAspectRatio& aValue,
+             const nsAString* aSerialized);
 
   /**
    * Sets this object with the string or atom representation of aValue.
@@ -393,6 +397,7 @@ private:
       const nsSVGLength2* mSVGLength;
       const mozilla::SVGLengthList* mSVGLengthList;
       const nsSVGNumberPair* mSVGNumberPair;
+      const mozilla::SVGAnimatedPreserveAspectRatio* mSVGPreserveAspectRatio;
     };
   };
 

content/svg/content/src/Makefile.in

@@ -174,6 +174,7 @@ EXPORTS =  			\
 	nsSVGLength2.h             \
 	nsSVGNumberPair.h          \
 	nsSVGRect.h                \
+	SVGAnimatedPreserveAspectRatio.h \
 	SVGLength.h                \
 	SVGLengthList.h            \
 	$(NULL)

content/svg/content/src/SVGAnimatedPreserveAspectRatio.cpp

@@ -250,7 +250,8 @@ SVGAnimatedPreserveAspectRatio::SetBaseValueString(
 }
 
 void
-SVGAnimatedPreserveAspectRatio::GetBaseValueString(nsAString & aValueAsString)
+SVGAnimatedPreserveAspectRatio::GetBaseValueString(
+  nsAString& aValueAsString) const
 {
   nsAutoString tmpString;
 
@@ -276,12 +277,17 @@ nsresult
 SVGAnimatedPreserveAspectRatio::SetBaseAlign(PRUint16 aAlign,
                                              nsSVGElement *aSVGElement)
 {
+  if (mIsBaseSet && mBaseVal.GetAlign() == aAlign) {
+    return NS_OK;
+  }
+
+  nsAttrValue emptyOrOldValue = aSVGElement->WillChangePreserveAspectRatio();
   nsresult rv = mBaseVal.SetAlign(aAlign);
   NS_ENSURE_SUCCESS(rv, rv);
   mIsBaseSet = true;
 
   mAnimVal.mAlign = mBaseVal.mAlign;
-  aSVGElement->DidChangePreserveAspectRatio(true);
+  aSVGElement->DidChangePreserveAspectRatio(emptyOrOldValue);
   if (mIsAnimated) {
     aSVGElement->AnimationNeedsResample();
   }
@@ -293,12 +299,17 @@ nsresult
 SVGAnimatedPreserveAspectRatio::SetBaseMeetOrSlice(PRUint16 aMeetOrSlice,
                                                    nsSVGElement *aSVGElement)
 {
+  if (mIsBaseSet && mBaseVal.GetMeetOrSlice() == aMeetOrSlice) {
+    return NS_OK;
+  }
+
+  nsAttrValue emptyOrOldValue = aSVGElement->WillChangePreserveAspectRatio();
   nsresult rv = mBaseVal.SetMeetOrSlice(aMeetOrSlice);
   NS_ENSURE_SUCCESS(rv, rv);
   mIsBaseSet = true;
 
   mAnimVal.mMeetOrSlice = mBaseVal.mMeetOrSlice;
-  aSVGElement->DidChangePreserveAspectRatio(true);
+  aSVGElement->DidChangePreserveAspectRatio(emptyOrOldValue);
   if (mIsAnimated) {
     aSVGElement->AnimationNeedsResample();
   }

content/svg/content/src/SVGAnimatedPreserveAspectRatio.h

@@ -118,7 +118,7 @@ public:
 
   nsresult SetBaseValueString(const nsAString& aValue,
                               nsSVGElement *aSVGElement);
-  void GetBaseValueString(nsAString& aValue);
+  void GetBaseValueString(nsAString& aValue) const;
 
   nsresult SetBaseAlign(PRUint16 aAlign, nsSVGElement *aSVGElement);
   nsresult SetBaseMeetOrSlice(PRUint16 aMeetOrSlice, nsSVGElement *aSVGElement);

content/svg/content/src/nsSVGElement.cpp

@@ -558,6 +558,9 @@ nsSVGElement::ParseAttribute(PRInt32 aNamespaceID,
           rv = preserveAspectRatio->SetBaseValueString(aValue, this);
           if (NS_FAILED(rv)) {
             preserveAspectRatio->Init();
+          } else {
+            aResult.SetTo(*preserveAspectRatio, &aValue);
+            didSetResult = true;
           }
           foundMatch = true;
         }
@@ -763,10 +766,9 @@ nsSVGElement::UnsetAttrInternal(PRInt32 aNamespaceID, nsIAtom* aName,
     if (aName == nsGkAtoms::preserveAspectRatio) {
       SVGAnimatedPreserveAspectRatio *preserveAspectRatio =
         GetPreserveAspectRatio();
-
       if (preserveAspectRatio) {
+        MaybeSerializeAttrBeforeRemoval(aName, aNotify);
         preserveAspectRatio->Init();
-        DidChangePreserveAspectRatio(false);
         return;
       }
     }
@@ -2192,24 +2194,26 @@ nsSVGElement::GetPreserveAspectRatio()
   return nsnull;
 }
 
-void
-nsSVGElement::DidChangePreserveAspectRatio(bool aDoSetAttr)
+nsAttrValue
+nsSVGElement::WillChangePreserveAspectRatio()
 {
-  if (!aDoSetAttr)
-    return;
+  return WillChangeValue(nsGkAtoms::preserveAspectRatio);
+}
 
+void
+nsSVGElement::DidChangePreserveAspectRatio(const nsAttrValue& aEmptyOrOldValue)
+{
   SVGAnimatedPreserveAspectRatio *preserveAspectRatio =
     GetPreserveAspectRatio();
 
   NS_ASSERTION(preserveAspectRatio,
-               "DidChangePreserveAspectRatio on element with no preserveAspectRatio attrib");
+               "DidChangePreserveAspectRatio on element with no "
+               "preserveAspectRatio attrib");
 
-  nsAutoString serializedValue;
-  preserveAspectRatio->GetBaseValueString(serializedValue);
+  nsAttrValue newValue;
+  newValue.SetTo(*preserveAspectRatio, nsnull);
 
-  nsAttrValue attrValue(serializedValue);
-  SetParsedAttr(kNameSpaceID_None, nsGkAtoms::preserveAspectRatio, nsnull,
-                attrValue, true);
+  DidChangeValue(nsGkAtoms::preserveAspectRatio, aEmptyOrOldValue, newValue);
 }
 
 void

content/svg/content/src/nsSVGElement.h

@@ -174,6 +174,7 @@ public:
   nsAttrValue WillChangeNumberPair(PRUint8 aAttrEnum);
   nsAttrValue WillChangeIntegerPair(PRUint8 aAttrEnum);
   nsAttrValue WillChangeAngle(PRUint8 aAttrEnum);
+  nsAttrValue WillChangePreserveAspectRatio();
   nsAttrValue WillChangeLengthList(PRUint8 aAttrEnum);
 
   void DidChangeLength(PRUint8 aAttrEnum, const nsAttrValue& aEmptyOrOldValue);
@@ -187,7 +188,7 @@ public:
   void DidChangeBoolean(PRUint8 aAttrEnum);
   void DidChangeEnum(PRUint8 aAttrEnum);
   virtual void DidChangeViewBox(bool aDoSetAttr);
-  virtual void DidChangePreserveAspectRatio(bool aDoSetAttr);
+  void DidChangePreserveAspectRatio(const nsAttrValue& aEmptyOrOldValue);
   virtual void DidChangeNumberList(PRUint8 aAttrEnum, bool aDoSetAttr);
   void DidChangeLengthList(PRUint8 aAttrEnum,
                            const nsAttrValue& aEmptyOrOldValue);

content/svg/content/test/test_dataTypes.html

@@ -235,6 +235,13 @@ function runTests()
   is(basePreserveAspectRatio.meetOrSlice, 2, "preserveAspectRatio.meetOrSlice baseVal");
   is(animPreserveAspectRatio.meetOrSlice, 2, "preserveAspectRatio.meetOrSlice animVal");
 
+  marker.setAttribute("preserveAspectRatio", "");
+  ok(marker.getAttribute("preserveAspectRatio") === "",
+     "empty preserveAspectRatio attribute");
+  marker.removeAttribute("preserveAspectRatio");
+  ok(marker.getAttribute("preserveAspectRatio") === null,
+     "removed preserveAspectRatio attribute");
+
   // viewBox attribute
   var baseViewBox = marker.viewBox.baseVal;
   var animViewBox = marker.viewBox.animVal;

content/svg/content/test/test_dataTypesModEvents.html

@@ -201,6 +201,22 @@ function runTests()
   convolve.setAttribute("result", "bar");
   convolve.result.baseVal = "bar";
 
+  // preserveAspectRatio attribute
+
+  eventChecker.watchAttr(marker, "preserveAspectRatio");
+  eventChecker.expect("add modify remove add");
+  marker.setAttribute("preserveAspectRatio", "xMaxYMid slice");
+  marker.preserveAspectRatio.baseVal.align =
+    SVGPreserveAspectRatio.SVG_PRESERVEASPECTRATIO_XMAXYMAX;
+  marker.removeAttribute("preserveAspectRatio");
+  marker.preserveAspectRatio.baseVal.align =
+    SVGPreserveAspectRatio.SVG_PRESERVEASPECTRATIO_XMIDYMIN;
+
+  eventChecker.expect("");
+  marker.preserveAspectRatio.baseVal.meetOrSlice =
+    SVGPreserveAspectRatio.SVG_MEETORSLICE_MEET;
+  marker.setAttribute("preserveAspectRatio", "xMidYMin meet");
+
   eventChecker.finish();
 
   SimpleTest.finish();