Bug 774388 - Patch 5: Wait for [CrossProcess]CompositorParent's to be gone before we tear down the compositor thread - r=mattwoodrow
☠☠ backed out by 1fd5a864e81d ☠ ☠
authorBenoit Jacob <bjacob@mozilla.com>
Thu, 03 Jul 2014 14:53:27 -0400
changeset 192176 2532e22d613508a3af62cb31078281e3d77aec2e
parent 192175 719844108f1a3cf1b6de996162c499f04219fa93
child 192177 99581dfb5ec491512f92953fbbcbd6789cf594e7
push id45776
push userbjacob@mozilla.com
push dateThu, 03 Jul 2014 18:57:18 +0000
treeherdermozilla-inbound@a54b05c9e4a1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow
bugs774388
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 774388 - Patch 5: Wait for [CrossProcess]CompositorParent's to be gone before we tear down the compositor thread - r=mattwoodrow
gfx/layers/ipc/CompositorParent.cpp
gfx/layers/ipc/CompositorParent.h
--- a/gfx/layers/ipc/CompositorParent.cpp
+++ b/gfx/layers/ipc/CompositorParent.cpp
@@ -48,16 +48,17 @@
 #include "mozilla/layers/CompositorD3D11.h"
 #include "mozilla/layers/CompositorD3D9.h"
 #endif
 #include "GeckoProfiler.h"
 #include "mozilla/ipc/ProtocolTypes.h"
 #include "mozilla/unused.h"
 #include "mozilla/Hal.h"
 #include "mozilla/HalTypes.h"
