Bug 775730 - Address missed review comments. r=dholbert

content/svg/content/src/DOMSVGLength.cpp

@@ -59,6 +59,7 @@ public:
     : mLength(aLength)
   {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    MOZ_ASSERT(mLength, "Expecting non-null length");
     MOZ_ASSERT(mLength->HasOwner(),
                "Expecting list to have an owner for notification");
     mEmptyOrOldValue =
@@ -75,7 +76,7 @@ public:
   }
 
 private:
-  DOMSVGLength* mLength;
+  DOMSVGLength* const mLength;
   nsAttrValue   mEmptyOrOldValue;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
@@ -162,11 +163,13 @@ DOMSVGLength::SetValue(float aUserUnitValue)
       return NS_OK;
     }
     float uuPerUnit = InternalItem().GetUserUnitsPerUnit(Element(), Axis());
-    float newValue = aUserUnitValue / uuPerUnit;
-    if (uuPerUnit > 0 && NS_finite(newValue)) {
-      AutoChangeLengthNotifier notifier(this);
-      InternalItem().SetValueAndUnit(newValue, InternalItem().GetUnit());
-      return NS_OK;
+    if (uuPerUnit > 0) {
+      float newValue = aUserUnitValue / uuPerUnit;
+      if (NS_finite(newValue)) {
+        AutoChangeLengthNotifier notifier(this);
+        InternalItem().SetValueAndUnit(newValue, InternalItem().GetUnit());
+        return NS_OK;
+      }
     }
   } else if (mUnit == nsIDOMSVGLength::SVG_LENGTHTYPE_NUMBER ||
              mUnit == nsIDOMSVGLength::SVG_LENGTHTYPE_PX) {

content/svg/content/src/DOMSVGLengthList.cpp

@@ -85,6 +85,7 @@ public:
     : mLengthList(aLengthList)
   {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    MOZ_ASSERT(mLengthList, "Expecting non-null lengthList");
     mEmptyOrOldValue =
       mLengthList->Element()->WillChangeLengthList(mLengthList->AttrEnum());
   }
@@ -99,7 +100,7 @@ public:
   }
 
 private:
-  DOMSVGLengthList* mLengthList;
+  DOMSVGLengthList* const mLengthList;
   nsAttrValue       mEmptyOrOldValue;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };

content/svg/content/src/DOMSVGNumber.cpp

@@ -14,7 +14,7 @@
 
 // See the architecture comment in DOMSVGAnimatedNumberList.h.
 
-using namespace mozilla;
+namespace mozilla {
 
 // We could use NS_IMPL_CYCLE_COLLECTION_1, except that in Unlink() we need to
 // clear our list's weak ref to us to be safe. (The other option would be to
@@ -35,18 +35,17 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(DOMSVGNumber)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(DOMSVGNumber)
+}
+DOMCI_DATA(SVGNumber, mozilla::DOMSVGNumber)
 
-DOMCI_DATA(SVGNumber, DOMSVGNumber)
-
+namespace mozilla {
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(DOMSVGNumber)
-  NS_INTERFACE_MAP_ENTRY(DOMSVGNumber) // pseudo-interface
+  NS_INTERFACE_MAP_ENTRY(mozilla::DOMSVGNumber) // pseudo-interface
   NS_INTERFACE_MAP_ENTRY(nsIDOMSVGNumber)
   NS_INTERFACE_MAP_ENTRY(nsISupports)
   NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(SVGNumber)
 NS_INTERFACE_MAP_END
 
-namespace mozilla {
-
 //----------------------------------------------------------------------
 // Helper class: AutoChangeNumberNotifier
 // Stack-based helper class to pair calls to WillChangeNumberList and
@@ -58,6 +57,7 @@ public:
     : mNumber(aNumber)
   {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    MOZ_ASSERT(mNumber, "Expecting non-null number");
     MOZ_ASSERT(mNumber->HasOwner(),
                "Expecting list to have an owner for notification");
     mEmptyOrOldValue =
@@ -74,13 +74,11 @@ public:
   }
 
 private:
-  DOMSVGNumber* mNumber;
+  DOMSVGNumber* const mNumber;
   nsAttrValue   mEmptyOrOldValue;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
-}
-
 DOMSVGNumber::DOMSVGNumber(DOMSVGNumberList *aList,
                            uint8_t aAttrEnum,
                            uint32_t aListIndex,
@@ -190,3 +188,4 @@ DOMSVGNumber::IndexIsValid()
 }
 #endif
 
+} // namespace mozilla

content/svg/content/src/DOMSVGNumberList.cpp

