Bug 1348361 - Part 3 - Do not block the thread when spawning a gecko child process; r?jld draft
authorStephen A Pohl <spohl.mozilla.bugs@gmail.com>
Fri, 16 Feb 2018 10:24:21 -0500
changeset 768520 035255f364359c3ddaf6525b72849d0649dcc97a
parent 768519 baa56a97e189296f761e7b251550e4aa2f873289
push id102893
push userbmo:agaynor@mozilla.com
push dateFri, 16 Mar 2018 13:06:46 +0000
reviewersjld
bugs1348361
milestone61.0a1
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;