+#include "mozilla/StaticPtr.h"
 
 namespace mozilla {
 namespace layers {
 
 using namespace base;
 using namespace mozilla::ipc;
 using namespace mozilla::gfx;
 using namespace std;
@@ -68,155 +69,203 @@ CompositorParent::LayerTreeState::LayerT
   , mCrossProcessParent(nullptr)
   , mLayerTree(nullptr)
 {
 }
 
 typedef map<uint64_t, CompositorParent::LayerTreeState> LayerTreeMap;
 static LayerTreeMap sIndirectLayerTrees;
 
-// FIXME/bug 774386: we're assuming that there's only one
-// CompositorParent, but that's not always true.  This assumption only
-// affects CrossProcessCompositorParent below.
-static Thread* sCompositorThread = nullptr;
-// manual reference count of the compositor thread.
-static int sCompositorThreadRefCount = 0;
-static MessageLoop* sMainLoop = nullptr;
+/**
+  * A global map referencing each compositor by ID.
+  *
+  * This map is used by the ImageBridge protocol to trigger
+  * compositions without having to keep references to the
+  * compositor
+  */
+typedef map<uint64_t,CompositorParent*> CompositorMap;
+static CompositorMap* sCompositorMap;
+
+static void CreateCompositorMap()
+{
+  MOZ_ASSERT(!sCompositorMap);
+  sCompositorMap = new CompositorMap;
+}
+
+static void DestroyCompositorMap()
+{
+  MOZ_ASSERT(sCompositorMap);
+  MOZ_ASSERT(sCompositorMap->empty());
+  delete sCompositorMap;
+  sCompositorMap = nullptr;
+}
 
 // See ImageBridgeChild.cpp
 void ReleaseImageBridgeParentSingleton();
 
-static void DeferredDeleteCompositorParent(CompositorParent* aNowReadyToDie)
-{
-  aNowReadyToDie->Release();
-}
-
-static void DeleteCompositorThread()
+class CompositorThreadHolder MOZ_FINAL
 {
-  if (NS_IsMainThread()){
-    ReleaseImageBridgeParentSingleton();
-    delete sCompositorThread;
-    sCompositorThread = nullptr;
-  } else {
-    sMainLoop->PostTask(FROM_HERE, NewRunnableFunction(&DeleteCompositorThread));
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CompositorThreadHolder)
+
+public:
+  CompositorThreadHolder()
+    : mCompositorThread(CreateCompositorThread())
+  {
+    MOZ_ASSERT(NS_IsMainThread());
+    MOZ_COUNT_CTOR(CompositorThreadHolder);
   }
-}
+
+  Thread* GetCompositorThread() const {
+    return mCompositorThread;
+  }
 
-static void ReleaseCompositorThread()
-{
-  if(--sCompositorThreadRefCount == 0) {
-    DeleteCompositorThread();
-  }
-}
+private:
+  ~CompositorThreadHolder()
+  {
+    MOZ_ASSERT(NS_IsMainThread());
+
+    MOZ_COUNT_DTOR(CompositorThreadHolder);
 
-static void SetThreadPriority()
-{
-  hal::SetCurrentThreadPriority(hal::THREAD_PRIORITY_COMPOSITOR);
-}
+    DestroyCompositorThread(mCompositorThread);
+  }
+
+  Thread* const mCompositorThread;
 
-void CompositorParent::StartUp()
-{
-  CreateCompositorMap();
-  CreateThread();
-  sMainLoop = MessageLoop::current();
-}
+  static Thread* CreateCompositorThread();
+  static void DestroyCompositorThread(Thread* aCompositorThread);
+
+  friend class CompositorParent;
+};
 
-void CompositorParent::ShutDown()
-{
-  DestroyThread();
-  DestroyCompositorMap();
-}
+static StaticRefPtr<CompositorThreadHolder> sCompositorThreadHolder;
+
+static MessageLoop* sMainLoop = nullptr;
 
-bool CompositorParent::CreateThread()
+/* static */ Thread*
+CompositorThreadHolder::CreateCompositorThread()
 {
-  NS_ASSERTION(NS_IsMainThread(), "Should be on the main Thread!");
-  MOZ_ASSERT(!sCompositorThread);
-  sCompositorThreadRefCount = 1;
-  sCompositorThread = new Thread("Compositor");
+  MOZ_ASSERT(NS_IsMainThread());
+  sMainLoop = MessageLoop::current();
+
+  MOZ_ASSERT(!sCompositorThreadHolder, "The compositor thread has already been started!");
+
+  Thread* compositorThread = new Thread("Compositor");
 
   Thread::Options options;
   /* Timeout values are powers-of-two to enable us get better data.
      128ms is chosen for transient hangs because 8Hz should be the minimally
      acceptable goal for Compositor responsiveness (normal goal is 60Hz). */
   options.transient_hang_timeout = 128; // milliseconds
   /* 8192ms is chosen for permanent hangs because it's several seconds longer
      than the default hang timeout on major platforms (about 5 seconds). */
   options.permanent_hang_timeout = 8192; // milliseconds
 
-  if (!sCompositorThread->StartWithOptions(options)) {
-    delete sCompositorThread;
-    sCompositorThread = nullptr;
-    return false;
+  if (!compositorThread->StartWithOptions(options)) {
+    delete compositorThread;
+    return nullptr;
   }
 
-  return true;
+  CreateCompositorMap();
+
+  return compositorThread;
+}
+
+/* static */ void
+CompositorThreadHolder::DestroyCompositorThread(Thread* aCompositorThread)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  MOZ_ASSERT(!sCompositorThreadHolder, "We shouldn't be destroying the compositor thread yet.");
+
+  DestroyCompositorMap();
+  ReleaseImageBridgeParentSingleton();
+  delete aCompositorThread;
 }
 
-void CompositorParent::DestroyThread()
+static Thread* CompositorThread() {
+  return sCompositorThreadHolder ? sCompositorThreadHolder->GetCompositorThread() : nullptr;
+}
+
+static void SetThreadPriority()
+{
+  hal::SetCurrentThreadPriority(hal::THREAD_PRIORITY_COMPOSITOR);
+}
+
+void CompositorParent::StartUp()
 {
-  NS_ASSERTION(NS_IsMainThread(), "Should be on the main Thread!");
-  ReleaseCompositorThread();
+  MOZ_ASSERT(NS_IsMainThread(), "Should be on the main Thread!");
+  MOZ_ASSERT(!sCompositorThreadHolder, "The compositor thread has already been started!");
+
+  sCompositorThreadHolder = new CompositorThreadHolder();
+}
+
+void CompositorParent::ShutDown()
+{
+  MOZ_ASSERT(NS_IsMainThread(), "Should be on the main Thread!");
+  MOZ_ASSERT(sCompositorThreadHolder, "The compositor thread has already been shut down!");
+
+  sCompositorThreadHolder = nullptr;
 }
 
 MessageLoop* CompositorParent::CompositorLoop()
 {
-  return sCompositorThread ? sCompositorThread->message_loop() : nullptr;
+  return CompositorThread() ? CompositorThread()->message_loop() : nullptr;
 }
 
 CompositorParent::CompositorParent(nsIWidget* aWidget,
                                    bool aUseExternalSurfaceSize,
                                    int aSurfaceWidth, int aSurfaceHeight)
   : mWidget(aWidget)
   , mCurrentCompositeTask(nullptr)
   , mIsTesting(false)
   , mPendingTransaction(0)
   , mPaused(false)
   , mUseExternalSurfaceSize(aUseExternalSurfaceSize)
   , mEGLSurfaceSize(aSurfaceWidth, aSurfaceHeight)
   , mPauseCompositionMonitor("PauseCompositionMonitor")
   , mResumeCompositionMonitor("ResumeCompositionMonitor")
   , mOverrideComposeReadiness(false)
   , mForceCompositionTask(nullptr)
+  , mCompositorThreadHolder(sCompositorThreadHolder)
 {
-  MOZ_ASSERT(sCompositorThread != nullptr,
-             "The compositor thread must be Initialized before instanciating a CmpositorParent.");
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(CompositorThread(),
+             "The compositor thread must be Initialized before instanciating a CompositorParent.");
   MOZ_COUNT_CTOR(CompositorParent);
   mCompositorID = 0;
   // FIXME: This holds on the the fact that right now the only thing that
   // can destroy this instance is initialized on the compositor thread after
   // this task has been processed.
   CompositorLoop()->PostTask(FROM_HERE, NewRunnableFunction(&AddCompositor,
                                                           this, &mCompositorID));
 
   CompositorLoop()->PostTask(FROM_HERE, NewRunnableFunction(SetThreadPriority));
 
   mRootLayerTreeID = AllocateLayerTreeId();
   sIndirectLayerTrees[mRootLayerTreeID].mParent = this;
 
   mApzcTreeManager = new APZCTreeManager();
-  ++sCompositorThreadRefCount;
 }
 
 bool
 CompositorParent::IsInCompositorThread()
 {
-  return sCompositorThread && sCompositorThread->thread_id() == PlatformThread::CurrentId();
+  return CompositorThread() && CompositorThread()->thread_id() == PlatformThread::CurrentId();
 }
 
 uint64_t
 CompositorParent::RootLayerTreeId()
 {
   return mRootLayerTreeID;
 }
 
 CompositorParent::~CompositorParent()
 {
+  MOZ_ASSERT(NS_IsMainThread());
   MOZ_COUNT_DTOR(CompositorParent);
-
-  ReleaseCompositorThread();
 }
 
 void
 CompositorParent::Destroy()
 {
   NS_ABORT_IF_FALSE(ManagedPLayerTransactionParent().Length() == 0,
                     "CompositorParent destroyed before managed PLayerTransactionParent");
 
@@ -259,29 +308,44 @@ CompositorParent::RecvWillStop()
     mLayerManager->Destroy();
     mLayerManager = nullptr;
     mCompositionManager = nullptr;
   }
 
   return true;
 }
 
