Bug 817256 - DOMSVGTransform should hold strong ref to matrix r=longsonr

content/svg/content/src/DOMSVGMatrix.cpp

@@ -21,9 +21,6 @@ namespace mozilla {
 // upon unlink.
 NS_IMPL_CYCLE_COLLECTION_CLASS(DOMSVGMatrix)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(DOMSVGMatrix)
-  if (tmp->mTransform) {
-    tmp->mTransform->ClearMatrixTearoff(tmp);
-  }
 NS_IMPL_CYCLE_COLLECTION_UNLINK(mTransform)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END

content/svg/content/src/DOMSVGMatrix.h

@@ -85,12 +85,6 @@ public:
     SetIsDOMBinding();
   }
 
-  ~DOMSVGMatrix() {
-    if (mTransform) {
-      mTransform->ClearMatrixTearoff(this);
-    }
-  }
-
   const gfxMatrix& Matrix() const {
     return mTransform ? mTransform->Matrixgfx() : mMatrix;
   }

content/svg/content/src/DOMSVGTransform.cpp

@@ -11,10 +11,13 @@
 #include <math.h>
 #include "nsContentUtils.h"
 #include "nsAttrValueInlines.h"
+#include "nsSVGAttrTearoffTable.h"
 #include "mozilla/dom/SVGTransformBinding.h"
 
 namespace mozilla {
 
+static nsSVGAttrTearoffTable<DOMSVGTransform, DOMSVGMatrix> sSVGMatrixTearoffTable;
+
 //----------------------------------------------------------------------
 // nsISupports methods:
 
@@ -67,7 +70,6 @@ DOMSVGTransform::DOMSVGTransform(DOMSVGTransformList *aList,
   , mListIndex(aListIndex)
   , mIsAnimValItem(aIsAnimValItem)
   , mTransform(nullptr)
-  , mMatrixTearoff(nullptr)
 {
   SetIsDOMBinding();
   // These shifts are in sync with the members in the header.
@@ -84,7 +86,6 @@ DOMSVGTransform::DOMSVGTransform()
   , mTransform(new SVGTransform()) // Default ctor for objects not in a list
                                    // initialises to matrix type with identity
                                    // matrix
-  , mMatrixTearoff(nullptr)
 {
   SetIsDOMBinding();
 }
@@ -94,7 +95,6 @@ DOMSVGTransform::DOMSVGTransform(const gfxMatrix &aMatrix)
   , mListIndex(0)
   , mIsAnimValItem(false)
   , mTransform(new SVGTransform(aMatrix))
-  , mMatrixTearoff(nullptr)
 {
   SetIsDOMBinding();
 }
@@ -104,11 +104,20 @@ DOMSVGTransform::DOMSVGTransform(const SVGTransform &aTransform)
   , mListIndex(0)
   , mIsAnimValItem(false)
   , mTransform(new SVGTransform(aTransform))
-  , mMatrixTearoff(nullptr)
 {
   SetIsDOMBinding();
 }
 
+DOMSVGTransform::~DOMSVGTransform()
+{
+  sSVGMatrixTearoffTable.RemoveTearoff(this);
+  // Our mList'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 mList is null.
+  if (mList) {
+    mList->mItems[mListIndex] = nullptr;
+  }
+}
 
 uint16_t
 DOMSVGTransform::Type() const
@@ -119,11 +128,13 @@ DOMSVGTransform::Type() const
 already_AddRefed<DOMSVGMatrix>
 DOMSVGTransform::Matrix()
 {
-  if (!mMatrixTearoff) {
-    mMatrixTearoff = new DOMSVGMatrix(*this);
+  nsRefPtr<DOMSVGMatrix> wrapper =
+    sSVGMatrixTearoffTable.GetTearoff(this);
+  if (!wrapper) {
+    wrapper = new DOMSVGMatrix(*this);
+    sSVGMatrixTearoffTable.AddTearoff(this, wrapper);
   }
-  nsRefPtr<DOMSVGMatrix> matrix = mMatrixTearoff;
-  return matrix.forget();
+  return wrapper.forget();
 }
 
 float
@@ -318,15 +329,6 @@ DOMSVGTransform::SetMatrix(const gfxMatrix& aMatrix)
   NotifyElementDidChange(emptyOrOldValue);
 }
 
-void
-DOMSVGTransform::ClearMatrixTearoff(DOMSVGMatrix* aMatrix)
-{
-  NS_ABORT_IF_FALSE(mMatrixTearoff == aMatrix,
-      "Unexpected matrix pointer to be cleared");
-  mMatrixTearoff = nullptr;
-}
-
-
 //----------------------------------------------------------------------
 // Implementation helpers
 

content/svg/content/src/DOMSVGTransform.h

@@ -69,19 +69,7 @@ public:
    */
   explicit DOMSVGTransform(const SVGTransform &aMatrix);
 
-  ~DOMSVGTransform() {
-    // Our matrix tear-off pointer should be cleared before we are destroyed
-    // (since matrix tear-offs keep an owning reference to their transform, and
-    // clear the tear-off pointer themselves if unlinked).
-    NS_ABORT_IF_FALSE(!mMatrixTearoff, "Matrix tear-off pointer not cleared."
-        " Transform being destroyed before matrix?");
-    // Our mList'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 mList is null.
-    if (mList) {
-      mList->mItems[mListIndex] = nullptr;
-    }
-  }
+  ~DOMSVGTransform();
 
   /**
    * Create an unowned copy of an owned transform. The caller is responsible for
@@ -145,7 +133,7 @@ public:
   uint16_t Type() const;
   already_AddRefed<DOMSVGMatrix> Matrix();
   float Angle() const;
-  void SetMatrix(mozilla::DOMSVGMatrix& matrix, ErrorResult& rv);
+  void SetMatrix(DOMSVGMatrix& matrix, ErrorResult& rv);
   void SetTranslate(float tx, float ty, ErrorResult& rv);
   void SetScale(float sx, float sy, ErrorResult& rv);
   void SetRotate(float angle, float cx, float cy, ErrorResult& rv);
@@ -162,7 +150,6 @@ protected:
     return Transform().Matrix();
   }
   void SetMatrix(const gfxMatrix& aMatrix);
-  void ClearMatrixTearoff(DOMSVGMatrix* aMatrix);
 
 private:
   nsSVGElement* Element() {
@@ -204,14 +191,6 @@ private:
   // with any particular list and thus, no internal SVGTransform object. In
   // that case we allocate an SVGTransform object on the heap to store the data.
   nsAutoPtr<SVGTransform> mTransform;
-
-  // Weak ref to DOMSVGMatrix tearoff. The DOMSVGMatrix object will take of
-  // clearing this pointer when it is destroyed (by calling ClearMatrixTearoff).
-  //
-  // If this extra pointer member proves undesirable, it can be replaced with
-  // a hashmap (nsSVGAttrTearoffTable) to map from DOMSVGTransform to
-  // DOMSVGMatrix.
-  DOMSVGMatrix* mMatrixTearoff;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(DOMSVGTransform, MOZILLA_DOMSVGTRANSFORM_IID)