Bug 880544 - Fix leak in DOMSVGLengthList. r=dzbarksy

content/svg/content/src/DOMSVGLengthList.cpp

@@ -176,7 +176,18 @@ DOMSVGLengthList::Initialize(nsIDOMSVGLength *newItem,
   return InsertItemBefore(newItem, 0, error);
 }
 
-nsIDOMSVGLength*
+already_AddRefed<nsIDOMSVGLength>
+DOMSVGLengthList::GetItem(uint32_t index, ErrorResult& error)
+{
+  bool found;
+  nsRefPtr<nsIDOMSVGLength> item = IndexedGetter(index, found, error);
+  if (!found) {
+    error.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR);
+  }
+  return item.forget();
+}
+
+already_AddRefed<nsIDOMSVGLength>
 DOMSVGLengthList::IndexedGetter(uint32_t index, bool& found, ErrorResult& error)
 {
   if (IsAnimValList()) {
@@ -184,8 +195,7 @@ DOMSVGLengthList::IndexedGetter(uint32_t index, bool& found, ErrorResult& error)
   }
   found = index < LengthNoFlush();
   if (found) {
-    EnsureItemAt(index);
-    return mItems[index];
+    return GetItemAt(index);
   }
   return nullptr;
 }
@@ -307,13 +317,12 @@ DOMSVGLengthList::RemoveItem(uint32_t index,
   // internal value.
   MaybeRemoveItemFromAnimValListAt(index);
 
-  // We have to return the removed item, so make sure it exists:
-  EnsureItemAt(index);
+  // We have to return the removed item, so get it, creating it if necessary:
+  nsCOMPtr<nsIDOMSVGLength> result = GetItemAt(index);
 
   // Notify the DOM item of removal *before* modifying the lists so that the
   // DOM item can copy its *old* value:
   mItems[index]->RemovingFromList();
-  nsCOMPtr<nsIDOMSVGLength> result = mItems[index];
 
   InternalList().RemoveItem(index);
   mItems.RemoveElementAt(index);
@@ -327,12 +336,16 @@ DOMSVGLengthList::RemoveItem(uint32_t index,
   return result.forget();
 }
 
-void
-DOMSVGLengthList::EnsureItemAt(uint32_t aIndex)
+already_AddRefed<nsIDOMSVGLength>
+DOMSVGLengthList::GetItemAt(uint32_t aIndex)
 {
+  MOZ_ASSERT(aIndex < mItems.Length());
+
   if (!mItems[aIndex]) {
     mItems[aIndex] = new DOMSVGLength(this, AttrEnum(), aIndex, IsAnimValList());
   }
+  nsRefPtr<nsIDOMSVGLength> result = mItems[aIndex];
+  return result.forget();
 }
 
 void

content/svg/content/src/DOMSVGLengthList.h

@@ -103,17 +103,10 @@ public:
   void Clear(ErrorResult& aError);
   already_AddRefed<nsIDOMSVGLength> Initialize(nsIDOMSVGLength *newItem,
                                                ErrorResult& error);
-  nsIDOMSVGLength* GetItem(uint32_t index, ErrorResult& error)
-  {
-    bool found;
-    nsIDOMSVGLength* item = IndexedGetter(index, found, error);
-    if (!found) {
-      error.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR);
-    }
-    return item;
-  }
-  nsIDOMSVGLength* IndexedGetter(uint32_t index, bool& found,
-                                 ErrorResult& error);
+  already_AddRefed<nsIDOMSVGLength> GetItem(uint32_t index,
+                                            ErrorResult& error);
+  already_AddRefed<nsIDOMSVGLength> IndexedGetter(uint32_t index, bool& found,
+                                                  ErrorResult& error);
   already_AddRefed<nsIDOMSVGLength> InsertItemBefore(nsIDOMSVGLength *newItem,
                                                      uint32_t index,
                                                      ErrorResult& error);
@@ -163,8 +156,8 @@ private:
    */
   SVGLengthList& InternalList() const;
 
-  /// Creates a DOMSVGLength for aIndex, if it doesn't already exist.
-  void EnsureItemAt(uint32_t aIndex);
+  /// Returns the nsIDOMSVGLength at aIndex, creating it if necessary.
+  already_AddRefed<nsIDOMSVGLength> GetItemAt(uint32_t aIndex);
 
   void MaybeInsertNullInAnimValListAt(uint32_t aIndex);
   void MaybeRemoveItemFromAnimValListAt(uint32_t aIndex);

content/svg/content/src/crashtests/880544-2.svg

@@ -0,0 +1,15 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <script>//<![CDATA[
+
+function add_watch()
+{
+  document.getElementById("e").x.baseVal.watch("0", function(){});
+}
+
+window.addEventListener("load", add_watch, false);
+
+  //]]></script>
+
+  <text id="e" x="10">foo</text>
+
+</svg>

content/svg/content/src/crashtests/crashtests.list

@@ -67,3 +67,4 @@ load 837450-1.svg
 load 842463-1.html
 load 864509.svg
 load 880544-1.svg
+load 880544-2.svg

dom/bindings/Bindings.conf

@@ -933,8 +933,7 @@ DOMInterfaces = {
 
 'SVGLengthList': {
     'nativeType': 'mozilla::DOMSVGLengthList',
-    'headerFile': 'DOMSVGLengthList.h',
-    'resultNotAddRefed': [ 'getItem' ]
+    'headerFile': 'DOMSVGLengthList.h'
 },
 
 'SVGLinearGradientElement': {