Backed out changeset 70c5f8576e43 (bug 1525887) for PLDHashTable failures CLOSED TREE
authorBogdan Tara <btara@mozilla.com>
Thu, 14 Feb 2019 12:30:34 +0200
changeset 459018 2045ba64df59a540532ca92c769d701264b96961
parent 459017 9d7a50ad7fc65340a4f66f0b98dcf27f8efcbf49
child 459019 dfeedf1391264cf677aa2e275ae02abd2d25483d
push id35554
push userrgurzau@mozilla.com
push dateThu, 14 Feb 2019 17:00:27 +0000
treeherdermozilla-central@db6bcdbe4040 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1525887
milestone67.0a1
backs out70c5f8576e43c6a5f27fc95c946f1bb4d901c666
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
Backed out changeset 70c5f8576e43 (bug 1525887) for PLDHashTable failures CLOSED TREE
docshell/base/BrowsingContext.cpp
docshell/base/BrowsingContextGroup.cpp
docshell/base/BrowsingContextGroup.h
docshell/base/CanonicalBrowsingContext.cpp
docshell/base/CanonicalBrowsingContext.h
docshell/build/nsDocShellModule.cpp
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@@ -143,18 +143,16 @@ CanonicalBrowsingContext* BrowsingContex
   MOZ_LOG(GetLog(), LogLevel::Debug,
           ("Creating 0x%08" PRIx64 " from IPC (origin=0x%08" PRIx64 ")", aId,
            aOriginProcess ? uint64_t(aOriginProcess->ChildID()) : 0));
 
   RefPtr<BrowsingContext> context;
   if (XRE_IsParentProcess()) {
     context = new CanonicalBrowsingContext(
         aParent, aOpener, aName, aId, aOriginProcess->ChildID(), Type::Content);
-
-    context->Group()->Subscribe(aOriginProcess);
   } else {
     context = new BrowsingContext(aParent, aOpener, aName, aId, Type::Content);
   }
 
   Register(context);
 
   context->Attach();
 
--- a/docshell/base/BrowsingContextGroup.cpp
+++ b/docshell/base/BrowsingContextGroup.cpp
@@ -6,58 +6,69 @@
 
 #include "mozilla/dom/BrowsingContextGroup.h"
 #include "mozilla/dom/BrowsingContextBinding.h"
 #include "mozilla/dom/BindingUtils.h"
 
 namespace mozilla {
 namespace dom {
 
+StaticAutoPtr<nsTArray<RefPtr<BrowsingContextGroup>>>
+    BrowsingContextGroup::sAllGroups;
+
+/* static */ void BrowsingContextGroup::Init() {
+  if (!sAllGroups) {
+    sAllGroups = new nsTArray<RefPtr<BrowsingContextGroup>>();
+    ClearOnShutdown(&sAllGroups);
+  }
+}
+
 bool BrowsingContextGroup::Contains(BrowsingContext* aBrowsingContext) {
   return aBrowsingContext->Group() == this;
 }
 
 void BrowsingContextGroup::Register(BrowsingContext* aBrowsingContext) {
-  MOZ_DIAGNOSTIC_ASSERT(aBrowsingContext);
   mContexts.PutEntry(aBrowsingContext);
 }
 
 void BrowsingContextGroup::Unregister(BrowsingContext* aBrowsingContext) {
-  MOZ_DIAGNOSTIC_ASSERT(aBrowsingContext);
   mContexts.RemoveEntry(aBrowsingContext);
 }
 
-void BrowsingContextGroup::Subscribe(ContentParent* aOriginProcess) {
-  MOZ_DIAGNOSTIC_ASSERT(aOriginProcess);
-  mSubscribers.PutEntry(aOriginProcess);
-  aOriginProcess->OnBrowsingContextGroupSubscribe(this);
-}
-
-void BrowsingContextGroup::Unsubscribe(ContentParent* aOriginProcess) {
-  MOZ_DIAGNOSTIC_ASSERT(aOriginProcess);
-  mSubscribers.RemoveEntry(aOriginProcess);
-  aOriginProcess->OnBrowsingContextGroupUnsubscribe(this);
+BrowsingContextGroup::BrowsingContextGroup() {
+  sAllGroups->AppendElement(this);
 }
 
 BrowsingContextGroup::~BrowsingContextGroup() {
-  for (auto iter = mSubscribers.Iter(); !iter.Done(); iter.Next()) {
-    nsRefPtrHashKey<ContentParent>* entry = iter.Get();
-    entry->GetKey()->OnBrowsingContextGroupUnsubscribe(this);
+  if (sAllGroups) {
+    sAllGroups->RemoveElement(this);
   }
 }
 
 nsISupports* BrowsingContextGroup::GetParentObject() const {
   return xpc::NativeGlobal(xpc::PrivilegedJunkScope());
 }
 
 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)
+NS_IMPL_CYCLE_COLLECTION_CLASS(BrowsingContextGroup)
+
+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(BrowsingContextGroup)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mContexts)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mToplevels)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
+NS_IMPL_CYCLE_COLLECTION_UNLINK_END
+
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(BrowsingContextGroup)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mContexts)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mToplevels)
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
+
+NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(BrowsingContextGroup)
 
 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
