Backed out changeset 8f1051f12365 (bug 1637085) for build bustages on selftest.py. CLOSED TREE
authorRazvan Maries <rmaries@mozilla.com>
Mon, 01 Jun 2020 21:52:34 +0300
changeset 533382 451b7934feb47bb748041cde53273afc5a567367
parent 533381 5d2d0c265789f64157d927aec5adf3f93ec9cdd8
child 533383 a7ee7cb5e8748d12a8247061792123a6365316f7
push id117700
push userrmaries@mozilla.com
push dateMon, 01 Jun 2020 18:53:43 +0000
treeherderautoland@451b7934feb4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1637085
milestone79.0a1
backs out8f1051f12365af7ad7c2a0dd1837e5960d88c8cc
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 8f1051f12365 (bug 1637085) for build bustages on selftest.py. CLOSED TREE
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -611,16 +611,18 @@ 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",
@@ -1466,18 +1468,16 @@ 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,18 +1719,16 @@ 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());
@@ -3078,87 +3076,40 @@ 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: remoteType=");
-  aName.Append(RemoteTypePrefix(mRemoteType));
-  aName.AppendPrintf(" id=%p", this);
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-ContentParent::GetState(nsIPropertyBag** aResult) {
-  *aResult = nullptr;
-  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;
@@ -3589,18 +3540,16 @@ 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,17 +31,16 @@
 #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"
@@ -128,17 +127,16 @@ 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,
@@ -337,17 +335,16 @@ 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;
 
@@ -702,19 +699,16 @@ 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.