Bug 1511054 - Make APZSampler more threadsafe. r=botond
authorKartikaya Gupta <kgupta@mozilla.com>
Fri, 30 Nov 2018 18:19:17 +0000
changeset 505504 4a7e5e05432e569e0d91a645a441ed65226cf5c7
parent 505503 0066c3d92589f5925c4a20d50bf25349d8613dd9
child 505505 96e1ddc0637be0c1166da92c34550ef418e4aa91
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond
bugs1511054
milestone65.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 1511054 - Make APZSampler more threadsafe. r=botond Differential Revision: https://phabricator.services.mozilla.com/D13556
gfx/layers/apz/public/APZSampler.h
gfx/layers/apz/src/APZSampler.cpp
gfx/layers/ipc/CompositorBridgeParent.cpp
--- a/gfx/layers/apz/public/APZSampler.h
+++ b/gfx/layers/apz/public/APZSampler.h
@@ -40,16 +40,20 @@ struct ScrollbarData;
  * the sampler thread.
  */
 class APZSampler {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(APZSampler)
 
  public:
   APZSampler(const RefPtr<APZCTreeManager>& aApz, bool aIsUsingWebRender);
 
+  // Whoever creates this sampler is responsible for calling Destroy() on it
+  // before releasing the owning refptr.
+  void Destroy();
+
   void SetWebRenderWindowId(const wr::WindowId& aWindowId);
 
   /**
    * This function is invoked from rust on the render backend thread when it
    * is created. It effectively tells the APZSampler "the current thread is
    * the sampler thread for this window id" and allows APZSampler to remember
    * which thread it is.
    */
@@ -118,17 +122,18 @@ class APZSampler {
 
   // Used to manage the mapping from a WR window id to APZSampler. These are
   // only used if WebRender is enabled. Both sWindowIdMap and mWindowId should
   // only be used while holding the sWindowIdLock. Note that we use a
   // StaticAutoPtr wrapper on sWindowIdMap to avoid a static initializer for the
   // unordered_map. This also avoids the initializer/memory allocation in cases
   // where we're not using WebRender.
   static StaticMutex sWindowIdLock;
-  static StaticAutoPtr<std::unordered_map<uint64_t, APZSampler*>> sWindowIdMap;
+  static StaticAutoPtr<std::unordered_map<uint64_t, RefPtr<APZSampler>>>
+      sWindowIdMap;
   Maybe<wr::WrWindowId> mWindowId;
 
   // Lock used to protected mSamplerThreadId
   mutable Mutex mThreadIdLock;
   // If WebRender is enabled, this holds the thread id of the render backend
   // thread (which is the sampler thread) for the compositor associated with
   // this APZSampler instance.
   Maybe<PlatformThreadId> mSamplerThreadId;
--- a/gfx/layers/apz/src/APZSampler.cpp
+++ b/gfx/layers/apz/src/APZSampler.cpp
@@ -16,45 +16,45 @@
 #include "mozilla/layers/SynchronousTask.h"
 #include "TreeTraversal.h"
 #include "mozilla/webrender/WebRenderAPI.h"
 
 namespace mozilla {
 namespace layers {
 
 StaticMutex APZSampler::sWindowIdLock;
-StaticAutoPtr<std::unordered_map<uint64_t, APZSampler*>>
+StaticAutoPtr<std::unordered_map<uint64_t, RefPtr<APZSampler>>>
     APZSampler::sWindowIdMap;
 
 APZSampler::APZSampler(const RefPtr<APZCTreeManager>& aApz,
                        bool aIsUsingWebRender)
     : mApz(aApz),
       mIsUsingWebRender(aIsUsingWebRender),
       mThreadIdLock("APZSampler::mThreadIdLock"),
       mSampleTimeLock("APZSampler::mSampleTimeLock") {
   MOZ_ASSERT(aApz);
   mApz->SetSampler(this);
 }
 
-APZSampler::~APZSampler() {
-  mApz->SetSampler(nullptr);
+APZSampler::~APZSampler() { mApz->SetSampler(nullptr); }
 
+void APZSampler::Destroy() {
   StaticMutexAutoLock lock(sWindowIdLock);
   if (mWindowId) {
     MOZ_ASSERT(sWindowIdMap);
     sWindowIdMap->erase(wr::AsUint64(*mWindowId));
   }
 }
 
 void APZSampler::SetWebRenderWindowId(const wr::WindowId& aWindowId) {
   StaticMutexAutoLock lock(sWindowIdLock);
   MOZ_ASSERT(!mWindowId);
   mWindowId = Some(aWindowId);
   if (!sWindowIdMap) {
-    sWindowIdMap = new std::unordered_map<uint64_t, APZSampler*>();
+    sWindowIdMap = new std::unordered_map<uint64_t, RefPtr<APZSampler>>();
     NS_DispatchToMainThread(NS_NewRunnableFunction(
         "APZUpdater::ClearOnShutdown", [] { ClearOnShutdown(&sWindowIdMap); }));
   }
   (*sWindowIdMap)[wr::AsUint64(aWindowId)] = this;
 }
 
 /*static*/ void APZSampler::SetSamplerThread(const wr::WrWindowId& aWindowId) {
   if (RefPtr<APZSampler> sampler = GetSampler(aWindowId)) {
--- a/gfx/layers/ipc/CompositorBridgeParent.cpp
+++ b/gfx/layers/ipc/CompositorBridgeParent.cpp
@@ -426,16 +426,17 @@ void CompositorBridgeParent::StopAndClea
   mPaused = true;
 
   // We need to clear the APZ tree before we destroy the WebRender API below,
   // because in the case of async scene building that will shut down the updater
   // thread and we need to run the task before that happens.
   MOZ_ASSERT((mApzSampler != nullptr) == (mApzcTreeManager != nullptr));
   MOZ_ASSERT((mApzUpdater != nullptr) == (mApzcTreeManager != nullptr));
   if (mApzUpdater) {
+    mApzSampler->Destroy();
     mApzSampler = nullptr;
     mApzUpdater->ClearTree(mRootLayerTreeID);
     mApzUpdater = nullptr;
     mApzcTreeManager = nullptr;
   }
 
   // Ensure that the layer manager is destroyed before CompositorBridgeChild.
   if (mLayerManager) {