Bug 880544 - Fix leak in DOMSVGTransformList. r=dzbarksy

content/svg/content/src/DOMSVGTransformList.cpp

@@ -173,7 +173,18 @@ DOMSVGTransformList::Initialize(SVGTransform& newItem, ErrorResult& error)
   return InsertItemBefore(*domItem, 0, error);
 }
 
-SVGTransform*
+already_AddRefed<SVGTransform>
+DOMSVGTransformList::GetItem(uint32_t index, ErrorResult& error)
+{
+  bool found;
+  nsRefPtr<SVGTransform> item = IndexedGetter(index, found, error);
+  if (!found) {
+    error.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR);
+  }
+  return item.forget();
+}
+
+already_AddRefed<SVGTransform>
 DOMSVGTransformList::IndexedGetter(uint32_t index, bool& found,
                                    ErrorResult& error)
 {
@@ -182,8 +193,7 @@ DOMSVGTransformList::IndexedGetter(uint32_t index, bool& found,
   }
   found = index < LengthNoFlush();
   if (found) {
-    EnsureItemAt(index);
-    return mItems[index];
+    return GetItemAt(index);
   }
   return nullptr;
 }
@@ -295,13 +305,12 @@ DOMSVGTransformList::RemoveItem(uint32_t index, ErrorResult& error)
   // 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:
+  nsRefPtr<SVGTransform> 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();
-  nsRefPtr<SVGTransform> result = mItems[index];
+  result->RemovingFromList();
 
   InternalList().RemoveItem(index);
   mItems.RemoveElementAt(index);
@@ -354,12 +363,16 @@ DOMSVGTransformList::Consolidate(ErrorResult& error)
 //----------------------------------------------------------------------
 // Implementation helpers:
 
-void
-DOMSVGTransformList::EnsureItemAt(uint32_t aIndex)
+already_AddRefed<SVGTransform>
+DOMSVGTransformList::GetItemAt(uint32_t aIndex)
 {
+  MOZ_ASSERT(aIndex < mItems.Length());
+
   if (!mItems[aIndex]) {
     mItems[aIndex] = new SVGTransform(this, aIndex, IsAnimValList());
   }
+  nsRefPtr<SVGTransform> result = mItems[aIndex];
+  return result.forget();
 }
 
 void

content/svg/content/src/DOMSVGTransformList.h

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

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

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

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

@@ -66,3 +66,4 @@ load 831561.html
 load 837450-1.svg
 load 842463-1.html
 load 864509.svg
+load 880544-1.svg

dom/bindings/Bindings.conf

@@ -1097,8 +1097,7 @@ DOMInterfaces = {
 
 'SVGTransformList': {
     'nativeType': 'mozilla::DOMSVGTransformList',
-    'headerFile': 'DOMSVGTransformList.h',
-    'resultNotAddRefed': [ 'getItem' ]
+    'headerFile': 'DOMSVGTransformList.h'
 },
 
 'SVGStringList': {