Bug 1288228 part 1: Move cleanup code from DOMSVGLength destructor into a helper-function. r=jwatt
authorDaniel Holbert <dholbert@cs.stanford.edu>
Thu, 21 Jul 2016 10:49:38 -0700
changeset 346904 45d06c2c094d57644c3ce4069a2e5e341e5b9134
parent 346903 7f6d8a4e0ee62118b035c955bcbd7eb7f2fd538f
child 346905 4d20e25a44381263adc79c2b01a63d2067417ece
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs1288228
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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;