+static void DeferredDeleteCompositorParentOnMainThread(CompositorParent* aNowReadyToDie)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  aNowReadyToDie->Release();
+}
+
+static void DeferredDeleteCompositorParentOnIOThread(CompositorParent* aToBeDeletedOnMainThread)
+{
+  MOZ_ASSERT(!NS_IsMainThread());
+  MOZ_ASSERT(sMainLoop);
+  sMainLoop->PostTask(FROM_HERE,
+                      NewRunnableFunction(&DeferredDeleteCompositorParentOnMainThread,
+                                          aToBeDeletedOnMainThread));
+}
+
 bool
 CompositorParent::RecvStop()
 {
   Destroy();
   // There are chances that the ref count reaches zero on the main thread shortly
   // after this function returns while some ipdl code still needs to run on
   // this thread.
   // We must keep the compositor parent alive untill the code handling message
   // reception is finished on this thread.
-  this->AddRef(); // Corresponds to DeferredDeleteCompositorParent's Release
-  CompositorLoop()->PostTask(FROM_HERE,
-                           NewRunnableFunction(&DeferredDeleteCompositorParent,
-                                               this));
+  this->AddRef(); // Corresponds to DeferredDeleteCompositorParentOnMainThread's Release
+  MessageLoop::current()->PostTask(FROM_HERE,
+                                   NewRunnableFunction(&DeferredDeleteCompositorParentOnIOThread,
+                                                       this));
   return true;
 }
 
 bool
 CompositorParent::RecvPause()
 {
   PauseComposition();
   return true;
@@ -916,37 +980,16 @@ CompositorParent::AllocPLayerTransaction
 
 bool
 CompositorParent::DeallocPLayerTransactionParent(PLayerTransactionParent* actor)
 {
   static_cast<LayerTransactionParent*>(actor)->ReleaseIPDLReference();
   return true;
 }
 
-
-typedef map<uint64_t,CompositorParent*> CompositorMap;
-static CompositorMap* sCompositorMap;
-
-void CompositorParent::CreateCompositorMap()
-{
-  if (sCompositorMap == nullptr) {
-    sCompositorMap = new CompositorMap;
-  }
-}
-
-void CompositorParent::DestroyCompositorMap()
-{
-  if (sCompositorMap != nullptr) {
-    NS_ASSERTION(sCompositorMap->empty(),
-                 "The Compositor map should be empty when destroyed>");
-    delete sCompositorMap;
-    sCompositorMap = nullptr;
-  }
-}
-
 CompositorParent* CompositorParent::GetCompositor(uint64_t id)
 {
   CompositorMap::iterator it = sCompositorMap->find(id);
   return it != sCompositorMap->end() ? it->second : nullptr;
 }
 
 void CompositorParent::AddCompositor(CompositorParent* compositor, uint64_t* outID)
 {
@@ -1077,17 +1120,20 @@ class CrossProcessCompositorParent MOZ_F
 {
   friend class CompositorParent;
 
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CrossProcessCompositorParent)
 public:
   CrossProcessCompositorParent(Transport* aTransport, ProcessId aOtherProcess)
     : mTransport(aTransport)
     , mChildProcessId(aOtherProcess)
-  {}
+    , mCompositorThreadHolder(sCompositorThreadHolder)
+  {
+    MOZ_ASSERT(NS_IsMainThread());
+  }
 
   // IToplevelProtocol::CloneToplevel()
   virtual IToplevelProtocol*
   CloneToplevel(const InfallibleTArray<mozilla::ipc::ProtocolFdMapping>& aFds,
                 base::ProcessHandle aPeerProcess,
                 mozilla::ipc::ProtocolCloneContext* aCtx) MOZ_OVERRIDE;
 
   virtual void ActorDestroy(ActorDestroyReason aWhy) MOZ_OVERRIDE;
@@ -1140,16 +1186,18 @@ private:
 
   // There can be many CPCPs, and IPDL-generated code doesn't hold a
   // reference to top-level actors.  So we hold a reference to
   // ourself.  This is released (deferred) in ActorDestroy().
   nsRefPtr<CrossProcessCompositorParent> mSelfRef;
   Transport* mTransport;
   // Child side's process Id.
   base::ProcessId mChildProcessId;
+
+  const nsRefPtr<CompositorThreadHolder> mCompositorThreadHolder;
 };
 
 void
 CompositorParent::DidComposite()
 {
   if (mPendingTransaction) {
     unused << SendDidComposite(0, mPendingTransaction);
     mPendingTransaction = 0;
@@ -1171,16 +1219,18 @@ OpenCompositor(CrossProcessCompositorPar
 {
   DebugOnly<bool> ok = aCompositor->Open(aTransport, aHandle, aIOLoop);
   MOZ_ASSERT(ok);
 }
 
 /*static*/ PCompositorParent*
 CompositorParent::Create(Transport* aTransport, ProcessId aOtherProcess)
 {
+  gfxPlatform::InitLayersIPC();
+
   nsRefPtr<CrossProcessCompositorParent> cpcp =
     new CrossProcessCompositorParent(aTransport, aOtherProcess);
   ProcessHandle handle;
   if (!base::OpenProcessHandle(aOtherProcess, &handle)) {
     // XXX need to kill |aOtherProcess|, it's boned
     return nullptr;
   }
 
@@ -1402,23 +1452,25 @@ CrossProcessCompositorParent::GetComposi
 }
 
 void
 CrossProcessCompositorParent::DeferredDestroy()
 {
   CrossProcessCompositorParent* self;
   mSelfRef.forget(&self);
 
-  nsCOMPtr<nsIRunnable> runnable =
-    NS_NewNonOwningRunnableMethod(self, &CrossProcessCompositorParent::Release);
-  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToMainThread(runnable)));
+  MOZ_ASSERT(sMainLoop);
+  sMainLoop->PostTask(FROM_HERE,
+                      NewRunnableMethod(self, &CrossProcessCompositorParent::Release));
 }
 
 CrossProcessCompositorParent::~CrossProcessCompositorParent()
 {
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(XRE_GetIOMessageLoop());
   XRE_GetIOMessageLoop()->PostTask(FROM_HERE,
                                    new DeleteTask<Transport>(mTransport));
 }
 
 IToplevelProtocol*
 CrossProcessCompositorParent::CloneToplevel(const InfallibleTArray<mozilla::ipc::ProtocolFdMapping>& aFds,
                                             base::ProcessHandle aPeerProcess,
                                             mozilla::ipc::ProtocolCloneContext* aCtx)
--- a/gfx/layers/ipc/CompositorParent.h
+++ b/gfx/layers/ipc/CompositorParent.h
@@ -58,16 +58,18 @@ struct ScopedLayerTreeRegistration
                               Layer* aRoot,
                               GeckoContentController* aController);
   ~ScopedLayerTreeRegistration();
 
 private:
   uint64_t mLayersId;
 };
 
