Bug 1532661 - Part 6: Clean up BrowsingContext references more reliably, r=farre
authorNika Layzell <nika@thelayzells.com>
Thu, 14 Mar 2019 18:51:13 +0000
changeset 524935 97c2ee22169c641e88278fd0801abfc18db6c43f
parent 524934 856a3d16a4ca856711ad9e6368cccd5aa1e22118
child 524936 2b18f28016cfe2aceb450afb585f8685f07dae5f
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfarre
bugs1532661
milestone67.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 1532661 - Part 6: Clean up BrowsingContext references more reliably, r=farre Depends on D23047 Differential Revision: https://phabricator.services.mozilla.com/D23048
docshell/base/BrowsingContext.cpp
dom/ipc/TabChild.cpp
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@@ -226,33 +226,38 @@ void BrowsingContext::Detach(bool aFromI
   RefPtr<BrowsingContext> kungFuDeathGrip(this);
 
   BrowsingContextMap<RefPtr>::Ptr p;
   if (sCachedBrowsingContexts && (p = sCachedBrowsingContexts->lookup(Id()))) {
     MOZ_DIAGNOSTIC_ASSERT(!mParent || !mParent->mChildren.Contains(this));
     MOZ_DIAGNOSTIC_ASSERT(!mGroup || !mGroup->Toplevels().Contains(this));
     sCachedBrowsingContexts->remove(p);
   } else {
-    Children& children = mParent ? mParent->mChildren : mGroup->Toplevels();
+    Children* children = nullptr;
+    if (mParent) {
+      children = &mParent->mChildren;
+    } else if (mGroup) {
+      children = &mGroup->Toplevels();
+    }
 
-    // TODO(farre): This assert looks extremely fishy, I know, but
-    // what we're actually saying is this: if we're detaching, but our
-    // parent doesn't have any children, it is because we're being
-    // detached by the cycle collector destroying docshells out of
-    // order.
-    MOZ_DIAGNOSTIC_ASSERT(children.IsEmpty() || children.Contains(this));
+    if (children) {
+      // TODO(farre): This assert looks extremely fishy, I know, but
+      // what we're actually saying is this: if we're detaching, but our
+      // parent doesn't have any children, it is because we're being
+      // detached by the cycle collector destroying docshells out of
+      // order.
+      MOZ_DIAGNOSTIC_ASSERT(children->IsEmpty() || children->Contains(this));
 
-    children.RemoveElement(this);
+      children->RemoveElement(this);
+    }
   }
 
-  Group()->Unregister(this);
-
-  // By definition, we no longer are the current process for this
-  // BrowsingContext - clear our now-dead nsDocShell reference.
-  mDocShell = nullptr;
+  if (mGroup) {
+    mGroup->Unregister(this);
+  }
 
   // As our nsDocShell is going away, this should implicitly mark us as closed.
   // We directly set our member, rather than using a transaction as we're going
   // to send a `Detach` message to other processes either way.
   mClosed = true;
 
   if (!aFromIPC && XRE_IsContentProcess()) {
     auto cc = ContentChild::GetSingleton();
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -630,20 +630,22 @@ void TabChild::UpdateFrameType() {
                              ? nsIDocShell::FRAME_TYPE_BROWSER
                              : nsIDocShell::FRAME_TYPE_REGULAR);
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(TabChild)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(TabChild, TabChildBase)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mWebNav)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mBrowsingContext)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(TabChild, TabChildBase)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mWebNav)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mBrowsingContext)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(TabChild, TabChildBase)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(TabChild)
   NS_INTERFACE_MAP_ENTRY(nsIWebBrowserChrome)
   NS_INTERFACE_MAP_ENTRY(nsIWebBrowserChrome2)
@@ -919,16 +921,20 @@ TabChild::ProvideWindow(mozIDOMWindowPro
   ContentChild* cc = ContentChild::GetSingleton();
   return cc->ProvideWindowCommon(
       this, aParent, iframeMoz, aChromeFlags, aCalledFromJS, aPositionSpecified,
       aSizeSpecified, aURI, aName, aFeatures, aForceNoOpener, aLoadState,
       aWindowIsNew, aReturn);
 }
 
 void TabChild::DestroyWindow() {
+  if (mBrowsingContext) {
+    mBrowsingContext = nullptr;
+  }
+
   if (mCoalescedMouseEventFlusher) {
     mCoalescedMouseEventFlusher->RemoveObserver();
     mCoalescedMouseEventFlusher = nullptr;
   }
 
   // In case we don't have chance to process all entries, clean all data in
   // the queue.
   while (mToBeDispatchedMouseData.GetSize() > 0) {