Bug 1529684 - Part 1: Allow Attaching BrowsingContext from parent to child, r=farre
☠☠ backed out by 1b825b6f3869 ☠ ☠
authorNika Layzell <nika@thelayzells.com>
Tue, 05 Mar 2019 17:33:19 +0000
changeset 462433 7f2bda80e47996b542d4c8f602f88a4af6b84f16
parent 462432 19047d153c2f5ddf5184def2be0745d2f1d936a8
child 462434 257d45117af301a610832d9d4b6f43ba55ecb9c4
push id79648
push usernlayzell@mozilla.com
push dateTue, 05 Mar 2019 19:51:46 +0000
treeherderautoland@9f3e65f6b1dd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfarre
bugs1529684
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 1529684 - Part 1: Allow Attaching BrowsingContext from parent to child, r=farre This is important to allow creating BrowsingContexts outside of the process where they are going to be used. This is largely a re-arrangement of existing code. There is currently no way to do this type of attaching for browsing contexts in existing BrowsingContextGroups, which creates some limitations, but happens to be sufficient for us in the current situation. Differential Revision: https://phabricator.services.mozilla.com/D21095
docshell/base/BrowsingContext.cpp
docshell/base/BrowsingContext.h
dom/ipc/ContentChild.cpp
dom/ipc/ContentChild.h
dom/ipc/ContentParent.cpp
dom/ipc/PContent.ipdl
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@@ -53,30 +53,16 @@ static StaticAutoPtr<BrowsingContextMap<
 
 static void Register(BrowsingContext* aBrowsingContext) {
   MOZ_ALWAYS_TRUE(
       sBrowsingContexts->putNew(aBrowsingContext->Id(), aBrowsingContext));
 
   aBrowsingContext->Group()->Register(aBrowsingContext);
 }
 
-static void Sync(BrowsingContext* aBrowsingContext) {
-  if (!XRE_IsContentProcess()) {
-    return;
-  }
-
-  auto cc = ContentChild::GetSingleton();
-  MOZ_DIAGNOSTIC_ASSERT(cc);
-  RefPtr<BrowsingContext> parent = aBrowsingContext->GetParent();
-  BrowsingContext* opener = aBrowsingContext->GetOpener();
-  cc->SendAttachBrowsingContext(parent, opener,
-                                BrowsingContextId(aBrowsingContext->Id()),
-                                aBrowsingContext->Name());
-}
-
 BrowsingContext* BrowsingContext::TopLevelBrowsingContext() {
   BrowsingContext* bc = this;
   while (bc->mParent) {
     bc = bc->mParent;
   }
   return bc;
 }
 
@@ -156,17 +142,17 @@ already_AddRefed<BrowsingContext> Browsi
 
     context->Group()->Subscribe(aOriginProcess);
   } else {
     context = new BrowsingContext(aParent, aOpener, aName, aId, Type::Content);
   }
 
   Register(context);
 
-  context->Attach();
+  // Caller handles attaching us to the tree.
 
   return context.forget();
 }
 
 BrowsingContext::BrowsingContext(BrowsingContext* aParent,
                                  BrowsingContext* aOpener,
                                  const nsAString& aName,
                                  uint64_t aBrowsingContextId, Type aType)