+class CompositorThreadHolder;
+
 class CompositorParent : public PCompositorParent,
                          public ShadowLayersManager
 {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CompositorParent)
 
 public:
   CompositorParent(nsIWidget* aWidget,
                    bool aUseExternalSurfaceSize = false,
@@ -161,17 +163,18 @@ public:
   static MessageLoop* CompositorLoop();
 
   /**
    * Creates the compositor thread and the global compositor map.
    */
   static void StartUp();
 
   /**
-   * Destroys the compositor thread and the global compositor map.
+   * Waits for all [CrossProcess]CompositorParent's to be gone,
+   * and destroys the compositor thread and global compositor map.
    */
   static void ShutDown();
 
   /**
    * Allocate an ID that can be used to refer to a layer tree and
    * associated resources that live only on the compositor thread.
    *
    * Must run on the content main thread.
@@ -255,46 +258,16 @@ protected:
   void InitializeLayerManager(const nsTArray<LayersBackend>& aBackendHints);
   void PauseComposition();
   void ResumeComposition();
   void ResumeCompositionAndResize(int width, int height);
   void ForceComposition();
   void CancelCurrentCompositeTask();
 
   /**
-   * Creates a global map referencing each compositor by ID.
-   *
-   * This map is used by the ImageBridge protocol to trigger
-   * compositions without having to keep references to the
-   * compositor
-   */
-  static void CreateCompositorMap();
-  static void DestroyCompositorMap();
-
-  /**
-   * Creates the compositor thread.
-   *
-   * All compositors live on the same thread.
-   * The thread is not lazily created on first access to avoid dealing with
-   * thread safety. Therefore it's best to create and destroy the thread when
-   * we know we areb't using it (So creating/destroying along with gfxPlatform
-   * looks like a good place).
-   */
-  static bool CreateThread();
-
-  /**
-   * Destroys the compositor thread.
-   *
-   * It is safe to call this fucntion more than once, although the second call
-   * will have no effect.
-   * This function is not thread-safe.
-   */
-  static void DestroyThread();
-
-  /**
    * Add a compositor to the global compositor map.
    */
   static void AddCompositor(CompositorParent* compositor, uint64_t* id);
   /**
    * Remove a compositor from the global compositor map.
    */
   static CompositorParent* RemoveCompositor(uint64_t id);
 
@@ -331,15 +304,17 @@ protected:
   uint64_t mCompositorID;
   uint64_t mRootLayerTreeID;
 
   bool mOverrideComposeReadiness;
   CancelableTask* mForceCompositionTask;
 
   nsRefPtr<APZCTreeManager> mApzcTreeManager;
 
+  const nsRefPtr<CompositorThreadHolder> mCompositorThreadHolder;
+
   DISALLOW_EVIL_CONSTRUCTORS(CompositorParent);
 };
 
 } // layers
 } // mozilla
 
 #endif // mozilla_layers_CompositorParent_h