Bug 1535835 - Remove global weak ref to BrowsingContext within Unlink(), r=farre a=jcristau
authorNika Layzell <nika@thelayzells.com>
Fri, 24 May 2019 20:15:53 +0000
changeset 536568 ab126e9e64632ac6ee92b9ee819cfdc9ec3abd51
parent 536567 dfe3f74cba935f205a6b160bd0084eaf584e2cc6
child 536569 4738da99fbc88df4ba50b494b58342d44fa3575b
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfarre, jcristau
bugs1535835
milestone68.0
Bug 1535835 - Remove global weak ref to BrowsingContext within Unlink(), r=farre a=jcristau Differential Revision: https://phabricator.services.mozilla.com/D30683
docshell/base/BrowsingContext.cpp
docshell/base/BrowsingContext.h
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@@ -45,56 +45,48 @@ namespace dom {
 
 extern mozilla::LazyLogModule gUserInteractionPRLog;
 
 #define USER_ACTIVATION_LOG(msg, ...) \
   MOZ_LOG(gUserInteractionPRLog, LogLevel::Debug, (msg, ##__VA_ARGS__))
 
 static LazyLogModule gBrowsingContextLog("BrowsingContext");
 
-template <template <typename> class PtrType>
-using BrowsingContextMap =
-    HashMap<uint64_t, PtrType<BrowsingContext>, DefaultHasher<uint64_t>,
-            InfallibleAllocPolicy>;
+typedef nsDataHashtable<nsUint64HashKey, BrowsingContext*> BrowsingContextMap;
 
-static StaticAutoPtr<BrowsingContextMap<WeakPtr>> sBrowsingContexts;
+static StaticAutoPtr<BrowsingContextMap> sBrowsingContexts;
 
 static void Register(BrowsingContext* aBrowsingContext) {
-  MOZ_ALWAYS_TRUE(
-      sBrowsingContexts->putNew(aBrowsingContext->Id(), aBrowsingContext));
+  sBrowsingContexts->Put(aBrowsingContext->Id(), aBrowsingContext);
 
   aBrowsingContext->Group()->Register(aBrowsingContext);
 }
 
 BrowsingContext* BrowsingContext::Top() {
   BrowsingContext* bc = this;
   while (bc->mParent) {
     bc = bc->mParent;
   }
   return bc;
 }
 
 /* static */
 void BrowsingContext::Init() {
   if (!sBrowsingContexts) {
-    sBrowsingContexts = new BrowsingContextMap<WeakPtr>();
+    sBrowsingContexts = new BrowsingContextMap();
     ClearOnShutdown(&sBrowsingContexts);
   }
 }
 
 /* static */
 LogModule* BrowsingContext::GetLog() { return gBrowsingContextLog; }
 
 /* static */
 already_AddRefed<BrowsingContext> BrowsingContext::Get(uint64_t aId) {
-  if (BrowsingContextMap<WeakPtr>::Ptr abc = sBrowsingContexts->lookup(aId)) {
-    return do_AddRef(abc->value().get());
-  }
-
-  return nullptr;
+  return do_AddRef(sBrowsingContexts->Get(aId));
 }
 
 CanonicalBrowsingContext* BrowsingContext::Canonical() {
   return CanonicalBrowsingContext::Cast(this);
 }
 
 /* static */
 already_AddRefed<BrowsingContext> BrowsingContext::Create(
@@ -356,17 +348,17 @@ void BrowsingContext::RestoreChildren(Ch
     }
     cc->SendRestoreBrowsingContextChildren(this, contexts);
   }
 }
 
 bool BrowsingContext::IsCached() { return Group()->IsContextCached(this); }
 
 bool BrowsingContext::HasOpener() const {
-  return sBrowsingContexts->has(mOpenerId);
+  return sBrowsingContexts->Contains(mOpenerId);
 }
 
 void BrowsingContext::GetChildren(Children& aChildren) {
   MOZ_ALWAYS_TRUE(aChildren.AppendElements(mChildren));
 }
 
 // FindWithName follows the rules for choosing a browsing context,
 // with the exception of sandboxing for iframes. The implementation
@@ -523,17 +515,17 @@ bool BrowsingContext::IsActive() const {
 }
 
 BrowsingContext::~BrowsingContext() {
   MOZ_DIAGNOSTIC_ASSERT(!mParent || !mParent->mChildren.Contains(this));
   MOZ_DIAGNOSTIC_ASSERT(!mGroup || !mGroup->Toplevels().Contains(this));
   MOZ_DIAGNOSTIC_ASSERT(!mGroup || !mGroup->IsContextCached(this));
 
   if (sBrowsingContexts) {
-    sBrowsingContexts->remove(Id());
+    sBrowsingContexts->Remove(Id());
   }
 }
 
 nsISupports* BrowsingContext::GetParentObject() const {
   return xpc::NativeGlobal(xpc::PrivilegedJunkScope());
 }
 
 JSObject* BrowsingContext::WrapObject(JSContext* aCx,
@@ -564,16 +556,20 @@ void BrowsingContext::NotifyResetUserGes
 bool BrowsingContext::GetUserGestureActivation() {
   RefPtr<BrowsingContext> topLevelBC = Top();
   return topLevelBC->GetIsActivatedByUserGesture();
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(BrowsingContext)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(BrowsingContext)
+  if (sBrowsingContexts) {
+    sBrowsingContexts->Remove(tmp->Id());
+  }
+
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocShell, mChildren, mParent, mGroup,
                                   mEmbedderElement)
   if (XRE_IsParentProcess()) {
     CanonicalBrowsingContext::Cast(tmp)->Unlink();
   }
   NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
--- a/docshell/base/BrowsingContext.h
+++ b/docshell/base/BrowsingContext.h
@@ -83,19 +83,17 @@ class BrowsingContextBase {
 // major difference is that BrowsingContexts are replicated (synced)
 // to the parent process, making it possible to traverse the
 // BrowsingContext tree for a tab, in both the parent and the child
 // process.
 //
 // Trees of BrowsingContexts should only ever contain nodes of the
 // same BrowsingContext::Type. This is enforced by asserts in the
 // BrowsingContext::Create* methods.
-class BrowsingContext : public nsWrapperCache,
-                        public SupportsWeakPtr<BrowsingContext>,
-                        public BrowsingContextBase {
+class BrowsingContext : public nsWrapperCache, public BrowsingContextBase {
  public:
   enum class Type { Chrome, Content };
 
   using Children = nsTArray<RefPtr<BrowsingContext>>;
 
   static void Init();
   static LogModule* GetLog();
   static void CleanupContexts(uint64_t aProcessId);