Bug 1637085: Use AsyncShutdown for ContentParent shutdown. r=nika
authorKris Maglione <maglione.k@gmail.com>
Tue, 02 Jun 2020 20:46:07 +0000
changeset 533612 925f89a10fc8beb0e7c5ca02c8b873b0ebd138d7
parent 533611 ce622c9ded1c21a30c7f4d4c75719fae9dd7f54d
child 533613 0880c9f6821008953ea4f4f12d2d6ee84767a636
push id37474
push userabutkovits@mozilla.com
push dateWed, 03 Jun 2020 09:29:05 +0000
treeherdermozilla-central@bf162b065e1f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika
bugs1637085
milestone79.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 1637085: Use AsyncShutdown for ContentParent shutdown. r=nika Differential Revision: https://phabricator.services.mozilla.com/D74746
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -611,18 +611,16 @@ static bool sCanLaunchSubprocesses;
 
 // Set to true when the first content process gets created.
 static bool sCreatedFirstContentProcess = false;
 
 // The first content child has ID 1, so the chrome process can have ID 0.
 static uint64_t gContentChildID = 1;
 
 static const char* sObserverTopics[] = {
-    "xpcom-shutdown",
-    "profile-before-change",
     NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC,
     NS_IPC_IOSERVICE_SET_CONNECTIVITY_TOPIC,
     NS_IPC_CAPTIVE_PORTAL_SET_STATE,
     "memory-pressure",
     "child-gc-request",
     "child-cc-request",
     "child-mmu-request",
     "child-ghost-request",
@@ -1468,16 +1466,18 @@ void ContentParent::Init() {
   nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
   if (obs) {
     size_t length = ArrayLength(sObserverTopics);
     for (size_t i = 0; i < length; ++i) {
       obs->AddObserver(this, sObserverTopics[i], false);
     }
   }
 
+  AddShutdownBlockers();
+
   // Flush any pref updates that happened during launch and weren't
   // included in the blobs set up in BeginSubprocessLaunch.
   for (const Pref& pref : mQueuedPrefs) {
     Unused << NS_WARN_IF(!SendPreferenceUpdate(pref));
   }
   mQueuedPrefs.Clear();
 
   if (obs) {
@@ -1719,16 +1719,18 @@ void ContentParent::ActorDestroy(ActorDe
     mForceKillTimer->Cancel();
     mForceKillTimer = nullptr;
   }
 
   // Signal shutdown completion regardless of error state, so we can
   // finish waiting in the xpcom-shutdown/profile-before-change observer.
   mIPCOpen = false;
 
+  RemoveShutdownBlockers();
+
   if (mHangMonitorActor) {
     ProcessHangMonitor::RemoveProcess(mHangMonitorActor);
     mHangMonitorActor = nullptr;
   }
 
   RefPtr<FileSystemSecurity> fss = FileSystemSecurity::Get();
   if (fss) {
     fss->Forget(ChildID());
@@ -3076,40 +3078,89 @@ NS_IMPL_CYCLE_COLLECTING_ADDREF(ContentP
 NS_IMPL_CYCLE_COLLECTING_RELEASE(ContentParent)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ContentParent)
   NS_INTERFACE_MAP_ENTRY_CONCRETE(ContentParent)
   NS_INTERFACE_MAP_ENTRY(nsIContentParent)
   NS_INTERFACE_MAP_ENTRY(nsIObserver)
   NS_INTERFACE_MAP_ENTRY(nsIDOMGeoPositionCallback)
   NS_INTERFACE_MAP_ENTRY(nsIDOMGeoPositionErrorCallback)
+  NS_INTERFACE_MAP_ENTRY(nsIAsyncShutdownBlocker)
   NS_INTERFACE_MAP_ENTRY(nsIInterfaceRequestor)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIContentParent)
 NS_INTERFACE_MAP_END
 
+// Async shutdown blocker
+NS_IMETHODIMP
+ContentParent::BlockShutdown(nsIAsyncShutdownClient* aClient) {
+  // Make sure that our process will get scheduled.
+  ProcessPriorityManager::SetProcessPriority(this, PROCESS_PRIORITY_FOREGROUND);
+
+  // Okay to call ShutDownProcess multiple times.
+  ShutDownProcess(SEND_SHUTDOWN_MESSAGE);
+  MarkAsDead();
+
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+ContentParent::GetName(nsAString& aName) {
+  aName.AssignLiteral("ContentParent:");
+  aName.AppendPrintf(" id=%p", this);
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+ContentParent::GetState(nsIPropertyBag** aResult) {
+  auto props = MakeRefPtr<nsHashPropertyBag>();
+  props->SetPropertyAsAString(NS_LITERAL_STRING("remoteTypePrefix"),
+                              RemoteTypePrefix(mRemoteType));
+  *aResult = props.forget().downcast<nsIWritablePropertyBag>().take();
+  return NS_OK;
+}
+
+static StaticRefPtr<nsIAsyncShutdownClient> sXPCOMShutdownClient;
+static StaticRefPtr<nsIAsyncShutdownClient> sProfileBeforeChangeClient;
+
+static void InitClients() {
+  if (!sXPCOMShutdownClient) {
+    nsresult rv;
+    nsCOMPtr<nsIAsyncShutdownService> svc = services::GetAsyncShutdown();
+
+    nsCOMPtr<nsIAsyncShutdownClient> client;
+    rv = svc->GetXpcomWillShutdown(getter_AddRefs(client));
+    sXPCOMShutdownClient = client.forget();
+    ClearOnShutdown(&sXPCOMShutdownClient);
+    MOZ_RELEASE_ASSERT(NS_SUCCEEDED(rv), "XPCOMShutdown shutdown blocker");
+
+    rv = svc->GetProfileBeforeChange(getter_AddRefs(client));
+    sProfileBeforeChangeClient = client.forget();
+    ClearOnShutdown(&sProfileBeforeChangeClient);
+    MOZ_RELEASE_ASSERT(NS_SUCCEEDED(rv),
+                       "profileBeforeChange shutdown blocker");
+  }
+}
+
+void ContentParent::AddShutdownBlockers() {
+  InitClients();
+
+  sXPCOMShutdownClient->AddBlocker(this, NS_LITERAL_STRING(__FILE__), __LINE__,
+                                   EmptyString());
+  sProfileBeforeChangeClient->AddBlocker(this, NS_LITERAL_STRING(__FILE__),
+                                         __LINE__, EmptyString());
+}
+
+void ContentParent::RemoveShutdownBlockers() {
+  Unused << sXPCOMShutdownClient->RemoveBlocker(this);
+  Unused << sProfileBeforeChangeClient->RemoveBlocker(this);
+}
+
 NS_IMETHODIMP
 ContentParent::Observe(nsISupports* aSubject, const char* aTopic,
                        const char16_t* aData) {
-  if (mSubprocess && (!strcmp(aTopic, "profile-before-change") ||
-                      !strcmp(aTopic, "xpcom-shutdown"))) {
-    // Make sure that our process will get scheduled.
-    ProcessPriorityManager::SetProcessPriority(this,
-                                               PROCESS_PRIORITY_FOREGROUND);
-
-    // Okay to call ShutDownProcess multiple times.
-    ShutDownProcess(SEND_SHUTDOWN_MESSAGE);
-    MarkAsDead();
-
-    // Wait for shutdown to complete, so that we receive any shutdown
-    // data (e.g. telemetry) from the child before we quit.
-    // This loop terminate prematurely based on mForceKillTimer.
-    SpinEventLoopUntil([&]() { return !mIPCOpen || mCalledKillHard; });
-    NS_ASSERTION(!mSubprocess, "Close should have nulled mSubprocess");
-  }
-
   if (IsDead() || !mSubprocess) {
     return NS_OK;
   }
 
   if (!strcmp(aTopic, "nsPref:changed")) {
     // A pref changed. If it's not on the blacklist, inform child processes.
     if (!ShouldSyncPreference(aData)) {
       return NS_OK;
@@ -3540,16 +3591,18 @@ void ContentParent::KillHard(const char*
   // process handle becomes invalid on the first call, causing a second call
   // to crash our process - more details in bug 890840.
   if (mCalledKillHard) {
     return;
   }
   mCalledKillHard = true;
   mForceKillTimer = nullptr;
 
+  RemoveShutdownBlockers();
+
   GeneratePairedMinidump(aReason);
 
   nsDependentCString reason(aReason);
   Telemetry::Accumulate(Telemetry::SUBPROCESS_KILL_HARD, reason, 1);
 
   ProcessHandle otherProcessHandle;
   if (!base::OpenProcessHandle(OtherPid(), &otherProcessHandle)) {
     NS_ERROR("Failed to open child process when attempting kill.");
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -31,16 +31,17 @@
 #include "mozilla/TimeStamp.h"
 #include "mozilla/Variant.h"
 #include "mozilla/UniquePtr.h"
 
 #include "nsDataHashtable.h"
 #include "nsPluginTags.h"
 #include "nsFrameMessageManager.h"
 #include "nsHashKeys.h"
+#include "nsIAsyncShutdown.h"
 #include "nsIContentParent.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsIObserver.h"
 #include "nsIRemoteTab.h"
 #include "nsIDOMGeoPositionCallback.h"
 #include "nsIDOMGeoPositionErrorCallback.h"
 #include "nsRefPtrHashtable.h"
 #include "PermissionMessageUtils.h"
@@ -127,16 +128,17 @@ struct CancelContentJSOptions;
   }
 
 class ContentParent final
     : public PContentParent,
       public nsIContentParent,
       public nsIObserver,
       public nsIDOMGeoPositionCallback,
       public nsIDOMGeoPositionErrorCallback,
+      public nsIAsyncShutdownBlocker,
       public nsIInterfaceRequestor,
       public gfx::gfxVarReceiver,
       public mozilla::LinkedListElement<ContentParent>,
       public gfx::GPUProcessListener,
       public mozilla::MemoryReportingProcess,
       public mozilla::dom::ipc::MessageManagerCallback,
       public mozilla::ipc::IShmemAllocator,
       public mozilla::ipc::ParentToChildStreamActorManager,
@@ -335,16 +337,17 @@ class ContentParent final
 
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(ContentParent, nsIObserver)
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_NSICONTENTPARENT
   NS_DECL_NSIOBSERVER
   NS_DECL_NSIDOMGEOPOSITIONCALLBACK
   NS_DECL_NSIDOMGEOPOSITIONERRORCALLBACK
+  NS_DECL_NSIASYNCSHUTDOWNBLOCKER
   NS_DECL_NSIINTERFACEREQUESTOR
 
   /**
    * MessageManagerCallback methods that we override.
    */
   virtual bool DoLoadMessageManagerScript(const nsAString& aURL,
                                           bool aRunInGlobalScope) override;
 
@@ -699,16 +702,19 @@ class ContentParent final
    */
   static nsClassHashtable<nsStringHashKey, nsTArray<ContentParent*>>*
       sBrowserContentParents;
   static nsTArray<ContentParent*>* sPrivateContent;
   static nsDataHashtable<nsUint32HashKey, ContentParent*>*
       sJSPluginContentParents;
   static StaticAutoPtr<LinkedList<ContentParent>> sContentParents;
 
+  void AddShutdownBlockers();
+  void RemoveShutdownBlockers();
+
 #if defined(XP_MACOSX) && defined(MOZ_SANDBOX)
   // Cached Mac sandbox params used when launching content processes.
   static StaticAutoPtr<std::vector<std::string>> sMacSandboxParams;
 #endif
 
   // Set aLoadUri to true to load aURIToLoad and to false to only create the
   // window. aURIToLoad should always be provided, if available, to ensure
   // compatibility with GeckoView.