Backed out changeset 6f2e001c5f39 (bug 774388)

gfx/layers/ipc/CompositorParent.cpp

@@ -53,7 +53,6 @@
 #include "mozilla/unused.h"
 #include "mozilla/Hal.h"
 #include "mozilla/HalTypes.h"
-#include "mozilla/StaticPtr.h"
 
 namespace mozilla {
 namespace layers {
@@ -74,65 +73,12 @@ CompositorParent::LayerTreeState::LayerTreeState()
 typedef map<uint64_t, CompositorParent::LayerTreeState> LayerTreeMap;
 static LayerTreeMap sIndirectLayerTrees;
 
-/**
+// FIXME/bug 774386: we're assuming that there's only one
-  * A global map referencing each compositor by ID.
+// CompositorParent, but that's not always true.  This assumption only
-  *
+// affects CrossProcessCompositorParent below.
-  * This map is used by the ImageBridge protocol to trigger
+static Thread* sCompositorThread = nullptr;
-  * compositions without having to keep references to the
+// manual reference count of the compositor thread.
-  * compositor
+static int sCompositorThreadRefCount = 0;
-  */
-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;
-}
-
-class CompositorThreadHolder
-{
-  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CompositorThreadHolder)
-
-public:
-  CompositorThreadHolder()
-    : mCompositorThread(CreateCompositorThread())
-  {
-    MOZ_COUNT_CTOR(CompositorThreadHolder);
-  }
-
-  Thread* GetCompositorThread() const {
-    return mCompositorThread;
-  }
-
-private:
-  ~CompositorThreadHolder() {
-    MOZ_COUNT_DTOR(CompositorThreadHolder);
-    DeleteCompositorThread(mCompositorThread);
-  }
-
-  Thread* const mCompositorThread;
-
-  static Thread* CreateCompositorThread();
-  static void DeleteCompositorThread(Thread* aCompositorThread);
-};
-
-static StaticRefPtr<CompositorThreadHolder> gCompositorThreadHolder;
-
-static Thread* CompositorThread() {
-  MOZ_ASSERT(gCompositorThreadHolder);
-  return gCompositorThreadHolder->GetCompositorThread();
-}
-
-
 static MessageLoop* sMainLoop = nullptr;
 
 // See ImageBridgeChild.cpp
@@ -143,16 +89,21 @@ static void DeferredDeleteCompositorParent(CompositorParent* aNowReadyToDie)
   aNowReadyToDie->Release();
 }
 
-/* static */ void CompositorThreadHolder::DeleteCompositorThread(Thread* aCompositorThread)
+static void DeleteCompositorThread()
 {
   if (NS_IsMainThread()){
-    DestroyCompositorMap();
     ReleaseImageBridgeParentSingleton();
-    delete aCompositorThread;
+    delete sCompositorThread;
+    sCompositorThread = nullptr;
   } else {
-    sMainLoop->PostTask(FROM_HERE,
+    sMainLoop->PostTask(FROM_HERE, NewRunnableFunction(&DeleteCompositorThread));
-                        NewRunnableFunction(&CompositorThreadHolder::DeleteCompositorThread,
+  }
-                                            CompositorThread()));
+}
+
+static void ReleaseCompositorThread()
+{
+  if(--sCompositorThreadRefCount == 0) {
+    DeleteCompositorThread();
   }
 }
 
@@ -161,28 +112,25 @@ static void SetThreadPriority()
   hal::SetCurrentThreadPriority(hal::THREAD_PRIORITY_COMPOSITOR);
 }
 
-void CompositorParent::StartUpCompositorThread()
+void CompositorParent::StartUp()
 {
-  MOZ_ASSERT(NS_IsMainThread(), "Should be on the main Thread!");
-  MOZ_ASSERT(!gCompositorThreadHolder, "The compositor thread has already been started!");
   CreateCompositorMap();
-  gCompositorThreadHolder = new CompositorThreadHolder();
+  CreateThread();
   sMainLoop = MessageLoop::current();
 }
 
-void CompositorParent::AllowShutdownOfCompositorThreadWhenUnreferenced()
+void CompositorParent::ShutDown()
 {
-  MOZ_ASSERT(NS_IsMainThread(), "Should be on the main Thread!");
+  DestroyThread();
-  MOZ_ASSERT(gCompositorThreadHolder, "The compositor thread has already been shut down!");
+  DestroyCompositorMap();
-  gCompositorThreadHolder = nullptr;
 }
 
