Bug 1561179 - P3: Merge EnsureRDDReady into LaunchRDDProcess. r=mattwoodrow
authorDan Glastonbury <dan.glastonbury@gmail.com>
Mon, 04 Nov 2019 03:41:54 +0000
changeset 500314 dc3f655d626efcbff67d819f0195d56e06f8cd43
parent 500313 8b9accb338043f9e7cdeb90f77b76927cd19a143
child 500315 e4d3547739db667cecbcb571c5f9a0cbb6a484a8
push id36761
push userdvarga@mozilla.com
push dateMon, 04 Nov 2019 09:41:18 +0000
treeherdermozilla-central@5647ec4ba6f2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow
bugs1561179
milestone72.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 1561179 - P3: Merge EnsureRDDReady into LaunchRDDProcess. r=mattwoodrow LaunchRDDProcess() and CreateContentBridge() create a sync creation. Merge the functions into one function. Keep the IPDL messaging async to avoid adding a exception for the message being sync. Differential Revision: https://phabricator.services.mozilla.com/D50400
dom/ipc/ContentParent.cpp
dom/media/ipc/PRDD.ipdl
dom/media/ipc/RDDChild.cpp
dom/media/ipc/RDDChild.h
dom/media/ipc/RDDParent.cpp
dom/media/ipc/RDDProcessManager.cpp
dom/media/ipc/RDDProcessManager.h
dom/media/ipc/RemoteDecoderManagerChild.cpp
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -1088,19 +1088,20 @@ mozilla::ipc::IPCResult ContentParent::R
     nsresult* aRv, Endpoint<PRemoteDecoderManagerChild>* aEndpoint) {
   *aRv = NS_OK;
 
   if (XRE_IsParentProcess() &&
       BrowserTabsRemoteAutostart() &&  // only do rdd process if e10s on
       Preferences::GetBool("media.rdd-process.enabled", false)) {
     RDDProcessManager* rdd = RDDProcessManager::Get();
     if (rdd) {
-      rdd->LaunchRDDProcess();
-
-      bool rddOpened = rdd->CreateContentBridge(OtherPid(), aEndpoint);
+      bool rddOpened = rdd->LaunchRDDProcess();
+      if (rddOpened) {
+        rddOpened = rdd->CreateContentBridge(OtherPid(), aEndpoint);
+      }
 
       if (NS_WARN_IF(!rddOpened)) {
         *aRv = NS_ERROR_NOT_AVAILABLE;
       }
     } else {
       *aRv = NS_ERROR_NOT_AVAILABLE;
     }
   }
--- a/dom/media/ipc/PRDD.ipdl
+++ b/dom/media/ipc/PRDD.ipdl
@@ -39,18 +39,16 @@ parent:
 
   async PreferenceUpdate(Pref pref);
 
   async UpdateVar(GfxVarUpdate var);
 
   async CreateVideoBridgeToParentProcess(Endpoint<PVideoBridgeChild> endpoint);
 
 child:
-  // args TBD, sent when init complete. Occurs once, after Init().
-  async InitComplete();
 
   async InitCrashReporter(Shmem shmem, NativeThreadId threadId);
 
   async AddMemoryReport(MemoryReport aReport);
   async FinishMemoryReport(uint32_t aGeneration);
 
 };
 
