Bug 1402592 - Ensure that ImageBridgeParent instances are closed by the parent during shutdown. r=dvander

We currently allow the content process to shutdown the IPDL objects on
behalf the parent, and we wait for all of these instances to be freed
before we complete shutdown. This is undesirable because it requires the
parent to trust the child rather than the other way around; the child
can hold shutdown hostage by simply not releasing its instances. The
child should already support the parent closing its graphics IPDL
objects because the GPU process itself can die abruptly and be restored
at a later time.

gfx/layers/ipc/CompositorThread.cpp

@@ -22,9 +22,6 @@ namespace layers {
 static StaticRefPtr<CompositorThreadHolder> sCompositorThreadHolder;
 static bool sFinishedCompositorShutDown = false;
 
-// See ImageBridgeChild.cpp
-void ReleaseImageBridgeParentSingleton();
-
 CompositorThreadHolder* GetCompositorThreadHolder()
 {
   return sCompositorThreadHolder;
@@ -126,7 +123,7 @@ CompositorThreadHolder::Shutdown()
   MOZ_ASSERT(NS_IsMainThread(), "Should be on the main Thread!");
   MOZ_ASSERT(sCompositorThreadHolder, "The compositor thread has already been shut down!");
 
-  ReleaseImageBridgeParentSingleton();
+  ImageBridgeParent::Shutdown();
   gfx::ReleaseVRManagerParentSingleton();
   MediaSystemResourceService::Shutdown();
   CompositorManagerParent::Shutdown();

gfx/layers/ipc/ImageBridgeChild.cpp

@@ -240,13 +240,16 @@ ImageBridgeChild::ShutdownStep2(SynchronousTask* aTask)
 
   MOZ_ASSERT(InImageBridgeChildThread(),
              "Should be in ImageBridgeChild thread.");
-  Close();
+  if (!mDestroyed) {
+    Close();
+  }
 }
 
 void
 ImageBridgeChild::ActorDestroy(ActorDestroyReason aWhy)
 {
   mCanSend = false;
+  mDestroyed = true;
   {
     MutexAutoLock lock(mContainerMapLock);
     mImageContainerListeners.Clear();
@@ -650,8 +653,6 @@ ImageBridgeChild::WillShutdown()
 
     task.Wait();
   }
-
-  mDestroyed = true;
 }
 
 void

gfx/layers/ipc/ImageBridgeParent.cpp

@@ -46,6 +46,8 @@ std::map<base::ProcessId, ImageBridgeParent*> ImageBridgeParent::sImageBridges;
 
 StaticAutoPtr<mozilla::Monitor> sImageBridgesLock;
 
+static StaticRefPtr<ImageBridgeParent> sImageBridgeParentSingleton;
+
 // defined in CompositorBridgeParent.cpp
 CompositorThreadHolder* GetCompositorThreadHolder();
 
@@ -81,12 +83,6 @@ ImageBridgeParent::~ImageBridgeParent()
 {
 }
 
-static StaticRefPtr<ImageBridgeParent> sImageBridgeParentSingleton;
-
-void ReleaseImageBridgeParentSingleton() {
-  sImageBridgeParentSingleton = nullptr;
-}
-
 /* static */ ImageBridgeParent*
 ImageBridgeParent::CreateSameProcess()
 {
@@ -116,6 +112,36 @@ ImageBridgeParent::CreateForGPUProcess(Endpoint<PImageBridgeParent>&& aEndpoint)
   return true;
 }
 
+/* static */ void
+ImageBridgeParent::ShutdownInternal()
+{
+  // We make a copy because we don't want to hold the lock while closing and we
+  // don't want the object to get freed underneath us.
+  nsTArray<RefPtr<ImageBridgeParent>> actors;
+  {
+    MonitorAutoLock lock(*sImageBridgesLock);
+    for (const auto& iter : sImageBridges) {
+      actors.AppendElement(iter.second);
+    }
+  }
+
+  for (auto const& actor : actors) {
+    MOZ_RELEASE_ASSERT(!actor->mClosed);
+    actor->Close();
+  }
+
+  sImageBridgeParentSingleton = nullptr;
+}
+
+/* static */ void
+ImageBridgeParent::Shutdown()
+{
+  CompositorThreadHolder::Loop()->PostTask(
+    NS_NewRunnableFunction("ImageBridgeParent::Shutdown", []() -> void {
+      ImageBridgeParent::ShutdownInternal();
+  }));
+}
+
 void
 ImageBridgeParent::ActorDestroy(ActorDestroyReason aWhy)
 {

gfx/layers/ipc/ImageBridgeParent.h

@@ -58,6 +58,7 @@ public:
   static ImageBridgeParent* CreateSameProcess();
   static bool CreateForGPUProcess(Endpoint<PImageBridgeParent>&& aEndpoint);
   static bool CreateForContent(Endpoint<PImageBridgeParent>&& aEndpoint);
+  static void Shutdown();
 
   virtual ShmemAllocator* AsShmemAllocator() override { return this; }
 
@@ -125,6 +126,8 @@ protected:
   void Bind(Endpoint<PImageBridgeParent>&& aEndpoint);
 
 private:
+  static void ShutdownInternal();
+
   void DeferredDestroy();
   MessageLoop* mMessageLoop;
   // This keeps us alive until ActorDestroy(), at which point we do a