Bug 1580565 - Part 6: Add a unique ID to each BrowsingContextGroup, r=kmag

This allows us to explicitly specify BrowsingContextGroups when synchronizing
them. A major advantage of this is that it means we can handle an attempt to
create a BrowsingContext with a parent which the content process is unaware of,
which is possible due to changes to the EnsureSubscribed logic in earlier
patches in this stack.

This is OK, because in the case where the content process cannot see its parent,
the parent must be imminently discarding.

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

docshell/base/BrowsingContext.cpp

@@ -297,13 +297,13 @@ void BrowsingContext::EnsureAttached() {
     Register(this);
 
     // Attach the browsing context to the tree.
-    Attach();
+    Attach(/* aFromIPC */ false, /* aOriginProcess */ nullptr);
   }
 }
 
 /* static */
-already_AddRefed<BrowsingContext> BrowsingContext::CreateFromIPC(
+void BrowsingContext::CreateFromIPC(BrowsingContext::IPCInitializer&& aInit,
-    BrowsingContext::IPCInitializer&& aInit, BrowsingContextGroup* aGroup,
+                                    BrowsingContextGroup* aGroup,
                                     ContentParent* aOriginProcess) {
   MOZ_DIAGNOSTIC_ASSERT(aOriginProcess || XRE_IsContentProcess());
   MOZ_DIAGNOSTIC_ASSERT(aGroup);
@@ -345,9 +345,7 @@ already_AddRefed<BrowsingContext> BrowsingContext::CreateFromIPC(
 
   Register(context);
 
-  // Caller handles attaching us to the tree.
+  context->Attach(/* aFromIPC */ true, aOriginProcess);
-
-  return context.forget();
 }
 
 BrowsingContext::BrowsingContext(WindowContext* aParentWindow,
@@ -460,7 +458,7 @@ void BrowsingContext::Embed() {
   }
 }
 
-void BrowsingContext::Attach(bool aFromIPC) {
+void BrowsingContext::Attach(bool aFromIPC, ContentParent* aOriginProcess) {
   MOZ_DIAGNOSTIC_ASSERT(!mEverAttached);
   mEverAttached = true;
 
@@ -489,17 +487,19 @@ void BrowsingContext::Attach(bool aFromIPC) {
     PopupBlocker::RegisterOpenPopupSpam();
   }
 
-  if (!aFromIPC) {
+  if (XRE_IsContentProcess() && !aFromIPC) {
     // Send attach to our parent if we need to.
-    if (XRE_IsContentProcess()) {
+    ContentChild::GetSingleton()->SendCreateBrowsingContext(
-      ContentChild::GetSingleton()->SendAttachBrowsingContext(
+        mGroup->Id(), GetIPCInitializer());
+  } else if (XRE_IsParentProcess()) {
+    mGroup->EachOtherParent(aOriginProcess, [&](ContentParent* aParent) {
+      MOZ_DIAGNOSTIC_ASSERT(IsContent(),
+                            "chrome BCG cannot be synced to content process");
+      if (!Canonical()->IsEmbeddedInProcess(aParent->ChildID())) {
+        Unused << aParent->SendCreateBrowsingContext(mGroup->Id(),
                                                      GetIPCInitializer());
-    } else if (IsContent()) {
-      MOZ_DIAGNOSTIC_ASSERT(XRE_IsParentProcess());
-      mGroup->EachParent([&](ContentParent* aParent) {
-        Unused << aParent->SendAttachBrowsingContext(GetIPCInitializer());
-      });
       }
+    });
   }
 }
 
@@ -539,11 +539,11 @@ void BrowsingContext::Detach(bool aFromIPC) {
         // destroyed.
         if (!Canonical()->IsEmbeddedInProcess(aParent->ChildID()) &&
             !Canonical()->IsOwnedByProcess(aParent->ChildID())) {
-          aParent->SendDetachBrowsingContext(Id(), callback, callback);
+          aParent->SendDiscardBrowsingContext(this, callback, callback);
         }
       });
     } else if (!aFromIPC) {
-      ContentChild::GetSingleton()->SendDetachBrowsingContext(Id(), callback,
+      ContentChild::GetSingleton()->SendDiscardBrowsingContext(this, callback,
                                                                callback);
     }
   }

docshell/base/BrowsingContext.h

@@ -238,12 +238,6 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
     return mDocShell ? mDocShell->GetWindow() : nullptr;
   }
 
-  // Attach the current BrowsingContext to its parent, in both the child and the
-  // parent process. BrowsingContext objects are created attached by default, so
-  // this method need only be called when restoring cached BrowsingContext
-  // objects.
-  void Attach(bool aFromIPC = false);
-
   // Detach the current BrowsingContext from its parent, in both the
   // child and the parent process.
   void Detach(bool aFromIPC = false);
@@ -540,8 +534,8 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
   IPCInitializer GetIPCInitializer();
 
   // Create a BrowsingContext object from over IPC.
-  static already_AddRefed<BrowsingContext> CreateFromIPC(
+  static void CreateFromIPC(IPCInitializer&& aInitializer,
-      IPCInitializer&& aInitializer, BrowsingContextGroup* aGroup,
+                            BrowsingContextGroup* aGroup,
                             ContentParent* aOriginProcess);
 
   // Performs access control to check that 'this' can access 'aTarget'.
@@ -569,6 +563,8 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
                   FieldTuple&& aFields);
 
  private:
+  void Attach(bool aFromIPC, ContentParent* aOriginProcess);
+
   // Find the special browsing context if aName is '_self', '_parent',
   // '_top', but not '_blank'. The latter is handled in FindWithName
   BrowsingContext* FindWithSpecialName(const nsAString& aName,

docshell/base/BrowsingContextGroup.cpp

@@ -16,13 +16,31 @@
 namespace mozilla {
 namespace dom {
 
-BrowsingContextGroup::BrowsingContextGroup() {
+static StaticRefPtr<BrowsingContextGroup> sChromeGroup;
-  if (XRE_IsContentProcess()) {
+
-    ContentChild::GetSingleton()->HoldBrowsingContextGroup(this);
+static StaticAutoPtr<
-  } else {
+    nsDataHashtable<nsUint64HashKey, RefPtr<BrowsingContextGroup>>>
-    ContentParent::HoldBrowsingContextGroup(this);
+    sBrowsingContextGroups;
+
+already_AddRefed<BrowsingContextGroup> BrowsingContextGroup::GetOrCreate(
+    uint64_t aId) {
+  if (!sBrowsingContextGroups) {
+    sBrowsingContextGroups =
+        new nsDataHashtable<nsUint64HashKey, RefPtr<BrowsingContextGroup>>();
+    ClearOnShutdown(&sBrowsingContextGroups);
   }
 
+  auto entry = sBrowsingContextGroups->LookupForAdd(aId);
+  RefPtr<BrowsingContextGroup> group =
+      entry.OrInsert([&] { return do_AddRef(new BrowsingContextGroup(aId)); });
+  return group.forget();
+}
+
+already_AddRefed<BrowsingContextGroup> BrowsingContextGroup::Create() {
+  return GetOrCreate(nsContentUtils::GenerateBrowsingContextId());
+}
+
+BrowsingContextGroup::BrowsingContextGroup(uint64_t aId) : mId(aId) {
   mTimerEventQueue = ThrottledEventQueue::Create(
       GetMainThreadSerialEventTarget(), "BrowsingContextGroup timer queue");
 
@@ -36,6 +54,10 @@ bool BrowsingContextGroup::Contains(BrowsingContext* aBrowsingContext) {
 
 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);
 }
 
@@ -48,11 +70,6 @@ void BrowsingContextGroup::Unregister(BrowsingContext* aBrowsingContext) {
     // all subscribers.
     UnsubscribeAllContentParents();
 
-    if (XRE_IsContentProcess()) {
-      ContentChild::GetSingleton()->ReleaseBrowsingContextGroup(this);
-    } else {
-      ContentParent::ReleaseBrowsingContextGroup(this);
-    }
     // We may have been deleted here as the ContentChild/Parent may
     // have held the last references to `this`.
     // Do not access any members at this point.
@@ -115,7 +132,7 @@ void BrowsingContextGroup::EnsureSubscribed(ContentParent* aProcess) {
   CollectContextInitializers(mToplevels, inits);
 
   // Send all of our contexts to the target content process.
-  Unused << aProcess->SendRegisterBrowsingContextGroup(inits);
+  Unused << aProcess->SendRegisterBrowsingContextGroup(Id(), inits);
 
   // If the focused or active BrowsingContexts belong in this group, tell the
   // newly subscribed process.
@@ -140,6 +157,10 @@ BrowsingContextGroup::~BrowsingContextGroup() {
 }
 
 void BrowsingContextGroup::UnsubscribeAllContentParents() {
+  if (sBrowsingContextGroups) {
+    sBrowsingContextGroups->Remove(Id());
+  }
+
   for (auto iter = mSubscribers.Iter(); !iter.Done(); iter.Next()) {
     nsRefPtrHashKey<ContentParent>* entry = iter.Get();
     entry->GetKey()->OnBrowsingContextGroupUnsubscribe(this);
@@ -196,13 +217,11 @@ void BrowsingContextGroup::FlushPostMessageEvents() {
   }
 }
 
-static StaticRefPtr<BrowsingContextGroup> sChromeGroup;
-
 /* static */
 BrowsingContextGroup* BrowsingContextGroup::GetChromeGroup() {
   MOZ_DIAGNOSTIC_ASSERT(XRE_IsParentProcess());
   if (!sChromeGroup && XRE_IsParentProcess()) {
-    sChromeGroup = new BrowsingContextGroup();
+    sChromeGroup = BrowsingContextGroup::Create();
     ClearOnShutdown(&sChromeGroup);
   }
 
@@ -251,18 +270,20 @@ already_AddRefed<BrowsingContextGroup> BrowsingContextGroup::Select(
   if (aOpener) {
     return do_AddRef(aOpener->Group());
   }
-  return MakeAndAddRef<BrowsingContextGroup>();
+  return Create();
 }
 
-already_AddRefed<BrowsingContextGroup> BrowsingContextGroup::Select(
+void BrowsingContextGroup::GetAllGroups(
-    uint64_t aParentId, uint64_t aOpenerId) {
+    nsTArray<RefPtr<BrowsingContextGroup>>& aGroups) {
-  RefPtr<WindowContext> parent = WindowContext::GetById(aParentId);
+  aGroups.Clear();
-  MOZ_RELEASE_ASSERT(parent || aParentId == 0);
+  if (!sBrowsingContextGroups) {
+    return;
+  }
 
-  RefPtr<BrowsingContext> opener = BrowsingContext::Get(aOpenerId);
+  aGroups.SetCapacity(sBrowsingContextGroups->Count());
-  MOZ_RELEASE_ASSERT(opener || aOpenerId == 0);
+  for (auto& group : *sBrowsingContextGroups) {
-
+    aGroups.AppendElement(group.GetData());
-  return Select(parent, opener);
+  }
 }
 
 NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(BrowsingContextGroup, mContexts,

docshell/base/BrowsingContextGroup.h

@@ -55,18 +55,18 @@ class BrowsingContextGroup final : public nsWrapperCache {
     aToplevels.AppendElements(mToplevels);
   }
 
+  uint64_t Id() { return mId; }
+
   nsISupports* GetParentObject() const;
   JSObject* WrapObject(JSContext* aCx,
                        JS::Handle<JSObject*> aGivenProto) override;
 
-  BrowsingContextGroup();
+  // Get or create a BrowsingContextGroup with the given ID.
-
+  static already_AddRefed<BrowsingContextGroup> GetOrCreate(uint64_t aId);
+  static already_AddRefed<BrowsingContextGroup> Create();
   static already_AddRefed<BrowsingContextGroup> Select(
       WindowContext* aParent, BrowsingContext* aOpener);
 
-  static already_AddRefed<BrowsingContextGroup> Select(uint64_t aParentId,
-                                                       uint64_t aOpenerId);
-
   // For each 'ContentParent', except for 'aExcludedParent',
   // associated with this group call 'aCallback'.
   template <typename Func>
@@ -114,13 +114,18 @@ class BrowsingContextGroup final : public nsWrapperCache {
     return mWorkerEventQueue;
   }
 
+  static void GetAllGroups(nsTArray<RefPtr<BrowsingContextGroup>>& aGroups);
+
  private:
   friend class CanonicalBrowsingContext;
 
+  explicit BrowsingContextGroup(uint64_t aId);
   ~BrowsingContextGroup();
 
   void UnsubscribeAllContentParents();
 
+  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.

dom/ipc/ContentChild.cpp

@@ -2201,8 +2201,6 @@ void ContentChild::ActorDestroy(ActorDestroyReason why) {
 
   mIdleObservers.Clear();
 
-  mBrowsingContextGroupHolder.Clear();
-
   nsCOMPtr<nsIConsoleService> svc(do_GetService(NS_CONSOLESERVICE_CONTRACTID));
   if (svc) {
     svc->UnregisterListener(mConsoleListener);
@@ -3614,42 +3612,46 @@ PContentChild::Result ContentChild::OnMessageReceived(const Message& aMsg,
   return result;
 }
 
-mozilla::ipc::IPCResult ContentChild::RecvAttachBrowsingContext(
+mozilla::ipc::IPCResult ContentChild::RecvCreateBrowsingContext(
-    BrowsingContext::IPCInitializer&& aInit) {
+    uint64_t aGroupId, BrowsingContext::IPCInitializer&& aInit) {
-  RefPtr<BrowsingContext> child = BrowsingContext::Get(aInit.mId);
+  // We can't already have a BrowsingContext with this ID.
-  MOZ_RELEASE_ASSERT(!child || child->IsCached());
+  if (RefPtr<BrowsingContext> existing = BrowsingContext::Get(aInit.mId)) {
-
+    return IPC_FAIL(this, "Browsing context already exists");
-  if (!child) {
-    // Determine the BrowsingContextGroup from our parent or opener fields.
-    RefPtr<BrowsingContextGroup> group =
-        BrowsingContextGroup::Select(aInit.mParentId, aInit.GetOpenerId());
-    child = BrowsingContext::CreateFromIPC(std::move(aInit), group, nullptr);
   }
 
-  child->Attach(/* aFromIPC */ true);
+  RefPtr<WindowContext> parent = WindowContext::GetById(aInit.mParentId);
-
+  if (!parent && aInit.mParentId != 0) {
+    // Handle this case by ignoring the request, as parent must be in the
+    // process of being discarded.
+    // In the future it would be nice to avoid sending this message to the child
+    // at all.
+    NS_WARNING("Attempt to attach BrowsingContext to discarded parent");
     return IPC_OK();
   }
 
-mozilla::ipc::IPCResult ContentChild::RecvDetachBrowsingContext(
+  RefPtr<BrowsingContextGroup> group =
-    uint64_t aContextId, DetachBrowsingContextResolver&& aResolve) {
+      BrowsingContextGroup::GetOrCreate(aGroupId);
-  // NOTE: Immediately resolve the promise, as we've received the message. This
+  BrowsingContext::CreateFromIPC(std::move(aInit), group, nullptr);
-  // will allow the parent process to discard references to this BC.
+  return IPC_OK();
-  aResolve(true);
-
-  // If we can't find a BrowsingContext with the given ID, it's already been
-  // collected and we can ignore the request.
-  RefPtr<BrowsingContext> context = BrowsingContext::Get(aContextId);
-  if (context) {
-    context->Detach(/* aFromIPC */ true);
 }
 
+mozilla::ipc::IPCResult ContentChild::RecvDiscardBrowsingContext(
+    const MaybeDiscarded<BrowsingContext>& aContext,
+    DiscardBrowsingContextResolver&& aResolve) {
+  if (!aContext.IsNullOrDiscarded()) {
+    aContext.get()->Detach(/* aFromIPC */ true);
+  }
+
+  // Immediately resolve the promise, as we've received the message. This will
+  // allow the parent process to discard references to this BC.
+  aResolve(true);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult ContentChild::RecvRegisterBrowsingContextGroup(
-    nsTArray<SyncedContextInitializer>&& aInits) {
+    uint64_t aGroupId, nsTArray<SyncedContextInitializer>&& aInits) {
-  RefPtr<BrowsingContextGroup> group = new BrowsingContextGroup();
+  RefPtr<BrowsingContextGroup> group =
+      BrowsingContextGroup::GetOrCreate(aGroupId);
 
   // Each of the initializers in aInits is sorted in pre-order, so our parent
   // should always be available before the element itself.
@@ -3665,9 +3667,7 @@ mozilla::ipc::IPCResult ContentChild::RecvRegisterBrowsingContextGroup(
         MOZ_ASSERT_IF(parent, parent->Group() == group);
 #endif
 
-        RefPtr<BrowsingContext> ctxt =
         BrowsingContext::CreateFromIPC(std::move(init), group, nullptr);
-        ctxt->Attach(/* aFromIPC */ true);
         break;
       }
       case SyncedContextInitializer::TWindowContextInitializer: {
@@ -3999,14 +3999,6 @@ mozilla::ipc::IPCResult ContentChild::RecvDiscardWindowContext(
   return IPC_OK();
 }
 
-void ContentChild::HoldBrowsingContextGroup(BrowsingContextGroup* aBCG) {
-  mBrowsingContextGroupHolder.AppendElement(aBCG);
-}
-
-void ContentChild::ReleaseBrowsingContextGroup(BrowsingContextGroup* aBCG) {
-  mBrowsingContextGroupHolder.RemoveElement(aBCG);
-}
-
 mozilla::ipc::IPCResult ContentChild::RecvScriptError(
     const nsString& aMessage, const nsString& aSourceName,
     const nsString& aSourceLine, const uint32_t& aLineNumber,

dom/ipc/ContentChild.h

@@ -691,10 +691,6 @@ class ContentChild final
   PFileDescriptorSetChild* SendPFileDescriptorSetConstructor(
       const FileDescriptor& aFD) override;
 
-  const nsTArray<RefPtr<BrowsingContextGroup>>& BrowsingContextGroups() const {
-    return mBrowsingContextGroupHolder;
-  }
-
  private:
   static void ForceKillTimerCallback(nsITimer* aTimer, void* aClosure);
   void StartForceKillTimer();
@@ -710,14 +706,15 @@ class ContentChild final
 
   virtual void OnChannelReceivedMessage(const Message& aMsg) override;
 
-  mozilla::ipc::IPCResult RecvAttachBrowsingContext(
+  mozilla::ipc::IPCResult RecvCreateBrowsingContext(
-      BrowsingContext::IPCInitializer&& aInit);
+      uint64_t aGroupId, BrowsingContext::IPCInitializer&& aInit);
 
-  mozilla::ipc::IPCResult RecvDetachBrowsingContext(
+  mozilla::ipc::IPCResult RecvDiscardBrowsingContext(
-      uint64_t aContextId, DetachBrowsingContextResolver&& aResolve);
+      const MaybeDiscarded<BrowsingContext>& aContext,
+      DiscardBrowsingContextResolver&& aResolve);
 
   mozilla::ipc::IPCResult RecvRegisterBrowsingContextGroup(
-      nsTArray<SyncedContextInitializer>&& aInits);
+      uint64_t aGroupId, nsTArray<SyncedContextInitializer>&& aInits);
 
   mozilla::ipc::IPCResult RecvWindowClose(
       const MaybeDiscarded<BrowsingContext>& aContext, bool aTrustedCaller);
@@ -871,8 +868,6 @@ class ContentChild final
 
   uint32_t mNetworkLinkType = 0;
 
-  nsTArray<RefPtr<BrowsingContextGroup>> mBrowsingContextGroupHolder;
-
   // See `BrowsingContext::mEpochs` for an explanation of this field.
   uint64_t mBrowsingContextFieldEpoch = 0;
 

dom/ipc/ContentParent.cpp

@@ -586,8 +586,6 @@ StaticAutoPtr<LinkedList<ContentParent>> ContentParent::sContentParents;
 UniquePtr<SandboxBrokerPolicyFactory>
     ContentParent::sSandboxBrokerPolicyFactory;
 #endif
-StaticAutoPtr<nsTArray<RefPtr<BrowsingContextGroup>>>
-    ContentParent::sBrowsingContextGroupHolder;
 #if defined(XP_MACOSX) && defined(MOZ_SANDBOX)
 StaticAutoPtr<std::vector<std::string>> ContentParent::sMacSandboxParams;
 #endif
@@ -646,9 +644,6 @@ void ContentParent::StartUp() {
   // child process
   sCanLaunchSubprocesses = true;
 
-  sBrowsingContextGroupHolder = new nsTArray<RefPtr<BrowsingContextGroup>>();
-  ClearOnShutdown(&sBrowsingContextGroupHolder);
-
   if (!XRE_IsParentProcess()) {
     return;
   }
@@ -5900,56 +5895,54 @@ mozilla::ipc::IPCResult ContentParent::RecvSessionStorageData(
   return IPC_OK();
 }
 
-mozilla::ipc::IPCResult ContentParent::RecvAttachBrowsingContext(
+mozilla::ipc::IPCResult ContentParent::RecvCreateBrowsingContext(
-    BrowsingContext::IPCInitializer&& aInit) {
+    uint64_t aGroupId, BrowsingContext::IPCInitializer&& aInit) {
   RefPtr<WindowGlobalParent> parent;
   if (aInit.mParentId != 0) {
     parent = WindowGlobalParent::GetByInnerWindowId(aInit.mParentId);
-    MOZ_RELEASE_ASSERT(parent, "Parent doesn't exist in parent process");
+    if (!parent) {
+      return IPC_FAIL(this, "Parent doesn't exist in parent process");
+    }
   }
 
   if (parent && parent->GetContentParent() != this) {
     // We're trying attach a child BrowsingContext to a parent
-    // BrowsingContext in another process. This is illegal since the
+    // WindowContext in another process. This is illegal since the
-    // only thing that could create that child BrowsingContext is a
+    // only thing that could create that child BrowsingContext is the parent
-    // parent docshell in the same process as that BrowsingContext.
+    // window's process.
-    MOZ_DIAGNOSTIC_ASSERT(false,
+    return IPC_FAIL(this,
-                          "Trying to attach to out of process parent context");
+                    "Must create BrowsingContext from the parent's process");
+  }
 
-    MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Warning,
+  RefPtr<BrowsingContext> opener;
-            ("ParentIPC: Trying to attach to out of process parent context "
+  if (aInit.GetOpenerId() != 0) {
-             "0x%08" PRIx64,
+    opener = BrowsingContext::Get(aInit.GetOpenerId());
-             aInit.mParentId));
+    if (!opener) {
-    return IPC_OK();
+      return IPC_FAIL(this, "Opener doesn't exist in parent process");
+    }
   }
 
   RefPtr<BrowsingContext> child = BrowsingContext::Get(aInit.mId);
-  if (child && !child->IsCached()) {
+  if (child) {
-    // This is highly suspicious. BrowsingContexts should only be
+    // This is highly suspicious. BrowsingContexts should only be created once,
-    // attached at most once, but finding one indicates that someone
+    // so finding one indicates that someone is doing something they shouldn't.
-    // is doing something they shouldn't.
+    return IPC_FAIL(this, "A BrowsingContext with this ID already exists");
-    MOZ_DIAGNOSTIC_ASSERT(false,
-                          "Trying to attach already attached browsing context");
-
-    MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Warning,
-            ("ParentIPC: Trying to attach already attached 0x%08" PRIx64
-             " to 0x%08" PRIx64,
-             aInit.mId, aInit.mParentId));
-    return IPC_OK();
   }
 
-  if (!child) {
+  // Ensure that the passed-in BrowsingContextGroup is valid.
   RefPtr<BrowsingContextGroup> group =
-        BrowsingContextGroup::Select(aInit.mParentId, aInit.GetOpenerId());
+      BrowsingContextGroup::GetOrCreate(aGroupId);
-    child = BrowsingContext::CreateFromIPC(std::move(aInit), group, this);
+  if (parent && parent->Group() != group) {
+    return IPC_FAIL(this, "Parent is not in the given group");
+  }
+  if (opener && opener->Group() != group) {
+    return IPC_FAIL(this, "Opener is not in the given group");
+  }
+  if (!parent && !opener && !group->Toplevels().IsEmpty()) {
+    return IPC_FAIL(this, "Unrelated context must be created in a new group");
   }
 
-  child->Attach(/* aFromIPC */ true);
+  BrowsingContext::CreateFromIPC(std::move(aInit), group, this);
-
-  child->Group()->EachOtherParent(this, [&](ContentParent* aParent) {
-    Unused << aParent->SendAttachBrowsingContext(child->GetIPCInitializer());
-  });
-
   return IPC_OK();
 }
 
@@ -5979,28 +5972,21 @@ bool ContentParent::CheckBrowsingContextEmbedder(CanonicalBrowsingContext* aBC,
   return true;
 }
 
-mozilla::ipc::IPCResult ContentParent::RecvDetachBrowsingContext(
+mozilla::ipc::IPCResult ContentParent::RecvDiscardBrowsingContext(
-    uint64_t aContextId, DetachBrowsingContextResolver&& aResolve) {
+    const MaybeDiscarded<BrowsingContext>& aContext,
-  // NOTE: Immediately resolve the promise, as we've received the message. This
+    DiscardBrowsingContextResolver&& aResolve) {
-  // will allow the content process to discard references to this BC.
+  if (!aContext.IsNullOrDiscarded()) {
-  aResolve(true);
+    RefPtr<CanonicalBrowsingContext> context = aContext.get_canonical();
-
+    if (!CheckBrowsingContextEmbedder(context, "discard")) {
-  // NOTE: It's OK if we don't have this context anymore. It was just already
+      return IPC_FAIL(this, "Illegal Discard attempt");
-  // detached, return.
-  RefPtr<CanonicalBrowsingContext> context =
-      CanonicalBrowsingContext::Get(aContextId);
-  if (!context || context->IsDiscarded()) {
-    MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Debug,
-            ("ParentIPC: Trying to detach already detached"));
-    return IPC_OK();
-  }
-
-  if (!CheckBrowsingContextEmbedder(context, "detach")) {
-    return IPC_FAIL(this, "Illegal Detach() attempt");
     }
 
     context->Detach(/* aFromIPC */ true);
+  }
 
+  // Resolve the promise, as we've received and handled the message. This will
+  // allow the content process to fully-discard references to this BC.
+  aResolve(true);
   return IPC_OK();
 }
 
@@ -6313,18 +6299,6 @@ mozilla::ipc::IPCResult ContentParent::RecvWindowPostMessage(
   return IPC_OK();
 }
 
-/* static */
-void ContentParent::HoldBrowsingContextGroup(BrowsingContextGroup* aBCG) {
-  sBrowsingContextGroupHolder->AppendElement(aBCG);
-}
-
-/* static */
-void ContentParent::ReleaseBrowsingContextGroup(BrowsingContextGroup* aBCG) {
-  if (sBrowsingContextGroupHolder) {
-    sBrowsingContextGroupHolder->RemoveElement(aBCG);
-  }
-}
-
 void ContentParent::OnBrowsingContextGroupSubscribe(
     BrowsingContextGroup* aGroup) {
   MOZ_DIAGNOSTIC_ASSERT(aGroup);

dom/ipc/ContentParent.h

@@ -640,11 +640,12 @@ class ContentParent final
 
   static bool IsInputEventQueueSupported();
 
-  mozilla::ipc::IPCResult RecvAttachBrowsingContext(
+  mozilla::ipc::IPCResult RecvCreateBrowsingContext(
-      BrowsingContext::IPCInitializer&& aInit);
+      uint64_t aGroupId, BrowsingContext::IPCInitializer&& aInit);
 
-  mozilla::ipc::IPCResult RecvDetachBrowsingContext(
+  mozilla::ipc::IPCResult RecvDiscardBrowsingContext(
-      uint64_t aContextId, DetachBrowsingContextResolver&& aResolve);
+      const MaybeDiscarded<BrowsingContext>& aContext,
+      DiscardBrowsingContextResolver&& aResolve);
 
   mozilla::ipc::IPCResult RecvWindowClose(
       const MaybeDiscarded<BrowsingContext>& aContext, bool aTrustedCaller);
@@ -1290,9 +1291,6 @@ class ContentParent final
                                const bool& aMinimizeMemoryUsage,
                                const Maybe<FileDescriptor>& aDMDFile) override;
 
-  static void HoldBrowsingContextGroup(BrowsingContextGroup* aBCG);
-  static void ReleaseBrowsingContextGroup(BrowsingContextGroup* aBCG);
-
   void OnBrowsingContextGroupSubscribe(BrowsingContextGroup* aGroup);
   void OnBrowsingContextGroupUnsubscribe(BrowsingContextGroup* aGroup);
 

dom/ipc/PContent.ipdl

@@ -832,7 +832,7 @@ child:
 
     // Begin subscribing to a new BrowsingContextGroup, sending down the current
     // value for every individual BrowsingContext.
-    async RegisterBrowsingContextGroup(SyncedContextInitializer[] aInits);
+    async RegisterBrowsingContextGroup(uint64_t aGroupId, SyncedContextInitializer[] aInits);
 
 #if defined(MOZ_SANDBOX) && defined(MOZ_DEBUG) && defined(ENABLE_TESTS)
     // Initialize top-level actor for testing content process sandbox.
@@ -1569,32 +1569,23 @@ both:
                     uint32_t flags);
 
     /**
-     * Sync the BrowsingContext with id 'aContextId' and name 'aName' to the
+     * Creates a new BrowsingContext, initialized with the values provided in
-     * parent, and attach it to the BrowsingContext 'aParentContext'. If
+     * `BrowsingContextInitializer`.
-     * 'aParentContext' is 'nullptr' the BrowsingContext is a root in the
+     *
-     * BrowsingContext tree. AttachBrowsingContext must only be called at most
+     * This message may only be sent to the parent in limited situations. If the
-     * once for any child BrowsingContext, and only for BrowsingContexts where
+     * new BrowsingContext has a parent window, it must be owned by the
-     * the parent and the child context contains their nsDocShell.
+     * embedding process, otherwise it must be owned by the opener, if set.
      */
-    async AttachBrowsingContext(BrowsingContextInitializer aInit);
+    async CreateBrowsingContext(uint64_t aGroupId, BrowsingContextInitializer aInit);
 
     /**
-     * Remove the synced BrowsingContext with id 'aContextId' from the parent.
+     * Discards the passed-in BrowsingContext. If the BrowsingContext has
-     * DetachBrowsingContext is only needed to be called once for any
+     * already been discarded, this message does nothing.
-     * BrowsingContext, since detaching a node in the BrowsingContext detaches
+     * The response promise is fulfilled when the process has flagged the
-     * the entire sub-tree rooted at that node. Calling DetachBrowsingContext
+     * BrowsingContext as discarded.
-     * with an already detached BrowsingContext effectively does nothing. Note
-     * that it is not an error to call DetachBrowsingContext on a
-     * BrowsingContext belonging to an already detached subtree.
-     *
-     * As the passed-in context is allowed to already be detached, it is passed
-     * by id, rather than using BrowsingContext's serializer.
-     *
-     * Callers should keep the BrowsingContext alive until this async request is
-     * resolved or rejected, in order to ensure that in-flight messages still
-     * have valid targets.
      */
-    async DetachBrowsingContext(uint64_t aContextId) returns (bool unused);
+    async DiscardBrowsingContext(MaybeDiscardedBrowsingContext aContext)
+            returns (bool unused);
 
     async WindowClose(MaybeDiscardedBrowsingContext aContext,
                       bool aTrustedCaller);

toolkit/components/perfmonitoring/PerformanceUtils.cpp

@@ -41,12 +41,7 @@ nsTArray<RefPtr<PerformanceInfoPromise>> CollectPerformanceInfo() {
   }
 
   nsTArray<RefPtr<BrowsingContextGroup>> groups;
-  if (XRE_IsContentProcess()) {
+  BrowsingContextGroup::GetAllGroups(groups);
-    groups.AppendElements(
-        ContentChild::GetSingleton()->BrowsingContextGroups());
-  } else {
-    groups.AppendElements(ContentParent::BrowsingContextGroups());
-  }
 
   nsTArray<DocGroup*> docGroups;
   for (auto& browsingContextGroup : groups) {

xpcom/tests/gtest/TestThreadMetrics.cpp

@@ -118,7 +118,8 @@ class ThreadMetrics : public ::testing::Test {
  protected:
   virtual void SetUp() {
     // building the DocGroup structure
-    RefPtr<dom::BrowsingContextGroup> group = new dom::BrowsingContextGroup();
+    RefPtr<dom::BrowsingContextGroup> group =
+        dom::BrowsingContextGroup::Create();
     mDocGroup = group->AddDocument(NS_LITERAL_CSTRING("key"), nullptr);
     mDocGroup2 = group->AddDocument(NS_LITERAL_CSTRING("key2"), nullptr);
     mCounter = mDocGroup->GetPerformanceCounter();