Bug 1389021 - Force CompositorManagerParent to close before shutting down the compositor thread. r=dvander,me

gfx/layers/ipc/CompositorBridgeChild.cpp

@@ -139,16 +139,6 @@ CompositorBridgeChild::Destroy()
   // happens.
   RefPtr<CompositorBridgeChild> selfRef = this;
 
-  if (!mCanSend) {
-    // We may have already called destroy but still have lingering references
-    // or CompositorBridgeChild::ActorDestroy was called. Ensure that we do our
-    // post destroy clean up no matter what. It is safe to call multiple times.
-    MessageLoop::current()->PostTask(NewRunnableMethod(
-      "CompositorBridgeChild::AfterDestroy",
-      selfRef, &CompositorBridgeChild::AfterDestroy));
-    return;
-  }
-
   for (size_t i = 0; i < mTexturePools.Length(); i++) {
     mTexturePools[i]->Destroy();
   }
@@ -163,6 +153,16 @@ CompositorBridgeChild::Destroy()
     mLayerManager = nullptr;
   }
 
+  if (!mCanSend) {
+    // We may have already called destroy but still have lingering references
+    // or CompositorBridgeChild::ActorDestroy was called. Ensure that we do our
+    // post destroy clean up no matter what. It is safe to call multiple times.
+    MessageLoop::current()->PostTask(NewRunnableMethod(
+      "CompositorBridgeChild::AfterDestroy",
+      selfRef, &CompositorBridgeChild::AfterDestroy));
+    return;
+  }
+
   AutoTArray<PLayerTransactionChild*, 16> transactions;
   ManagedPLayerTransactionChild(transactions);
   for (int i = transactions.Length() - 1; i >= 0; --i) {

gfx/layers/ipc/CompositorManagerParent.cpp

@@ -9,6 +9,7 @@
 #include "mozilla/layers/CompositorBridgeParent.h"
 #include "mozilla/layers/CrossProcessCompositorBridgeParent.h"
 #include "mozilla/layers/CompositorThread.h"
+#include "nsAutoPtr.h"
 #include "VsyncSource.h"
 
 namespace mozilla {
@@ -17,6 +18,10 @@ namespace layers {
 StaticRefPtr<CompositorManagerParent> CompositorManagerParent::sInstance;
 StaticMutex CompositorManagerParent::sMutex;
 
+#ifdef COMPOSITOR_MANAGER_PARENT_EXPLICIT_SHUTDOWN
+StaticAutoPtr<nsTArray<CompositorManagerParent*>> CompositorManagerParent::sActiveActors;
+#endif
+
 /* static */ already_AddRefed<CompositorManagerParent>
 CompositorManagerParent::CreateSameProcess()
 {
@@ -42,6 +47,13 @@ CompositorManagerParent::CreateSameProcess()
   // 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();
 }
 
@@ -124,6 +136,14 @@ CompositorManagerParent::Bind(Endpoint<PCompositorManagerParent>&& aEndpoint)
   // 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 (!sActiveActors) {
+    sActiveActors = new nsTArray<CompositorManagerParent*>();
+  }
+  sActiveActors->AppendElement(this);
+#endif
 }
 
 void
@@ -143,6 +163,12 @@ CompositorManagerParent::DeallocPCompositorManagerParent()
                             this,
                             &CompositorManagerParent::DeferredDestroy));
 
+#ifdef COMPOSITOR_MANAGER_PARENT_EXPLICIT_SHUTDOWN
+  StaticMutexAutoLock lock(sMutex);
+  if (sActiveActors) {
+    sActiveActors->RemoveElement(this);
+  }
+#endif
   Release();
 }
 
@@ -152,6 +178,40 @@ CompositorManagerParent::DeferredDestroy()
   mCompositorThreadHolder = nullptr;
 }
 
+#ifdef COMPOSITOR_MANAGER_PARENT_EXPLICIT_SHUTDOWN
+/* static */ void
+CompositorManagerParent::ShutdownInternal()
+{
+  nsAutoPtr<nsTArray<CompositorManagerParent*>> actors;
+
+  // We move here because we may attempt to acquire the same lock during the
+  // destroy to remove the reference in sActiveActors.
+  {
+    StaticMutexAutoLock lock(sMutex);
+    actors = sActiveActors.forget();
+  }
+
+  if (actors) {
+    for (auto& actor : *actors) {
+      actor->Close();
+    }
+  }
+}
+#endif // COMPOSITOR_MANAGER_PARENT_EXPLICIT_SHUTDOWN
+
+/* static */ void
+CompositorManagerParent::Shutdown()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+#ifdef COMPOSITOR_MANAGER_PARENT_EXPLICIT_SHUTDOWN
+  CompositorThreadHolder::Loop()->PostTask(
+      NS_NewRunnableFunction("layers::CompositorManagerParent::Shutdown", []() -> void {
+        CompositorManagerParent::ShutdownInternal();
+      }));
+#endif
+}
+
 PCompositorBridgeParent*
 CompositorManagerParent::AllocPCompositorBridgeParent(const CompositorBridgeOptions& aOpt)
 {

gfx/layers/ipc/CompositorManagerParent.h

@@ -20,6 +20,10 @@ namespace layers {
 class CompositorBridgeParent;
 class CompositorThreadHolder;
 
+#ifndef DEBUG
+#define COMPOSITOR_MANAGER_PARENT_EXPLICIT_SHUTDOWN
+#endif
+
 class CompositorManagerParent final : public PCompositorManagerParent
 {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CompositorManagerParent)
@@ -27,6 +31,7 @@ class CompositorManagerParent final : public PCompositorManagerParent
 public:
   static already_AddRefed<CompositorManagerParent> CreateSameProcess();
   static void Create(Endpoint<PCompositorManagerParent>&& aEndpoint);
+  static void Shutdown();
 
   static already_AddRefed<CompositorBridgeParent>
   CreateSameProcessWidgetCompositorBridge(CSSToLayoutDeviceScale aScale,
@@ -43,6 +48,11 @@ private:
   static StaticRefPtr<CompositorManagerParent> sInstance;
   static StaticMutex sMutex;
 
+#ifdef COMPOSITOR_MANAGER_PARENT_EXPLICIT_SHUTDOWN
+  static StaticAutoPtr<nsTArray<CompositorManagerParent*>> sActiveActors;
+  static void ShutdownInternal();
+#endif
+
   CompositorManagerParent();
   ~CompositorManagerParent() override;
 

gfx/layers/ipc/CompositorThread.cpp

@@ -129,6 +129,7 @@ CompositorThreadHolder::Shutdown()
   ReleaseImageBridgeParentSingleton();
   gfx::ReleaseVRManagerParentSingleton();
   MediaSystemResourceService::Shutdown();
+  CompositorManagerParent::Shutdown();
 
   sCompositorThreadHolder = nullptr;