-/* static */ Thread* CompositorThreadHolder::CreateCompositorThread()
+bool CompositorParent::CreateThread()
 {
-  MOZ_ASSERT(NS_IsMainThread(), "Should be on the main Thread!");
+  NS_ASSERTION(NS_IsMainThread(), "Should be on the main Thread!");
-  MOZ_ASSERT(!gCompositorThreadHolder, "The compositor thread has already been started!");
+  MOZ_ASSERT(!sCompositorThread);
-
+  sCompositorThreadRefCount = 1;
-  Thread* compositorThread = new Thread("Compositor");
+  sCompositorThread = new Thread("Compositor");
 
   Thread::Options options;
   /* Timeout values are powers-of-two to enable us get better data.
@@ -193,17 +141,24 @@ void CompositorParent::AllowShutdownOfCompositorThreadWhenUnreferenced()
      than the default hang timeout on major platforms (about 5 seconds). */
   options.permanent_hang_timeout = 8192; // milliseconds
 
-  if (!compositorThread->StartWithOptions(options)) {
+  if (!sCompositorThread->StartWithOptions(options)) {
-    delete compositorThread;
+    delete sCompositorThread;
-    compositorThread = nullptr;
+    sCompositorThread = nullptr;
+    return false;
   }
 
-  return compositorThread;
+  return true;
+}
+
+void CompositorParent::DestroyThread()
+{
+  NS_ASSERTION(NS_IsMainThread(), "Should be on the main Thread!");
+  ReleaseCompositorThread();
 }
 
 MessageLoop* CompositorParent::CompositorLoop()
 {
-  return CompositorThread() ? CompositorThread()->message_loop() : nullptr;
+  return sCompositorThread ? sCompositorThread->message_loop() : nullptr;
 }
 
 CompositorParent::CompositorParent(nsIWidget* aWidget,
@@ -220,10 +175,9 @@ CompositorParent::CompositorParent(nsIWidget* aWidget,
   , mResumeCompositionMonitor("ResumeCompositionMonitor")
   , mOverrideComposeReadiness(false)
   , mForceCompositionTask(nullptr)
-  , mCompositorThreadHolder(gCompositorThreadHolder)
 {
-  MOZ_ASSERT(CompositorThread(),
+  MOZ_ASSERT(sCompositorThread != nullptr,
-             "The compositor thread must be Initialized before instanciating a CompositorParent.");
+             "The compositor thread must be Initialized before instanciating a CmpositorParent.");
   MOZ_COUNT_CTOR(CompositorParent);
   mCompositorID = 0;
   // FIXME: This holds on the the fact that right now the only thing that
@@ -238,12 +192,13 @@ CompositorParent::CompositorParent(nsIWidget* aWidget,
   sIndirectLayerTrees[mRootLayerTreeID].mParent = this;
 
   mApzcTreeManager = new APZCTreeManager();
+  ++sCompositorThreadRefCount;
 }
 
 bool
 CompositorParent::IsInCompositorThread()
 {
-  return CompositorThread() && CompositorThread()->thread_id() == PlatformThread::CurrentId();
+  return sCompositorThread && sCompositorThread->thread_id() == PlatformThread::CurrentId();
 }
 
 uint64_t
@@ -255,6 +210,8 @@ CompositorParent::RootLayerTreeId()
 CompositorParent::~CompositorParent()
 {
   MOZ_COUNT_DTOR(CompositorParent);
+
+  ReleaseCompositorThread();
 }
 
 void
@@ -962,6 +919,27 @@ CompositorParent::DeallocPLayerTransactionParent(PLayerTransactionParent* actor)
   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);

gfx/layers/ipc/CompositorParent.h

@@ -63,8 +63,6 @@ private:
   uint64_t mLayersId;
 };
 
-class CompositorThreadHolder;
-
 class CompositorParent : public PCompositorParent,
                          public ShadowLayersManager
 {
@@ -165,14 +163,12 @@ public:
   /**
    * Creates the compositor thread and the global compositor map.
    */
-  static void StartUpCompositorThread();
+  static void StartUp();
 
   /**
-   * Drops the static reference to the compositor thread holder,
+   * Destroys the compositor thread and the global compositor map.
-   * allowing its shutdown (also destroying the compositor map) when it
-   * will no longer be referenced by any CompositorParent.
    */
-  static void AllowShutdownOfCompositorThreadWhenUnreferenced();
+  static void ShutDown();
 
   /**
    * Allocate an ID that can be used to refer to a layer tree and
@@ -263,6 +259,36 @@ protected:
   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.
    */
@@ -310,8 +336,6 @@ protected:
 
   nsRefPtr<APZCTreeManager> mApzcTreeManager;
 
-  const nsRefPtr<CompositorThreadHolder> mCompositorThreadHolder;
-
   DISALLOW_EVIL_CONSTRUCTORS(CompositorParent);
 };
 

gfx/thebes/gfxPlatform.cpp

@@ -370,7 +370,7 @@ gfxPlatform::Init()
     if (UsesOffMainThreadCompositing() &&
         XRE_GetProcessType() == GeckoProcessType_Default)
     {
-        mozilla::layers::CompositorParent::StartUpCompositorThread();
+        mozilla::layers::CompositorParent::StartUp();
         if (gfxPrefs::AsyncVideoEnabled()) {
             mozilla::layers::ImageBridgeChild::StartUp();
         }
@@ -521,7 +521,7 @@ gfxPlatform::ShutdownLayersIPC()
         layers::SharedBufferManagerChild::ShutDown();
 #endif
 
-        layers::CompositorParent::AllowShutdownOfCompositorThreadWhenUnreferenced();
+        layers::CompositorParent::ShutDown();
     }
     gPlatform->mAlreadyShutDownLayersIPC = true;
 }