Bug 1288228 part 1: Move cleanup code from DOMSVGLength destructor into a helper-function. r?jwatt draft
authorDaniel Holbert <dholbert@cs.stanford.edu>
Thu, 21 Jul 2016 10:49:38 -0700
changeset 390761 d7392829df14381cd5cd334285c919634db8a615
parent 390122 234f9780a4cd5caa3f09cd4d0a7dbcf507d478cd
child 390762 1bfd3bf4e03ce01cfa4c3c40c9c7305f5b676f19
child 390763 b909dfff12d8a7404511609329a283e1a8fa5c10
push id23741
push userdholbert@mozilla.com
push dateThu, 21 Jul 2016 17:59:04 +0000
reviewersjwatt
bugs1288228
milestone50.0a1
Bug 1288228 part 1: Move cleanup code from DOMSVGLength destructor into a helper-function. r?jwatt MozReview-Commit-ID: 8CQCZZP8NUn
dom/svg/DOMSVGLength.cpp
dom/svg/DOMSVGLength.h
--- a/dom/svg/DOMSVGLength.cpp
+++ b/dom/svg/DOMSVGLength.cpp
@@ -135,31 +135,42 @@ DOMSVGLength::DOMSVGLength(nsSVGLength2*
   , mIsAnimValItem(aAnimVal)
   , mUnit(nsIDOMSVGLength::SVG_LENGTHTYPE_NUMBER)
   , mValue(0.0f)
   , mVal(aVal)
   , mSVGElement(aSVGElement)
 {
 }
 
-DOMSVGLength::~DOMSVGLength()
+void
+DOMSVGLength::CleanupWeakRefs()
 {
-  // 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.
+  // Our mList's weak ref to us must be nulled out when we die (or when we're
+  // cycle collected), so we that don't leave behind a pointer to
+  // free / soon-to-be-free memory.
   if (mList) {
+    MOZ_ASSERT(mList->mItems[mListIndex] == this,
+               "Clearing out the wrong list index...?");
     mList->mItems[mListIndex] = nullptr;
   }
 
+  // Similarly, we must update the tearoff table to remove its (non-owning)
+  // pointer to mVal.
   if (mVal) {
-    auto& table = mIsAnimValItem ? sAnimSVGLengthTearOffTable : sBaseSVGLengthTearOffTable;
+    auto& table = mIsAnimValItem ?
+      sAnimSVGLengthTearOffTable : sBaseSVGLengthTearOffTable;
     table.RemoveTearoff(mVal);
   }
 }
 
+DOMSVGLength::~DOMSVGLength()
+{
+  CleanupWeakRefs();
+}
+
 already_AddRefed<DOMSVGLength>
 DOMSVGLength::GetTearOff(nsSVGLength2* aVal, nsSVGElement* aSVGElement,
                          bool aAnimVal)
 {
   auto& table = aAnimVal ? sAnimSVGLengthTearOffTable : sBaseSVGLengthTearOffTable;
   RefPtr<DOMSVGLength> domLength = table.GetTearoff(aVal);
   if (!domLength) {
     domLength = new DOMSVGLength(aVal, aSVGElement, aAnimVal);
--- a/dom/svg/DOMSVGLength.h
+++ b/dom/svg/DOMSVGLength.h
@@ -218,16 +218,23 @@ private:
    * animVal items.
    */
   SVGLength& InternalItem();
 
 #ifdef DEBUG
   bool IndexIsValid();
 #endif
 
+  /**
+   * Clears soon-to-be-invalid weak references in external objects that were
+   * set up during the creation of this object. This should be called during
+   * destruction and during cycle collection.
+   */
+  void CleanupWeakRefs();
+
   RefPtr<DOMSVGLengthList> mList;
 
   // Bounds for the following are checked in the ctor, so be sure to update
   // that if you change the capacity of any of the following.
 
   uint32_t mListIndex:MOZ_SVG_LIST_INDEX_BIT_COUNT;
   uint32_t mAttrEnum:4; // supports up to 16 attributes
   uint32_t mIsAnimValItem:1;