Bug 1389759 - Ensure we tear down CompositorManagerChild correctly if init or the GPU process fail. r=dvander, a=sledru
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 26 Sep 2017 13:21:52 -0400
changeset 432020 b7b894f5fad8b4fb79e60b6bbdcce52da1281714
parent 432019 b108bdcaff9d3ec05cd3eba32800b3393409126c
child 432021 eef9559e379b51c1c5ac6a22a1e29ee2c2111f8d
push id7870
push userryanvm@gmail.com
push dateMon, 02 Oct 2017 13:47:52 +0000
treeherdermozilla-beta@b7b894f5fad8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander, sledru
bugs1389759
milestone57.0
Bug 1389759 - Ensure we tear down CompositorManagerChild correctly if init or the GPU process fail. r=dvander, a=sledru
gfx/ipc/GPUProcessManager.cpp
gfx/layers/ipc/CompositorManagerChild.cpp
gfx/layers/ipc/CompositorManagerChild.h
gfx/layers/ipc/CompositorManagerParent.cpp
gfx/layers/ipc/CompositorManagerParent.h
--- a/gfx/ipc/GPUProcessManager.cpp
+++ b/gfx/ipc/GPUProcessManager.cpp
@@ -220,43 +220,41 @@ GPUProcessManager::EnsureProtocolsReady(
   EnsureCompositorManagerChild();
   EnsureImageBridgeChild();
   EnsureVRManager();
 }
 
 void
 GPUProcessManager::EnsureCompositorManagerChild()
 {
-  base::ProcessId gpuPid = EnsureGPUReady()
-                           ? mGPUChild->OtherPid()
-                           : base::GetCurrentProcId();
-
-  if (CompositorManagerChild::IsInitialized(gpuPid)) {
+  bool gpuReady = EnsureGPUReady();
+  if (CompositorManagerChild::IsInitialized(mProcessToken)) {
     return;
   }
 
-  if (!EnsureGPUReady()) {
-    CompositorManagerChild::InitSameProcess(AllocateNamespace());
+  if (!gpuReady) {
+    CompositorManagerChild::InitSameProcess(AllocateNamespace(), mProcessToken);
     return;
   }
 
   ipc::Endpoint<PCompositorManagerParent> parentPipe;
   ipc::Endpoint<PCompositorManagerChild> childPipe;
   nsresult rv = PCompositorManager::CreateEndpoints(
     mGPUChild->OtherPid(),
     base::GetCurrentProcId(),
     &parentPipe,
     &childPipe);
   if (NS_FAILED(rv)) {
     DisableGPUProcess("Failed to create PCompositorManager endpoints");
     return;
   }
 
   mGPUChild->SendInitCompositorManager(Move(parentPipe));
-  CompositorManagerChild::Init(Move(childPipe), AllocateNamespace());
+  CompositorManagerChild::Init(Move(childPipe), AllocateNamespace(),
+                               mProcessToken);
 }
 
 void
 GPUProcessManager::EnsureImageBridgeChild()
 {
   if (ImageBridgeChild::GetSingleton()) {
     return;
   }
@@ -518,17 +516,17 @@ GPUProcessManager::NotifyListenersOnComp
   }
 }
 
 void
 GPUProcessManager::OnProcessUnexpectedShutdown(GPUProcessHost* aHost)
 {
   MOZ_ASSERT(mProcess && mProcess == aHost);
 
-  CompositorManagerChild::OnGPUProcessLost();
+  CompositorManagerChild::OnGPUProcessLost(aHost->GetProcessToken());
   DestroyProcess();
 
   if (mNumProcessAttempts > uint32_t(gfxPrefs::GPUProcessMaxRestarts())) {
     char disableMessage[64];
     SprintfLiteral(disableMessage, "GPU process disabled after %d attempts",
                    mNumProcessAttempts);
     DisableGPUProcess(disableMessage);
   } else if (mNumProcessAttempts > uint32_t(gfxPrefs::GPUProcessMaxRestartsWithDecoder()) &&
--- a/gfx/layers/ipc/CompositorManagerChild.cpp
+++ b/gfx/layers/ipc/CompositorManagerChild.cpp
@@ -19,47 +19,58 @@
 namespace mozilla {
 namespace layers {
 
 using gfx::GPUProcessManager;
 
 StaticRefPtr<CompositorManagerChild> CompositorManagerChild::sInstance;
 
 /* static */ bool
-CompositorManagerChild::IsInitialized(base::ProcessId aGPUPid)
+CompositorManagerChild::IsInitialized(uint64_t aProcessToken)
 {
   MOZ_ASSERT(NS_IsMainThread());
-  return sInstance && sInstance->CanSend() && sInstance->OtherPid() == aGPUPid;
+  return sInstance && sInstance->CanSend() &&
+         sInstance->mProcessToken == aProcessToken;
 }
 
-/* static */ bool
-CompositorManagerChild::InitSameProcess(uint32_t aNamespace)
+/* static */ void
+CompositorManagerChild::InitSameProcess(uint32_t aNamespace,
+                                        uint64_t aProcessToken)
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (NS_WARN_IF(IsInitialized(base::GetCurrentProcId()))) {
     MOZ_ASSERT_UNREACHABLE("Already initialized same process");
-    return false;
+    return;
   }
 
   RefPtr<CompositorManagerParent> parent =
     CompositorManagerParent::CreateSameProcess();
-  sInstance = new CompositorManagerChild(parent, aNamespace);
-  return true;
+  RefPtr<CompositorManagerChild> child =
+    new CompositorManagerChild(parent, aProcessToken, aNamespace);
+  if (NS_WARN_IF(!child->CanSend())) {
+    MOZ_DIAGNOSTIC_ASSERT(false, "Failed to open same process protocol");
+    return;
+  }
+
+  parent->BindComplete();
+  sInstance = child.forget();
 }
 
 /* static */ bool
 CompositorManagerChild::Init(Endpoint<PCompositorManagerChild>&& aEndpoint,
-                             uint32_t aNamespace)
+                             uint32_t aNamespace,
+                             uint64_t aProcessToken /* = 0 */)
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (sInstance) {
     MOZ_ASSERT(sInstance->mNamespace != aNamespace);
   }
 
-  sInstance = new CompositorManagerChild(Move(aEndpoint), aNamespace);
+  sInstance = new CompositorManagerChild(Move(aEndpoint), aProcessToken,
+                                         aNamespace);
   return sInstance->CanSend();
 }
 
 /* static */ void
 CompositorManagerChild::Shutdown()
 {
   MOZ_ASSERT(NS_IsMainThread());
   CompositorBridgeChild::ShutDown();
@@ -68,24 +79,24 @@ CompositorManagerChild::Shutdown()
     return;
   }
 
   sInstance->Close();
   sInstance = nullptr;
 }
 
 /* static */ void
-CompositorManagerChild::OnGPUProcessLost()
+CompositorManagerChild::OnGPUProcessLost(uint64_t aProcessToken)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   // Since GPUChild and CompositorManagerChild will race on ActorDestroy, we
   // cannot know if the CompositorManagerChild is about to be released but has
   // yet to be. As such, we want to pre-emptively set mCanSend to false.
-  if (sInstance) {
+  if (sInstance && sInstance->mProcessToken == aProcessToken) {
     sInstance->mCanSend = false;
   }
 }
 
 /* static */ bool
 CompositorManagerChild::CreateContentCompositorBridge(uint32_t aNamespace)
 {
   MOZ_ASSERT(NS_IsMainThread());
@@ -157,40 +168,44 @@ CompositorManagerChild::CreateSameProces
 
   RefPtr<CompositorBridgeChild> bridge =
     static_cast<CompositorBridgeChild*>(pbridge);
   bridge->InitForWidget(1, aLayerManager, aNamespace);
   return bridge.forget();
 }
 
 CompositorManagerChild::CompositorManagerChild(CompositorManagerParent* aParent,
+                                               uint64_t aProcessToken,
                                                uint32_t aNamespace)
-  : mCanSend(false)
+  : mProcessToken(aProcessToken)
   , mNamespace(aNamespace)
   , mResourceId(0)
+  , mCanSend(false)
 {
   MOZ_ASSERT(aParent);
 
   SetOtherProcessId(base::GetCurrentProcId());
   MessageLoop* loop = CompositorThreadHolder::Loop();
   ipc::MessageChannel* channel = aParent->GetIPCChannel();
   if (NS_WARN_IF(!Open(channel, loop, ipc::ChildSide))) {
     return;
   }
 
   mCanSend = true;
   AddRef();
   SetReplyTimeout();
 }
 
 CompositorManagerChild::CompositorManagerChild(Endpoint<PCompositorManagerChild>&& aEndpoint,
+                                               uint64_t aProcessToken,
                                                uint32_t aNamespace)
-  : mCanSend(false)
+  : mProcessToken(aProcessToken)
   , mNamespace(aNamespace)
   , mResourceId(0)
+  , mCanSend(false)
 {
   if (NS_WARN_IF(!aEndpoint.Bind(this))) {
     return;
   }
 
   mCanSend = true;
   AddRef();
   SetReplyTimeout();
--- a/gfx/layers/ipc/CompositorManagerChild.h
+++ b/gfx/layers/ipc/CompositorManagerChild.h
@@ -19,22 +19,23 @@ namespace layers {
 class CompositorManagerParent;
 class LayerManager;
 
 class CompositorManagerChild : public PCompositorManagerChild
 {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CompositorManagerChild)
 
 public:
-  static bool IsInitialized(base::ProcessId aPid);
-  static bool InitSameProcess(uint32_t aNamespace);
+  static bool IsInitialized(uint64_t aProcessToken);
+  static void InitSameProcess(uint32_t aNamespace, uint64_t aProcessToken);
   static bool Init(Endpoint<PCompositorManagerChild>&& aEndpoint,
-                   uint32_t aNamespace);
+                   uint32_t aNamespace,
+                   uint64_t aProcessToken = 0);
   static void Shutdown();
-  static void OnGPUProcessLost();
+  static void OnGPUProcessLost(uint64_t aProcessToken);
 
   static bool
   CreateContentCompositorBridge(uint32_t aNamespace);
 
   static already_AddRefed<CompositorBridgeChild>
   CreateWidgetCompositorBridge(uint64_t aProcessToken,
                                LayerManager* aLayerManager,
                                uint32_t aNamespace,
@@ -68,19 +69,21 @@ public:
   bool DeallocPCompositorBridgeChild(PCompositorBridgeChild* aActor) override;
 
   bool ShouldContinueFromReplyTimeout() override;
 
 private:
   static StaticRefPtr<CompositorManagerChild> sInstance;
 
   CompositorManagerChild(CompositorManagerParent* aParent,
+                         uint64_t aProcessToken,
                          uint32_t aNamespace);
 
   CompositorManagerChild(Endpoint<PCompositorManagerChild>&& aEndpoint,
+                         uint64_t aProcessToken,
                          uint32_t aNamespace);
 
   ~CompositorManagerChild() override
   {
   }
 
   bool CanSend() const
   {
@@ -90,17 +93,18 @@ private:
 
   void DeallocPCompositorManagerChild() override;
 
   already_AddRefed<nsIEventTarget>
   GetSpecificMessageEventTarget(const Message& aMsg) override;
 
   void SetReplyTimeout();
 
-  bool mCanSend;
+  uint64_t mProcessToken;
   uint32_t mNamespace;
   uint32_t mResourceId;
+  bool mCanSend;
 };
 
 } // namespace layers
 } // namespace mozilla
 
 #endif
--- a/gfx/layers/ipc/CompositorManagerParent.cpp
+++ b/gfx/layers/ipc/CompositorManagerParent.cpp
@@ -37,28 +37,16 @@ CompositorManagerParent::CreateSameProce
     return nullptr;
   }
 
   // The child is responsible for setting up the IPC channel in the same
   // process case because if we open from the child perspective, we can do it
   // on the main thread and complete before we return the manager handles.
   RefPtr<CompositorManagerParent> parent = new CompositorManagerParent();
   parent->SetOtherProcessId(base::GetCurrentProcId());
