Bug 1533447 - Part 2: Move bc caching to BrowsingContextGroup. r=nika
authorAndreas Farre <farre@mozilla.com>
Mon, 29 Apr 2019 07:01:50 +0000
changeset 530563 27b0545208f8a12f284d2c8eb515d21b6f8e9cdb
parent 530562 7cd8cc12be3d0929bb4a106e76a7596598676770
child 530564 b98bad3d505ebd093c06c74bf0b4d17b33174d6c
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika
bugs1533447
milestone68.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 1533447 - Part 2: Move bc caching to BrowsingContextGroup. r=nika Differential Revision: https://phabricator.services.mozilla.com/D28669
docshell/base/BrowsingContext.cpp
docshell/base/BrowsingContext.h
docshell/base/BrowsingContextGroup.cpp
docshell/base/BrowsingContextGroup.h
dom/ipc/ContentChild.cpp
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@@ -52,20 +52,16 @@ static LazyLogModule gBrowsingContextLog
 
 template <template <typename> class PtrType>
 using BrowsingContextMap =
     HashMap<uint64_t, PtrType<BrowsingContext>, DefaultHasher<uint64_t>,
             InfallibleAllocPolicy>;
 
 static StaticAutoPtr<BrowsingContextMap<WeakPtr>> sBrowsingContexts;
 
-// TODO(farre): This duplicates some of the work performed by the
-// bfcache. This should be unified. [Bug 1471601]
-static StaticAutoPtr<BrowsingContextMap<RefPtr>> sCachedBrowsingContexts;
-
 static void Register(BrowsingContext* aBrowsingContext) {
   MOZ_ALWAYS_TRUE(
       sBrowsingContexts->putNew(aBrowsingContext->Id(), aBrowsingContext));
 
   aBrowsingContext->Group()->Register(aBrowsingContext);
 }
 
 BrowsingContext* BrowsingContext::Top() {
@@ -77,21 +73,16 @@ BrowsingContext* BrowsingContext::Top() 
 }
 
 /* static */
 void BrowsingContext::Init() {
   if (!sBrowsingContexts) {
     sBrowsingContexts = new BrowsingContextMap<WeakPtr>();
     ClearOnShutdown(&sBrowsingContexts);
   }
-
-  if (!sCachedBrowsingContexts) {
-    sCachedBrowsingContexts = new BrowsingContextMap<RefPtr>();
-    ClearOnShutdown(&sCachedBrowsingContexts);
-  }
 }
 
 /* static */
 LogModule* BrowsingContext::GetLog() { return gBrowsingContextLog; }
 
 /* static */
 already_AddRefed<BrowsingContext> BrowsingContext::Get(uint64_t aId) {
   if (BrowsingContextMap<WeakPtr>::Ptr abc = sBrowsingContexts->lookup(aId)) {
@@ -216,19 +207,18 @@ void BrowsingContext::SetEmbedderElement
     //
     // XXX: This is a workaround to some parent edges not being immutable in the
     // parent process. It can be fixed once bug 1539979 has been fixed.
     if (mParent && mEmbedderElement && mEmbedderElement != aEmbedder) {
       NS_WARNING("Non root content frameLoader swap! This will crash soon!");
 
       MOZ_DIAGNOSTIC_ASSERT(mType == Type::Chrome, "must be chrome");
       MOZ_DIAGNOSTIC_ASSERT(XRE_IsParentProcess(), "must be in parent");
-      MOZ_DIAGNOSTIC_ASSERT(
-          !sCachedBrowsingContexts || !sCachedBrowsingContexts->has(Id()),
-          "cannot be in bfcache");
+      MOZ_DIAGNOSTIC_ASSERT(!Group()->IsContextCached(this),
+                            "cannot be in bfcache");
 
       RefPtr<BrowsingContext> kungFuDeathGrip(this);
       RefPtr<BrowsingContext> newParent;
       container->GetBrowsingContext(getter_AddRefs(newParent));
       mParent->mChildren.RemoveElement(this);
       if (newParent) {
         newParent->mChildren.AppendElement(this);
       }
@@ -253,20 +243,20 @@ void BrowsingContext::SetEmbedderElement
 
   mEmbedderElement = aEmbedder;
 }
 
 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",
+           Group()->IsContextCached(this) ? "Re-connecting" : "Connecting",
            Id(), mParent ? mParent->Id() : 0));
 
