Bug 629200 part 17 - Remove unnecessary serialisation from setting nsSVGPreserveAspectRatio; r=jwatt
authorBrian Birtles <birtles@gmail.com>
Thu, 16 Feb 2012 08:40:46 +0900
changeset 86953 0cfd2c0264ecdf5bcfafb71ee19a9840ca0958ef
parent 86952 ae97ed40a24f33a1e4f167a64dc471f2e7bffe79
child 86954 b397e9d603348257f499d2daed842146fd37911e
push id537
push usertim.taubert@gmx.de
push dateThu, 16 Feb 2012 11:54:26 +0000
treeherderfx-team@6989376471f7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs629200
milestone13.0a1
Bug 629200 part 17 - Remove unnecessary serialisation from setting nsSVGPreserveAspectRatio; r=jwatt
content/base/src/nsAttrValue.cpp
content/base/src/nsAttrValue.h
content/svg/content/src/Makefile.in
content/svg/content/src/SVGAnimatedPreserveAspectRatio.cpp
content/svg/content/src/SVGAnimatedPreserveAspectRatio.h
content/svg/content/src/nsSVGElement.cpp
content/svg/content/src/nsSVGElement.h
content/svg/content/test/test_dataTypes.html
content/svg/content/test/test_dataTypesModEvents.html
--- a/content/base/src/nsAttrValue.cpp
+++ b/content/base/src/nsAttrValue.cpp
@@ -50,16 +50,17 @@
 #include "nsIDocument.h"
 #include "nsContentUtils.h"
 #include "nsReadableUtils.h"
 #include "prprf.h"
 #include "nsSVGAngle.h"
 #include "nsSVGIntegerPair.h"
 #include "nsSVGLength2.h"
 #include "nsSVGNumberPair.h"
+#include "SVGAnimatedPreserveAspectRatio.h"
 #include "SVGLengthList.h"
 
 namespace css = mozilla::css;
 
 #define MISC_STR_PTR(_cont) \
   reinterpret_cast<void*>((_cont)->mStringBits & NS_ATTRVALUE_POINTERVALUE_MASK)
 
 nsTArray<const nsAttrValue::EnumTable*>* nsAttrValue::sEnumTableArray = nsnull;
@@ -289,16 +290,21 @@ nsAttrValue::SetTo(const nsAttrValue& aO
       cont->mSVGLengthList = otherCont->mSVGLengthList;
       break;
     }
     case eSVGNumberPair:
     {
       cont->mSVGNumberPair = otherCont->mSVGNumberPair;
       break;
     }
+    case eSVGPreserveAspectRatio:
+    {
+      cont->mSVGPreserveAspectRatio = otherCont->mSVGPreserveAspectRatio;
+      break;
+    }
     default:
     {
       NS_NOTREACHED("unknown type stored in MiscContainer");
       break;
     }
   }
 
   void* otherPtr = MISC_STR_PTR(otherCont);
@@ -447,16 +453,28 @@ nsAttrValue::SetTo(const nsSVGNumberPair
     MiscContainer* cont = GetMiscContainer();
     cont->mSVGNumberPair = &aValue;
     cont->mType = eSVGNumberPair;
     SetMiscAtomOrString(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)
 {
   PtrBits tmp = aOther.mBits;
   aOther.mBits = mBits;
   mBits = tmp;
 }
 
 void
@@ -568,16 +586,21 @@ nsAttrValue::ToString(nsAString& aResult
       GetMiscContainer()->mSVGLengthList->GetValueAsString(aResult);
       break;
     }
     case eSVGNumberPair:
     {
       GetMiscContainer()->mSVGNumberPair->GetBaseValueString(aResult);
       break;
     }
+    case eSVGPreserveAspectRatio:
+    {
+      GetMiscContainer()->mSVGPreserveAspectRatio->GetBaseValueString(aResult);
+      break;
+    }
     default:
     {
       aResult.Truncate();
       break;
     }
   }
 }
 
