Bug 1453463 - Keep an APZC map on APZCTreeManager. r?botond draft
authorKartikaya Gupta <kgupta@mozilla.com>
Mon, 16 Apr 2018 09:33:08 -0400
changeset 782960 f9adaf38ed14651ff24ece84bcf19a62e2e706ab
parent 782959 3dfcc29d63c9eb2019618378b31a5b99ec176562
child 782961 ff014e1e9633fc0a8d7b65bcdba31ed2a5256bb0
push id106583
push userkgupta@mozilla.com
push dateMon, 16 Apr 2018 13:33:33 +0000
reviewersbotond
bugs1453463
milestone61.0a1
Bug 1453463 - Keep an APZC map on APZCTreeManager. r?botond We are already building an almost-what-we-want map in the TreeBuildingState, so I modified to be exactly what we want, and then just move it to the APZCTreeManager once the tree build is done. MozReview-Commit-ID: 40RVwYv93wR
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/layers/apz/src/APZCTreeManager.h
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -96,20 +96,24 @@ struct APZCTreeManager::TreeBuildingStat
 
   // A list of nodes that need to be destroyed at the end of the tree building.
   // This is initialized with all nodes in the old tree, and nodes are removed
   // from it as we reuse them in the new tree.
   nsTArray<RefPtr<HitTestingTreeNode>> mNodesToDestroy;
 
   // This map is populated as we place APZCs into the new tree. Its purpose is
   // to facilitate re-using the same APZC for different layers that scroll
-  // together (and thus have the same ScrollableLayerGuid).
+  // together (and thus have the same ScrollableLayerGuid). The presShellId
+  // doesn't matter for this purpose, and we move the map to the APZCTreeManager
+  // after we're done building, so it's useful to have the presshell-ignoring
+  // map for that.
   std::unordered_map<ScrollableLayerGuid,
-                     AsyncPanZoomController*,
-                     ScrollableLayerGuid::HashFn> mApzcMap;
+                     RefPtr<AsyncPanZoomController>,
+                     ScrollableLayerGuid::HashIgnoringPresShellFn,
+                     ScrollableLayerGuid::EqualIgnoringPresShellFn> mApzcMap;
 
   // As the tree is traversed, the top element of this stack tracks whether
   // the parent scroll node has a perspective transform.
   std::stack<bool> mParentHasPerspective;
 
   // During the tree building process, the perspective transform component
   // of the ancestor transforms of some APZCs can be "deferred" to their
   // children, meaning they are added to the children's ancestor transforms
@@ -226,16 +230,17 @@ private:
 };
 
 APZCTreeManager::APZCTreeManager(LayersId aRootLayersId)
     : mInputQueue(new InputQueue()),
       mRootLayersId(aRootLayersId),
       mSampler(nullptr),
       mUpdater(nullptr),
       mTreeLock("APZCTreeLock"),
+      mMapLock("APZCMapLock"),
       mHitResultForInputBlock(CompositorHitTestInfo::eInvisibleToHitTest),
       mRetainedTouchIdentifier(-1),
       mInScrollbarTouchDrag(false),
       mApzcTreeLog("apzctree"),
       mTestDataLock("APZTestDataLock"),
       mDPI(160.0)
 {
   RefPtr<APZCTreeManager> self(this);
@@ -470,16 +475,22 @@ APZCTreeManager::UpdateHitTestingTreeImp
             }
           });
     }
   }
 
   // We do not support tree structures where the root node has siblings.
   MOZ_ASSERT(!(mRootNode && mRootNode->GetPrevSibling()));
 
+  { // scope lock and update our mApzcMap before we destroy all the unused
+    // APZC instances
+    MutexAutoLock lock(mMapLock);
+    mApzcMap = Move(state.mApzcMap);
+  }
+
   for (size_t i = 0; i < state.mNodesToDestroy.Length(); i++) {
     APZCTM_LOG("Destroying node at %p with APZC %p\n",
         state.mNodesToDestroy[i].get(),
         state.mNodesToDestroy[i]->GetApzc());
     state.mNodesToDestroy[i]->Destroy();
   }
 
 #if ENABLE_APZCTM_LOGGING
--- a/gfx/layers/apz/src/APZCTreeManager.h
+++ b/gfx/layers/apz/src/APZCTreeManager.h
@@ -60,21 +60,22 @@ class WebRenderScrollDataWrapper;
 struct AncestorTransform;
 struct ScrollThumbData;
 
 /**
  * ****************** NOTE ON LOCK ORDERING IN APZ **************************
  *
  * There are two main kinds of locks used by APZ: APZCTreeManager::mTreeLock
  * ("the tree lock") and AsyncPanZoomController::mRecursiveMutex ("APZC locks").
- * There is also the APZCTreeManager::mTestDataLock ("test lock").
+ * There is also the APZCTreeManager::mTestDataLock ("test lock") and
+ * APZCTreeManager::mMapLock ("map lock").
  *
  * To avoid deadlock, we impose a lock ordering between these locks, which is:
  *
- *      tree lock -> APZC locks -> test lock
+ *      tree lock -> map lock -> APZC locks -> test lock
  *
  * The interpretation of the lock ordering is that if lock A precedes lock B
  * in the ordering sequence, then you must NOT wait on A while holding B.
  *
  * **************************************************************************
  */
 
 /**
@@ -727,16 +728,26 @@ private:
    * This lock does not need to be held while manipulating a single APZC instance in
    * isolation (that is, if its tree pointers are not being accessed or mutated). The
    * lock also needs to be held when accessing the mRootNode instance variable, as that
    * is considered part of the APZC tree management state.
    * IMPORTANT: See the note about lock ordering at the top of this file. */
   mutable mozilla::RecursiveMutex mTreeLock;
   RefPtr<HitTestingTreeNode> mRootNode;
 
+  /* A map for quick access to get APZC instances by guid, without having to
+   * acquire the tree lock. mMapLock must be acquired while accessing or
+   * modifying mApzcMap.
+   */
+  mutable mozilla::Mutex mMapLock;
+  std::unordered_map<ScrollableLayerGuid,
+                     RefPtr<AsyncPanZoomController>,
+                     ScrollableLayerGuid::HashIgnoringPresShellFn,
+                     ScrollableLayerGuid::EqualIgnoringPresShellFn> mApzcMap;
+
   /* Holds the zoom constraints for scrollable layers, as determined by the
    * the main-thread gecko code. This can only be accessed on the updater
    * thread. */
   std::unordered_map<ScrollableLayerGuid,
                      ZoomConstraints,
                      ScrollableLayerGuid::HashFn> mZoomConstraints;
   /* A list of keyboard shortcuts to use for translating keyboard inputs into
    * keyboard actions. This is gathered on the main thread from XBL bindings.