-  sCachedBrowsingContexts->remove(Id());
+  Unused << Group()->EvictCachedContext(this);
 
   auto* children = mParent ? &mParent->mChildren : &mGroup->Toplevels();
   MOZ_DIAGNOSTIC_ASSERT(!children->Contains(this));
 
   children->AppendElement(this);
 
   if (!aFromIPC) {
     // Send attach to our parent if we need to.
@@ -289,22 +279,17 @@ void BrowsingContext::Attach(bool aFromI
 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;
-  if (sCachedBrowsingContexts && (p = sCachedBrowsingContexts->lookup(Id()))) {
-    MOZ_DIAGNOSTIC_ASSERT(!mParent || !mParent->mChildren.Contains(this));
-    MOZ_DIAGNOSTIC_ASSERT(!mGroup || !mGroup->Toplevels().Contains(this));
-    sCachedBrowsingContexts->remove(p);
-  } else {
+  if (!Group()->EvictCachedContext(this)) {
     Children* children = nullptr;
     if (mParent) {
       children = &mParent->mChildren;
     } else if (mGroup) {
       children = &mGroup->Toplevels();
     }
 
     if (children) {
@@ -331,39 +316,31 @@ void BrowsingContext::Detach(bool aFromI
   if (!aFromIPC && XRE_IsContentProcess()) {
     auto cc = ContentChild::GetSingleton();
     MOZ_DIAGNOSTIC_ASSERT(cc);
     cc->SendDetachBrowsingContext(this, false /* aMoveToBFCache */);
   }
 }
 
 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));
-  }
+  Group()->CacheContexts(mChildren);
   mChildren.Clear();
 
   if (!aFromIPC && XRE_IsContentProcess()) {
     auto cc = ContentChild::GetSingleton();
     MOZ_DIAGNOSTIC_ASSERT(cc);
     cc->SendDetachBrowsingContext(this, true /* aMoveToBFCache */);
   }
 }
 
-bool BrowsingContext::IsCached() { return sCachedBrowsingContexts->has(Id()); }
+bool BrowsingContext::IsCached() { return Group()->IsContextCached(this); }
 
 bool BrowsingContext::HasOpener() const {
   return sBrowsingContexts->has(mOpenerId);
 }
 
 void BrowsingContext::GetChildren(
     nsTArray<RefPtr<BrowsingContext>>& aChildren) {
   MOZ_ALWAYS_TRUE(aChildren.AppendElements(mChildren));
@@ -521,18 +498,17 @@ bool BrowsingContext::IsActive() const {
   }
 
   return false;
 }
 
 BrowsingContext::~BrowsingContext() {
   MOZ_DIAGNOSTIC_ASSERT(!mParent || !mParent->mChildren.Contains(this));
   MOZ_DIAGNOSTIC_ASSERT(!mGroup || !mGroup->Toplevels().Contains(this));
-  MOZ_DIAGNOSTIC_ASSERT(!sCachedBrowsingContexts ||
-                        !sCachedBrowsingContexts->has(Id()));
+  MOZ_DIAGNOSTIC_ASSERT(!mGroup || !mGroup->IsContextCached(this));
 
   if (sBrowsingContexts) {
     sBrowsingContexts->remove(Id());
   }
 }
 
 nsISupports* BrowsingContext::GetParentObject() const {
   return xpc::NativeGlobal(xpc::PrivilegedJunkScope());
@@ -958,30 +934,36 @@ bool IPDLParamTraits<dom::BrowsingContex
 
 void IPDLParamTraits<dom::BrowsingContext::IPCInitializer>::Write(
     IPC::Message* aMessage, IProtocol* aActor,
     const dom::BrowsingContext::IPCInitializer& aInit) {
   // Write actor ID parameters.
   WriteIPDLParam(aMessage, aActor, aInit.mId);
   WriteIPDLParam(aMessage, aActor, aInit.mParentId);
 
+  WriteIPDLParam(aMessage, aActor, aInit.mCached);
+
   // Write other synchronized fields.
 #define MOZ_BC_FIELD(name, ...) WriteIPDLParam(aMessage, aActor, aInit.m##name);
 #include "mozilla/dom/BrowsingContextFieldList.h"
 }
 
 bool IPDLParamTraits<dom::BrowsingContext::IPCInitializer>::Read(
     const IPC::Message* aMessage, PickleIterator* aIterator, IProtocol* aActor,
     dom::BrowsingContext::IPCInitializer* aInit) {
   // Read actor ID parameters.
   if (!ReadIPDLParam(aMessage, aIterator, aActor, &aInit->mId) ||
       !ReadIPDLParam(aMessage, aIterator, aActor, &aInit->mParentId)) {
     return false;
   }
 
+  if (!ReadIPDLParam(aMessage, aIterator, aActor, &aInit->mCached)) {
+    return false;
+  }
+
   // Read other synchronized fields.
 #define MOZ_BC_FIELD(name, ...)                                       \
   if (!ReadIPDLParam(aMessage, aIterator, aActor, &aInit->m##name)) { \
     return false;                                                     \
   }
 #include "mozilla/dom/BrowsingContextFieldList.h"
 
   return true;
--- a/docshell/base/BrowsingContext.h
+++ b/docshell/base/BrowsingContext.h
@@ -330,27 +330,29 @@ class BrowsingContext : public nsWrapper
 
     // IDs are used for Parent and Opener to allow for this object to be
     // deserialized before other BrowsingContext in the BrowsingContextGroup
     // have been initialized.
     uint64_t mParentId;
     already_AddRefed<BrowsingContext> GetParent();
     already_AddRefed<BrowsingContext> GetOpener();
 
+    bool mCached;
     // Include each field, skipping mOpener, as we want to handle it
     // separately.
 #define MOZ_BC_FIELD(name, type) type m##name;
 #include "mozilla/dom/BrowsingContextFieldList.h"
   };
 
   // Create an IPCInitializer object for this BrowsingContext.
   IPCInitializer GetIPCInitializer() {
     IPCInitializer init;
     init.mId = Id();
     init.mParentId = mParent ? mParent->Id() : 0;
+    init.mCached = IsCached();
 
 #define MOZ_BC_FIELD(name, type) init.m##name = m##name;
 #include "mozilla/dom/BrowsingContextFieldList.h"
     return init;
   }
 
   // Create a BrowsingContext object from over IPC.
   static already_AddRefed<BrowsingContext> CreateFromIPC(
--- a/docshell/base/BrowsingContextGroup.cpp
+++ b/docshell/base/BrowsingContextGroup.cpp
@@ -66,16 +66,36 @@ void BrowsingContextGroup::EnsureSubscri
       inits.AppendElement(aContext->GetIPCInitializer());
     });
   }
 
   // Send all of our contexts to the target content process.
   Unused << aProcess->SendRegisterBrowsingContextGroup(inits);
 }
 