@@ -86,6 +86,7 @@ public:
     : mNumberList(aNumberList)
   {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    MOZ_ASSERT(mNumberList, "Expecting non-null numberList");
     mEmptyOrOldValue =
       mNumberList->Element()->WillChangeNumberList(mNumberList->AttrEnum());
   }
@@ -100,7 +101,7 @@ public:
   }
 
 private:
-  DOMSVGNumberList* mNumberList;
+  DOMSVGNumberList* const mNumberList;
   nsAttrValue       mEmptyOrOldValue;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };

content/svg/content/src/DOMSVGPathSeg.cpp

@@ -11,7 +11,7 @@
 
 // See the architecture comment in DOMSVGPathSegList.h.
 
-using namespace mozilla;
+namespace mozilla {
 
 // We could use NS_IMPL_CYCLE_COLLECTION_1, except that in Unlink() we need to
 // clear our list's weak ref to us to be safe. (The other option would be to
@@ -40,8 +40,6 @@ NS_IMPL_CYCLE_COLLECTION_TRACE_END
 NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(DOMSVGPathSeg, AddRef)
 NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(DOMSVGPathSeg, Release)
 
-namespace mozilla {
-
 //----------------------------------------------------------------------
 // Helper class: AutoChangePathSegNotifier
 // Stack-based helper class to pair calls to WillChangePathSegList
@@ -53,6 +51,7 @@ public:
     : mPathSeg(aPathSeg)
   {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    MOZ_ASSERT(mPathSeg, "Expecting non-null pathSeg");
     MOZ_ASSERT(mPathSeg->HasOwner(),
                "Expecting list to have an owner for notification");
     mEmptyOrOldValue =
@@ -68,13 +67,11 @@ public:
   }
 
 private:
-  DOMSVGPathSeg* mPathSeg;
+  DOMSVGPathSeg* const mPathSeg;
   nsAttrValue    mEmptyOrOldValue;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
-}
-
 DOMSVGPathSeg::DOMSVGPathSeg(DOMSVGPathSegList *aList,
                              uint32_t aListIndex,
                              bool aIsAnimValItem)
@@ -386,3 +383,4 @@ DOMSVGPathSeg::CreateFor(DOMSVGPathSegList *aList,
   }
 }
 
+} // namespace mozilla

content/svg/content/src/DOMSVGPathSegList.cpp

@@ -61,6 +61,7 @@ public:
     : mPathSegList(aPathSegList)
   {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    MOZ_ASSERT(mPathSegList, "Expecting non-null pathSegList");
     mEmptyOrOldValue =
       mPathSegList->Element()->WillChangePathSegList();
   }
@@ -74,7 +75,7 @@ public:
   }
 
 private:
-  DOMSVGPathSegList* mPathSegList;
+  DOMSVGPathSegList* const mPathSegList;
   nsAttrValue        mEmptyOrOldValue;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };

content/svg/content/src/DOMSVGPoint.cpp

@@ -27,6 +27,7 @@ public:
     : mPoint(aPoint)
   {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    MOZ_ASSERT(mPoint, "Expecting non-null point");
     MOZ_ASSERT(mPoint->HasOwner(),
                "Expecting list to have an owner for notification");
     mEmptyOrOldValue =
@@ -42,7 +43,7 @@ public:
   }
 
 private:
-  DOMSVGPoint* mPoint;
+  DOMSVGPoint* const mPoint;
   nsAttrValue  mEmptyOrOldValue;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };

content/svg/content/src/DOMSVGPointList.cpp

@@ -79,6 +79,7 @@ public:
     : mPointList(aPointList)
   {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    MOZ_ASSERT(mPointList, "Expecting non-null pointList");
     mEmptyOrOldValue =
       mPointList->Element()->WillChangePointList();
   }
@@ -92,7 +93,7 @@ public:
   }
 
 private:
-  DOMSVGPointList* mPointList;
+  DOMSVGPointList* const mPointList;
   nsAttrValue      mEmptyOrOldValue;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };

content/svg/content/src/DOMSVGStringList.cpp

@@ -48,6 +48,7 @@ public:
     : mStringList(aStringList)
   {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    MOZ_ASSERT(mStringList, "Expecting non-null stringList");
     mEmptyOrOldValue =
       mStringList->mElement->WillChangeStringList(mStringList->mIsConditionalProcessingAttribute,
                                                   mStringList->mAttrEnum);
@@ -60,7 +61,7 @@ public:
   }
 
 private:
-  DOMSVGStringList* mStringList;
+  DOMSVGStringList* const mStringList;
   nsAttrValue       mEmptyOrOldValue;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };

