Bug 1265619 - Fix potential race condition when APZCTreeManager::PrepareNodeForLayer() is called before layer is initialized.; r?Kats. draft
authorKilik Kuo <kikuo@mozilla.com>
Tue, 19 Apr 2016 16:33:15 +0800
changeset 353629 2548af8586c2700ef733429608997f771781bc10
parent 353628 fee39861e88064b0238ae08bd19d61ad89f194bb
child 354524 32406f98791dbd0c7ebedc70f7651e178fd66353
push id15883
push userkikuo@mozilla.com
push dateTue, 19 Apr 2016 08:34:53 +0000
reviewersKats
bugs1265619
milestone44.0
Bug 1265619 - Fix potential race condition when APZCTreeManager::PrepareNodeForLayer() is called before layer is initialized.; r?Kats. MozReview-Commit-ID: LdTgqtFqUGL
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/layers/apz/src/APZCTreeManager.h
gfx/layers/ipc/CompositorParent.cpp
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -93,17 +93,18 @@ APZCTreeManager::CalculatePendingDisplay
     aFrameMetrics, aVelocity, aEstimatedPaintDuration);
 }
 
 APZCTreeManager::APZCTreeManager()
     : mInputQueue(new InputQueue()),
       mTreeLock("APZCTreeLock"),
       mHitResultForInputBlock(HitNothing),
       mRetainedTouchIdentifier(-1),
-      mApzcTreeLog("apzctree")
+      mApzcTreeLog("apzctree"),
+      mPaintThrottlerMapLock("PaintThrottlerMapLock")
 {
   MOZ_ASSERT(NS_IsMainThread());
   AsyncPanZoomController::InitializeGlobalState();
   mApzcTreeLog.ConditionOnPrefFunction(gfxPrefs::APZPrintTree);
 }
 
 APZCTreeManager::~APZCTreeManager()
 {
@@ -207,31 +208,35 @@ APZCTreeManager::UpdateHitTestingTree(Co
   mRootNode->Dump("  ");
 #endif
 }
 
 void
 APZCTreeManager::InitializeForLayersId(uint64_t aLayersId)
 {
   MOZ_ASSERT(NS_IsMainThread());
+  MutexAutoLock lock(mPaintThrottlerMapLock);
+
   auto throttlerInsertResult = mPaintThrottlerMap.insert(
     std::make_pair(aLayersId, RefPtr<TaskThrottler>()));
   if (throttlerInsertResult.second) {
     throttlerInsertResult.first->second = new TaskThrottler(
       GetFrameTime(), TimeDuration::FromMilliseconds(500));
   }
 }
 
 void
 APZCTreeManager::AdoptLayersId(uint64_t aLayersId, APZCTreeManager* aOldManager)
 {
   MOZ_ASSERT(aOldManager);
   if (aOldManager == this) {
     return;
   }
+  MutexAutoLock lock(mPaintThrottlerMapLock);
+
   auto iter = aOldManager->mPaintThrottlerMap.find(aLayersId);
   if (iter != aOldManager->mPaintThrottlerMap.end()) {
     mPaintThrottlerMap[aLayersId] = iter->second;
     aOldManager->mPaintThrottlerMap.erase(iter);
   }
 }
 
 // Compute the clip region to be used for a layer with an APZC. This function
@@ -466,16 +471,18 @@ APZCTreeManager::PrepareNodeForLayer(con
     // The APZC we get off the layer may have been destroyed previously if the
     // layer was inactive or omitted from the layer tree for whatever reason
     // from a layers update. If it later comes back it will have a reference to
     // a destroyed APZC and so we need to throw that out and make a new one.
     bool newApzc = (apzc == nullptr || apzc->IsDestroyed());
     if (newApzc) {
       // Look up the paint throttler for this layers id, or create it if
       // this is the first APZC for this layers id.
+      MutexAutoLock lock(mPaintThrottlerMapLock);
+
       RefPtr<TaskThrottler> throttler = mPaintThrottlerMap[aLayersId];
       MOZ_ASSERT(throttler);
 
       apzc = NewAPZCInstance(aLayersId, state->mController, throttler);
       apzc->SetCompositorParent(aState.mCompositor);
       if (state->mCrossProcessParent != nullptr) {
         apzc->ShareFrameMetricsAcrossProcesses();
       }
--- a/gfx/layers/apz/src/APZCTreeManager.h
+++ b/gfx/layers/apz/src/APZCTreeManager.h
@@ -13,16 +13,17 @@
 #include "gfxPoint.h"                   // for gfxPoint
 #include "mozilla/Assertions.h"         // for MOZ_ASSERT_HELPER2
 #include "mozilla/EventForwards.h"      // for WidgetInputEvent, nsEventStatus
 #include "mozilla/gfx/Logging.h"        // for gfx::TreeLog
 #include "mozilla/gfx/Matrix.h"         // for Matrix4x4
 #include "mozilla/layers/APZUtils.h"    // for HitTestResult
 #include "mozilla/layers/TouchCounter.h"// for TouchCounter
 #include "mozilla/Monitor.h"            // for Monitor
+#include "mozilla/Mutex.h"              // for Mutex
 #include "mozilla/TimeStamp.h"          // for mozilla::TimeStamp
 #include "mozilla/Vector.h"             // for mozilla::Vector
 #include "nsAutoPtr.h"                  // for nsRefPtr
 #include "nsCOMPtr.h"                   // for already_AddRefed
 #include "nsISupportsImpl.h"            // for MOZ_COUNT_CTOR, etc
 #include "nsTArrayForwardDeclare.h"     // for nsTArray, nsTArray_Impl, etc
 #include "Units.h"                      // for CSSPoint, CSSRect, etc
 
@@ -556,14 +557,18 @@ private:
   /* Tracks the number of touch points we are tracking that are currently on
    * the screen. */
   TouchCounter mTouchCounter;
   /* For logging the APZC tree for debugging (enabled by the apz.printtree
    * pref). */
   gfx::TreeLog mApzcTreeLog;
 
   static float sDPI;
+
+  /* Provide atomic operation to mPaintThrottlerMap.
+   */
+  mozilla::Mutex mPaintThrottlerMapLock;
 };
 
 } // namespace layers
 } // namespace mozilla
 
 #endif // mozilla_layers_PanZoomController_h
--- a/gfx/layers/ipc/CompositorParent.cpp
+++ b/gfx/layers/ipc/CompositorParent.cpp
@@ -1529,18 +1529,19 @@ CompositorParent::RecvNotifyChildCreated
   NotifyChildCreated(child);
   return true;
 }
 
 void
 CompositorParent::NotifyChildCreated(const uint64_t& aChild)
 {
   if (mApzcTreeManager) {
-    NS_DispatchToMainThread(NS_NewRunnableMethodWithArg<uint64_t>(
-        mApzcTreeManager, &APZCTreeManager::InitializeForLayersId, aChild));
+    // Directly initialize paintThrottler for layer Id to avoid race condition
+    // in APZCTreeManager::PrepareNodeForLayer.
+    mApzcTreeManager->InitializeForLayersId(aChild);
   }
   sIndirectLayerTreesLock->AssertCurrentThreadOwns();
   sIndirectLayerTrees[aChild].mParent = this;
   sIndirectLayerTrees[aChild].mLayerManager = mLayerManager;
 }
 
 bool
 CompositorParent::RecvAdoptChild(const uint64_t& child)