Bug 1262898: Keep the GeckoChildProcessHost alive for the lifetime of the CompositorBridge and ImageBridge parent actors. r=jimm r=nical

MozReview-Commit-ID: 1rsWqRpbhgN

dom/ipc/ContentParent.cpp

@@ -1999,14 +1999,6 @@ ContentParent::RecvDeallocateLayerTreeId(const uint64_t& aId)
 
 namespace {
 
-void
-DelayedDeleteSubprocess(GeckoChildProcessHost* aSubprocess)
-{
-  XRE_GetIOMessageLoop()
-    ->PostTask(FROM_HERE,
-           new DeleteTask<GeckoChildProcessHost>(aSubprocess));
-}
-
 // This runnable only exists to delegate ownership of the
 // ContentParent to this runnable, until it's deleted by the event
 // system.
@@ -2138,10 +2130,10 @@ ContentParent::ActorDestroy(ActorDestroyReason why)
   }
   mIdleListeners.Clear();
 
-  MessageLoop::current()->
-    PostTask(FROM_HERE,
-             NewRunnableFunction(DelayedDeleteSubprocess, mSubprocess));
-  mSubprocess = nullptr;
+  if (mSubprocess) {
+    mSubprocess->DissociateActor();
+    mSubprocess = nullptr;
+  }
 
   // IPDL rules require actors to live on past ActorDestroy, but it
   // may be that the kungFuDeathGrip above is the last reference to
@@ -3353,7 +3345,7 @@ PCompositorBridgeParent*
 ContentParent::AllocPCompositorBridgeParent(mozilla::ipc::Transport* aTransport,
                                             base::ProcessId aOtherProcess)
 {
-  return CompositorBridgeParent::Create(aTransport, aOtherProcess);
+  return CompositorBridgeParent::Create(aTransport, aOtherProcess, mSubprocess);
 }
 
 gfx::PVRManagerParent*
@@ -3367,7 +3359,7 @@ PImageBridgeParent*
 ContentParent::AllocPImageBridgeParent(mozilla::ipc::Transport* aTransport,
                                        base::ProcessId aOtherProcess)
 {
-  return ImageBridgeParent::Create(aTransport, aOtherProcess);
+  return ImageBridgeParent::Create(aTransport, aOtherProcess, mSubprocess);
 }
 
 PBackgroundParent*

gfx/layers/ipc/CompositorBridgeParent.cpp

@@ -67,6 +67,7 @@
 #endif
 #include "GeckoProfiler.h"
 #include "mozilla/ipc/ProtocolTypes.h"
+#include "mozilla/ipc/GeckoChildProcessHost.h"
 #include "mozilla/unused.h"
 #include "mozilla/Hal.h"
 #include "mozilla/HalTypes.h"
@@ -1907,6 +1908,7 @@ class CrossProcessCompositorBridgeParent final : public PCompositorBridgeParent,
 public:
   explicit CrossProcessCompositorBridgeParent(Transport* aTransport)
     : mTransport(aTransport)
