Bug 1446161 - Remove the earlier attempt at async launch. r=spohl,mccr8
authorJed Davis <jld@mozilla.com>
Wed, 28 Nov 2018 20:42:24 +0000
changeset 448585 3cbd2cf23c867eddeaeccdfffc70485f2ad3c9c0
parent 448584 25b4fcd4382dfa21ee08f1b7604c649132f52500
child 448586 9f5e62b653028ed2b31c16c2556343739a0f55b8
push id35119
push userccoroiu@mozilla.com
push dateThu, 29 Nov 2018 04:26:53 +0000
treeherdermozilla-central@c35dea45f131 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersspohl, mccr8
bugs1446161
milestone65.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 1446161 - Remove the earlier attempt at async launch. r=spohl,mccr8 The first attempt at async launch tried to hide the asynchrony inside IPC, by making the process seem to be launched enough to construct new channels and send it messages, and lazily blocking on the pid/handle. Unfortunately, in practice we wind up needing the pid/handle immediately, and this requirement is too deeply embedded in IPC for that to be viable. (The alternative that will be used instead -- exposing process launch via an explicitly asynchronous promise interface -- is made simpler by Project Fission's upcoming rewrite of how the DOM requests new content processes.) Depends on D8941 Differential Revision: https://phabricator.services.mozilla.com/D8942
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
dom/ipc/ContentProcessHost.cpp
dom/ipc/ContentProcessHost.h
dom/ipc/moz.build
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
@@ -1612,19 +1612,17 @@ ContentParent::OnChannelError()
   PContentParent::OnChannelError();
 }
 
 void
 ContentParent::OnChannelConnected(int32_t pid)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-#ifndef ASYNC_CONTENTPROC_LAUNCH
   SetOtherProcessId(pid);
-#endif
 
 #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) {
@@ -1639,21 +1637,16 @@ ContentParent::OnChannelConnected(int32_
     if (NS_FAILED(rv)) {
       cpus = 1;
     }
     if (nice != 0 && cpus == 1) {
       setpriority(PRIO_PROCESS, pid, getpriority(PRIO_PROCESS, pid) + nice);
     }
   }
 #endif
-
-#if defined(MOZ_CODE_COVERAGE) && defined(ASYNC_CONTENTPROC_LAUNCH)
-  Unused << SendShareCodeCoverageMutex(
-              CodeCoverageHandler::Get()->GetMutexHandle(pid));
-#endif
 }
 
 void
 ContentParent::ProcessingError(Result aCode, const char* aReason)
 {
   if (MsgDropped == aCode) {
     return;
   }
@@ -2314,41 +2307,32 @@ ContentParent::LaunchSubprocess(ProcessP
                               : (int) recordreplay::ProcessKind::MiddlemanReplaying);
     extraArgs.push_back(recordreplay::gProcessKindOption);
     extraArgs.push_back(buf.get());
 
     extraArgs.push_back(recordreplay::gRecordingFileOption);
     extraArgs.push_back(NS_ConvertUTF16toUTF8(mRecordingFile).get());
   }
 