content/svg/content/src/DOMSVGTransformList.cpp

@@ -87,6 +87,7 @@ public:
     : mTransformList(aTransformList)
   {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    MOZ_ASSERT(mTransformList, "Expecting non-null transformList");
     mEmptyOrOldValue =
       mTransformList->Element()->WillChangeTransformList();
   }
@@ -100,7 +101,7 @@ public:
   }
 
 private:
-  DOMSVGTransformList* mTransformList;
+  DOMSVGTransformList* const mTransformList;
   nsAttrValue          mEmptyOrOldValue;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };

content/svg/content/src/SVGTransform.cpp

@@ -14,7 +14,7 @@
 #include "nsSVGAttrTearoffTable.h"
 
 namespace {
-  const double radPerDegree = 2.0 * M_PI / 360.0;
+  const double kRadPerDegree = 2.0 * M_PI / 360.0;
 }
 
 namespace mozilla {
@@ -76,6 +76,7 @@ public:
     : mTransform(aTransform)
   {
     MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    MOZ_ASSERT(mTransform, "Expecting non-null transform");
     if (mTransform->HasOwner()) {
       mEmptyOrOldValue =
         mTransform->Element()->WillChangeTransformList();
@@ -93,7 +94,7 @@ public:
   }
 
 private:
-  SVGTransform* mTransform;
+  SVGTransform* const mTransform;
   nsAttrValue   mEmptyOrOldValue;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
@@ -261,13 +262,14 @@ SVGTransform::SetSkewX(float angle, ErrorResult& rv)
     return;
   }
 
-  if (!NS_finite(tan(angle * radPerDegree))) {
+  if (!NS_finite(tan(angle * kRadPerDegree))) {
     rv.Throw(NS_ERROR_RANGE_ERR);
     return;
   }
 
   AutoChangeTransformNotifier notifier(this);
-  Transform().SetSkewX(angle);
+  DebugOnly<nsresult> result = Transform().SetSkewX(angle);
+  MOZ_ASSERT(NS_SUCCEEDED(result), "SetSkewX unexpectedly failed");
 }
 
 void
@@ -283,13 +285,14 @@ SVGTransform::SetSkewY(float angle, ErrorResult& rv)
     return;
   }
 
-  if (!NS_finite(tan(angle * radPerDegree))) {
+  if (!NS_finite(tan(angle * kRadPerDegree))) {
     rv.Throw(NS_ERROR_RANGE_ERR);
     return;
   }
 
   AutoChangeTransformNotifier notifier(this);
-  Transform().SetSkewY(angle);
+  DebugOnly<nsresult> result = Transform().SetSkewY(angle);
+  MOZ_ASSERT(NS_SUCCEEDED(result), "SetSkewY unexpectedly failed");
 }
 
 //----------------------------------------------------------------------

content/svg/content/src/nsSVGTransform.cpp

@@ -10,7 +10,7 @@
 #include "nsTextFormatter.h"
 
 namespace {
-  const double radPerDegree = 2.0 * M_PI / 360.0;
+  const double kRadPerDegree = 2.0 * M_PI / 360.0;
 }
 
 namespace mozilla {
@@ -115,7 +115,7 @@ nsSVGTransform::SetRotate(float aAngle, float aCx, float aCy)
   mType    = SVG_TRANSFORM_ROTATE;
   mMatrix.Reset();
   mMatrix.Translate(gfxPoint(aCx, aCy));
-  mMatrix.Rotate(aAngle*radPerDegree);
+  mMatrix.Rotate(aAngle*kRadPerDegree);
   mMatrix.Translate(gfxPoint(-aCx, -aCy));
   mAngle   = aAngle;
   mOriginX = aCx;
@@ -125,7 +125,7 @@ nsSVGTransform::SetRotate(float aAngle, float aCx, float aCy)
 nsresult
 nsSVGTransform::SetSkewX(float aAngle)
 {
-  double ta = tan(aAngle*radPerDegree);
+  double ta = tan(aAngle*kRadPerDegree);
   NS_ENSURE_FINITE(ta, NS_ERROR_RANGE_ERR);
 
   mType    = SVG_TRANSFORM_SKEWX;
@@ -140,7 +140,7 @@ nsSVGTransform::SetSkewX(float aAngle)
 nsresult
 nsSVGTransform::SetSkewY(float aAngle)
 {
-  double ta = tan(aAngle*radPerDegree);
+  double ta = tan(aAngle*kRadPerDegree);
   NS_ENSURE_FINITE(ta, NS_ERROR_RANGE_ERR);
 
   mType    = SVG_TRANSFORM_SKEWY;