@@ -3,17 +3,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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 {
@@ -27,54 +26,48 @@ class BrowsingContext;
 //
 // A BrowsingContext may not hold references to other BrowsingContext objects
 // which are not in the same BrowsingContextGroup.
 class BrowsingContextGroup final : public nsWrapperCache {
  public:
   NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(BrowsingContextGroup)
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_NATIVE_CLASS(BrowsingContextGroup)
 
-  typedef nsTHashtable<nsRefPtrHashKey<ContentParent>> ContentParents;
+  static void Init();
 
   // Interact with the list of BrowsingContexts.
   bool Contains(BrowsingContext* aContext);
   void Register(BrowsingContext* aContext);
   void Unregister(BrowsingContext* aContext);
 
-  // Interact with the list of ContentParents
-  void Subscribe(ContentParent* aOriginProcess);
-  void Unsubscribe(ContentParent* aOriginProcess);
-
-  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);
   }
 
   nsISupports* GetParentObject() const;
   JSObject* WrapObject(JSContext* aCx,
                        JS::Handle<JSObject*> aGivenProto) override;
 
-  BrowsingContextGroup() = default;
+  BrowsingContextGroup();
 
  private:
   friend class CanonicalBrowsingContext;
 
   ~BrowsingContextGroup();
 
+  static StaticAutoPtr<nsTArray<RefPtr<BrowsingContextGroup>>> sAllGroups;
+
   // 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.
   nsTHashtable<nsRefPtrHashKey<BrowsingContext>> mContexts;
 
   // The set of toplevel browsing contexts in the current BrowsingContextGroup.
   BrowsingContext::Children mToplevels;
-
-  ContentParents mSubscribers;
 };
 
 }  // namespace dom
 }  // namespace mozilla
 
 #endif  // !defined(mozilla_dom_BrowsingContextGroup_h)
--- a/docshell/base/CanonicalBrowsingContext.cpp
+++ b/docshell/base/CanonicalBrowsingContext.cpp
@@ -25,16 +25,37 @@ CanonicalBrowsingContext::CanonicalBrows
                                                    BrowsingContext::Type aType)
     : BrowsingContext(aParent, aOpener, aName, aBrowsingContextId, aType),
       mProcessId(aProcessId) {
   // You are only ever allowed to create CanonicalBrowsingContexts in the
   // parent process.
   MOZ_RELEASE_ASSERT(XRE_IsParentProcess());
 }
 
