Bug 629200 part 18 - Remove unnecessary serialisation from setting nsSVGViewBox; r=jwatt
authorBrian Birtles <birtles@gmail.com>
Thu, 16 Feb 2012 08:40:46 +0900
changeset 87114 b397e9d603348257f499d2daed842146fd37911e
parent 87113 0cfd2c0264ecdf5bcfafb71ee19a9840ca0958ef
child 87115 e6b4f0dbd601a0c04fde82090855237aa14b1418
push id129
push userbturner@mozilla.com
push dateSat, 18 Feb 2012 14:29:45 +0000
reviewersjwatt
bugs629200
milestone13.0a1
Bug 629200 part 18 - Remove unnecessary serialisation from setting nsSVGViewBox; r=jwatt
content/base/src/nsAttrValue.cpp
content/base/src/nsAttrValue.h
content/svg/content/src/Makefile.in
content/svg/content/src/nsSVGElement.cpp
content/svg/content/src/nsSVGElement.h
content/svg/content/src/nsSVGViewBox.cpp
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 "nsSVGViewBox.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)
 
@@ -295,16 +296,21 @@ nsAttrValue::SetTo(const nsAttrValue& aO
       cont->mSVGNumberPair = otherCont->mSVGNumberPair;
       break;
     }
     case eSVGPreserveAspectRatio:
     {
       cont->mSVGPreserveAspectRatio = otherCont->mSVGPreserveAspectRatio;
       break;
     }
+    case eSVGViewBox:
+    {
+      cont->mSVGViewBox = otherCont->mSVGViewBox;
+      break;
+    }
     default:
     {
       NS_NOTREACHED("unknown type stored in MiscContainer");
       break;
     }
   }
 
   void* otherPtr = MISC_STR_PTR(otherCont);
@@ -465,16 +471,27 @@ nsAttrValue::SetTo(const mozilla::SVGAni
     MiscContainer* cont = GetMiscContainer();
     cont->mSVGPreserveAspectRatio = &aValue;
     cont->mType = eSVGPreserveAspectRatio;
     SetMiscAtomOrString(aSerialized);
   }
 }
 
 void
+nsAttrValue::SetTo(const nsSVGViewBox& aValue, const nsAString* aSerialized)
+{
+  if (EnsureEmptyMiscContainer()) {
+    MiscContainer* cont = GetMiscContainer();
+    cont->mSVGViewBox = &aValue;
+    cont->mType = eSVGViewBox;
+    SetMiscAtomOrString(aSerialized);
+  }
+}
+
+void
 nsAttrValue::SwapValueWith(nsAttrValue& aOther)
 {
   PtrBits tmp = aOther.mBits;
   aOther.mBits = mBits;
   mBits = tmp;
 }
 
 void
@@ -591,16 +608,21 @@ nsAttrValue::ToString(nsAString& aResult
       GetMiscContainer()->mSVGNumberPair->GetBaseValueString(aResult);
       break;
     }
     case eSVGPreserveAspectRatio:
     {
       GetMiscContainer()->mSVGPreserveAspectRatio->GetBaseValueString(aResult);
       break;
     }
+    case eSVGViewBox:
+    {
+      GetMiscContainer()->mSVGViewBox->GetBaseValueString(aResult);
+      break;
+    }
     default:
     {
       aResult.Truncate();
       break;
     }
   }
 }
 
@@ -799,16 +821,20 @@ nsAttrValue::HashValue() const
     case eSVGNumberPair:
     {
       return NS_PTR_TO_INT32(cont->mSVGNumberPair);
     }
     case eSVGPreserveAspectRatio:
     {
       return NS_PTR_TO_INT32(cont->mSVGPreserveAspectRatio);
     }
+    case eSVGViewBox:
+    {
+      return NS_PTR_TO_INT32(cont->mSVGViewBox);
+    }
     default:
     {
       NS_NOTREACHED("unknown type stored in MiscContainer");
       return 0;
     }
   }
 }
 
@@ -916,16 +942,20 @@ nsAttrValue::Equals(const nsAttrValue& a
     {
       return thisCont->mSVGNumberPair == otherCont->mSVGNumberPair;
     }
     case eSVGPreserveAspectRatio:
     {
       return thisCont->mSVGPreserveAspectRatio ==
         otherCont->mSVGPreserveAspectRatio;
     }
+    case eSVGViewBox:
+    {
+      return thisCont->mSVGViewBox == otherCont->mSVGViewBox;
+    }
     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
@@ -57,16 +57,17 @@ class nsAString;
 class nsIAtom;
 class nsIDocument;
 template<class E, class A> class nsTArray;
 struct nsTArrayDefaultAllocator;
 class nsSVGAngle;
 class nsSVGIntegerPair;
 class nsSVGLength2;
 class nsSVGNumberPair;
+class nsSVGViewBox;
 
 namespace mozilla {
 namespace css {
 class StyleRule;
 }
 class SVGAnimatedPreserveAspectRatio;
 class SVGLengthList;
 }
@@ -134,16 +135,17 @@ public:
     ,eDoubleValue  =   0x12
     ,eIntMarginValue = 0x13
     ,eSVGAngle =       0x14
     ,eSVGIntegerPair = 0x15
     ,eSVGLength =      0x16
     ,eSVGLengthList =  0x17
     ,eSVGNumberPair =  0x18
     ,eSVGPreserveAspectRatio = 0x19
+    ,eSVGViewBox =     0x20
   };
 
   ValueType Type() const;
 
   void Reset();
 
   void SetTo(const nsAttrValue& aOther);
   void SetTo(const nsAString& aValue);
