Bug 1650257: Part 1 - Stop discarding BCs from the parent on WindowGl…

…obal destruction. r=nika Differential Revision: https://phabricator.services.mozilla.com/D87485
ambroff · Aug 31, 2020 · 2debb11 · 2debb11
1 parent aab2137
commit 2debb11
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 37 deletions.
diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp
@@ -632,13 +632,14 @@ void BrowsingContext::Attach(bool aFromIPC, ContentParent* aOriginProcess) {
 }
 
 void BrowsingContext::Detach(bool aFromIPC) {
-  MOZ_DIAGNOSTIC_ASSERT(mEverAttached);
-
   MOZ_LOG(GetLog(), LogLevel::Debug,
           ("%s: Detaching 0x%08" PRIx64 " from 0x%08" PRIx64,
            XRE_IsParentProcess() ? "Parent" : "Child", Id(),
            GetParent() ? GetParent()->Id() : 0));
 
+  MOZ_DIAGNOSTIC_ASSERT(mEverAttached);
+  MOZ_DIAGNOSTIC_ASSERT(!mIsDiscarded);
+
   nsCOMPtr<nsIRequestContextService> rcsvc =
       net::RequestContextService::GetOrCreate();
   if (rcsvc) {
@@ -1162,6 +1163,25 @@ BrowsingContext::~BrowsingContext() {
   UnregisterBrowserId(this);
 }
 
+/* static */
+void BrowsingContext::DiscardFromContentParent(ContentParent* aCP) {
+  MOZ_ASSERT(XRE_IsParentProcess());
+
+  if (sBrowsingContexts) {
+    AutoTArray<RefPtr<BrowsingContext>, 8> toDiscard;
+    for (const auto& entry : *sBrowsingContexts) {
+      auto* bc = entry.GetData()->Canonical();
+      if (!bc->IsDiscarded() && bc->IsEmbeddedInProcess(aCP->ChildID())) {
+        toDiscard.AppendElement(bc);
+      }
+    }
+
+    for (BrowsingContext* bc : toDiscard) {
+      bc->Detach(/* aFromIPC */ true);
+    }
+  }
+}
+
 nsISupports* BrowsingContext::GetParentObject() const {
   return xpc::NativeGlobal(xpc::PrivilegedJunkScope());
 }

diff --git a/docshell/base/BrowsingContext.h b/docshell/base/BrowsingContext.h
@@ -193,6 +193,8 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
     return GetFromWindow(aProxy);
   }
 
+  static void DiscardFromContentParent(ContentParent* aCP);
+
   // Create a brand-new toplevel BrowsingContext with no relationships to other
   // BrowsingContexts, and which is not embedded within any <browser> or frame
   // element.

diff --git a/docshell/base/BrowsingContextGroup.cpp b/docshell/base/BrowsingContextGroup.cpp
@@ -172,20 +172,6 @@ void BrowsingContextGroup::Unsubscribe(ContentParent* aProcess) {
   MOZ_DIAGNOSTIC_ASSERT(!hostEntry || hostEntry.Data() != aProcess,
                         "Unsubscribing existing host entry");
 #endif
-
-  // If this origin process embeds any non-discarded windowless
-  // BrowsingContexts, make sure to discard them, as this process is going away.
-  // Nested subframes will be discarded by WindowGlobalParent when it is
-  // destroyed by IPC.
-  nsTArray<RefPtr<BrowsingContext>> toDiscard;
-  for (auto& context : mToplevels) {
-    if (context->Canonical()->IsEmbeddedInProcess(aProcess->ChildID())) {
-      toDiscard.AppendElement(context);
-    }
-  }
-  for (auto& context : toDiscard) {
-    context->Detach(/* aFromIPC */ true);
-  }
 }
 
 ContentParent* BrowsingContextGroup::GetHostProcess(

diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp
@@ -60,6 +60,7 @@
 #include "mozilla/HangDetails.h"
 #include "mozilla/LoginReputationIPC.h"
 #include "mozilla/LookAndFeel.h"
+#include "mozilla/NullPrincipal.h"
 #include "mozilla/PerformanceMetricsCollector.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/PresShell.h"
@@ -227,6 +228,7 @@
 #include "nsPluginTags.h"
 #include "nsQueryObject.h"
 #include "nsReadableUtils.h"
+#include "nsSHistory.h"
 #include "nsScriptError.h"
 #include "nsSerializationHelper.h"
 #include "nsServiceManagerUtils.h"
@@ -1932,8 +1934,11 @@ void ContentParent::ActorDestroy(ActorDestroyReason why) {
   a11y::AccessibleWrap::ReleaseContentProcessIdFor(ChildID());
 #endif
 
-  // As this process is going away, ensure that every BrowsingContextGroup has
-  // been fully unsubscribed.
+  // As this process is going away, ensure that every BrowsingContext hosted by
+  // it has been detached, and every BrowsingContextGroup has been fully
+  // unsubscribed.
+  BrowsingContext::DiscardFromContentParent(this);
+
   nsTHashtable<nsRefPtrHashKey<BrowsingContextGroup>> groups;
   mGroups.SwapElements(groups);
   for (auto& group : groups) {

diff --git a/dom/ipc/WindowGlobalParent.cpp b/dom/ipc/WindowGlobalParent.cpp
@@ -740,14 +740,6 @@ void WindowGlobalParent::ActorDestroy(ActorDestroyReason aWhy) {
     }
   }
 
-  // If there are any non-discarded nested contexts when this WindowContext is
-  // destroyed, tear them down.
-  nsTArray<RefPtr<dom::BrowsingContext>> toDiscard;
-  toDiscard.AppendElements(Children());
-  for (auto& context : toDiscard) {
-    context->Detach(/* aFromIPC */ true);
-  }
-
   // Note that our WindowContext has become discarded.
   WindowContext::Discard();
 

diff --git a/xpfe/appshell/nsAppShellService.cpp b/xpfe/appshell/nsAppShellService.cpp
@@ -321,18 +321,8 @@ class BrowserDestroyer final : public Runnable {
         mContainer(aContainer) {}
 
   static nsresult Destroy(nsIWebBrowser* aBrowser) {
-    RefPtr<BrowsingContext> bc;
-    if (nsCOMPtr<nsIDocShell> docShell = do_GetInterface(aBrowser)) {
-      bc = docShell->GetBrowsingContext();
-    }
-
     nsCOMPtr<nsIBaseWindow> window(do_QueryInterface(aBrowser));
-    nsresult rv = window->Destroy();
-    MOZ_ASSERT(bc);
-    if (bc) {
-      bc->Detach();
-    }
-    return rv;
+    return window->Destroy();
   }
 
   NS_IMETHOD