@@ -772,16 +795,20 @@ nsAttrValue::HashValue() const
     case eSVGLengthList:
     {
       return NS_PTR_TO_INT32(cont->mSVGLengthList);
     }
     case eSVGNumberPair:
     {
       return NS_PTR_TO_INT32(cont->mSVGNumberPair);
     }
+    case eSVGPreserveAspectRatio:
+    {
+      return NS_PTR_TO_INT32(cont->mSVGPreserveAspectRatio);
+    }
     default:
     {
       NS_NOTREACHED("unknown type stored in MiscContainer");
       return 0;
     }
   }
 }
 
@@ -884,16 +911,21 @@ nsAttrValue::Equals(const nsAttrValue& a
     case eSVGLengthList:
     {
       return thisCont->mSVGLengthList == otherCont->mSVGLengthList;
     }
     case eSVGNumberPair:
     {
       return thisCont->mSVGNumberPair == otherCont->mSVGNumberPair;
     }
+    case eSVGPreserveAspectRatio:
+    {
+      return thisCont->mSVGPreserveAspectRatio ==
+        otherCont->mSVGPreserveAspectRatio;
+    }
     default:
     {
       NS_NOTREACHED("unknown type stored in MiscContainer");
       return false;
     }
   }
   if (needsStringComparison) {
     if (thisCont->mStringBits == otherCont->mStringBits) {
--- a/content/base/src/nsAttrValue.h
+++ b/content/base/src/nsAttrValue.h
@@ -62,16 +62,17 @@ class nsSVGAngle;
 class nsSVGIntegerPair;
 class nsSVGLength2;
 class nsSVGNumberPair;
 
 namespace mozilla {
 namespace css {
 class StyleRule;
 }
+class SVGAnimatedPreserveAspectRatio;
 class SVGLengthList;
 }
 
 #define NS_ATTRVALUE_MAX_STRINGLENGTH_ATOM 12
 
 #define NS_ATTRVALUE_BASETYPE_MASK (PtrBits(3))
 #define NS_ATTRVALUE_POINTERVALUE_MASK (~NS_ATTRVALUE_BASETYPE_MASK)
 
@@ -132,16 +133,17 @@ public:
     ,eAtomArray =      0x11
     ,eDoubleValue  =   0x12
     ,eIntMarginValue = 0x13
     ,eSVGAngle =       0x14
     ,eSVGIntegerPair = 0x15
     ,eSVGLength =      0x16
     ,eSVGLengthList =  0x17
     ,eSVGNumberPair =  0x18
+    ,eSVGPreserveAspectRatio = 0x19
   };
 
   ValueType Type() const;
 
   void Reset();
 
   void SetTo(const nsAttrValue& aOther);
   void SetTo(const nsAString& aValue);
@@ -152,16 +154,18 @@ public:
   void SetTo(mozilla::css::StyleRule* aValue, const nsAString* aSerialized);
   void SetTo(const nsIntMargin& aValue);
   void SetTo(const nsSVGAngle& aValue, const nsAString* aSerialized);
   void SetTo(const nsSVGIntegerPair& aValue, const nsAString* aSerialized);
   void SetTo(const nsSVGLength2& aValue, const nsAString* aSerialized);
   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.
    *
    * After calling this method, this object will have type eString unless the
    * type of aValue is eAtom, in which case this object will also have type
    * eAtom.
    */
@@ -388,16 +392,17 @@ private:
       AtomArray* mAtomArray;
       double mDoubleValue;
       nsIntMargin* mIntMargin;
       const nsSVGAngle* mSVGAngle;
       const nsSVGIntegerPair* mSVGIntegerPair;
       const nsSVGLength2* mSVGLength;
       const mozilla::SVGLengthList* mSVGLengthList;
       const nsSVGNumberPair* mSVGNumberPair;
+      const mozilla::SVGAnimatedPreserveAspectRatio* mSVGPreserveAspectRatio;
     };
   };
 
   inline ValueBaseType BaseType() const;
 
   /**
    * Get the index of an EnumTable in the sEnumTableArray.
    * If the EnumTable is not in the sEnumTableArray, it is added.
--- a/content/svg/content/src/Makefile.in
+++ b/content/svg/content/src/Makefile.in
@@ -169,16 +169,17 @@ FORCE_STATIC_LIB = 1
 EXPORTS =  			\
 	nsSVGAngle.h               \
 	nsSVGElement.h             \
 	nsSVGFeatures.h            \
 	nsSVGIntegerPair.h         \
 	nsSVGLength2.h             \
 	nsSVGNumberPair.h          \
 	nsSVGRect.h                \
+	SVGAnimatedPreserveAspectRatio.h \
 	SVGLength.h                \
 	SVGLengthList.h            \
 	$(NULL)
 
 include $(topsrcdir)/config/rules.mk
 
 INCLUDES += 	\
 		-I$(srcdir)/../../../xml/content/src \
--- a/content/svg/content/src/SVGAnimatedPreserveAspectRatio.cpp
+++ b/content/svg/content/src/SVGAnimatedPreserveAspectRatio.cpp
@@ -245,17 +245,18 @@ SVGAnimatedPreserveAspectRatio::SetBaseV
 
   // We don't need to call DidChange* here - we're only called by
   // nsSVGElement::ParseAttribute under nsGenericElement::SetAttr,
   // which takes care of notifying.
   return NS_OK;
 }
 
 void
-SVGAnimatedPreserveAspectRatio::GetBaseValueString(nsAString & aValueAsString)
+SVGAnimatedPreserveAspectRatio::GetBaseValueString(
+  nsAString& aValueAsString) const
 {
   nsAutoString tmpString;
 
   aValueAsString.Truncate();
 
   if (mBaseVal.mDefer) {
     aValueAsString.AppendLiteral("defer ");
   }
@@ -271,39 +272,49 @@ SVGAnimatedPreserveAspectRatio::GetBaseV
     aValueAsString.Append(tmpString);
   }
 }
 
 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();
   }
   
   return NS_OK;
 }
 
 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();
   }
   
   return NS_OK;
 }
 
 void
--- a/content/svg/content/src/SVGAnimatedPreserveAspectRatio.h
+++ b/content/svg/content/src/SVGAnimatedPreserveAspectRatio.h
@@ -113,17 +113,17 @@ public:
     mBaseVal.mDefer = false;
     mAnimVal = mBaseVal;
     mIsAnimated = false;
     mIsBaseSet = false;
   }
 
   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);
   void SetAnimValue(PRUint64 aPackedValue, nsSVGElement *aSVGElement);
 
   const SVGPreserveAspectRatio &GetBaseValue() const
     { return mBaseVal; }
   const SVGPreserveAspectRatio &GetAnimValue() const
--- a/content/svg/content/src/nsSVGElement.cpp
+++ b/content/svg/content/src/nsSVGElement.cpp
@@ -553,16 +553,19 @@ nsSVGElement::ParseAttribute(PRInt32 aNa
       // Check for SVGAnimatedPreserveAspectRatio attribute
       } else if (aAttribute == nsGkAtoms::preserveAspectRatio) {
         SVGAnimatedPreserveAspectRatio *preserveAspectRatio =
           GetPreserveAspectRatio();
         if (preserveAspectRatio) {
           rv = preserveAspectRatio->SetBaseValueString(aValue, this);
           if (NS_FAILED(rv)) {
             preserveAspectRatio->Init();
+          } else {
+            aResult.SetTo(*preserveAspectRatio, &aValue);
+            didSetResult = true;
           }
           foundMatch = true;
         }
       // Check for SVGAnimatedTransformList attribute
       } else if (GetTransformListAttrName() == aAttribute) {
         SVGAnimatedTransformList *transformList = GetAnimatedTransformList();
         if (transformList) {
           rv = transformList->SetBaseValueString(aValue);
@@ -758,20 +761,19 @@ nsSVGElement::UnsetAttrInternal(PRInt32 
         return;
       }
     }
 
     // Check if this is a preserveAspectRatio attribute going away
     if (aName == nsGkAtoms::preserveAspectRatio) {
       SVGAnimatedPreserveAspectRatio *preserveAspectRatio =
         GetPreserveAspectRatio();
-
       if (preserveAspectRatio) {
+        MaybeSerializeAttrBeforeRemoval(aName, aNotify);
         preserveAspectRatio->Init();
-        DidChangePreserveAspectRatio(false);
         return;
       }
     }
 
     // Check if this is a transform list attribute going away
     if (GetTransformListAttrName() == aName) {
       SVGAnimatedTransformList *transformList = GetAnimatedTransformList();
       if (transformList) {
@@ -2187,34 +2189,36 @@ nsSVGElement::DidAnimateViewBox()
 }
 
 SVGAnimatedPreserveAspectRatio *
 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
 nsSVGElement::DidAnimatePreserveAspectRatio()
 {
   nsIFrame* frame = GetPrimaryFrame();
   
   if (frame) {
--- a/content/svg/content/src/nsSVGElement.h
+++ b/content/svg/content/src/nsSVGElement.h
@@ -169,30 +169,31 @@ public:
     return GetNumberInfo().mNumberInfo[aAttrEnum].mPercentagesAllowed;
   }
   void SetLength(nsIAtom* aName, const nsSVGLength2 &aLength);
 
   nsAttrValue WillChangeLength(PRUint8 aAttrEnum);
   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);
   void DidChangeNumber(PRUint8 aAttrEnum);
   void DidChangeNumberPair(PRUint8 aAttrEnum,
                            const nsAttrValue& aEmptyOrOldValue);
   void DidChangeInteger(PRUint8 aAttrEnum);
   void DidChangeIntegerPair(PRUint8 aAttrEnum,
                             const nsAttrValue& aEmptyOrOldValue);
   void DidChangeAngle(PRUint8 aAttrEnum, const nsAttrValue& aEmptyOrOldValue);
   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);
   virtual void DidChangePointList(bool aDoSetAttr);
   virtual void DidChangePathSegList(bool aDoSetAttr);
   virtual void DidChangeTransformList(bool aDoSetAttr);
   void DidChangeString(PRUint8 aAttrEnum) {}
   void DidChangeStringList(bool aIsConditionalProcessingAttribute,
--- a/content/svg/content/test/test_dataTypes.html
+++ b/content/svg/content/test/test_dataTypes.html
@@ -230,16 +230,23 @@ function runTests()
   var basePreserveAspectRatio = marker.preserveAspectRatio.baseVal;
   var animPreserveAspectRatio = marker.preserveAspectRatio.animVal;
   marker.setAttribute("preserveAspectRatio", "xMaxYMid slice");
   is(basePreserveAspectRatio.align, 7, "preserveAspectRatio.align baseVal");
   is(animPreserveAspectRatio.align, 7, "preserveAspectRatio.align animVal");
   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;
   is(baseViewBox.x, 0, "viewBox baseVal");
   is(animViewBox.x, 0, "viewBox baseVal");
   is(baseViewBox.y, 0, "viewBox baseVal");
   is(animViewBox.y, 0, "viewBox baseVal");
   is(baseViewBox.width, 0, "viewBox baseVal");
--- a/content/svg/content/test/test_dataTypesModEvents.html
+++ b/content/svg/content/test/test_dataTypesModEvents.html
@@ -196,16 +196,32 @@ function runTests()
   convolve.removeAttribute("result");
   convolve.result.baseVal = "bar";
 
   eventChecker.expect("");
   convolve.result.baseVal = "bar";
   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();
 }
 
 window.addEventListener("load", runTests, false);
 </script>
 </pre>