@@ -156,16 +158,17 @@ public:
   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);
+  void SetTo(const nsSVGViewBox& 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.
    */
@@ -393,16 +396,17 @@ private:
       double mDoubleValue;
       nsIntMargin* mIntMargin;
       const nsSVGAngle* mSVGAngle;
       const nsSVGIntegerPair* mSVGIntegerPair;
       const nsSVGLength2* mSVGLength;
       const mozilla::SVGLengthList* mSVGLengthList;
       const nsSVGNumberPair* mSVGNumberPair;
       const mozilla::SVGAnimatedPreserveAspectRatio* mSVGPreserveAspectRatio;
+      const nsSVGViewBox* mSVGViewBox;
     };
   };
 
   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                \
+	nsSVGViewBox.h             \
 	SVGAnimatedPreserveAspectRatio.h \
 	SVGLength.h                \
 	SVGLengthList.h            \
 	$(NULL)
 
 include $(topsrcdir)/config/rules.mk
 
 INCLUDES += 	\
--- a/content/svg/content/src/nsSVGElement.cpp
+++ b/content/svg/content/src/nsSVGElement.cpp
@@ -542,16 +542,19 @@ nsSVGElement::ParseAttribute(PRInt32 aNa
     if (!foundMatch) {
       // Check for nsSVGViewBox attribute
       if (aAttribute == nsGkAtoms::viewBox) {
         nsSVGViewBox* viewBox = GetViewBox();
         if (viewBox) {
           rv = viewBox->SetBaseValueString(aValue, this);
           if (NS_FAILED(rv)) {
             viewBox->Init();
+          } else {
+            aResult.SetTo(*viewBox, &aValue);
+            didSetResult = true;
           }
           foundMatch = true;
         }
       // Check for SVGAnimatedPreserveAspectRatio attribute
       } else if (aAttribute == nsGkAtoms::preserveAspectRatio) {
         SVGAnimatedPreserveAspectRatio *preserveAspectRatio =
           GetPreserveAspectRatio();
         if (preserveAspectRatio) {
@@ -751,18 +754,18 @@ nsSVGElement::UnsetAttrInternal(PRInt32 
         return;
       }
     }
 
     // Check if this is a nsViewBox attribute going away
     if (aName == nsGkAtoms::viewBox) {
       nsSVGViewBox* viewBox = GetViewBox();
       if (viewBox) {
+        MaybeSerializeAttrBeforeRemoval(aName, aNotify);
         viewBox->Init();
-        DidChangeViewBox(false);
         return;
       }
     }
 
     // Check if this is a preserveAspectRatio attribute going away
     if (aName == nsGkAtoms::preserveAspectRatio) {
       SVGAnimatedPreserveAspectRatio *preserveAspectRatio =
         GetPreserveAspectRatio();
@@ -2153,32 +2156,33 @@ nsSVGElement::DidAnimateEnum(PRUint8 aAt
 }
 
 nsSVGViewBox *
 nsSVGElement::GetViewBox()
 {
   return nsnull;
 }
 
-void
-nsSVGElement::DidChangeViewBox(bool aDoSetAttr)
+nsAttrValue
+nsSVGElement::WillChangeViewBox()
 {
-  if (!aDoSetAttr)
-    return;
+  return WillChangeValue(nsGkAtoms::viewBox);
+}
 
+void
+nsSVGElement::DidChangeViewBox(const nsAttrValue& aEmptyOrOldValue)
+{
   nsSVGViewBox *viewBox = GetViewBox();
 
   NS_ASSERTION(viewBox, "DidChangeViewBox on element with no viewBox attrib");
 
-  nsAutoString serializedValue;
-  viewBox->GetBaseValueString(serializedValue);
+  nsAttrValue newValue;
+  newValue.SetTo(*viewBox, nsnull);
 
-  nsAttrValue attrValue(serializedValue);
-  SetParsedAttr(kNameSpaceID_None, nsGkAtoms::viewBox, nsnull,
-                attrValue, true);
+  DidChangeValue(nsGkAtoms::viewBox, aEmptyOrOldValue, newValue);
 }
 
 void
 nsSVGElement::DidAnimateViewBox()
 {
   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 WillChangeViewBox();
   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);
+  void DidChangeViewBox(const nsAttrValue& aEmptyOrOldValue);
   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) {}
--- a/content/svg/content/src/nsSVGViewBox.cpp
+++ b/content/svg/content/src/nsSVGViewBox.cpp
@@ -122,20 +122,26 @@ nsSVGViewBox::SetAnimValue(float aX, flo
   }
   aSVGElement->DidAnimateViewBox();
 }
 
 void
 nsSVGViewBox::SetBaseValue(float aX, float aY, float aWidth, float aHeight,
                            nsSVGElement *aSVGElement)
 {
+  if (mHasBaseVal && mBaseVal == nsSVGViewBoxRect(aX, aY, aWidth, aHeight)) {
+    return;
+  }
+
+  nsAttrValue emptyOrOldValue = aSVGElement->WillChangeViewBox();
+
   mBaseVal = nsSVGViewBoxRect(aX, aY, aWidth, aHeight);
   mHasBaseVal = true;
 
-  aSVGElement->DidChangeViewBox(true);
+  aSVGElement->DidChangeViewBox(emptyOrOldValue);
   if (mAnimVal) {
     aSVGElement->AnimationNeedsResample();
   }
 }
 
 static nsresult
 ToSVGViewBoxRect(const nsAString& aStr, nsSVGViewBoxRect *aViewBox)
 {
@@ -180,17 +186,17 @@ nsSVGViewBox::SetBaseValueString(const n
   nsresult res = ToSVGViewBoxRect(aValue, &viewBox);
   if (NS_SUCCEEDED(res)) {
     mBaseVal = nsSVGViewBoxRect(viewBox.x, viewBox.y, viewBox.width, viewBox.height);
     mHasBaseVal = true;
 
     if (mAnimVal) {
       aSVGElement->AnimationNeedsResample();
     }
-    // We don't need to call DidChange* here - we're only called by
+    // We don't need to call Will/DidChange* here - we're only called by
     // nsSVGElement::ParseAttribute under nsGenericElement::SetAttr,
     // which takes care of notifying.
   }
   return res;
 }
 
 void
 nsSVGViewBox::GetBaseValueString(nsAString& aValue) const
--- a/content/svg/content/test/test_dataTypes.html
+++ b/content/svg/content/test/test_dataTypes.html
@@ -274,16 +274,20 @@ function runTests()
   is(marker.viewBox.animVal.y, 6, "viewBox.y animVal");
   marker.viewBox.baseVal.width = 7;
   is(marker.viewBox.animVal.width, 7, "viewBox.width animVal");
   marker.viewBox.baseVal.height = 8;
   is(marker.viewBox.animVal.height, 8, "viewBox.height animVal");
   is(marker.getAttribute("viewBox"), "5 6 7 8", "viewBox attribute");
   marker.removeAttribute("viewBox");
   is(marker.hasAttribute("viewBox"), false, "viewBox hasAttribute");
+  ok(marker.getAttribute("viewBox") === null, "removed viewBox attribute");
+
+  marker.setAttribute("viewBox", "");
+  ok(marker.getAttribute("viewBox") === "", "empty viewBox attribute");
 
   SimpleTest.finish();
 }
 
 window.addEventListener("load", runTests, false);
 </script>
 </pre>
 </body>
--- a/content/svg/content/test/test_dataTypesModEvents.html
+++ b/content/svg/content/test/test_dataTypesModEvents.html
@@ -212,16 +212,32 @@ function runTests()
   marker.preserveAspectRatio.baseVal.align =
     SVGPreserveAspectRatio.SVG_PRESERVEASPECTRATIO_XMIDYMIN;
 
   eventChecker.expect("");
   marker.preserveAspectRatio.baseVal.meetOrSlice =
     SVGPreserveAspectRatio.SVG_MEETORSLICE_MEET;
   marker.setAttribute("preserveAspectRatio", "xMidYMin meet");
 
+  // viewBox attribute
+
+  eventChecker.watchAttr(marker, "viewBox");
+  eventChecker.expect("add modify remove add");
+  marker.setAttribute("viewBox", "1 2 3 4");
+  marker.viewBox.baseVal.height = 5;
+  marker.removeAttribute("viewBox");
+  marker.viewBox.baseVal.height = 4;
+
+  eventChecker.ignoreEvents();
+  marker.setAttribute("viewBox", "1 2 3 4");
+  eventChecker.expect("");
+  marker.viewBox.baseVal.height = 4;
+  marker.viewBox.baseVal.x = 1;
+  marker.setAttribute("viewBox", "1 2 3 4");
+
   eventChecker.finish();
 
   SimpleTest.finish();
 }
 
 window.addEventListener("load", runTests, false);
 </script>
 </pre>