Bug 1471869 - Re-attach cached contexts in bulk. r=nika

This makes restoring from bfcache send less messages. It also separates detach/attach from the caching logic, making it more clear that caching is going on. Differential Revision: https://phabricator.services.mozilla.com/D28670 --HG-- extra : moz-landing-system : lando
2019-04-29 11:38:45 +00:00 · 2019-04-29 11:38:45 +00:00 · e3410c54b8
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@ -246,12 +246,11 @@ void BrowsingContext::SetEmbedderElement(Element* aEmbedder) {

 void BrowsingContext::Attach(bool aFromIPC) {
  MOZ_LOG(GetLog(), LogLevel::Debug,
-          ("%s: %s 0x%08" PRIx64 " to 0x%08" PRIx64,
-           XRE_IsParentProcess() ? "Parent" : "Child",
-           Group()->IsContextCached(this) ? "Re-connecting" : "Connecting",
-           Id(), mParent ? mParent->Id() : 0));
+          ("%s: Connecting 0x%08" PRIx64 " to 0x%08" PRIx64,
+           XRE_IsParentProcess() ? "Parent" : "Child", Id(),
+           mParent ? mParent->Id() : 0));

-  Unused << Group()->EvictCachedContext(this);
+  MOZ_DIAGNOSTIC_ASSERT(!Group()->IsContextCached(this));

  auto* children = mParent ? &mParent->mChildren : &mGroup->Toplevels();
  MOZ_DIAGNOSTIC_ASSERT(!children->Contains(this));
@ -316,7 +315,7 @@ void BrowsingContext::Detach(bool aFromIPC) {
  if (!aFromIPC && XRE_IsContentProcess()) {
    auto cc = ContentChild::GetSingleton();
    MOZ_DIAGNOSTIC_ASSERT(cc);
-    cc->SendDetachBrowsingContext(this, false /* aMoveToBFCache */);
+    cc->SendDetachBrowsingContext(this);
  }
 }

@ -331,7 +330,33 @@ void BrowsingContext::CacheChildren(bool aFromIPC) {
  if (!aFromIPC && XRE_IsContentProcess()) {
    auto cc = ContentChild::GetSingleton();
    MOZ_DIAGNOSTIC_ASSERT(cc);
-    cc->SendDetachBrowsingContext(this, true /* aMoveToBFCache */);
+    cc->SendCacheBrowsingContextChildren(this);
+  }
+}
+
+void BrowsingContext::RestoreChildren(Children&& aChildren, bool aFromIPC) {
+  MOZ_LOG(GetLog(), LogLevel::Debug,
+          ("%s: Restoring children of 0x%08" PRIx64 "",
+           XRE_IsParentProcess() ? "Parent" : "Child", Id()));
+
+  MOZ_DIAGNOSTIC_ASSERT(mChildren.IsEmpty());
+
+  for (BrowsingContext* child : mChildren) {
+    MOZ_DIAGNOSTIC_ASSERT(child->GetParent() == this);
+    Unused << Group()->EvictCachedContext(child);
+  }
+
+  mChildren.SwapElements(aChildren);
+
+  if (!aFromIPC && XRE_IsContentProcess()) {
+    auto cc = ContentChild::GetSingleton();
+    MOZ_DIAGNOSTIC_ASSERT(cc);
+
+    nsTArray<BrowsingContextId> contexts(mChildren.Length());
+    for (BrowsingContext* child : mChildren) {
+      contexts.AppendElement(child->Id());
+    }
+    cc->SendRestoreBrowsingContextChildren(this, contexts);
  }
 }

@ -341,8 +366,7 @@ bool BrowsingContext::HasOpener() const {
  return sBrowsingContexts->has(mOpenerId);
 }

-void BrowsingContext::GetChildren(
-    nsTArray<RefPtr<BrowsingContext>>& aChildren) {
+void BrowsingContext::GetChildren(Children& aChildren) {
  MOZ_ALWAYS_TRUE(aChildren.AppendElements(mChildren));
 }

--- a/docshell/base/BrowsingContext.h
+++ b/docshell/base/BrowsingContext.h
@ -94,6 +94,8 @@ class BrowsingContext : public nsWrapperCache,
 public:
  enum class Type { Chrome, Content };

+  using Children = nsTArray<RefPtr<BrowsingContext>>;
+
  static void Init();
  static LogModule* GetLog();
  static void CleanupContexts(uint64_t aProcessId);
@ -149,6 +151,9 @@ class BrowsingContext : public nsWrapperCache,
  // them to allow them to be attached again.
  void CacheChildren(bool aFromIPC = false);

+  // Restore cached browsing contexts.
+  void RestoreChildren(Children&& aChildren, bool aFromIPC = false);
+
  // Determine if the current BrowsingContext was 'cached' by the logic in
  // CacheChildren.
  bool IsCached();
@ -172,7 +177,7 @@ class BrowsingContext : public nsWrapperCache,

  bool HasOpener() const;

-  void GetChildren(nsTArray<RefPtr<BrowsingContext>>& aChildren);
+  void GetChildren(Children& aChildren);

  BrowsingContextGroup* Group() { return mGroup; }

@ -221,7 +226,6 @@ class BrowsingContext : public nsWrapperCache,
  NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(BrowsingContext)
  NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_NATIVE_CLASS(BrowsingContext)

-  using Children = nsTArray<RefPtr<BrowsingContext>>;
  const Children& GetChildren() { return mChildren; }

  // Perform a pre-order walk of this BrowsingContext subtree.
@ -479,6 +483,7 @@ extern bool GetRemoteOuterWindowProxy(JSContext* aCx, BrowsingContext* aContext,
 typedef BrowsingContext::Transaction BrowsingContextTransaction;
 typedef BrowsingContext::FieldEpochs BrowsingContextFieldEpochs;
 typedef BrowsingContext::IPCInitializer BrowsingContextInitializer;
+typedef BrowsingContext::Children BrowsingContextChildren;

 }  // namespace dom

--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@ -7810,6 +7810,7 @@ nsresult nsDocShell::RestoreFromHistory() {
  // <head> is parsed.
  document->NotifyPossibleTitleChange(false);

+  BrowsingContext::Children contexts(childShells.Count());
  // Now we simulate appending child docshells for subframes.
  for (i = 0; i < childShells.Count(); ++i) {
    nsIDocShellTreeItem* childItem = childShells.ObjectAt(i);
@ -7849,12 +7850,7 @@ nsresult nsDocShell::RestoreFromHistory() {
    // child inherits our mIsActive mPrivateBrowsingId, which is what we want.
    AddChild(childItem);

-    // TODO(farre): This results in sending an IPC message to
-    // re-attach the 'BrowsingContext' to the 'ContentParent'. We
-    // would much rather do this in bulk. See Bug 1410260.
-    RefPtr<BrowsingContext> childContext =
-        nsDocShell::Cast(childShell)->GetBrowsingContext();
-    childContext->Attach();
+    contexts.AppendElement(nsDocShell::Cast(childShell)->GetBrowsingContext());

    childShell->SetAllowPlugins(allowPlugins);
    childShell->SetAllowJavascript(allowJavascript);
@ -7872,6 +7868,8 @@ nsresult nsDocShell::RestoreFromHistory() {
    NS_ENSURE_SUCCESS(rv, rv);
  }

+  GetBrowsingContext()->RestoreChildren(std::move(contexts));
+
  // Make sure to restore the window state after adding the child shells back
  // to the tree.  This is necessary for Thaw() and Resume() to propagate
  // properly.
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@ -3741,15 +3741,36 @@ mozilla::ipc::IPCResult ContentChild::RecvAttachBrowsingContext(
 }

 mozilla::ipc::IPCResult ContentChild::RecvDetachBrowsingContext(
-    BrowsingContext* aContext, bool aMoveToBFCache) {
+    BrowsingContext* aContext) {
  MOZ_RELEASE_ASSERT(aContext);

-  if (aMoveToBFCache) {
-    aContext->CacheChildren(/* aFromIPC */ true);
-  } else {
-    aContext->Detach(/* aFromIPC */ true);
+  aContext->Detach(/* aFromIPC */ true);
+
+  return IPC_OK();
+}
+
+mozilla::ipc::IPCResult ContentChild::RecvCacheBrowsingContextChildren(
+    BrowsingContext* aContext) {
+  MOZ_RELEASE_ASSERT(aContext);
+
+  aContext->CacheChildren(/* aFromIPC */ true);
+
+  return IPC_OK();
+}
+
+mozilla::ipc::IPCResult ContentChild::RecvRestoreBrowsingContextChildren(
+    BrowsingContext* aContext, nsTArray<BrowsingContextId>&& aChildren) {
+  MOZ_DIAGNOSTIC_ASSERT(aContext);
+
+  BrowsingContext::Children children(aChildren.Length());
+
+  for (auto id : aChildren) {
+    RefPtr<BrowsingContext> child = BrowsingContext::Get(id);
+    children.AppendElement(child);
  }

+  aContext->RestoreChildren(std::move(children), /* aFromIPC */ true);
+
  return IPC_OK();
 }

--- a/dom/ipc/ContentChild.h
+++ b/dom/ipc/ContentChild.h
@ -698,8 +698,13 @@ class ContentChild final : public PContentChild,
  mozilla::ipc::IPCResult RecvAttachBrowsingContext(
      BrowsingContext::IPCInitializer&& aInit);

-  mozilla::ipc::IPCResult RecvDetachBrowsingContext(BrowsingContext* aContext,
-                                                    bool aMoveToBFCache);
+  mozilla::ipc::IPCResult RecvDetachBrowsingContext(BrowsingContext* aContext);
+
+  mozilla::ipc::IPCResult RecvCacheBrowsingContextChildren(
+      BrowsingContext* aContext);
+
+  mozilla::ipc::IPCResult RecvRestoreBrowsingContextChildren(
+      BrowsingContext* aContext, nsTArray<BrowsingContextId>&& aChildren);

  mozilla::ipc::IPCResult RecvRegisterBrowsingContextGroup(
      nsTArray<BrowsingContext::IPCInitializer>&& aInits);
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@ -5674,7 +5674,7 @@ mozilla::ipc::IPCResult ContentParent::RecvAttachBrowsingContext(
 }

 mozilla::ipc::IPCResult ContentParent::RecvDetachBrowsingContext(
-    BrowsingContext* aContext, bool aMoveToBFCache) {
+    BrowsingContext* aContext) {
  if (!aContext) {
    MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Debug,
            ("ParentIPC: Trying to detach already detached"));
@ -5694,11 +5694,7 @@ mozilla::ipc::IPCResult ContentParent::RecvDetachBrowsingContext(
    return IPC_OK();
  }

-  if (aMoveToBFCache) {
-    aContext->CacheChildren(/* aFromIPC */ true);
-  } else {
-    aContext->Detach(/* aFromIPC */ true);
-  }
+  aContext->Detach(/* aFromIPC */ true);

  for (auto iter = aContext->Group()->ContentParentsIter(); !iter.Done();
       iter.Next()) {
@ -5707,8 +5703,87 @@ mozilla::ipc::IPCResult ContentParent::RecvDetachBrowsingContext(
      continue;
    }

-    Unused << entry->GetKey()->SendDetachBrowsingContext(aContext,
-                                                         aMoveToBFCache);
+    Unused << entry->GetKey()->SendDetachBrowsingContext(aContext);
+  }
+
+  return IPC_OK();
+}
+
+mozilla::ipc::IPCResult ContentParent::RecvCacheBrowsingContextChildren(
+    BrowsingContext* aContext) {
+  if (!aContext) {
+    MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Debug,
+            ("ParentIPC: Trying to cache already detached"));
+    return IPC_OK();
+  }
+
+  if (!aContext->Canonical()->IsOwnedByProcess(ChildID())) {
+    // Where trying to cache a child BrowsingContext in another child
+    // process. This is illegal since the owner of the BrowsingContext
+    // is the proccess with the in-process docshell, which is tracked
+    // by OwnerProcessId.
+    MOZ_DIAGNOSTIC_ASSERT(false, "Trying to cache out of process context");
+
+    MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Warning,
+            ("ParentIPC: Trying to cache out of process context 0x%08" PRIx64,
+             aContext->Id()));
+    return IPC_OK();
+  }
+
+  aContext->CacheChildren(/* aFromIPC */ true);
+
+  for (auto iter = aContext->Group()->ContentParentsIter(); !iter.Done();
+       iter.Next()) {
+    nsRefPtrHashKey<ContentParent>* entry = iter.Get();
+    if (entry->GetKey() == this) {
+      continue;
+    }
+
+    Unused << entry->GetKey()->SendCacheBrowsingContextChildren(aContext);
+  }
+
+  return IPC_OK();
+}
+
+mozilla::ipc::IPCResult ContentParent::RecvRestoreBrowsingContextChildren(
+    BrowsingContext* aContext, nsTArray<BrowsingContextId>&& aChildren) {
+  if (!aContext) {
+    MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Debug,
+            ("ParentIPC: Trying to restore already detached"));
+    return IPC_OK();
+  }
+
+  if (!aContext->Canonical()->IsOwnedByProcess(ChildID())) {
+    // Where trying to cache a child BrowsingContext in another child
+    // process. This is illegal since the owner of the BrowsingContext
+    // is the proccess with the in-process docshell, which is tracked
+    // by OwnerProcessId.
+    MOZ_DIAGNOSTIC_ASSERT(false, "Trying to restore out of process context");
+
+    MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Warning,
+            ("ParentIPC: Trying to restore out of process context 0x%08" PRIx64,
+             aContext->Id()));
+    return IPC_OK();
+  }
+
+  BrowsingContext::Children children(aChildren.Length());
+
+  for (auto id : aChildren) {
+    RefPtr<BrowsingContext> child = BrowsingContext::Get(id);
+    children.AppendElement(child);
+  }
+
+  aContext->RestoreChildren(std::move(children), /* aFromIPC */ true);
+
+  for (auto iter = aContext->Group()->ContentParentsIter(); !iter.Done();
+       iter.Next()) {
+    nsRefPtrHashKey<ContentParent>* entry = iter.Get();
+    if (entry->GetKey() == this) {
+      continue;
+    }
+
+    Unused << entry->GetKey()->SendRestoreBrowsingContextChildren(aContext,
+                                                                  aChildren);
  }

  return IPC_OK();
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@ -614,8 +614,13 @@ class ContentParent final : public PContentParent,
  mozilla::ipc::IPCResult RecvAttachBrowsingContext(
      BrowsingContext::IPCInitializer&& aInit);

-  mozilla::ipc::IPCResult RecvDetachBrowsingContext(BrowsingContext* aContext,
-                                                    bool aMoveToBFCache);
+  mozilla::ipc::IPCResult RecvDetachBrowsingContext(BrowsingContext* aContext);
+
+  mozilla::ipc::IPCResult RecvCacheBrowsingContextChildren(
+      BrowsingContext* aContext);
+
+  mozilla::ipc::IPCResult RecvRestoreBrowsingContextChildren(
+      BrowsingContext* aContext, nsTArray<BrowsingContextId>&& aChildren);

  mozilla::ipc::IPCResult RecvWindowClose(BrowsingContext* aContext,
                                          bool aTrustedCaller);
--- a/dom/ipc/PContent.ipdl
+++ b/dom/ipc/PContent.ipdl
@ -1272,8 +1272,19 @@ both:
     * should move it to the bfcache allowing it to be re-attached if navigated
     * to.
     */
-    async DetachBrowsingContext(BrowsingContext aContext,
-                                bool aMoveToBFCache);
+    async DetachBrowsingContext(BrowsingContext aContext);
+
+    /**
+     * Removes all of 'aContext'\'s children, and caches them in the
+     * BrowsingContextGroup.
+     */
+    async CacheBrowsingContextChildren(BrowsingContext aContext);
+
+    /**
+     * Re-attach all BrowsingContexts in a 'aContext'.
+     */
+    async RestoreBrowsingContextChildren(BrowsingContext aContext,
+                                         BrowsingContextId[] aChildren);

    async WindowClose(BrowsingContext aContext, bool aTrustedCaller);
    async WindowFocus(BrowsingContext aContext);