+bool BrowsingContextGroup::IsContextCached(BrowsingContext* aContext) const {
+  MOZ_DIAGNOSTIC_ASSERT(aContext);
+  return mCachedContexts.Contains(aContext);
+}
+
+void BrowsingContextGroup::CacheContext(BrowsingContext* aContext) {
+  mCachedContexts.PutEntry(aContext);
+}
+
+void BrowsingContextGroup::CacheContexts(
+    const BrowsingContext::Children& aContexts) {
+  for (BrowsingContext* child : aContexts) {
+    mCachedContexts.PutEntry(child);
+  }
+}
+
+bool BrowsingContextGroup::EvictCachedContext(BrowsingContext* aContext) {
+  return mCachedContexts.EnsureRemoved(aContext);
+}
+
 BrowsingContextGroup::~BrowsingContextGroup() {
   for (auto iter = mSubscribers.Iter(); !iter.Done(); iter.Next()) {
     nsRefPtrHashKey<ContentParent>* entry = iter.Get();
     entry->GetKey()->OnBrowsingContextGroupUnsubscribe(this);
   }
 }
 
 nsISupports* BrowsingContextGroup::GetParentObject() const {
@@ -83,15 +103,15 @@ nsISupports* BrowsingContextGroup::GetPa
 }
 
 JSObject* BrowsingContextGroup::WrapObject(JSContext* aCx,
                                            JS::Handle<JSObject*> aGivenProto) {
   return BrowsingContextGroup_Binding::Wrap(aCx, this, aGivenProto);
 }
 
 NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(BrowsingContextGroup, mContexts,
-                                      mToplevels, mSubscribers)
+                                      mToplevels, mSubscribers, mCachedContexts)
 
 NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(BrowsingContextGroup, AddRef)
 NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(BrowsingContextGroup, Release)
 
 }  // namespace dom
 }  // namespace mozilla
