Bug 1389759 - Ensure we tear down CompositorManagerChild correctly if init or the GPU process fail. r=dvander

gfx/ipc/GPUProcessManager.cpp

@@ -225,16 +225,13 @@ GPUProcessManager::EnsureProtocolsReady()
 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;
   }
 
@@ -251,7 +248,8 @@ GPUProcessManager::EnsureCompositorManagerChild()
   }
 
   mGPUChild->SendInitCompositorManager(Move(parentPipe));
-  CompositorManagerChild::Init(Move(childPipe), AllocateNamespace());
+  CompositorManagerChild::Init(Move(childPipe), AllocateNamespace(),
+                               mProcessToken);
 }
 
 void
@@ -523,7 +521,7 @@ GPUProcessManager::OnProcessUnexpectedShutdown(GPUProcessHost* aHost)
 {
   MOZ_ASSERT(mProcess && mProcess == aHost);
 
-  CompositorManagerChild::OnGPUProcessLost();
+  CompositorManagerChild::OnGPUProcessLost(aHost->GetProcessToken());
   DestroyProcess();
 
   if (mNumProcessAttempts > uint32_t(gfxPrefs::GPUProcessMaxRestarts())) {

gfx/layers/ipc/CompositorManagerChild.cpp

@@ -24,37 +24,48 @@ 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();
 }
 
@@ -73,14 +84,14 @@ CompositorManagerChild::Shutdown()
 }
 
 /* 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;
   }
 }
@@ -162,10 +173,12 @@ CompositorManagerChild::CreateSameProcessWidgetCompositorBridge(LayerManager* aL
 }
 
 CompositorManagerChild::CompositorManagerChild(CompositorManagerParent* aParent,
+                                               uint64_t aProcessToken,
                                                uint32_t aNamespace)
-  : mCanSend(false)
+  : mProcessToken(aProcessToken)
   , mNamespace(aNamespace)
   , mResourceId(0)
+  , mCanSend(false)
 {
   MOZ_ASSERT(aParent);
 
@@ -182,10 +195,12 @@ CompositorManagerChild::CompositorManagerChild(CompositorManagerParent* aParent,
 }
 
 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;

gfx/layers/ipc/CompositorManagerChild.h

@@ -24,12 +24,13 @@ 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);
@@ -73,9 +74,11 @@ 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
@@ -95,9 +98,10 @@ private:
 
   void SetReplyTimeout();
 
-  bool mCanSend;
+  uint64_t mProcessToken;
   uint32_t mNamespace;
   uint32_t mResourceId;
+  bool mCanSend;
 };
 
 } // namespace layers

gfx/layers/ipc/CompositorManagerParent.cpp

@@ -42,18 +42,6 @@ CompositorManagerParent::CreateSameProcess()
   // 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();
 }
 
@@ -133,12 +121,22 @@ CompositorManagerParent::Bind(Endpoint<PCompositorManagerParent>&& aEndpoint)
     return;
   }
 
+  BindComplete();
+}
+
+void
+CompositorManagerParent::BindComplete()
+{
   // Add the IPDL reference to ourself, so we can't get freed until IPDL is
   // done with us.
   AddRef();
 
-#ifdef COMPOSITOR_MANAGER_PARENT_EXPLICIT_SHUTDOWN
   StaticMutexAutoLock lock(sMutex);
+  if (OtherPid() == base::GetCurrentProcId()) {
+    sInstance = this;
+  }
+
+#ifdef COMPOSITOR_MANAGER_PARENT_EXPLICIT_SHUTDOWN
   if (!sActiveActors) {
     sActiveActors = new nsTArray<CompositorManagerParent*>();
   }

gfx/layers/ipc/CompositorManagerParent.h

@@ -39,6 +39,7 @@ public:
                                           bool aUseExternalSurfaceSize,
                                           const gfx::IntSize& aSurfaceSize);
 
+  void BindComplete();
   void ActorDestroy(ActorDestroyReason aReason) override;
 
   bool DeallocPCompositorBridgeParent(PCompositorBridgeParent* aActor) override;