Bug 1633204 - Live WindowContexts should keep BrowsingContextGroup alive, r=farre

Differential Revision: https://phabricator.services.mozilla.com/D72893

docshell/base/BrowsingContextGroup.cpp

@@ -48,25 +48,17 @@ BrowsingContextGroup::BrowsingContextGroup(uint64_t aId) : mId(aId) {
       GetMainThreadSerialEventTarget(), "BrowsingContextGroup worker queue");
 }
 
-bool BrowsingContextGroup::Contains(BrowsingContext* aBrowsingContext) {
-  return aBrowsingContext->Group() == this;
+void BrowsingContextGroup::Register(nsISupports* aContext) {
+  MOZ_DIAGNOSTIC_ASSERT(aContext);
+  mContexts.PutEntry(aContext);
 }
 
-void BrowsingContextGroup::Register(BrowsingContext* aBrowsingContext) {
-  MOZ_DIAGNOSTIC_ASSERT(aBrowsingContext);
-  MOZ_DIAGNOSTIC_ASSERT(this == sChromeGroup ? aBrowsingContext->IsChrome()
-                                             : aBrowsingContext->IsContent(),
-                        "Only chrome BCs may exist in the chrome group, and "
-                        "only content BCs may exist in other groups");
-  mContexts.PutEntry(aBrowsingContext);
-}
-
-void BrowsingContextGroup::Unregister(BrowsingContext* aBrowsingContext) {
-  MOZ_DIAGNOSTIC_ASSERT(aBrowsingContext);
-  mContexts.RemoveEntry(aBrowsingContext);
+void BrowsingContextGroup::Unregister(nsISupports* aContext) {
+  MOZ_DIAGNOSTIC_ASSERT(aContext);
+  mContexts.RemoveEntry(aContext);
 
   if (mContexts.IsEmpty()) {
-    // There are no browsing context still referencing this group. We can clear
+    // There are no synced contexts still referencing this group. We can clear
     // all subscribers.
     UnsubscribeAllContentParents();
 
@@ -87,14 +79,14 @@ void BrowsingContextGroup::Unsubscribe(ContentParent* aOriginProcess) {
   mSubscribers.RemoveEntry(aOriginProcess);
   aOriginProcess->OnBrowsingContextGroupUnsubscribe(this);
 
-  // If this origin process still embeds any non-discarded BrowsingContexts in
-  // this BrowsingContextGroup, make sure to discard them, as this process is
-  // going away.
+  // 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 : mContexts) {
-    if (context.GetKey()->Canonical()->IsEmbeddedInProcess(
-            aOriginProcess->ChildID())) {
-      toDiscard.AppendElement(context.GetKey());
+  for (auto& context : mToplevels) {
+    if (context->Canonical()->IsEmbeddedInProcess(aOriginProcess->ChildID())) {
+      toDiscard.AppendElement(context);
     }
   }
   for (auto& context : toDiscard) {
@@ -128,7 +120,7 @@ void BrowsingContextGroup::EnsureSubscribed(ContentParent* aProcess) {
   // FIXME: This won't send non-discarded children of discarded BCs, but those
   // BCs will be in the process of being destroyed anyway.
   // FIXME: Prevent that situation from occuring.
-  nsTArray<SyncedContextInitializer> inits(mContexts.Count() * 2);
+  nsTArray<SyncedContextInitializer> inits(mContexts.Count());
   CollectContextInitializers(mToplevels, inits);
 
   // Send all of our contexts to the target content process.

docshell/base/BrowsingContextGroup.h

@@ -36,10 +36,10 @@ class BrowsingContextGroup final : public nsWrapperCache {
 
   typedef nsTHashtable<nsRefPtrHashKey<ContentParent>> ContentParents;
 
-  // Interact with the list of BrowsingContexts.
-  bool Contains(BrowsingContext* aContext);
-  void Register(BrowsingContext* aContext);
-  void Unregister(BrowsingContext* aContext);
+  // Interact with the list of synced contexts. This controls the lifecycle of
+  // the BrowsingContextGroup and contexts loaded within them.
+  void Register(nsISupports* aContext);
+  void Unregister(nsISupports* aContext);
 
   // Interact with the list of ContentParents
   void Subscribe(ContentParent* aOriginProcess);
@@ -126,10 +126,15 @@ class BrowsingContextGroup final : public nsWrapperCache {
 
   uint64_t mId;
 
-  // A BrowsingContextGroup contains a series of BrowsingContext objects. They
-  // are addressed using a hashtable to avoid linear lookup when adding or
-  // removing elements from the set.
-  nsTHashtable<nsRefPtrHashKey<BrowsingContext>> mContexts;
+  // A BrowsingContextGroup contains a series of {Browsing,Window}Context
+  // objects. They are addressed using a hashtable to avoid linear lookup when
+  // adding or removing elements from the set.
+  //
+  // FIXME: This list is only required over a counter to keep nested
+  // non-discarded contexts within discarded contexts alive. It should be
+  // removed in the future.
+  // FIXME: Consider introducing a better common base than `nsISupports`?
+  nsTHashtable<nsRefPtrHashKey<nsISupports>> mContexts;
 
   // The set of toplevel browsing contexts in the current BrowsingContextGroup.
   nsTArray<RefPtr<BrowsingContext>> mToplevels;

docshell/base/WindowContext.cpp

@@ -154,6 +154,7 @@ void WindowContext::Init() {
 
   // Register this to the browsing context.
   mBrowsingContext->RegisterWindowContext(this);
+  Group()->Register(this);
 }
 
 void WindowContext::Discard() {
@@ -167,6 +168,7 @@ void WindowContext::Discard() {
   mIsDiscarded = true;
   gWindowContexts->Remove(InnerWindowId());
   mBrowsingContext->UnregisterWindowContext(this);
+  Group()->Unregister(this);
 }
 
 WindowContext::WindowContext(BrowsingContext* aBrowsingContext,

dom/ipc/WindowGlobalParent.cpp

@@ -632,6 +632,14 @@ already_AddRefed<Promise> WindowGlobalParent::GetSecurityInfo(
 }
 
 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();