@@ -192,34 +178,40 @@ BrowsingContext::BrowsingContext(Browsin
 
 void BrowsingContext::SetDocShell(nsIDocShell* aDocShell) {
   // XXX(nika): We should communicate that we are now an active BrowsingContext
   // process to the parent & do other validation here.
   MOZ_RELEASE_ASSERT(nsDocShell::Cast(aDocShell)->GetBrowsingContext() == this);
   mDocShell = aDocShell;
 }
 
-void BrowsingContext::Attach() {
+void BrowsingContext::Attach(bool aFromIPC) {
   MOZ_LOG(GetLog(), LogLevel::Debug,
           ("%s: %s 0x%08" PRIx64 " to 0x%08" PRIx64,
            XRE_IsParentProcess() ? "Parent" : "Child",
            sCachedBrowsingContexts->has(Id()) ? "Re-connecting" : "Connecting",
            Id(), mParent ? mParent->Id() : 0));
 
   sCachedBrowsingContexts->remove(Id());
 
   auto* children = mParent ? &mParent->mChildren : &mGroup->Toplevels();
   MOZ_DIAGNOSTIC_ASSERT(!children->Contains(this));
 
   children->AppendElement(this);
 
-  Sync(this);
+  // Send attach to our parent if we need to.
+  if (!aFromIPC && XRE_IsContentProcess()) {
+    auto cc = ContentChild::GetSingleton();
+    MOZ_DIAGNOSTIC_ASSERT(cc);
+    cc->SendAttachBrowsingContext(
+        mParent, mOpener, BrowsingContextId(mBrowsingContextId), Name());
+  }
 }
 
-void BrowsingContext::Detach() {
+void BrowsingContext::Detach(bool aFromIPC) {
   MOZ_LOG(GetLog(), LogLevel::Debug,
           ("%s: Detaching 0x%08" PRIx64 " from 0x%08" PRIx64,
            XRE_IsParentProcess() ? "Parent" : "Child", Id(),
            mParent ? mParent->Id() : 0));
 
   RefPtr<BrowsingContext> kungFuDeathGrip(this);
 
   BrowsingContextMap<RefPtr>::Ptr p;
@@ -237,48 +229,44 @@ void BrowsingContext::Detach() {
     // order.
     MOZ_DIAGNOSTIC_ASSERT(children.IsEmpty() || children.Contains(this));
 
     children.RemoveElement(this);
   }
 
   Group()->Unregister(this);
 
-  if (!XRE_IsContentProcess()) {
-    return;
+  if (!aFromIPC && XRE_IsContentProcess()) {
+    auto cc = ContentChild::GetSingleton();
+    MOZ_DIAGNOSTIC_ASSERT(cc);
+    cc->SendDetachBrowsingContext(this, false /* aMoveToBFCache */);
   }
-
-  auto cc = ContentChild::GetSingleton();
-  MOZ_DIAGNOSTIC_ASSERT(cc);
-  cc->SendDetachBrowsingContext(this, false /* aMoveToBFCache */);
 }
 
-void BrowsingContext::CacheChildren() {
+void BrowsingContext::CacheChildren(bool aFromIPC) {
   if (mChildren.IsEmpty()) {
     return;
   }
 
   MOZ_LOG(GetLog(), LogLevel::Debug,
           ("%s: Caching children of 0x%08" PRIx64 "",
            XRE_IsParentProcess() ? "Parent" : "Child", Id()));
 
   MOZ_ALWAYS_TRUE(sCachedBrowsingContexts->reserve(mChildren.Length()));
 
   for (BrowsingContext* child : mChildren) {
     MOZ_ALWAYS_TRUE(sCachedBrowsingContexts->putNew(child->Id(), child));
   }
   mChildren.Clear();
 
-  if (!XRE_IsContentProcess()) {
-    return;
+  if (!aFromIPC && XRE_IsContentProcess()) {
+    auto cc = ContentChild::GetSingleton();
+    MOZ_DIAGNOSTIC_ASSERT(cc);
+    cc->SendDetachBrowsingContext(this, true /* aMoveToBFCache */);
   }
-
-  auto cc = ContentChild::GetSingleton();
-  MOZ_DIAGNOSTIC_ASSERT(cc);
-  cc->SendDetachBrowsingContext(this, true /* aMoveToBFCache */);
 }
 
 bool BrowsingContext::IsCached() { return sCachedBrowsingContexts->has(Id()); }
 
 void BrowsingContext::GetChildren(
     nsTArray<RefPtr<BrowsingContext>>& aChildren) {
   MOZ_ALWAYS_TRUE(aChildren.AppendElements(mChildren));
 }
--- a/docshell/base/BrowsingContext.h
+++ b/docshell/base/BrowsingContext.h
@@ -173,25 +173,25 @@ class BrowsingContext : public nsWrapper
   nsPIDOMWindowOuter* GetDOMWindow() const {
     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();
+  void Attach(bool aFromIPC = false);
 
   // Detach the current BrowsingContext from its parent, in both the
   // child and the parent process.
-  void Detach();
+  void Detach(bool aFromIPC = false);
 
   // Remove all children from the current BrowsingContext and cache
   // them to allow them to be attached again.
-  void CacheChildren();
+  void CacheChildren(bool aFromIPC = false);
 
   // Determine if the current BrowsingContext was 'cached' by the logic in
   // CacheChildren.
   bool IsCached();
 
   const nsString& Name() const { return mName; }
   void GetName(nsAString& aName) { aName = mName; }
   bool NameEquals(const nsAString& aName) { return mName.Equals(aName); }
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -3674,16 +3674,45 @@ PContentChild::Result ContentChild::OnMe
     MOZ_ASSERT(!aMsg.is_reply());
 
     LSObject::OnSyncMessageHandled();
   }
 
   return result;
 }
 
+mozilla::ipc::IPCResult ContentChild::RecvAttachBrowsingContext(
+    BrowsingContext* aParent, BrowsingContext* aOpener,
+    BrowsingContextId aChildId, const nsString& aName) {
+  RefPtr<BrowsingContext> child = BrowsingContext::Get(aChildId);
+  MOZ_RELEASE_ASSERT(!child || child->IsCached());
+
+  if (!child) {
+    child = BrowsingContext::CreateFromIPC(aParent, aOpener, aName,
+                                           (uint64_t)aChildId, nullptr);
+  }
+
+  child->Attach(/* aFromIPC */ true);
+
+  return IPC_OK();
+}
+
+mozilla::ipc::IPCResult ContentChild::RecvDetachBrowsingContext(
+    BrowsingContext* aContext, bool aMoveToBFCache) {
+  MOZ_RELEASE_ASSERT(aContext);
+
+  if (aMoveToBFCache) {
+    aContext->CacheChildren(/* aFromIPC */ true);
+  } else {
+    aContext->Detach(/* aFromIPC */ true);
+  }
+
+  return IPC_OK();
+}
+
 mozilla::ipc::IPCResult ContentChild::RecvWindowClose(BrowsingContext* aContext,
                                                       bool aTrustedCaller) {
   if (!aContext) {
     MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Debug,
             ("ChildIPC: Trying to send a message to dead or detached context"));
     return IPC_OK();
   }
 
--- a/dom/ipc/ContentChild.h
+++ b/dom/ipc/ContentChild.h
@@ -707,16 +707,23 @@ class ContentChild final : public PConte
   virtual already_AddRefed<nsIEventTarget> GetConstructedEventTarget(
       const Message& aMsg) override;
 
   virtual already_AddRefed<nsIEventTarget> GetSpecificMessageEventTarget(
       const Message& aMsg) override;
 
   virtual void OnChannelReceivedMessage(const Message& aMsg) override;
 
+  mozilla::ipc::IPCResult RecvAttachBrowsingContext(
+      BrowsingContext* aParentContext, BrowsingContext* aOpener,
+      BrowsingContextId aContextId, const nsString& aName);
+
+  mozilla::ipc::IPCResult RecvDetachBrowsingContext(BrowsingContext* aContext,
+                                                    bool aMoveToBFCache);
+
   mozilla::ipc::IPCResult RecvWindowClose(BrowsingContext* aContext,
                                           bool aTrustedCaller);
   mozilla::ipc::IPCResult RecvWindowFocus(BrowsingContext* aContext);
   mozilla::ipc::IPCResult RecvWindowBlur(BrowsingContext* aContext);
   mozilla::ipc::IPCResult RecvWindowPostMessage(
       BrowsingContext* aContext, const ClonedMessageData& aMessage,
       const PostMessageData& aData);
 
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -5703,16 +5703,18 @@ mozilla::ipc::IPCResult ContentParent::R
     return IPC_OK();
   }
 
   if (!child) {
     child = BrowsingContext::CreateFromIPC(aParent, aOpener, aName,
                                            (uint64_t)aChildId, this);
   }
 
+  child->Attach(/* aFromIPC */ true);
+
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult ContentParent::RecvDetachBrowsingContext(
     BrowsingContext* aContext, bool aMoveToBFCache) {
   if (!aContext) {
     MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Debug,
             ("ParentIPC: Trying to detach already detached"));
@@ -5729,19 +5731,19 @@ mozilla::ipc::IPCResult ContentParent::R
     // above TODO. [Bug 1471598]
     MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Warning,
             ("ParentIPC: Trying to detach out of process context 0x%08" PRIx64,
              aContext->Id()));
     return IPC_OK();
   }
 
   if (aMoveToBFCache) {
-    aContext->CacheChildren();
+    aContext->CacheChildren(/* aFromIPC */ true);
   } else {
-    aContext->Detach();
+    aContext->Detach(/* aFromIPC */ true);
   }
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult ContentParent::RecvSetOpenerBrowsingContext(
     BrowsingContext* aContext, BrowsingContext* aOpener) {
   if (!aContext) {
--- a/dom/ipc/PContent.ipdl
+++ b/dom/ipc/PContent.ipdl
@@ -1223,44 +1223,16 @@ parent:
                                                   nsCString aTrackingOrigin,
                                                   nsCString aGrantedOrigin,
                                                   int aAllowMode)
           returns (bool unused);
 
     async StoreUserInteractionAsPermission(Principal aPrincipal);
 
     /**
-     * Sync the BrowsingContext with id 'aContextId' and name 'aName' to the
-     * parent, and attach it to the BrowsingContext 'aParentContext'. If
-     * 'aParentContext' is 'nullptr' the BrowsingContext is a root in the
-     * BrowsingContext tree. AttachBrowsingContext must only be called at most
-     * once for any child BrowsingContext, and only for BrowsingContexts where
-     * the parent and the child context contains their nsDocShell.
-     */
-    async AttachBrowsingContext(BrowsingContext aParentContext,
-                                BrowsingContext aOpener,
-                                BrowsingContextId aContextId,
-                                nsString aName);
-
-    /**
-     * Remove the synced BrowsingContext 'aContext' from the parent.
-     * DetachBrowsingContext is only needed to be called once for any
-     * BrowsingContext, since detaching a node in the BrowsingContext detaches
-     * the entire sub-tree rooted at that node. Calling DetachBrowsingContext
-     * 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. The
-     * 'aMoveToBFCache' paramater controls if detaching a BrowsingContext
-     * should move it to the bfcache allowing it to be re-attached if navigated
-     * to.
-     */
-    async DetachBrowsingContext(BrowsingContext aContext,
-                                bool aMoveToBFCache);
-
-    /**
      * Set the opener of browsing context 'aContext' to the browsing context
      * with id 'aOpenerId'.
      */
     async SetOpenerBrowsingContext(BrowsingContext aContext,
                                    BrowsingContext aOpenerContext);
 
     /**
      * Notify parent to update user gesture activation flag.
@@ -1283,16 +1255,44 @@ both:
 
     /**
      * Send a Push error message to all service worker clients in the parent or
      * child.
      */
     async PushError(nsCString scope, Principal principal, nsString message,
                     uint32_t flags);
 
+    /**
+     * Sync the BrowsingContext with id 'aContextId' and name 'aName' to the
+     * parent, and attach it to the BrowsingContext 'aParentContext'. If
+     * 'aParentContext' is 'nullptr' the BrowsingContext is a root in the
+     * BrowsingContext tree. AttachBrowsingContext must only be called at most
+     * once for any child BrowsingContext, and only for BrowsingContexts where
+     * the parent and the child context contains their nsDocShell.
+     */
+    async AttachBrowsingContext(BrowsingContext aParentContext,
+                                BrowsingContext aOpener,
+                                BrowsingContextId aContextId,
+                                nsString aName);
+
+    /**
+     * Remove the synced BrowsingContext 'aContext' from the parent.
+     * DetachBrowsingContext is only needed to be called once for any
+     * BrowsingContext, since detaching a node in the BrowsingContext detaches
+     * the entire sub-tree rooted at that node. Calling DetachBrowsingContext
+     * 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. The
+     * 'aMoveToBFCache' paramater controls if detaching a BrowsingContext
+     * should move it to the bfcache allowing it to be re-attached if navigated
+     * to.
+     */
+    async DetachBrowsingContext(BrowsingContext aContext,
+                                bool aMoveToBFCache);
+
     async WindowClose(BrowsingContext aContext, bool aTrustedCaller);
     async WindowFocus(BrowsingContext aContext);
     async WindowBlur(BrowsingContext aContext);
     async WindowPostMessage(BrowsingContext aContext, ClonedMessageData aMessage,
                             PostMessageData aData);
 };
 
 }