Backed out changeset 9bedd57840ed (bug 1637085) for WPT failures. CLOSED TREE
authorDorel Luca <dluca@mozilla.com>
Fri, 15 May 2020 05:37:57 +0300
changeset 530213 31e3dab240a28c162943acad26fb543c54144dc0
parent 530212 dc88ebc9fed6b2b7330816947c32a72ef65b69f9
child 530214 3de26cd60e6263a404c70b2f2e3b0c86a537f9d0
push id116043
push userdluca@mozilla.com
push dateFri, 15 May 2020 02:38:47 +0000
treeherderautoland@31e3dab240a2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1637085
milestone78.0a1
backs out9bedd57840edbba0a91f41e8347a6a724473d13a
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 9bedd57840ed (bug 1637085) for WPT failures. CLOSED TREE
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -601,16 +601,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",
@@ -1360,18 +1362,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) {
@@ -1609,18 +1609,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());
@@ -2873,87 +2871,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;
@@ -3394,18 +3345,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"
@@ -131,17 +130,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,
@@ -338,17 +336,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;
 
@@ -720,19 +717,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.