Issue #1643 - Follow-up: Ensure we properly clear our pointers when the

Presentation of a document is destroyed.

3 files changed, 24 insertions, 9 deletions

diff --git a/dom/base/ResizeObserverController.cpp b/dom/base/ResizeObserverController.cpp
index acc401a5e2..d2162bb62e 100644
--- a/dom/base/ResizeObserverController.cpp
+++ b/dom/base/ResizeObserverController.cpp
@@ -15,7 +15,7 @@ namespace dom {
 void
 ResizeObserverNotificationHelper::WillRefresh(TimeStamp aTime)
 {
-  MOZ_ASSERT(mOwner, "Why is mOwner already dead when this RefreshObserver is still registered?");
+  MOZ_DIAGNOSTIC_ASSERT(mOwner, "RefreshObserver should have been de-registered on time, but isn't.");
   if (mOwner) {
     mOwner->Notify();
   }
@@ -69,10 +69,8 @@ ResizeObserverNotificationHelper::Unregister()
   }
 
   nsRefreshDriver* refreshDriver = GetRefreshDriver();
-  if (!refreshDriver) {
-    // We can't access RefreshDriver now. Just abort the Unregister().
-    return;
-  }
+  MOZ_RELEASE_ASSERT(refreshDriver,
+                     "We should not leave a dangling reference to the observer around");
 
   refreshDriver->RemoveRefreshObserver(this, Flush_Display);
   mRegistered = false;
@@ -81,9 +79,8 @@ ResizeObserverNotificationHelper::Unregister()
 void
 ResizeObserverNotificationHelper::Disconnect()
 {
-  Unregister();
-  // Our owner is dying. Clear our pointer to it, in case we outlive it.
-  mOwner = nullptr;
+  MOZ_RELEASE_ASSERT(!mRegistered, "How can we die when registered?");
+  MOZ_RELEASE_ASSERT(!mOwner, "Forgot to clear weak pointer?");
 }
 
 ResizeObserverNotificationHelper::~ResizeObserverNotificationHelper()
@@ -111,6 +108,10 @@ ResizeObserverController::AddResizeObserver(ResizeObserver* aObserver)
   mResizeObservers.AppendElement(aObserver);
 }
 
+void ResizeObserverController::DetachFromDocument() {
+  mResizeObserverNotificationHelper->Unregister();
+}
+
 void
 ResizeObserverController::Notify()
 {
@@ -240,7 +241,10 @@ ResizeObserverController::GetShell() const
 
 ResizeObserverController::~ResizeObserverController()
 {
-  mResizeObserverNotificationHelper->Disconnect();
+  MOZ_RELEASE_ASSERT(
+      !mResizeObserverNotificationHelper->IsRegistered(),
+      "Nothing else should keep a reference to our notification helper when we go away");
+  mResizeObserverNotificationHelper->DetachFromOwner();
 }
 
 } // namespace dom
diff --git a/dom/base/ResizeObserverController.h b/dom/base/ResizeObserverController.h
index 5d7912b438..01169112aa 100644
--- a/dom/base/ResizeObserverController.h
+++ b/dom/base/ResizeObserverController.h
@@ -41,6 +41,10 @@ public:
 
   void Disconnect();
 
+  bool IsRegistered() const { return mRegistered; }

+

+  void DetachFromOwner() { mOwner = nullptr; }

+
 protected:
   virtual ~ResizeObserverNotificationHelper();
 
@@ -68,6 +72,7 @@ public:
   void Traverse(nsCycleCollectionTraversalCallback& aCb);
   void Unlink();
 
+  void DetachFromDocument();
   void AddResizeObserver(ResizeObserver* aObserver);
 
   /*
diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp
index ce37c77b2c..0396848a99 100644
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -3644,6 +3644,12 @@ nsDocument::DeleteShell()
   // objects for @font-face rules that came from the style set.
   RebuildUserFontSet();
 
+  if (mResizeObserverController) {
+    // If the shell is going away, we need to remove any links to this document
+    // from the observer.
+    mResizeObserverController->DetachFromDocument();
+  }
+
   nsIPresShell* oldShell = mPresShell;
   mPresShell = nullptr;
   UpdateFrameRequestCallbackSchedulingState(oldShell);