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 521937 97c2ee22169c
parent 521936 856a3d16a4ca
child 521938 2b18f28016cf
push id10870
push usernbeleuzu@mozilla.com
push dateFri, 15 Mar 2019 20:00:07 +0000
treeherdermozilla-beta@c594aee5b7a4 [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) {