+// TODO(farre): CanonicalBrowsingContext::CleanupContexts starts from the
+// list of root BrowsingContexts. This isn't enough when separate
+// BrowsingContext nodes of a BrowsingContext tree, not in a crashing
+// child process, are from that process and thus needs to be
+// cleaned. [Bug 1472108]
+/* static */ void CanonicalBrowsingContext::CleanupContexts(
+    uint64_t aProcessId) {
+  nsTArray<RefPtr<BrowsingContext>> contexts;
+  for (auto& group : *BrowsingContextGroup::sAllGroups) {
+    for (auto& context : group->Toplevels()) {
+      if (Cast(context)->IsOwnedByProcess(aProcessId)) {
+        contexts.AppendElement(context);
+      }
+    }
+  }
+
+  for (auto& context : contexts) {
+    context->Detach();
+  }
+}
+
 /* static */ already_AddRefed<CanonicalBrowsingContext>
 CanonicalBrowsingContext::Get(uint64_t aId) {
   MOZ_RELEASE_ASSERT(XRE_IsParentProcess());
   return BrowsingContext::Get(aId).downcast<CanonicalBrowsingContext>();
 }
 
 /* static */ CanonicalBrowsingContext* CanonicalBrowsingContext::Cast(
     BrowsingContext* aContext) {
--- a/docshell/base/CanonicalBrowsingContext.h
+++ b/docshell/base/CanonicalBrowsingContext.h
@@ -21,16 +21,17 @@ namespace dom {
 
 class WindowGlobalParent;
 
 // CanonicalBrowsingContext is a BrowsingContext living in the parent
 // process, with whatever extra data that a BrowsingContext in the
 // parent needs.
 class CanonicalBrowsingContext final : public BrowsingContext {
  public:
+  static void CleanupContexts(uint64_t aProcessId);
   static already_AddRefed<CanonicalBrowsingContext> Get(uint64_t aId);
   static CanonicalBrowsingContext* Cast(BrowsingContext* aContext);
   static const CanonicalBrowsingContext* Cast(const BrowsingContext* aContext);
 
   bool IsOwnedByProcess(uint64_t aProcessId) const {
     return mProcessId == aProcessId;
   }
   uint64_t OwnerProcessId() const { return mProcessId; }
--- a/docshell/build/nsDocShellModule.cpp
+++ b/docshell/build/nsDocShellModule.cpp
@@ -11,16 +11,17 @@
 #include "nsSHEntryShared.h"
 #include "nsSHistory.h"
 
 namespace mozilla {
 
 // The one time initialization for this module
 nsresult InitDocShellModule() {
   mozilla::dom::BrowsingContext::Init();
+  mozilla::dom::BrowsingContextGroup::Init();
   nsresult rv = nsSHistory::Startup();
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
 void UnloadDocShellModule() {
   nsSHistory::Shutdown();
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -34,17 +34,16 @@
 #include "mozilla/BasePrincipal.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/Components.h"
 #include "mozilla/StyleSheetInlines.h"
 #include "mozilla/DataStorage.h"
 #include "mozilla/devtools/HeapSnapshotTempFileHelperParent.h"
 #include "mozilla/docshell/OfflineCacheUpdateParent.h"
 #include "mozilla/dom/BrowsingContext.h"
-#include "mozilla/dom/BrowsingContextGroup.h"
 #include "mozilla/dom/CanonicalBrowsingContext.h"
 #include "mozilla/dom/ClientManager.h"
 #include "mozilla/dom/ClientOpenWindowOpActors.h"
 #include "mozilla/dom/DataTransfer.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/File.h"
 #include "mozilla/dom/FileCreatorHelper.h"
 #include "mozilla/dom/FileSystemSecurity.h"
@@ -1739,22 +1738,17 @@ void ContentParent::ActorDestroy(ActorDe
   }
 
   mBlobURLs.Clear();
 
 #if defined(XP_WIN32) && defined(ACCESSIBILITY)
   a11y::AccessibleWrap::ReleaseContentProcessIdFor(ChildID());
 #endif
 
-  for (auto iter = mGroups.Iter(); !iter.Done(); iter.Next()) {
-    nsRefPtrHashKey<mozilla::dom::BrowsingContextGroup>* entry = iter.Get();
-    entry->GetKey()->Unsubscribe(this);
-  }
-
-  MOZ_DIAGNOSTIC_ASSERT(mGroups.IsEmpty());
+  CanonicalBrowsingContext::CleanupContexts(ChildID());
 }
 
 bool ContentParent::TryToRecycle() {
   // This life time check should be replaced by a memory health check (memory
   // usage + fragmentation).
   const double kMaxLifeSpan = 5;
   if (mShutdownPending || mCalledKillHard || !IsAlive() ||
       !mRemoteType.EqualsLiteral(DEFAULT_REMOTE_TYPE) ||
@@ -5821,26 +5815,16 @@ mozilla::ipc::IPCResult ContentParent::R
   if (!BuildClonedMessageDataForParent(cp, messageFromChild, message)) {
     // FIXME Logging?
     return IPC_OK();
   }
   Unused << cp->SendWindowPostMessage(aContext, message, aData);
   return IPC_OK();
 }
 
-void ContentParent::OnBrowsingContextGroupSubscribe(BrowsingContextGroup* aGroup) {
-  MOZ_DIAGNOSTIC_ASSERT(aGroup);
-  mGroups.PutEntry(aGroup);
-}
-
-void ContentParent::OnBrowsingContextGroupUnsubscribe(BrowsingContextGroup* aGroup) {
-  MOZ_DIAGNOSTIC_ASSERT(aGroup);
-  mGroups.RemoveEntry(aGroup);
-}
-
 }  // namespace dom
 }  // namespace mozilla
 
 NS_IMPL_ISUPPORTS(ParentIdleListener, nsIObserver)
 
 NS_IMETHODIMP
 ParentIdleListener::Observe(nsISupports*, const char* aTopic,
                             const char16_t* aData) {
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -91,17 +91,16 @@ class PJavaScriptParent;
 }  // namespace jsipc
 
 namespace layers {
 struct TextureFactoryIdentifier;
 }  // namespace layers
 
 namespace dom {
 
-class BrowsingContextGroup;
 class Element;
 class TabParent;
 class ClonedMessageData;
 class MemoryReport;
 class TabContext;
 class ContentBridgeParent;
 class GetFilesHelper;
 class MemoryReportRequestHost;
@@ -118,17 +117,16 @@ class ContentParent final : public PCont
                             public mozilla::MemoryReportingProcess {
   typedef mozilla::ipc::GeckoChildProcessHost GeckoChildProcessHost;
   typedef mozilla::ipc::OptionalURIParams OptionalURIParams;
   typedef mozilla::ipc::PFileDescriptorSetParent PFileDescriptorSetParent;
   typedef mozilla::ipc::TestShellParent TestShellParent;
   typedef mozilla::ipc::URIParams URIParams;
   typedef mozilla::ipc::PrincipalInfo PrincipalInfo;
   typedef mozilla::dom::ClonedMessageData ClonedMessageData;
-  typedef mozilla::dom::BrowsingContextGroup BrowsingContextGroup;
 
   friend class mozilla::PreallocatedProcessManagerImpl;
   friend class PContentParent;
 #ifdef FUZZING
   friend class mozilla::ipc::ProtocolFuzzerHelper;
 #endif
 
  public:
@@ -1188,19 +1186,16 @@ class ContentParent final : public PCont
   bool CanCommunicateWith(ContentParentId aOtherProcess);
 
   nsresult SaveRecording(nsIFile* aFile, bool* aRetval);
 
   bool IsRecordingOrReplaying() const {
     return mRecordReplayState != eNotRecordingOrReplaying;
   }
 
-  void OnBrowsingContextGroupSubscribe(BrowsingContextGroup* aGroup);
-  void OnBrowsingContextGroupUnsubscribe(BrowsingContextGroup* aGroup);
-
  private:
   // Released in ActorDestroy; deliberately not exposed to the CC.
   RefPtr<ContentParent> mSelfRef;
 
   // If you add strong pointers to cycle collected objects here, be sure to
   // release these objects in ShutDownProcess.  See the comment there for more
   // details.
 
@@ -1326,18 +1321,16 @@ class ContentParent final : public PCont
   static nsDataHashtable<nsUint64HashKey, TabParent*> sNextTabParents;
 
 #if defined(XP_MACOSX) && defined(MOZ_CONTENT_SANDBOX)
   // When set to true, indicates that content processes should
   // initialize their sandbox during startup instead of waiting
   // for the SetProcessSandbox IPDL message.
   static bool sEarlySandboxInit;
 #endif
-
-  nsTHashtable<nsRefPtrHashKey<BrowsingContextGroup>> mGroups;
 };
 
 }  // namespace dom
 }  // namespace mozilla
 
 class ParentIdleListener : public nsIObserver {
   friend class mozilla::dom::ContentParent;