--- a/dom/media/ipc/RDDChild.cpp
+++ b/dom/media/ipc/RDDChild.cpp
@@ -19,17 +19,17 @@
 #endif
 #include "RDDProcessHost.h"
 
 namespace mozilla {
 
 using namespace layers;
 using namespace gfx;
 
-RDDChild::RDDChild(RDDProcessHost* aHost) : mHost(aHost), mRDDReady(false) {
+RDDChild::RDDChild(RDDProcessHost* aHost) : mHost(aHost) {
   MOZ_COUNT_CTOR(RDDChild);
 }
 
 RDDChild::~RDDChild() { MOZ_COUNT_DTOR(RDDChild); }
 
 bool RDDChild::Init(bool aStartMacSandbox) {
   Maybe<FileDescriptor> brokerFd;
 
@@ -56,35 +56,16 @@ bool RDDChild::Init(bool aStartMacSandbo
   Unused << SendInitProfiler(ProfilerParent::CreateForProcess(OtherPid()));
 #endif
 
   gfxVars::AddReceiver(this);
 
   return true;
 }
 
-bool RDDChild::EnsureRDDReady() {
-  if (mRDDReady) {
-    return true;
-  }
-
-  mRDDReady = true;
-  return true;
-}
-
-mozilla::ipc::IPCResult RDDChild::RecvInitComplete() {
-  // We synchronously requested RDD parameters before this arrived.
-  if (mRDDReady) {
-    return IPC_OK();
-  }
-
-  mRDDReady = true;
-  return IPC_OK();
-}
-
 bool RDDChild::SendRequestMemoryReport(const uint32_t& aGeneration,
                                        const bool& aAnonymize,
                                        const bool& aMinimizeMemoryUsage,
                                        const Maybe<FileDescriptor>& aDMDFile) {
   mMemoryReportRequest = MakeUnique<MemoryReportRequestHost>(aGeneration);
   Unused << PRDDChild::SendRequestMemoryReport(aGeneration, aAnonymize,
                                                aMinimizeMemoryUsage, aDMDFile);
   return true;
--- a/dom/media/ipc/RDDChild.h
+++ b/dom/media/ipc/RDDChild.h
@@ -29,23 +29,18 @@ class RDDChild final : public PRDDChild,
   typedef mozilla::dom::MemoryReportRequestHost MemoryReportRequestHost;
 
  public:
   explicit RDDChild(RDDProcessHost* aHost);
   ~RDDChild();
 
   bool Init(bool aStartMacSandbox);
 
-  bool EnsureRDDReady();
-
   void OnVarChanged(const GfxVarUpdate& aVar) override;
 
-  // PRDDChild overrides.
-  mozilla::ipc::IPCResult RecvInitComplete();
-
   void ActorDestroy(ActorDestroyReason aWhy) override;
 
   mozilla::ipc::IPCResult RecvAddMemoryReport(const MemoryReport& aReport);
   mozilla::ipc::IPCResult RecvFinishMemoryReport(const uint32_t& aGeneration);
 
   bool SendRequestMemoryReport(const uint32_t& aGeneration,
                                const bool& aAnonymize,
                                const bool& aMinimizeMemoryUsage,
@@ -54,14 +49,13 @@ class RDDChild final : public PRDDChild,
   static void Destroy(UniquePtr<RDDChild>&& aChild);
 
  private:
   RDDProcessHost* mHost;
   UniquePtr<MemoryReportRequestHost> mMemoryReportRequest;
 #if defined(XP_LINUX) && defined(MOZ_SANDBOX)
   UniquePtr<SandboxBroker> mSandboxBroker;
 #endif
-  bool mRDDReady;
 };
 
 }  // namespace mozilla
 
 #endif  // _include_dom_media_ipc_RDDChild_h_
--- a/dom/media/ipc/RDDParent.cpp
+++ b/dom/media/ipc/RDDParent.cpp
@@ -116,18 +116,16 @@ static void StartRDDMacSandbox() {
     MOZ_CRASH("mozilla::StartMacSandbox failed");
   }
 }
 #endif
 
 mozilla::ipc::IPCResult RDDParent::RecvInit(
     nsTArray<GfxVarUpdate>&& vars, const Maybe<FileDescriptor>& aBrokerFd,
     bool aStartMacSandbox) {
-  Unused << SendInitComplete();
-
   for (const auto& var : vars) {
     gfxVars::ApplyUpdate(var);
   }
 
 #if defined(MOZ_SANDBOX)
 #  if defined(XP_MACOSX)
   // Close all current connections to the WindowServer. This ensures that the
   // Activity Monitor will not label the content process as "Not responding"
--- a/dom/media/ipc/RDDProcessManager.cpp
+++ b/dom/media/ipc/RDDProcessManager.cpp
@@ -96,57 +96,51 @@ void RDDProcessManager::OnPreferenceChan
   if (!!mRDDChild) {
     MOZ_ASSERT(mQueuedPrefs.IsEmpty());
     mRDDChild->SendPreferenceUpdate(pref);
   } else {
     mQueuedPrefs.AppendElement(pref);
   }
 }
 
-void RDDProcessManager::LaunchRDDProcess() {
+bool RDDProcessManager::LaunchRDDProcess() {
   if (mProcess) {
-    return;
+    return true;
   }
 
   mNumProcessAttempts++;
 
   std::vector<std::string> extraArgs;
   nsCString parentBuildID(mozilla::PlatformBuildID());
   extraArgs.push_back("-parentBuildID");
   extraArgs.push_back(parentBuildID.get());
 
   // The subprocess is launched asynchronously, so we wait for a callback to
   // acquire the IPDL actor.
   mProcess = new RDDProcessHost(this);
   if (!mProcess->Launch(extraArgs)) {
     DestroyProcess();
+    return false;
   }
+  if (!EnsureRDDReady()) {
+    return false;
+  }
+
+  return true;
 }
 
 bool RDDProcessManager::EnsureRDDReady() {
-  if (mProcess && !mProcess->IsConnected()) {
-    if (!mProcess->WaitForLaunch()) {
-      // If this fails, we should have fired OnProcessLaunchComplete and
-      // removed the process.
-      MOZ_ASSERT(!mProcess && !mRDDChild);
-      return false;
-    }
+  if (mProcess && !mProcess->IsConnected() && !mProcess->WaitForLaunch()) {
+    // If this fails, we should have fired OnProcessLaunchComplete and
+    // removed the process.
+    MOZ_ASSERT(!mProcess && !mRDDChild);
+    return false;
   }
 
-  if (mRDDChild) {
-    if (mRDDChild->EnsureRDDReady()) {
-      return true;
-    }
-
-    // If the initialization above fails, we likely have a RDD process teardown
-    // waiting in our message queue (or will soon).
-    DestroyProcess();
-  }
-
-  return false;
+  return true;
 }
 
 void RDDProcessManager::OnProcessLaunchComplete(RDDProcessHost* aHost) {
   MOZ_ASSERT(mProcess && mProcess == aHost);
 
   if (!mProcess->IsConnected()) {
     DestroyProcess();
     return;
@@ -235,20 +229,16 @@ void RDDProcessManager::DestroyProcess()
   CrashReporter::AnnotateCrashReport(
       CrashReporter::Annotation::RDDProcessStatus,
       NS_LITERAL_CSTRING("Destroyed"));
 }
 
 bool RDDProcessManager::CreateContentBridge(
     base::ProcessId aOtherProcess,
     ipc::Endpoint<PRemoteDecoderManagerChild>* aOutRemoteDecoderManager) {
-  if (!EnsureRDDReady() || !StaticPrefs::media_rdd_process_enabled()) {
-    return false;
-  }
-
   ipc::Endpoint<PRemoteDecoderManagerParent> parentPipe;
   ipc::Endpoint<PRemoteDecoderManagerChild> childPipe;
 
   nsresult rv = PRemoteDecoderManager::CreateEndpoints(
       mRDDChild->OtherPid(), aOtherProcess, &parentPipe, &childPipe);
   if (NS_FAILED(rv)) {
     MOZ_LOG(sPDMLog, LogLevel::Debug,
             ("Could not create content remote decoder: %d", int(rv)));
--- a/dom/media/ipc/RDDProcessManager.h
+++ b/dom/media/ipc/RDDProcessManager.h
@@ -22,17 +22,17 @@ class RDDProcessManager final : public R
  public:
   static void Initialize();
   static void Shutdown();
   static RDDProcessManager* Get();
 
   ~RDDProcessManager();
 
   // If not using a RDD process, launch a new RDD process asynchronously.
-  void LaunchRDDProcess();
+  bool LaunchRDDProcess();
 
   // Ensure that RDD-bound methods can be used. If no RDD process is being
   // used, or one is launched and ready, this function returns immediately.
   // Otherwise it blocks until the RDD process has finished launching.
   bool EnsureRDDReady();
 
   bool CreateContentBridge(base::ProcessId aOtherProcess,
                            mozilla::ipc::Endpoint<PRemoteDecoderManagerChild>*
--- a/dom/media/ipc/RemoteDecoderManagerChild.cpp
+++ b/dom/media/ipc/RemoteDecoderManagerChild.cpp
@@ -169,31 +169,33 @@ void RemoteDecoderManagerChild::OpenForR
   // 2) if ActorDestroy was called (mCanSend is false) meaning the other
   // end of the ipc channel was torn down
   if (sRemoteDecoderManagerChildForRDDProcess &&
       sRemoteDecoderManagerChildForRDDProcess->mCanSend) {
     return;
   }
   sRemoteDecoderManagerChildForRDDProcess = nullptr;
   if (aEndpoint.IsValid()) {
-    RefPtr<RemoteDecoderManagerChild> manager = new RemoteDecoderManagerChild(VideoBridgeSource::RddProcess);
+    RefPtr<RemoteDecoderManagerChild> manager =
+        new RemoteDecoderManagerChild(VideoBridgeSource::RddProcess);
     if (aEndpoint.Bind(manager)) {
       sRemoteDecoderManagerChildForRDDProcess = manager;
       manager->InitIPDL();
     }
   }
 }
 
 void RemoteDecoderManagerChild::OpenForGPUProcess(
     Endpoint<PRemoteDecoderManagerChild>&& aEndpoint) {
   // Make sure we always dispatch everything in sRecreateTasks, even if we
   // fail since this is as close to being recreated as we will ever be.
   sRemoteDecoderManagerChildForGPUProcess = nullptr;
   if (aEndpoint.IsValid()) {
-    RefPtr<RemoteDecoderManagerChild> manager = new RemoteDecoderManagerChild(VideoBridgeSource::GpuProcess);
+    RefPtr<RemoteDecoderManagerChild> manager =
+        new RemoteDecoderManagerChild(VideoBridgeSource::GpuProcess);
     if (aEndpoint.Bind(manager)) {
       sRemoteDecoderManagerChildForGPUProcess = manager;
       manager->InitIPDL();
     }
   }
   for (Runnable* task : *sRecreateTasks) {
     task->Run();
   }