--- a/docshell/base/BrowsingContextGroup.h
+++ b/docshell/base/BrowsingContextGroup.h
@@ -4,17 +4,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_dom_BrowsingContextGroup_h
 #define mozilla_dom_BrowsingContextGroup_h
 
 #include "mozilla/dom/BrowsingContext.h"
 #include "mozilla/dom/ContentParent.h"
-#include "mozilla/StaticPtr.h"
 #include "nsHashKeys.h"
 #include "nsTArray.h"
 #include "nsTHashtable.h"
 #include "nsWrapperCache.h"
 
 namespace mozilla {
 namespace dom {
 
@@ -41,16 +40,22 @@ class BrowsingContextGroup final : publi
 
   // Interact with the list of ContentParents
   void Subscribe(ContentParent* aOriginProcess);
   void Unsubscribe(ContentParent* aOriginProcess);
 
   // Force the given ContentParent to subscribe to our BrowsingContextGroup.
   void EnsureSubscribed(ContentParent* aProcess);
 
+  // Methods interacting with cached contexts.
+  bool IsContextCached(BrowsingContext* aContext) const;
+  void CacheContext(BrowsingContext* aContext);
+  void CacheContexts(const BrowsingContext::Children& aContexts);
+  bool EvictCachedContext(BrowsingContext* aContext);
+
   ContentParents::Iterator ContentParentsIter() { return mSubscribers.Iter(); }
 
   // Get a reference to the list of toplevel contexts in this
   // BrowsingContextGroup.
   BrowsingContext::Children& Toplevels() { return mToplevels; }
   void GetToplevels(BrowsingContext::Children& aToplevels) {
     aToplevels.AppendElements(mToplevels);
   }
@@ -92,14 +97,17 @@ class BrowsingContextGroup final : publi
   // are addressed using a hashtable to avoid linear lookup when adding or
   // removing elements from the set.
   nsTHashtable<nsRefPtrHashKey<BrowsingContext>> mContexts;
 
   // The set of toplevel browsing contexts in the current BrowsingContextGroup.
   BrowsingContext::Children mToplevels;
 
   ContentParents mSubscribers;
+
+  // Map of cached contexts that need to stay alive due to bfcache.
+  nsTHashtable<nsRefPtrHashKey<BrowsingContext>> mCachedContexts;
 };
 
 }  // namespace dom
 }  // namespace mozilla
 
 #endif  // !defined(mozilla_dom_BrowsingContextGroup_h)
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -3763,21 +3763,27 @@ mozilla::ipc::IPCResult ContentChild::Re
 #ifdef DEBUG
     RefPtr<BrowsingContext> existing = BrowsingContext::Get(init.mId);
     MOZ_ASSERT(!existing, "BrowsingContext must not exist yet!");
 
     RefPtr<BrowsingContext> parent = init.GetParent();
     MOZ_ASSERT_IF(parent, parent->Group() == group);
 #endif
 
+    bool cached = init.mCached;
     RefPtr<BrowsingContext> ctxt =
         BrowsingContext::CreateFromIPC(std::move(init), group, nullptr);
 
-    // FIXME: We should deal with cached & detached contexts as well.
-    ctxt->Attach(/* aFromIPC */ true);
+    // If the browsing context is cached don't attach it, but add it
+    // to the cache here as well
+    if (cached) {
+      ctxt->Group()->CacheContext(ctxt);
+    } else {
+      ctxt->Attach(/* aFromIPC */ true);
+    }
   }
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult ContentChild::RecvWindowClose(BrowsingContext* aContext,
                                                       bool aTrustedCaller) {
   if (!aContext) {