-  SetOtherProcessId(kInvalidProcessId, ProcessIdState::ePending);
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-  if (!mSubprocess->Launch(extraArgs)) {
-#else
   if (!mSubprocess->LaunchAndWaitForProcessHandle(extraArgs)) {
-#endif
     NS_ERROR("failed to launch child in the parent");
     MarkAsDead();
     return false;
   }
 
   // See also ActorDestroy.
   mSelfRef = this;
 
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-  OpenWithAsyncPid(mSubprocess->GetChannel());
-#else
   base::ProcessId procId =
     base::GetProcId(mSubprocess->GetChildProcessHandle());
   Open(mSubprocess->GetChannel(), procId);
 #ifdef MOZ_CODE_COVERAGE
   Unused << SendShareCodeCoverageMutex(
               CodeCoverageHandler::Get()->GetMutexHandle(procId));
 #endif
-#endif // ASYNC_CONTENTPROC_LAUNCH
 
   InitInternal(aInitialPriority);
 
   ContentProcessManager::GetSingleton()->AddContentProcess(this);
 
   mHangMonitorActor = ProcessHangMonitor::AddProcess(this);
 
   // Set a reply timeout for CPOWs.
@@ -2416,17 +2400,17 @@ ContentParent::ContentParent(ContentPare
   // Request Windows message deferral behavior on our side of the PContent
   // channel. Generally only applies to the situation where we get caught in
   // a deadlock with the plugin process when sending CPOWs.
   GetIPCChannel()->SetChannelFlags(MessageChannel::REQUIRE_DEFERRED_MESSAGE_PROTECTION);
 #endif
 
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
   bool isFile = mRemoteType.EqualsLiteral(FILE_REMOTE_TYPE);
-  mSubprocess = new ContentProcessHost(this, isFile);
+  mSubprocess = new GeckoChildProcessHost(GeckoProcessType_Content, isFile);
 
 #if defined(XP_MACOSX) && defined(MOZ_CONTENT_SANDBOX)
   // sEarlySandboxInit is statically initialized to false.
   // Once we've set it to true due to the pref, avoid checking the
   // pref on subsequent calls. As a result, changing the earlyinit
   // pref requires restarting the browser to take effect.
   if (!ContentParent::sEarlySandboxInit) {
     ContentParent::sEarlySandboxInit =
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -6,26 +6,26 @@
 
 #ifndef mozilla_dom_ContentParent_h
 #define mozilla_dom_ContentParent_h
 
 #include "mozilla/dom/PContentParent.h"
 #include "mozilla/dom/nsIContentParent.h"
 #include "mozilla/gfx/gfxVarReceiver.h"
 #include "mozilla/gfx/GPUProcessListener.h"
+#include "mozilla/ipc/GeckoChildProcessHost.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/FileUtils.h"
 #include "mozilla/HalTypes.h"
 #include "mozilla/LinkedList.h"
 #include "mozilla/MemoryReportingProcess.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/UniquePtr.h"
 
-#include "ContentProcessHost.h"
 #include "nsDataHashtable.h"
 #include "nsPluginTags.h"
 #include "nsFrameMessageManager.h"
 #include "nsHashKeys.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsIObserver.h"
 #include "nsIThreadInternal.h"
 #include "nsIDOMGeoPositionCallback.h"
@@ -109,24 +109,24 @@ class ContentParent final : public PCont
                           , public nsIDOMGeoPositionCallback
                           , public nsIDOMGeoPositionErrorCallback
                           , public nsIInterfaceRequestor
                           , public gfx::gfxVarReceiver
                           , public mozilla::LinkedListElement<ContentParent>
                           , public gfx::GPUProcessListener
                           , public mozilla::MemoryReportingProcess
 {
+  typedef mozilla::ipc::GeckoChildProcessHost GeckoChildProcessHost;
   typedef mozilla::ipc::OptionalURIParams OptionalURIParams;
   typedef mozilla::ipc::PFileDescriptorSetParent PFileDescriptorSetParent;
   typedef mozilla::ipc::TestShellParent TestShellParent;
   typedef mozilla::ipc::URIParams URIParams;
   typedef mozilla::ipc::PrincipalInfo PrincipalInfo;
   typedef mozilla::dom::ClonedMessageData ClonedMessageData;
 
-  friend class ContentProcessHost;
   friend class mozilla::PreallocatedProcessManagerImpl;
 #ifdef FUZZING
   friend class mozilla::ipc::ProtocolFuzzerHelper;
 #endif
 
 public:
 
   virtual bool IsContentParent() const override { return true; }
@@ -388,17 +388,17 @@ public:
   {
     return mIsForBrowser;
   }
   virtual bool IsForJSPlugin() const override
   {
     return mJSPluginID != nsFakePluginTag::NOT_JSPLUGIN;
   }
 
-  ContentProcessHost* Process() const
+  GeckoChildProcessHost* Process() const
   {
     return mSubprocess;
   }
 
   ContentParent* Opener() const
   {
     return mOpener;
   }
@@ -1287,17 +1287,17 @@ public:
 private:
   // Released in ActorDestroy; deliberately not exposed to the CC.
   RefPtr<ContentParent> mSelfRef;
 
   // If you add strong pointers to cycle collected objects here, be sure to
   // release these objects in ShutDownProcess.  See the comment there for more
   // details.
 
-  ContentProcessHost* mSubprocess;
+  GeckoChildProcessHost* mSubprocess;
   const TimeStamp mLaunchTS; // used to calculate time to start content process
   TimeStamp mActivateTS;
   ContentParent* mOpener;
 
   nsString mRemoteType;
 
   ContentParentId mChildID;
   int32_t mGeolocationWatchID;
deleted file mode 100644
--- a/dom/ipc/ContentProcessHost.cpp
+++ /dev/null
@@ -1,63 +0,0 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-
- * vim: sts=8 sw=2 ts=2 tw=99 et :
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#include "ContentProcessHost.h"
-
-namespace mozilla {
-namespace dom {
-
-using namespace ipc;
-
-ContentProcessHost::ContentProcessHost(ContentParent* aContentParent,
-                                       bool aIsFileContent)
- : GeckoChildProcessHost(GeckoProcessType_Content, aIsFileContent),
-   mHasLaunched(false),
-   mContentParent(aContentParent)
-{
-  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
deleted file mode 100644
--- a/dom/ipc/ContentProcessHost.h
+++ /dev/null
@@ -1,51 +0,0 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-
- * vim: sts=8 sw=2 ts=2 tw=99 et :
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#ifndef _include_mozilla_dom_ipc_ContentProcessHost_h_
-#define _include_mozilla_dom_ipc_ContentProcessHost_h_
-
-#include "mozilla/ipc/GeckoChildProcessHost.h"
-
-namespace mozilla {
-namespace dom {
-
-class ContentParent;
-
-// ContentProcessHost is the "parent process" container for a subprocess handle
-// and IPC connection. It owns the parent process IPDL actor, which in this
-// case, is a ContentParent.
-class ContentProcessHost final : public ::mozilla::ipc::GeckoChildProcessHost
-{
-  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
-};
-
-} // namespace dom
-} // namespace mozilla
-
-#endif // _include_mozilla_dom_ipc_ContentProcessHost_h_
--- a/dom/ipc/moz.build
+++ b/dom/ipc/moz.build
@@ -31,17 +31,16 @@ EXPORTS.mozilla.dom += [
     'CoalescedInputData.h',
     'CoalescedMouseData.h',
     'CoalescedWheelData.h',
     'ContentBridgeChild.h',
     'ContentBridgeParent.h',
     'ContentChild.h',
     'ContentParent.h',
     'ContentProcess.h',
-    'ContentProcessHost.h',
     'ContentProcessManager.h',
     'CPOWManagerGetter.h',
     'FilePickerParent.h',
     'MemoryReportRequest.h',
     'nsIContentChild.h',
     'nsIContentParent.h',
     'PermissionMessageUtils.h',
     'TabChild.h',
@@ -62,17 +61,16 @@ EXPORTS.mozilla += [
 UNIFIED_SOURCES += [
     'CoalescedMouseData.cpp',
     'CoalescedWheelData.cpp',
     'ColorPickerParent.cpp',
     'ContentBridgeChild.cpp',
     'ContentBridgeParent.cpp',
     'ContentParent.cpp',
     'ContentProcess.cpp',
-    'ContentProcessHost.cpp',
     'ContentProcessManager.cpp',
     'FilePickerParent.cpp',
     'MemMapSnapshot.cpp',
     'MemoryReportRequest.cpp',
     'nsIContentChild.cpp',
     'nsIContentParent.cpp',
     'PermissionMessageUtils.cpp',
     'PreallocatedProcessManager.cpp',
--- a/ipc/glue/GeckoChildProcessHost.cpp
+++ b/ipc/glue/GeckoChildProcessHost.cpp
@@ -490,27 +490,20 @@ 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();
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-    OnProcessLaunchError();
-#endif
     CHROMIUM_LOG(ERROR) << "Failed to launch " <<
       XRE_ChildProcessTypeToString(mProcessType) << " subprocess";
     Telemetry::Accumulate(Telemetry::SUBPROCESS_LAUNCH_FAILURE,
       nsDependentCString(XRE_ChildProcessTypeToString(mProcessType)));
-#ifdef ASYNC_CONTENTPROC_LAUNCH
-  } else {
-    OnProcessHandleReady(mChildProcessHandle);
-#endif
   }
   return ok;
 }
 
 void
 #if defined(XP_WIN)
 AddAppDirToCommandLine(CommandLine& aCmdLine)
 #else
@@ -1135,24 +1128,16 @@ 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
@@ -67,18 +67,16 @@ public:
   bool LaunchAndWaitForProcessHandle(StringVector aExtraOpts=StringVector());
 
   // 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 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
@@ -670,20 +670,18 @@ IProtocol::ManagedState::GetActorEventTa
 {
   return mProtocol->Manager()->GetActorEventTarget(aActor);
 }
 
 IToplevelProtocol::IToplevelProtocol(const char* aName,
                                      ProtocolId aProtoId,
                                      Side aSide)
   : IProtocol(aSide, MakeUnique<ToplevelState>(aName, this, aSide))
-  , mMonitor("mozilla.ipc.IToplevelProtocol.mMonitor")
   , mProtocolId(aProtoId)
   , mOtherPid(mozilla::ipc::kInvalidProcessId)
-  , mOtherPidState(ProcessIdState::eUnstarted)
   , mIsMainThreadProtocol(false)
 {
 }
 
 IToplevelProtocol::~IToplevelProtocol()
 {
   mState = nullptr;
   if (mTrans) {
@@ -698,48 +696,31 @@ IToplevelProtocol::OtherPid() const
   base::ProcessId pid = OtherPidMaybeInvalid();
   MOZ_RELEASE_ASSERT(pid != kInvalidProcessId);
   return pid;
 }
 
 base::ProcessId
 IToplevelProtocol::OtherPidMaybeInvalid() const
 {
-  MonitorAutoLock lock(mMonitor);
-
-  if (mOtherPidState == ProcessIdState::eUnstarted) {
-    // If you're asking for the pid of a process we haven't even tried to
-    // start, you get an invalid pid back immediately.
-    return kInvalidProcessId;
-  }
-
-  while (mOtherPidState < ProcessIdState::eReady) {
-    lock.Wait();
-  }
-  MOZ_RELEASE_ASSERT(mOtherPidState == ProcessIdState::eReady);
-
   return mOtherPid;
 }
 
 void
-IToplevelProtocol::SetOtherProcessId(base::ProcessId aOtherPid,
-                                     ProcessIdState aState)
+IToplevelProtocol::SetOtherProcessId(base::ProcessId aOtherPid)
 {
-  MonitorAutoLock lock(mMonitor);
   // When recording an execution, all communication we do is forwarded from
   // the middleman to the parent process, so use its pid instead of the
   // middleman's pid.
   if (recordreplay::IsRecordingOrReplaying() &&
       aOtherPid == recordreplay::child::MiddlemanProcessId()) {
     mOtherPid = recordreplay::child::ParentProcessId();
   } else {
     mOtherPid = aOtherPid;
   }
-  mOtherPidState = aState;
-  lock.NotifyAll();
 }
 
 bool
 IToplevelProtocol::TakeMinidump(nsIFile** aDump, uint32_t* aSequence)
 {
   MOZ_RELEASE_ASSERT(GetSide() == ParentSide);
   return XRE_TakeMinidumpForChild(OtherPid(), aDump, aSequence);
 }
--- a/ipc/glue/ProtocolUtils.h
+++ b/ipc/glue/ProtocolUtils.h
@@ -414,23 +414,16 @@ class IToplevelProtocol : public IProtoc
     template<class PFooSide> friend class Endpoint;
 
 protected:
     explicit IToplevelProtocol(const char* aName, ProtocolId aProtoId,
                                Side aSide);
     ~IToplevelProtocol();
 
 public:
-    enum ProcessIdState {
-        eUnstarted,
-        ePending,
-        eReady,
-        eError
-    };
-
     class ToplevelState final : public ProtocolState
     {
 #ifdef FUZZING
       friend class mozilla::ipc::ProtocolFuzzerHelper;
 #endif
 
     public:
         ToplevelState(const char* aName, IToplevelProtocol* aProtocol, Side aSide);
@@ -482,18 +475,17 @@ public:
         mTrans = std::move(aTrans);
     }
 
     Transport* GetTransport() const { return mTrans.get(); }
 
     ProtocolId GetProtocolId() const { return mProtocolId; }
 
     base::ProcessId OtherPid() const final;
-    void SetOtherProcessId(base::ProcessId aOtherPid,
-                           ProcessIdState aState = ProcessIdState::eReady);
+    void SetOtherProcessId(base::ProcessId aOtherPid);
 
     bool TakeMinidump(nsIFile** aDump, uint32_t* aSequence);
 
     virtual void OnChannelClose() = 0;
     virtual void OnChannelError() = 0;
     virtual void ProcessingError(Result aError, const char* aMsgName) {}
     virtual void OnChannelConnected(int32_t peer_pid) {}
 
@@ -623,26 +615,22 @@ protected:
     virtual already_AddRefed<nsIEventTarget>
     GetConstructedEventTarget(const Message& aMsg) { return nullptr; }
 
     // Override this method in top-level protocols to change the event target
     // for specific messages.
     virtual already_AddRefed<nsIEventTarget>
     GetSpecificMessageEventTarget(const Message& aMsg) { return nullptr; }
 
-    // This monitor protects mOtherPid and mOtherPidState. All other fields
-    // should only be accessed on the worker thread.
-    mutable mozilla::Monitor mMonitor;
   private:
     base::ProcessId OtherPidMaybeInvalid() const;
 
     ProtocolId mProtocolId;
     UniquePtr<Transport> mTrans;
     base::ProcessId mOtherPid;
-    ProcessIdState mOtherPidState;
     bool mIsMainThreadProtocol;
 };
 
 class IShmemAllocator
 {
 public:
   virtual bool AllocShmem(size_t aSize,
                           mozilla::ipc::SharedMemory::SharedMemoryType aShmType,