Bug 1348361 - Part 3 - Do not block the thread when spawning a gecko child process; r=jld
authorStephen A Pohl <spohl.mozilla.bugs@gmail.com>
Fri, 16 Feb 2018 10:24:21 -0500
changeset 408647 0520f0545678b71542a77780a1ec7a21ad5613fb
parent 408646 d29a6c667ebae10002cbb3c9a0913cf0befc5e69
child 408648 8ef404b01c4aa86801c51097f6a524fc21ca1017
push id100996
push userbtara@mozilla.com
push dateSat, 17 Mar 2018 10:37:43 +0000
treeherdermozilla-inbound@97160a734959 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjld
bugs1348361
milestone61.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 1348361 - Part 3 - Do not block the thread when spawning a gecko child process; r=jld They are not yet fully async because ContentParent::InitInternal calls OtherPid(), which will block until the process is spawned. Deferring the calls to OtherPid() will be a subject of a follow up patch. MozReview-Commit-ID: 4TFkMpdQtRw
dom/ipc/ContentParent.cpp
dom/ipc/ContentProcessHost.cpp
dom/ipc/ContentProcessHost.h
ipc/glue/GeckoChildProcessHost.cpp
ipc/glue/GeckoChildProcessHost.h
ipc/glue/ProtocolUtils.cpp
ipc/glue/ProtocolUtils.h
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -1518,17 +1518,17 @@ ContentParent::OnChannelError()
 {
   RefPtr<ContentParent> content(this);
   PContentParent::OnChannelError();
 }
 
 void
 ContentParent::OnChannelConnected(int32_t pid)
 {
-  SetOtherProcessId(pid);
+  MOZ_ASSERT(NS_IsMainThread());
 
 #if defined(ANDROID) || defined(LINUX)
   // Check nice preference
   int32_t nice = Preferences::GetInt("dom.ipc.content.nice", 0);
 
   // Environment variable overrides preference
   char* relativeNicenessStr = getenv("MOZ_CHILD_PROCESS_RELATIVE_NICENESS");
   if (relativeNicenessStr) {
@@ -1543,16 +1543,21 @@ ContentParent::OnChannelConnected(int32_
     if (NS_FAILED(rv)) {
       cpus = 1;
     }
     if (nice != 0 && cpus == 1) {
       setpriority(PRIO_PROCESS, pid, getpriority(PRIO_PROCESS, pid) + nice);
     }
   }
 #endif
+
+#ifdef MOZ_CODE_COVERAGE
+  Unused << SendShareCodeCoverageMutex(
+              CodeCoverageHandler::Get()->GetMutexHandle(pid));
+#endif
 }
 
 void
 ContentParent::ProcessingError(Result aCode, const char* aReason)
 {
   if (MsgDropped == aCode) {
     return;
   }
@@ -2052,39 +2057,35 @@ ContentParent::LaunchSubprocess(ProcessP
   extraArgs.push_back("-schedulerPrefs");
   extraArgs.push_back(schedulerPrefs.get());
 
   if (gSafeMode) {
     extraArgs.push_back("-safeMode");
   }
 
   SetOtherProcessId(kInvalidProcessId, ProcessIdState::ePending);
-  if (!mSubprocess->LaunchAndWaitForProcessHandle(extraArgs)) {
+  if (!mSubprocess->Launch(extraArgs)) {
     NS_ERROR("failed to launch child in the parent");
     MarkAsDead();
     return false;
   }
 
-  base::ProcessId procId = base::GetProcId(mSubprocess->GetChildProcessHandle());
-
-  Open(mSubprocess->GetChannel(), procId);
-
-#ifdef MOZ_CODE_COVERAGE
-  Unused << SendShareCodeCoverageMutex(CodeCoverageHandler::Get()->GetMutexHandle(procId));
-#endif
+  OpenWithAsyncPid(mSubprocess->GetChannel());
 
   InitInternal(aInitialPriority);
 
   ContentProcessManager::GetSingleton()->AddContentProcess(this);
 
   mHangMonitorActor = ProcessHangMonitor::AddProcess(this);
 
   // Set a reply timeout for CPOWs.
   SetReplyTimeoutMs(Preferences::GetInt("dom.ipc.cpow.timeout", 0));
 
+  // TODO: If OtherPid() is not called between mSubprocess->Launch() and this,
+  // then we're not really measuring how long it took to spawn the process.
   Telemetry::Accumulate(Telemetry::CONTENT_PROCESS_LAUNCH_TIME_MS,
                         static_cast<uint32_t>((TimeStamp::Now() - mLaunchTS)
                                               .ToMilliseconds()));
 
   nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
   if (obs) {
     nsAutoString cpId;
     cpId.AppendInt(static_cast<uint64_t>(this->ChildID()));
--- a/dom/ipc/ContentProcessHost.cpp
+++ b/dom/ipc/ContentProcessHost.cpp
@@ -20,10 +20,44 @@ ContentProcessHost::ContentProcessHost(C
   MOZ_COUNT_CTOR(ContentProcessHost);
 }
 
 ContentProcessHost::~ContentProcessHost()
 {
   MOZ_COUNT_DTOR(ContentProcessHost);
 }
 
+bool
+ContentProcessHost::Launch(StringVector aExtraOpts)
+{
+  MOZ_ASSERT(!mHasLaunched);
+  MOZ_ASSERT(mContentParent);
+
+  bool res = GeckoChildProcessHost::AsyncLaunch(aExtraOpts);
+  MOZ_RELEASE_ASSERT(res);
+  return true;
+}
+
+void
+ContentProcessHost::OnProcessHandleReady(ProcessHandle aProcessHandle)
+{
+  MOZ_ASSERT(!NS_IsMainThread());
+
+  // This will wake up the main thread if it is waiting for the process to
+  // launch.
+  mContentParent->SetOtherProcessId(base::GetProcId(aProcessHandle));
+
+  mHasLaunched = true;
+  GeckoChildProcessHost::OnProcessHandleReady(aProcessHandle);
+}
+
+void
+ContentProcessHost::OnProcessLaunchError()
+{
+  MOZ_ASSERT(!NS_IsMainThread());
+  mContentParent->SetOtherProcessId(mozilla::ipc::kInvalidProcessId,
+                                    ContentParent::ProcessIdState::eError);
+  mHasLaunched = true;
+  GeckoChildProcessHost::OnProcessLaunchError();
+}
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/ipc/ContentProcessHost.h
+++ b/dom/ipc/ContentProcessHost.h
@@ -22,16 +22,26 @@ class ContentProcessHost final : public 
   friend class ContentParent;
 
 public:
   explicit
   ContentProcessHost(ContentParent* aContentParent,
                      bool aIsFileContent = false);
   ~ContentProcessHost();
 
+  // Launch the subprocess asynchronously. On failure, false is returned.
+  // Otherwise, true is returned, and either the OnProcessHandleReady method is
+  // called when the process is created, or OnProcessLaunchError will be called
+  // if the process could not be spawned due to an asynchronous error.
+  bool Launch(StringVector aExtraOpts);
+
+  // Called on the IO thread.
+  void OnProcessHandleReady(ProcessHandle aProcessHandle) override;
+  void OnProcessLaunchError() override;
+
 private:
   DISALLOW_COPY_AND_ASSIGN(ContentProcessHost);
 
   bool mHasLaunched;
 
   ContentParent* mContentParent; // weak
 };
 
--- a/ipc/glue/GeckoChildProcessHost.cpp
+++ b/ipc/glue/GeckoChildProcessHost.cpp
@@ -549,20 +549,23 @@ GeckoChildProcessHost::RunPerformAsyncLa
 
   bool ok = PerformAsyncLaunch(aExtraOpts);
   if (!ok) {
     // WaitUntilConnected might be waiting for us to signal.
     // If something failed let's set the error state and notify.
     MonitorAutoLock lock(mMonitor);
     mProcessState = PROCESS_ERROR;
     lock.Notify();
+    OnProcessLaunchError();
     CHROMIUM_LOG(ERROR) << "Failed to launch " <<
       XRE_ChildProcessTypeToString(mProcessType) << " subprocess";
     Telemetry::Accumulate(Telemetry::SUBPROCESS_LAUNCH_FAILURE,
       nsDependentCString(XRE_ChildProcessTypeToString(mProcessType)));
+  } else {
+    OnProcessHandleReady(mChildProcessHandle);
   }
   return ok;
 }
 
 void
 #if defined(XP_WIN)
 AddAppDirToCommandLine(CommandLine& aCmdLine)
 #else
@@ -1126,16 +1129,24 @@ GeckoChildProcessHost::OpenPrivilegedHan
     MOZ_ASSERT(aPid == base::GetProcId(mChildProcessHandle));
     return true;
   }
 
   return base::OpenPrivilegedProcessHandle(aPid, &mChildProcessHandle);
 }
 
 void
+GeckoChildProcessHost::OnProcessHandleReady(ProcessHandle aProcessHandle)
+{}
+
+void
+GeckoChildProcessHost::OnProcessLaunchError()
+{}
+
+void
 GeckoChildProcessHost::OnChannelConnected(int32_t peer_pid)
 {
   if (!OpenPrivilegedHandle(peer_pid)) {
     MOZ_CRASH("can't open handle to child process");
   }
   MonitorAutoLock lock(mMonitor);
   mProcessState = PROCESS_CONNECTED;
   lock.Notify();
--- a/ipc/glue/GeckoChildProcessHost.h
+++ b/ipc/glue/GeckoChildProcessHost.h
@@ -69,16 +69,18 @@ public:
   // Block until the child process has been created and it connects to
   // the IPC channel, meaning it's fully initialized.  (Or until an
   // error occurs.)
   bool SyncLaunch(StringVector aExtraOpts=StringVector(),
                   int32_t timeoutMs=0);
 
   virtual bool PerformAsyncLaunch(StringVector aExtraOpts=StringVector());
 
+  virtual void OnProcessHandleReady(ProcessHandle aProcessHandle);
+  virtual void OnProcessLaunchError();
   virtual void OnChannelConnected(int32_t peer_pid) override;
   virtual void OnMessageReceived(IPC::Message&& aMsg) override;
   virtual void OnChannelError() override;
   virtual void GetQueuedMessages(std::queue<IPC::Message>& queue) override;
 
   virtual void InitializeChannel();
 
   virtual bool CanShutdown() override { return true; }
--- a/ipc/glue/ProtocolUtils.cpp
+++ b/ipc/glue/ProtocolUtils.cpp
@@ -663,16 +663,24 @@ bool
 IToplevelProtocol::Open(MessageChannel* aChannel,
                         nsIEventTarget* aEventTarget,
                         mozilla::ipc::Side aSide)
 {
   SetOtherProcessId(base::GetCurrentProcId());
   return GetIPCChannel()->Open(aChannel, aEventTarget, aSide);
 }
 
+bool
+IToplevelProtocol::OpenWithAsyncPid(mozilla::ipc::Transport* aTransport,
+                                    MessageLoop* aThread,
+                                    mozilla::ipc::Side aSide)
+{
+  return GetIPCChannel()->Open(aTransport, aThread, aSide);
+}
+
 void
 IToplevelProtocol::Close()
 {
   GetIPCChannel()->Close();
 }
 
 void
 IToplevelProtocol::SetReplyTimeoutMs(int32_t aTimeoutMs)
--- a/ipc/glue/ProtocolUtils.h
+++ b/ipc/glue/ProtocolUtils.h
@@ -307,16 +307,20 @@ public:
     bool Open(MessageChannel* aChannel,
               MessageLoop* aMessageLoop,
               mozilla::ipc::Side aSide = mozilla::ipc::UnknownSide);
 
     bool Open(MessageChannel* aChannel,
               nsIEventTarget* aEventTarget,
               mozilla::ipc::Side aSide = mozilla::ipc::UnknownSide);
 
+    bool OpenWithAsyncPid(mozilla::ipc::Transport* aTransport,
+                          MessageLoop* aThread = nullptr,
+                          mozilla::ipc::Side aSide = mozilla::ipc::UnknownSide);
+
     void Close();
 
     void SetReplyTimeoutMs(int32_t aTimeoutMs);
 
     virtual int32_t Register(IProtocol*) override;
     virtual int32_t RegisterID(IProtocol*, int32_t) override;
     virtual IProtocol* Lookup(int32_t) override;
     virtual void Unregister(int32_t) override;