-
-  // CompositorManagerParent::Bind would normally add a reference for IPDL but
-  // we don't use that in the same process case.
-  parent.get()->AddRef();
-  sInstance = parent;
-
-#ifdef COMPOSITOR_MANAGER_PARENT_EXPLICIT_SHUTDOWN
-  if (!sActiveActors) {
-    sActiveActors = new nsTArray<CompositorManagerParent*>();
-  }
-  sActiveActors->AppendElement(parent);
-#endif
   return parent.forget();
 }
 
 /* static */ void
 CompositorManagerParent::Create(Endpoint<PCompositorManagerParent>&& aEndpoint)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
@@ -128,22 +116,32 @@ CompositorManagerParent::~CompositorMana
 void
 CompositorManagerParent::Bind(Endpoint<PCompositorManagerParent>&& aEndpoint)
 {
   MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread());
   if (NS_WARN_IF(!aEndpoint.Bind(this))) {
     return;
   }
 
+  BindComplete();
+}
+
+void
+CompositorManagerParent::BindComplete()
+{
   // Add the IPDL reference to ourself, so we can't get freed until IPDL is
   // done with us.
   AddRef();
 
+  StaticMutexAutoLock lock(sMutex);
+  if (OtherPid() == base::GetCurrentProcId()) {
+    sInstance = this;
+  }
+
 #ifdef COMPOSITOR_MANAGER_PARENT_EXPLICIT_SHUTDOWN
-  StaticMutexAutoLock lock(sMutex);
   if (!sActiveActors) {
     sActiveActors = new nsTArray<CompositorManagerParent*>();
   }
   sActiveActors->AppendElement(this);
 #endif
 }
 
 void
--- a/gfx/layers/ipc/CompositorManagerParent.h
+++ b/gfx/layers/ipc/CompositorManagerParent.h
@@ -34,16 +34,17 @@ public:
   static void Shutdown();
 
   static already_AddRefed<CompositorBridgeParent>
   CreateSameProcessWidgetCompositorBridge(CSSToLayoutDeviceScale aScale,
                                           const CompositorOptions& aOptions,
                                           bool aUseExternalSurfaceSize,
                                           const gfx::IntSize& aSurfaceSize);
 
+  void BindComplete();
   void ActorDestroy(ActorDestroyReason aReason) override;
 
   bool DeallocPCompositorBridgeParent(PCompositorBridgeParent* aActor) override;
   PCompositorBridgeParent* AllocPCompositorBridgeParent(const CompositorBridgeOptions& aOpt) override;
 
 private:
   static StaticRefPtr<CompositorManagerParent> sInstance;
   static StaticMutex sMutex;