Bug 1215265. Don't leak CompositorChild in content processes. r=nical

This makes the shutdown sequence of CrossProcessCompositorParent
work very much like the shutdown sequence of CompositorParent:
the child does SendStop(); the parent's RecvStop does some cleanup
and then queues a DeferredDestroy task; DeferredDestroy releases
the parent, destroying it; IPDL sends a message to destroy the child.

gfx/layers/ipc/CompositorChild.cpp

@@ -49,14 +49,9 @@ CompositorChild::~CompositorChild()
   if (mCanSend) {
     gfxCriticalError() << "CompositorChild was not deinitialized";
   }
-}
-
-static void DeferredDestroyCompositor(RefPtr<CompositorParent> aCompositorParent,
-                                      RefPtr<CompositorChild> aCompositorChild)
-{
-    // Bug 848949 needs to be fixed before
-    // we can close the channel properly
-    //aCompositorChild->Close();
+  if (sCompositor == this) {
+    sCompositor = nullptr;
+  }
 }
 
 void
@@ -101,11 +96,17 @@ CompositorChild::Destroy()
   }
 
   SendStop();
+}
 
-  // The DeferredDestroyCompositor task takes ownership of compositorParent and
-  // will release them when it runs.
-  MessageLoop::current()->PostTask(FROM_HERE,
-             NewRunnableFunction(DeferredDestroyCompositor, mCompositorParent, selfRef));
+/*static*/ void
+CompositorChild::ShutdownLayersIPC()
+{
+  if (sCompositor) {
+    sCompositor->Destroy();
+    do {
+      NS_ProcessNextEvent(nullptr, true);
+    } while (sCompositor);
+  }
 }
 
 bool

gfx/layers/ipc/CompositorChild.h

@@ -123,6 +123,8 @@ public:
   bool SendNotifyRegionInvalidated(const nsIntRegion& region);
   bool SendRequestNotifyAfterRemotePaint();
 
+  static void ShutdownLayersIPC();
+
 private:
   // Private destructor, to discourage deletion outside of Release():
   virtual ~CompositorChild();

gfx/layers/ipc/CompositorParent.cpp

@@ -1750,7 +1750,7 @@ public:
   // FIXME/bug 774388: work out what shutdown protocol we need.
   virtual bool RecvRequestOverfill() override { return true; }
   virtual bool RecvWillStop() override { return true; }
-  virtual bool RecvStop() override { return true; }
+  virtual bool RecvStop() override;
   virtual bool RecvPause() override { return true; }
   virtual bool RecvResume() override { return true; }
   virtual bool RecvNotifyHidden(const uint64_t& id) override;
@@ -1830,6 +1830,7 @@ private:
   // Private destructor, to discourage deletion outside of Release():
   virtual ~CrossProcessCompositorParent();
 
+  void Destroy();
   void DeferredDestroy();
 
   // There can be many CPCPs, and IPDL-generated code doesn't hold a
@@ -1941,14 +1942,27 @@ CrossProcessCompositorParent::RecvRequestNotifyAfterRemotePaint()
 }
 
 void
-CrossProcessCompositorParent::ActorDestroy(ActorDestroyReason aWhy)
+CrossProcessCompositorParent::Destroy()
 {
   RefPtr<CompositorLRU> lru = CompositorLRU::GetSingleton();
   lru->Remove(this);
+}
 
-  MessageLoop::current()->PostTask(
-    FROM_HERE,
-    NewRunnableMethod(this, &CrossProcessCompositorParent::DeferredDestroy));
+void
+CrossProcessCompositorParent::ActorDestroy(ActorDestroyReason aWhy)
+{
+  Destroy();
+}
+
+bool
+CrossProcessCompositorParent::RecvStop()
+{
+  Destroy();
+  // We must keep this object alive untill the code handling message
+  // reception is finished on this thread.
+  MessageLoop::current()->PostTask(FROM_HERE,
+      NewRunnableMethod(this, &CrossProcessCompositorParent::DeferredDestroy));
+  return true;
 }
 
 PLayerTransactionParent*

gfx/thebes/gfxPlatform.cpp

@@ -745,8 +745,7 @@ gfxPlatform::ShutdownLayersIPC()
     }
     sLayersIPCIsUp = false;
 
-    if (XRE_IsParentProcess())
-    {
+    if (XRE_IsParentProcess()) {
         // This must happen after the shutdown of media and widgets, which
         // are triggered by the NS_XPCOM_SHUTDOWN_OBSERVER_ID notification.
         layers::ImageBridgeChild::ShutDown();
@@ -755,6 +754,8 @@ gfxPlatform::ShutdownLayersIPC()
 #endif
 
         layers::CompositorParent::ShutDown();
+    } else {
+        CompositorChild::ShutdownLayersIPC();
     }
 }
 

widget/nsBaseWidget.cpp

@@ -251,12 +251,14 @@ void nsBaseWidget::DestroyCompositor()
     RefPtr<CompositorChild> compositorChild = mCompositorChild;
     RefPtr<CompositorParent> compositorParent = mCompositorParent;
     mCompositorChild->Destroy();
+    mCompositorChild = nullptr;
   }
 
   // Can have base widgets that are things like tooltips
   // which don't have CompositorVsyncDispatchers
   if (mCompositorVsyncDispatcher) {
     mCompositorVsyncDispatcher->Shutdown();
+    mCompositorVsyncDispatcher = nullptr;
   }
 }