Bug 630760 - DOMSVGLengthList ctor synchronizes animVal instances to the baseVal internal list, plus sync some divergence between comments and code in list types. r=roc, a=blocking.
authorJonathan Watt <jwatt@jwatt.org>
Sun, 06 Feb 2011 22:11:26 +1300
changeset 61998 1e03f7e81637fa6c66ce0dabc5f6d9703d550300
parent 61997 eb35c4e6d50e1fb397d950072873e02c206adf60
child 61999 d907bea5f40f202217bc0e3532770401e16c83ad
push idunknown
push userunknown
push dateunknown
reviewersroc, blocking
bugs630760
milestone2.0b12pre
first release with
nightly win64
1e03f7e81637 / 4.0b12pre / 20110206030201 / files
nightly linux32
nightly linux64
nightly mac
nightly win32
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly win64
Bug 630760 - DOMSVGLengthList ctor synchronizes animVal instances to the baseVal internal list, plus sync some divergence between comments and code in list types. r=roc, a=blocking.
content/svg/content/src/DOMSVGAnimatedLengthList.cpp
content/svg/content/src/DOMSVGLengthList.cpp
content/svg/content/src/DOMSVGLengthList.h
content/svg/content/src/DOMSVGNumberList.cpp
content/svg/content/src/DOMSVGNumberList.h
content/svg/content/src/DOMSVGPathSegList.cpp
content/svg/content/src/DOMSVGPathSegList.h
content/svg/content/src/DOMSVGPointList.cpp
content/svg/content/src/DOMSVGPointList.h
content/svg/content/test/Makefile.in
content/svg/content/test/test_SVGLengthList-2.xhtml
--- a/content/svg/content/src/DOMSVGAnimatedLengthList.cpp
+++ b/content/svg/content/src/DOMSVGAnimatedLengthList.cpp
@@ -62,27 +62,27 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
   NS_INTERFACE_MAP_ENTRY(nsISupports)
   NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(SVGAnimatedLengthList)
 NS_INTERFACE_MAP_END
 
 NS_IMETHODIMP
 DOMSVGAnimatedLengthList::GetBaseVal(nsIDOMSVGLengthList **_retval)
 {
   if (!mBaseVal) {
-    mBaseVal = new DOMSVGLengthList(this);
+    mBaseVal = new DOMSVGLengthList(this, InternalAList().GetBaseValue());
   }
   NS_ADDREF(*_retval = mBaseVal);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 DOMSVGAnimatedLengthList::GetAnimVal(nsIDOMSVGLengthList **_retval)
 {
   if (!mAnimVal) {
-    mAnimVal = new DOMSVGLengthList(this);
+    mAnimVal = new DOMSVGLengthList(this, InternalAList().GetAnimValue());
   }
   NS_ADDREF(*_retval = mAnimVal);
   return NS_OK;
 }
 
 /* static */ already_AddRefed<DOMSVGAnimatedLengthList>
 DOMSVGAnimatedLengthList::GetDOMWrapper(SVGAnimatedLengthList *aList,
                                         nsSVGElement *aElement,
--- a/content/svg/content/src/DOMSVGLengthList.cpp
+++ b/content/svg/content/src/DOMSVGLengthList.cpp
@@ -80,17 +80,19 @@ DOMSVGLengthList::InternalListLengthWill
 
   // If our length will decrease, notify the items that will be removed:
   for (PRUint32 i = aNewLength; i < oldLength; ++i) {
     if (mItems[i]) {
       mItems[i]->RemovingFromList();
     }
   }
 
-  if (!mItems.SetLength(aNewLength)) { // OOM
+  if (!mItems.SetLength(aNewLength)) {
+    // We silently ignore SetLength OOM failure since being out of sync is safe
+    // so long as we have *fewer* items than our internal list.
     mItems.Clear();
     return;
   }
 
   // If our length has increased, null out the new pointers:
   for (PRUint32 i = oldLength; i < aNewLength; ++i) {
     mItems[i] = nsnull;
   }
@@ -199,36 +201,41 @@ DOMSVGLengthList::InsertItemBefore(nsIDO
   if (IsAnimValList()) {
     return NS_ERROR_DOM_NO_MODIFICATION_ALLOWED_ERR;
   }
 
   nsCOMPtr<DOMSVGLength> domItem = do_QueryInterface(newItem);
   if (!domItem) {
     return NS_ERROR_DOM_SVG_WRONG_TYPE_ERR;
   }
+  if (domItem->HasOwner()) {
+    domItem = domItem->Copy(); // must do this before changing anything!
+  }
   index = NS_MIN(index, Length());
-  SVGLength length = domItem->ToSVGLength(); // get before setting domItem
-  if (domItem->HasOwner()) {
-    domItem = new DOMSVGLength();
-  }
-  PRBool ok = !!InternalList().InsertItem(index, length);
-  if (!ok) {
+
+  // Ensure we have enough memory so we can avoid complex error handling below:
+  if (!mItems.SetCapacity(mItems.Length() + 1) ||
+      !InternalList().SetCapacity(InternalList().Length() + 1)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
+
+  InternalList().InsertItem(index, domItem->ToSVGLength());
+  mItems.InsertElementAt(index, domItem.get());
+
+  // This MUST come after the insertion into InternalList(), or else under the
+  // insertion into InternalList() the values read from domItem would be bad
+  // data from InternalList() itself!:
   domItem->InsertingIntoList(this, AttrEnum(), index, IsAnimValList());
-  ok = !!mItems.InsertElementAt(index, domItem.get());
-  if (!ok) {
-    InternalList().RemoveItem(index);
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
+
   for (PRUint32 i = index + 1; i < Length(); ++i) {
     if (mItems[i]) {
       mItems[i]->UpdateListIndex(i);
     }
   }
+
   Element()->DidChangeLengthList(AttrEnum(), PR_TRUE);
 #ifdef MOZ_SMIL
   if (mAList->IsAnimating()) {
     Element()->AnimationNeedsResample();
   }
 #endif
   *_retval = domItem.forget().get();
   return NS_OK;
@@ -246,28 +253,32 @@ DOMSVGLengthList::ReplaceItem(nsIDOMSVGL
 
   nsCOMPtr<DOMSVGLength> domItem = do_QueryInterface(newItem);
   if (!domItem) {
     return NS_ERROR_DOM_SVG_WRONG_TYPE_ERR;
   }
   if (index >= Length()) {
     return NS_ERROR_DOM_INDEX_SIZE_ERR;
   }
-  SVGLength length = domItem->ToSVGLength(); // get before setting domItem
   if (domItem->HasOwner()) {
-    domItem = new DOMSVGLength();
+    domItem = domItem->Copy(); // must do this before changing anything!
   }
+
   if (mItems[index]) {
     // Notify any existing DOM item of removal *before* modifying the lists so
     // that the DOM item can copy the *old* value at its index:
     mItems[index]->RemovingFromList();
   }
-  InternalList()[index] = length;
+
+  InternalList()[index] = domItem->ToSVGLength();
+  mItems[index] = domItem;
+
+  // This MUST come after the ToSVGPoint() call, otherwise that call
+  // would end up reading bad data from InternalList()!
   domItem->InsertingIntoList(this, AttrEnum(), index, IsAnimValList());
-  mItems[index] = domItem;
 
   Element()->DidChangeLengthList(AttrEnum(), PR_TRUE);
 #ifdef MOZ_SMIL
   if (mAList->IsAnimating()) {
     Element()->AnimationNeedsResample();
   }
 #endif
   NS_ADDREF(*_retval = domItem.get());
@@ -287,26 +298,27 @@ DOMSVGLengthList::RemoveItem(PRUint32 in
     return NS_ERROR_DOM_INDEX_SIZE_ERR;
   }
   // We have to return the removed item, so make sure it exists:
   EnsureItemAt(index);
 
   // Notify the DOM item of removal *before* modifying the lists so that the
   // DOM item can copy its *old* value:
   mItems[index]->RemovingFromList();
+  NS_ADDREF(*_retval = mItems[index]);
 
   InternalList().RemoveItem(index);
+  mItems.RemoveElementAt(index);
 
-  NS_ADDREF(*_retval = mItems[index]);
-  mItems.RemoveElementAt(index);
   for (PRUint32 i = index; i < Length(); ++i) {
     if (mItems[i]) {
       mItems[i]->UpdateListIndex(i);
     }
   }
+
   Element()->DidChangeLengthList(AttrEnum(), PR_TRUE);
 #ifdef MOZ_SMIL
   if (mAList->IsAnimating()) {
     Element()->AnimationNeedsResample();
   }
 #endif
   return NS_OK;
 }
--- a/content/svg/content/src/DOMSVGLengthList.h
+++ b/content/svg/content/src/DOMSVGLengthList.h
@@ -71,27 +71,26 @@ class DOMSVGLengthList : public nsIDOMSV
 {
   friend class DOMSVGLength;
 
 public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS(DOMSVGLengthList)
   NS_DECL_NSIDOMSVGLENGTHLIST
 
-  DOMSVGLengthList(DOMSVGAnimatedLengthList *aAList)
+  DOMSVGLengthList(DOMSVGAnimatedLengthList *aAList,
+                   const SVGLengthList &aInternalList)
     : mAList(aAList)
   {
-    // We silently ignore SetLength OOM failure since being out of sync is safe
-    // so long as we have *fewer* items than our internal list.
-
-    mItems.SetLength(InternalList().Length());
-    for (PRUint32 i = 0; i < Length(); ++i) {
-      // null out all the pointers - items are created on-demand
-      mItems[i] = nsnull;
-    }
+    // aInternalList must be passed in explicitly because we can't use
+    // InternalList() here. (Because it depends on IsAnimValList, which depends
+    // on this object having been assigned to aAList's mBaseVal or mAnimVal,
+    // which hasn't happend yet.)
+    
+    InternalListLengthWillChange(aInternalList.Length()); // Sync mItems
   }
 
   ~DOMSVGLengthList() {
     // Our mAList's weak ref to us must be nulled out when we die. If GC has
     // unlinked us using the cycle collector code, then that has already
     // happened, and mAList is null.
     if (mAList) {
       ( IsAnimValList() ? mAList->mAnimVal : mAList->mBaseVal ) = nsnull;
@@ -124,16 +123,18 @@ private:
   }
 
   PRUint8 Axis() const {
     return mAList->mAxis;
   }
 
   /// Used to determine if this list is the baseVal or animVal list.
   PRBool IsAnimValList() const {
+    NS_ABORT_IF_FALSE(this == mAList->mBaseVal || this == mAList->mAnimVal,
+                      "Calling IsAnimValList() too early?!");
     return this == mAList->mAnimVal;
   }
 
   /**
    * Get a reference to this object's corresponding internal SVGLengthList.
    *
    * To simplify the code we just have this one method for obtaining both
    * baseVal and animVal internal lists. This means that animVal lists don't
--- a/content/svg/content/src/DOMSVGNumberList.cpp
+++ b/content/svg/content/src/DOMSVGNumberList.cpp
@@ -199,36 +199,41 @@ DOMSVGNumberList::InsertItemBefore(nsIDO
   if (IsAnimValList()) {
     return NS_ERROR_DOM_NO_MODIFICATION_ALLOWED_ERR;
   }
 
   nsCOMPtr<DOMSVGNumber> domItem = do_QueryInterface(newItem);
   if (!domItem) {
     return NS_ERROR_DOM_SVG_WRONG_TYPE_ERR;
   }
+  if (domItem->HasOwner()) {
+    domItem = domItem->Clone(); // must do this before changing anything!
+  }
   index = NS_MIN(index, Length());
-  float value = domItem->ToSVGNumber(); // get before setting domItem
-  if (domItem->HasOwner()) {
-    domItem = new DOMSVGNumber();
-  }
-  PRBool ok = !!InternalList().InsertItem(index, value);
-  if (!ok) {
+
+  // Ensure we have enough memory so we can avoid complex error handling below:
+  if (!mItems.SetCapacity(mItems.Length() + 1) ||
+      !InternalList().SetCapacity(InternalList().Length() + 1)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
+
+  InternalList().InsertItem(index, domItem->ToSVGNumber());
+  mItems.InsertElementAt(index, domItem.get());
+
+  // This MUST come after the insertion into InternalList(), or else under the
+  // insertion into InternalList() the values read from domItem would be bad
+  // data from InternalList() itself!:
   domItem->InsertingIntoList(this, AttrEnum(), index, IsAnimValList());
-  ok = !!mItems.InsertElementAt(index, domItem.get());
-  if (!ok) {
-    InternalList().RemoveItem(index);
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
+
   for (PRUint32 i = index + 1; i < Length(); ++i) {
     if (mItems[i]) {
       mItems[i]->UpdateListIndex(i);
     }
   }
+
   Element()->DidChangeNumberList(AttrEnum(), PR_TRUE);
 #ifdef MOZ_SMIL
   if (mAList->IsAnimating()) {
     Element()->AnimationNeedsResample();
   }
 #endif
   *_retval = domItem.forget().get();
   return NS_OK;
@@ -246,28 +251,32 @@ DOMSVGNumberList::ReplaceItem(nsIDOMSVGN
 
   nsCOMPtr<DOMSVGNumber> domItem = do_QueryInterface(newItem);
   if (!domItem) {
     return NS_ERROR_DOM_SVG_WRONG_TYPE_ERR;
   }
   if (index >= Length()) {
     return NS_ERROR_DOM_INDEX_SIZE_ERR;
   }
-  float length = domItem->ToSVGNumber(); // get before setting domItem
   if (domItem->HasOwner()) {
-    domItem = new DOMSVGNumber();
+    domItem = domItem->Clone(); // must do this before changing anything!
   }
+
   if (mItems[index]) {
     // Notify any existing DOM item of removal *before* modifying the lists so
     // that the DOM item can copy the *old* value at its index:
     mItems[index]->RemovingFromList();
   }
-  InternalList()[index] = length;
+
+  InternalList()[index] = domItem->ToSVGNumber();
+  mItems[index] = domItem;
+
+  // This MUST come after the ToSVGPoint() call, otherwise that call
+  // would end up reading bad data from InternalList()!
   domItem->InsertingIntoList(this, AttrEnum(), index, IsAnimValList());
-  mItems[index] = domItem;
 
   Element()->DidChangeNumberList(AttrEnum(), PR_TRUE);
 #ifdef MOZ_SMIL
   if (mAList->IsAnimating()) {
     Element()->AnimationNeedsResample();
   }
 #endif
   NS_ADDREF(*_retval = domItem.get());
@@ -287,26 +296,27 @@ DOMSVGNumberList::RemoveItem(PRUint32 in
     return NS_ERROR_DOM_INDEX_SIZE_ERR;
   }
   // We have to return the removed item, so make sure it exists:
   EnsureItemAt(index);
 
   // Notify the DOM item of removal *before* modifying the lists so that the
   // DOM item can copy its *old* value:
   mItems[index]->RemovingFromList();
+  NS_ADDREF(*_retval = mItems[index]);
 
   InternalList().RemoveItem(index);
+  mItems.RemoveElementAt(index);
 
-  NS_ADDREF(*_retval = mItems[index]);
-  mItems.RemoveElementAt(index);
   for (PRUint32 i = index; i < Length(); ++i) {
     if (mItems[i]) {
       mItems[i]->UpdateListIndex(i);
     }
   }
+
   Element()->DidChangeNumberList(AttrEnum(), PR_TRUE);
 #ifdef MOZ_SMIL
   if (mAList->IsAnimating()) {
     Element()->AnimationNeedsResample();
   }
 #endif
   return NS_OK;
 }
--- a/content/svg/content/src/DOMSVGNumberList.h
+++ b/content/svg/content/src/DOMSVGNumberList.h
@@ -74,22 +74,22 @@ public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS(DOMSVGNumberList)
   NS_DECL_NSIDOMSVGNUMBERLIST
 
   DOMSVGNumberList(DOMSVGAnimatedNumberList *aAList,
                    const SVGNumberList &aInternalList)
     : mAList(aAList)
   {
-    // We can NOT use InternalList() here - it depends on IsAnimValList, which
-    // in turn depends on this object having been assigned to either aAList's
-    // mBaseVal or mAnimVal. At this point we haven't been assigned to either!
-    // This is why our caller must pass in aInternalList.
+    // aInternalList must be passed in explicitly because we can't use
+    // InternalList() here. (Because it depends on IsAnimValList, which depends
+    // on this object having been assigned to aAList's mBaseVal or mAnimVal,
+    // which hasn't happend yet.)
 
-    InternalListLengthWillChange(aInternalList.Length()); // Initialize mItems
+    InternalListLengthWillChange(aInternalList.Length()); // Sync mItems
   }
 
   ~DOMSVGNumberList() {
     // Our mAList's weak ref to us must be nulled out when we die. If GC has
     // unlinked us using the cycle collector code, then that has already
     // happened, and mAList is null.
     if (mAList) {
       ( IsAnimValList() ? mAList->mAnimVal : mAList->mBaseVal ) = nsnull;
--- a/content/svg/content/src/DOMSVGPathSegList.cpp
+++ b/content/svg/content/src/DOMSVGPathSegList.cpp
@@ -81,20 +81,18 @@ DOMSVGPathSegList::GetDOMWrapper(void *a
 /* static */ DOMSVGPathSegList*
 DOMSVGPathSegList::GetDOMWrapperIfExists(void *aList)
 {
   return sSVGPathSegListTearoffTable.GetTearoff(aList);
 }
 
 DOMSVGPathSegList::~DOMSVGPathSegList()
 {
-  // We no longer have any list items, and there are no script references to
-  // us.
-  //
-  // Do NOT use InternalList() here! That's different!
+  // There are now no longer any references to us held by script or list items.
+  // Note we must use GetAnimValKey/GetBaseValKey here, NOT InternalList()!
   void *key = mIsAnimValList ?
     InternalAList().GetAnimValKey() :
     InternalAList().GetBaseValKey();
   sSVGPathSegListTearoffTable.RemoveTearoff(key);
 }
 
 void
 DOMSVGPathSegList::InternalListWillChangeTo(const SVGPathData& aNewValue)
@@ -340,18 +338,18 @@ DOMSVGPathSegList::InsertItemBefore(nsID
 
   float segAsRaw[1 + NS_SVG_PATH_SEG_MAX_ARGS];
   domItem->ToSVGPathSegEncodedData(segAsRaw);
 
   InternalList().mData.InsertElementsAt(internalIndex, segAsRaw, 1 + argCount);
   mItems.InsertElementAt(aIndex, ItemProxy(domItem.get(), internalIndex));
 
   // This MUST come after the insertion into InternalList(), or else under the
-  // insertion into InternalList() the data read from domItem would be bad data
-  // from InternalList() itself!:
+  // insertion into InternalList() the values read from domItem would be bad
+  // data from InternalList() itself!:
   domItem->InsertingIntoList(this, aIndex, IsAnimValList());
 
   for (PRUint32 i = aIndex + 1; i < Length(); ++i) {
     mItems[i].mInternalDataIndex += 1 + argCount;
     if (ItemAt(i)) {
       ItemAt(i)->UpdateListIndex(i);
     }
   }
@@ -406,17 +404,17 @@ DOMSVGPathSegList::ReplaceItem(nsIDOMSVG
   PRBool ok = !!InternalList().mData.ReplaceElementsAt(
                   internalIndex, 1 + oldArgCount,
                   segAsRaw, 1 + newArgCount);
   if (!ok) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
   ItemAt(aIndex) = domItem;
 
-  // This MUST come after the ToSVGPathSegEncodedData call otherwise that call
+  // This MUST come after the ToSVGPathSegEncodedData call, otherwise that call
   // would end up reading bad data from InternalList()!
   domItem->InsertingIntoList(this, aIndex, IsAnimValList());
 
   PRUint32 delta = newArgCount - oldArgCount;
   if (delta != 0) {
     for (PRUint32 i = aIndex + 1; i < Length(); ++i) {
       mItems[i].mInternalDataIndex += delta;
     }
--- a/content/svg/content/src/DOMSVGPathSegList.h
+++ b/content/svg/content/src/DOMSVGPathSegList.h
@@ -155,22 +155,17 @@ private:
   /**
    * Only our static GetDOMWrapper() factory method may create objects of our
    * type.
    */
   DOMSVGPathSegList(nsSVGElement *aElement, PRBool aIsAnimValList)
     : mElement(aElement)
     , mIsAnimValList(aIsAnimValList)
   {
-    // This call populates mItems with the same number of items as there are
-    // segments contained in the internal list. We ignore OOM failure since
-    // being out of sync is safe so long as we have *fewer* items than our
-    // internal list.
-
-    InternalListWillChangeTo(InternalList());
+    InternalListWillChangeTo(InternalList()); // Sync mItems
   }
 
   ~DOMSVGPathSegList();
 
   nsSVGElement* Element() {
     return mElement.get();
   }
 
--- a/content/svg/content/src/DOMSVGPointList.cpp
+++ b/content/svg/content/src/DOMSVGPointList.cpp
@@ -81,20 +81,18 @@ DOMSVGPointList::GetDOMWrapper(void *aLi
 /* static */ DOMSVGPointList*
 DOMSVGPointList::GetDOMWrapperIfExists(void *aList)
 {
   return sSVGPointListTearoffTable.GetTearoff(aList);
 }
 
 DOMSVGPointList::~DOMSVGPointList()
 {
-  // We no longer have any list items, and there are no script references to
-  // us.
-  //
-  // Do NOT use InternalList() as the key here! That's different!
+  // There are now no longer any references to us held by script or list items.
+  // Note we must use GetAnimValKey/GetBaseValKey here, NOT InternalList()!
   void *key = mIsAnimValList ?
     InternalAList().GetAnimValKey() :
     InternalAList().GetBaseValKey();
   sSVGPointListTearoffTable.RemoveTearoff(key);
 }
 
 void
 DOMSVGPointList::InternalListWillChangeTo(const SVGPointList& aNewValue)
@@ -265,18 +263,19 @@ DOMSVGPointList::InsertItemBefore(nsIDOM
   if (!mItems.SetCapacity(mItems.Length() + 1) ||
       !InternalList().SetCapacity(InternalList().Length() + 1)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   InternalList().InsertItem(aIndex, domItem->ToSVGPoint());
   mItems.InsertElementAt(aIndex, domItem.get());
 
-  // This MUST come after the insertion into InternalList(), or else the data
-  // read from domItem would be bad data from InternalList() itself!
+  // This MUST come after the insertion into InternalList(), or else under the
+  // insertion into InternalList() the values read from domItem would be bad
+  // data from InternalList() itself!:
   domItem->InsertingIntoList(this, aIndex, IsAnimValList());
 
   for (PRUint32 i = aIndex + 1; i < Length(); ++i) {
     if (mItems[i]) {
       mItems[i]->UpdateListIndex(i);
     }
   }
 
@@ -315,17 +314,17 @@ DOMSVGPointList::ReplaceItem(nsIDOMSVGPo
     // Notify any existing DOM item of removal *before* modifying the lists so
     // that the DOM item can copy the *old* value at its index:
     mItems[aIndex]->RemovingFromList();
   }
 
   InternalList()[aIndex] = domItem->ToSVGPoint();
   mItems[aIndex] = domItem;
 
-  // This MUST come after the assignment to InternalList, otherwise that call
+  // This MUST come after the ToSVGPoint() call, otherwise that call
   // would end up reading bad data from InternalList()!
   domItem->InsertingIntoList(this, aIndex, IsAnimValList());
 
   Element()->DidChangePointList(PR_TRUE);
 #ifdef MOZ_SMIL
   if (AttrIsAnimating()) {
     Element()->AnimationNeedsResample();
   }
--- a/content/svg/content/src/DOMSVGPointList.h
+++ b/content/svg/content/src/DOMSVGPointList.h
@@ -155,21 +155,17 @@ private:
   /**
    * Only our static GetDOMWrapper() factory method may create objects of our
    * type.
    */
   DOMSVGPointList(nsSVGElement *aElement, PRBool aIsAnimValList)
     : mElement(aElement)
     , mIsAnimValList(aIsAnimValList)
   {
-    // This call populates mItems with the same number of items as there are
-    // points in the internal list. We ignore OOM failure since being out of
-    // sync is safe so long as we have *fewer* items than our internal list.
-
-    InternalListWillChangeTo(InternalList());
+    InternalListWillChangeTo(InternalList()); // Sync mItems
   }
 
   ~DOMSVGPointList();
 
   nsSVGElement* Element() {
     return mElement.get();
   }
 
@@ -185,18 +181,17 @@ private:
    * base val and anim val internal lists. This means that anim val lists don't
    * get const protection, but our setter methods guard against changing
    * anim val lists.
    */
   SVGPointList& InternalList();
 
   SVGAnimatedPointList& InternalAList();
 
-  /// Creates an instance of the appropriate DOMSVGPoint sub-class for
-  // aIndex, if it doesn't already exist.
+  /// Creates a DOMSVGPoint for aIndex, if it doesn't already exist.
   void EnsureItemAt(PRUint32 aIndex);
 
   // Weak refs to our DOMSVGPoint items. The items are friends and take care
   // of clearing our pointer to them when they die.
   nsTArray<DOMSVGPoint*> mItems;
 
   // Strong ref to our element to keep it alive. We hold this not only for
   // ourself, but also for our DOMSVGPoint items too.
--- a/content/svg/content/test/Makefile.in
+++ b/content/svg/content/test/Makefile.in
@@ -68,16 +68,17 @@ include $(topsrcdir)/config/rules.mk
 		test_pointer-events.xhtml \
 		test_pointer-events-2.xhtml \
 		test_scientific.html \
 		scientific-helper.svg \
 		test_SVGAnimatedImageSMILDisabled.html \
 		animated-svg-image-helper.html \
 		animated-svg-image-helper.svg \
 		test_SVGLengthList.xhtml \
+		test_SVGLengthList-2.xhtml \
 		test_SVGPathSegList.xhtml \
 		test_SVGStyleElement.xhtml \
 		test_SVGxxxList.xhtml \
 		test_switch.xhtml \
 		switch-helper.svg \
 		test_text.html \
 		text-helper.svg \
 		test_transform.xhtml \
new file mode 100644
--- /dev/null
+++ b/content/svg/content/test/test_SVGLengthList-2.xhtml
@@ -0,0 +1,50 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=630760
+-->
+<head>
+  <title>Tests specific to SVGLengthList</title>
+  <script type="text/javascript" src="/MochiKit/packed.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=515116">Mozilla Bug 630760</a>
+<p id="display"></p>
+<div id="content" style="display:none;">
+<svg id="svg" xmlns="http://www.w3.org/2000/svg" width="100" height="100">
+  <text id="text">
+    <set attributeName="x" to="10 20 30 40" begin="0" dur="indefinite"/>
+  </text>
+</svg>
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+<![CDATA[
+
+SimpleTest.waitForExplicitFinish();
+
+/*
+Check that the animVal list for 'x' on <text> gives the correct number of
+items when examined for the FIRST time DURING animation.
+*/
+
+function run_tests()
+{
+  document.getElementById('svg').pauseAnimations();
+
+  var text = document.getElementById("text");
+  var list = text.x.animVal;
+
+  is(list.numberOfItems, 4, 'Checking numberOfItems');
+
+  SimpleTest.finish();
+}
+
+window.addEventListener("load", run_tests, false);
+
+]]>
+</script>
+</pre>
+</body>
+</html>