+    , mSubprocess(nullptr)
     , mNotifyAfterRemotePaint(false)
     , mDestroyCalled(false)
   {
@@ -2046,6 +2048,7 @@ private:
   // ourself.  This is released (deferred) in ActorDestroy().
   RefPtr<CrossProcessCompositorBridgeParent> mSelfRef;
   Transport* mTransport;
+  ipc::GeckoChildProcessHost* mSubprocess;
 
   RefPtr<CompositorThreadHolder> mCompositorThreadHolder;
   // If true, we should send a RemotePaintIsReady message when the layer transaction
@@ -2204,13 +2207,18 @@ OpenCompositor(CrossProcessCompositorBridgeParent* aCompositor,
 }
 
 /*static*/ PCompositorBridgeParent*
-CompositorBridgeParent::Create(Transport* aTransport, ProcessId aOtherPid)
+CompositorBridgeParent::Create(Transport* aTransport, ProcessId aOtherPid, GeckoChildProcessHost* aProcessHost)
 {
   gfxPlatform::InitLayersIPC();
 
   RefPtr<CrossProcessCompositorBridgeParent> cpcp =
     new CrossProcessCompositorBridgeParent(aTransport);
 
+  if (aProcessHost) {
+    cpcp->mSubprocess = aProcessHost;
+    aProcessHost->AssociateActor();
+  }
+
   cpcp->mSelfRef = cpcp;
   CompositorLoop()->PostTask(
     FROM_HERE,
@@ -2268,6 +2276,12 @@ CrossProcessCompositorBridgeParent::ActorDestroy(ActorDestroyReason aWhy)
 {
   RefPtr<CompositorLRU> lru = CompositorLRU::GetSingleton();
   lru->Remove(this);
+
+  if (mSubprocess) {
+    mSubprocess->DissociateActor();
+    mSubprocess = nullptr;
+  }
+
   // We must keep this object alive untill the code handling message
   // reception is finished on this thread.
   MessageLoop::current()->PostTask(FROM_HERE,
@@ -2743,7 +2757,7 @@ CrossProcessCompositorBridgeParent::CloneToplevel(
       Transport* transport = OpenDescriptor(aFds[i].fd(),
                                             Transport::MODE_SERVER);
       PCompositorBridgeParent* compositor =
-        CompositorBridgeParent::Create(transport, base::GetProcId(aPeerProcess));
+        CompositorBridgeParent::Create(transport, base::GetProcId(aPeerProcess), mSubprocess);
       compositor->CloneManagees(this, aCtx);
       compositor->IToplevelProtocol::SetTransport(transport);
       // The reference to the compositor thread is held in OnChannelConnected().

gfx/layers/ipc/CompositorBridgeParent.h

@@ -48,6 +48,10 @@ namespace gfx {
 class DrawTarget;
 } // namespace gfx
 
+namespace ipc {
+class GeckoChildProcessHost;
+} // namespace ipc
+
 namespace layers {
 
 class APZCTreeManager;
@@ -416,7 +420,7 @@ public:
    * directly to us.  Transport is to its thread context.
    */
   static PCompositorBridgeParent*
-  Create(Transport* aTransport, ProcessId aOtherProcess);
+  Create(Transport* aTransport, ProcessId aOtherProcess, mozilla::ipc::GeckoChildProcessHost* aProcessHost);
 
   struct LayerTreeState {
     LayerTreeState();

gfx/layers/ipc/ImageBridgeParent.cpp

@@ -17,6 +17,7 @@
 #include "mozilla/ipc/MessageChannel.h" // for MessageChannel, etc
 #include "mozilla/ipc/ProtocolUtils.h"
 #include "mozilla/ipc/Transport.h"      // for Transport
+#include "mozilla/ipc/GeckoChildProcessHost.h"
 #include "mozilla/media/MediaSystemResourceManagerParent.h" // for MediaSystemResourceManagerParent
 #include "mozilla/layers/CompositableTransactionParent.h"
 #include "mozilla/layers/CompositorBridgeParent.h"  // for CompositorBridgeParent
@@ -59,6 +60,7 @@ ImageBridgeParent::ImageBridgeParent(MessageLoop* aLoop,
   , mTransport(aTransport)
   , mSetChildThreadPriority(false)
   , mClosed(false)
+  , mSubprocess(nullptr)
 {
   MOZ_ASSERT(NS_IsMainThread());
   sMainLoop = MessageLoop::current();
@@ -100,6 +102,11 @@ ImageBridgeParent::ActorDestroy(ActorDestroyReason aWhy)
   // Can't alloc/dealloc shmems from now on.
   mClosed = true;
 
+  if (mSubprocess) {
+    mSubprocess->DissociateActor();
+    mSubprocess = nullptr;
+  }
+
   MessageLoop::current()->PostTask(
     FROM_HERE,
     NewRunnableMethod(this, &ImageBridgeParent::DeferredDestroy));
@@ -194,10 +201,16 @@ ConnectImageBridgeInParentProcess(ImageBridgeParent* aBridge,
 }
 
 /*static*/ PImageBridgeParent*
-ImageBridgeParent::Create(Transport* aTransport, ProcessId aChildProcessId)
+ImageBridgeParent::Create(Transport* aTransport, ProcessId aChildProcessId, GeckoChildProcessHost* aProcessHost)
 {
   MessageLoop* loop = CompositorBridgeParent::CompositorLoop();
   RefPtr<ImageBridgeParent> bridge = new ImageBridgeParent(loop, aTransport, aChildProcessId);
+
+  if (aProcessHost) {
+    bridge->mSubprocess = aProcessHost;
+    aProcessHost->AssociateActor();
+  }
+
   loop->PostTask(FROM_HERE,
                  NewRunnableFunction(ConnectImageBridgeInParentProcess,
                                      bridge.get(), aTransport, aChildProcessId));
@@ -359,7 +372,7 @@ ImageBridgeParent::CloneToplevel(const InfallibleTArray<ProtocolFdMapping>& aFds
     if (aFds[i].protocolId() == unsigned(GetProtocolId())) {
       Transport* transport = OpenDescriptor(aFds[i].fd(),
                                             Transport::MODE_SERVER);
-      PImageBridgeParent* bridge = Create(transport, base::GetProcId(aPeerProcess));
+      PImageBridgeParent* bridge = Create(transport, base::GetProcId(aPeerProcess), mSubprocess);
       bridge->CloneManagees(this, aCtx);
       bridge->IToplevelProtocol::SetTransport(transport);
       // The reference to the compositor thread is held in OnChannelConnected().

gfx/layers/ipc/ImageBridgeParent.h

@@ -29,6 +29,7 @@ class Thread;
 namespace mozilla {
 namespace ipc {
 class Shmem;
+class GeckoChildProcessHost;
 } // namespace ipc
 
 namespace layers {
@@ -56,7 +57,7 @@ public:
   virtual void ActorDestroy(ActorDestroyReason aWhy) override;
 
   static PImageBridgeParent*
-  Create(Transport* aTransport, ProcessId aChildProcessId);
+  Create(Transport* aTransport, ProcessId aChildProcessId, ipc::GeckoChildProcessHost* aProcessHost);
 
   // CompositableParentManager
   virtual void SendFenceHandleIfPresent(PTextureParent* aTexture) override;
@@ -156,6 +157,8 @@ private:
   bool mSetChildThreadPriority;
   bool mClosed;
 
+  ipc::GeckoChildProcessHost* mSubprocess;
+
   /**
    * Map of all living ImageBridgeParent instances
    */

gfx/thebes/gfxPlatform.cpp

@@ -879,8 +879,8 @@ gfxPlatform::ShutdownLayersIPC()
         gfx::VRManagerChild::ShutDown();
         // cf bug 1215265.
         if (gfxPrefs::ChildProcessShutdown()) {
-          layers::ImageBridgeChild::ShutDown();
           layers::CompositorBridgeChild::ShutDown();
+          layers::ImageBridgeChild::ShutDown();
         }
 
     } else if (XRE_IsParentProcess()) {

ipc/glue/GeckoChildProcessHost.cpp

@@ -112,6 +112,7 @@ GeckoChildProcessHost::GeckoChildProcessHost(GeckoProcessType aProcessType,
 #if defined(MOZ_WIDGET_COCOA)
   , mChildTask(MACH_PORT_NULL)
 #endif
+  , mAssociatedActors(1)
 {
     MOZ_COUNT_CTOR(GeckoChildProcessHost);
 }
@@ -473,6 +474,28 @@ GeckoChildProcessHost::SetAlreadyDead()
   mChildProcessHandle = 0;
 }
 
+namespace {
+
+void
+DelayedDeleteSubprocess(GeckoChildProcessHost* aSubprocess)
+{
+  XRE_GetIOMessageLoop()
+    ->PostTask(FROM_HERE,
+       new DeleteTask<GeckoChildProcessHost>(aSubprocess));
+}
+
+}
+
+void
+GeckoChildProcessHost::DissociateActor()
+{
+  if (!--mAssociatedActors) {
+    MessageLoop::current()->
+      PostTask(FROM_HERE,
+        NewRunnableFunction(DelayedDeleteSubprocess, this));
+  }
+}
+
 int32_t GeckoChildProcessHost::mChildCounter = 0;
 
 void

ipc/glue/GeckoChildProcessHost.h

@@ -10,6 +10,7 @@
 #include "base/waitable_event.h"
 #include "chrome/common/child_process_host.h"
 
+#include "mozilla/Atomics.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/ipc/FileDescriptor.h"
 #include "mozilla/Monitor.h"
@@ -125,6 +126,14 @@ public:
   // For bug 943174: Skip the EnsureProcessTerminated call in the destructor.
   void SetAlreadyDead();
 
+  // This associates an actor telling the process host to stay alive at least
+  // until DissociateActor has been called.
+  void AssociateActor() { mAssociatedActors++; }
+
+  // This gets called when actors get destroyed and will schedule the object
+  // for deletion when all actors have cleared their associations.
+  void DissociateActor();
+
 protected:
   GeckoProcessType mProcessType;
   ChildPrivileges mPrivileges;
@@ -202,6 +211,10 @@ private:
   // FIXME/cjones: this strongly indicates bad design.  Shame on us.
   std::queue<IPC::Message> mQueue;
 
+  // This tracks how many actors are associated with this process that require
+  // it to stay alive and have not yet been destroyed.
+  Atomic<int32_t> mAssociatedActors;
+
   static uint32_t sNextUniqueID;
 };
 

modules/libpref/init/all.js

@@ -4505,8 +4505,6 @@ pref("layers.prefer-d3d9", false);
 pref("layers.d3d11.force-warp", false);
 pref("layers.d3d11.disable-warp", true);
 
-// cf. Bug 1215265
-pref("layers.child-process-shutdown", false);
 #endif
 
 // Force all possible layers to be always active layers