Bug 1453463 - Refactor to make unordered_maps with guid keys a little cleaner. r?botond draft
authorKartikaya Gupta <kgupta@mozilla.com>
Mon, 16 Apr 2018 09:33:07 -0400
changeset 782959 3dfcc29d63c9eb2019618378b31a5b99ec176562
parent 782958 58cb39cde8f4ff0bda6e08cf4ccf80ffdce8b005
child 782960 f9adaf38ed14651ff24ece84bcf19a62e2e706ab
push id106583
push userkgupta@mozilla.com
push dateMon, 16 Apr 2018 13:33:33 +0000
reviewersbotond
bugs1453463
milestone61.0a1
Bug 1453463 - Refactor to make unordered_maps with guid keys a little cleaner. r?botond This is mostly just moving the existing hash function and introducing additional helpers to create maps with presshell-ignoring guid keys. We can use this in one place trivially so I did that as well. MozReview-Commit-ID: G8nMS1PECT4
gfx/layers/FrameMetrics.h
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/layers/apz/src/APZCTreeManager.h
--- a/gfx/layers/FrameMetrics.h
+++ b/gfx/layers/FrameMetrics.h
@@ -1100,20 +1100,52 @@ struct ScrollableLayerGuid {
       }
       if (mPresShellId == other.mPresShellId) {
         return mScrollId < other.mScrollId;
       }
     }
     return false;
   }
 
-  PLDHashNumber Hash() const
+  // Helper structs to use as hash/equality functions in std::unordered_map. e.g.
+  // std::unordered_map<ScrollableLayerGuid,
+  //                    ValueType,
+  //                    ScrollableLayerGuid::HashFn> myMap;
+  // std::unordered_map<ScrollableLayerGuid,
+  //                    ValueType,
+  //                    ScrollableLayerGuid::HashIgnoringPresShellFn,
+  //                    ScrollableLayerGuid::EqualIgnoringPresShellFn> myMap;
+
+  struct HashFn
   {
-    return HashGeneric(uint64_t(mLayersId), mPresShellId, mScrollId);
-  }
+    std::size_t operator()(const ScrollableLayerGuid& aGuid) const
+    {
+      return HashGeneric(uint64_t(aGuid.mLayersId),
+                         aGuid.mPresShellId,
+                         aGuid.mScrollId);
+    }
+  };
+
+  struct HashIgnoringPresShellFn
+  {
+    std::size_t operator()(const ScrollableLayerGuid& aGuid) const
+    {
+      return HashGeneric(uint64_t(aGuid.mLayersId),
+                         aGuid.mScrollId);
+    }
+  };
+
+  struct EqualIgnoringPresShellFn
+  {
+    bool operator()(const ScrollableLayerGuid& lhs, const ScrollableLayerGuid& rhs) const
+    {
+      return lhs.mLayersId == rhs.mLayersId
+          && lhs.mScrollId == rhs.mScrollId;
+    }
+  };
 };
 
 template <int LogLevel>
 gfx::Log<LogLevel>& operator<<(gfx::Log<LogLevel>& log, const ScrollableLayerGuid& aGuid) {
   return log << '(' << uint64_t(aGuid.mLayersId) << ',' << aGuid.mPresShellId << ',' << aGuid.mScrollId << ')';
 }
 
 struct ZoomConstraints {
@@ -1164,23 +1196,15 @@ struct ZoomConstraints {
   }
 
   bool operator!=(const ZoomConstraints& other) const
   {
     return !(*this == other);
   }
 };
 
-struct ScrollableLayerGuidHash
-{
-  std::size_t operator()(const ScrollableLayerGuid& Guid) const
-  {
-    return Guid.Hash();
-  }
-};
-
 
 typedef Maybe<ZoomConstraints> MaybeZoomConstraints;
 
 } // namespace layers
 } // namespace mozilla
 
 #endif /* GFX_FRAMEMETRICS_H */
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -97,17 +97,19 @@ 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).
-  std::unordered_map<ScrollableLayerGuid, AsyncPanZoomController*, ScrollableLayerGuidHash> mApzcMap;
+  std::unordered_map<ScrollableLayerGuid,
+                     AsyncPanZoomController*,
+                     ScrollableLayerGuid::HashFn> 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
@@ -538,17 +540,22 @@ APZCTreeManager::PushStateToWR(wr::Trans
 
   RecursiveMutexAutoLock lock(mTreeLock);
 
   // During the first pass through the tree, we build a cache of guid->HTTN so
   // that we can find the relevant APZC instances quickly in subsequent passes,
   // such as the one below to generate scrollbar transforms. Without this, perf
   // could end up being O(n^2) instead of O(n log n) because we'd have to search
   // the tree to find the corresponding APZC every time we hit a thumb node.
-  std::unordered_map<ScrollableLayerGuid, HitTestingTreeNode*, ScrollableLayerGuidHash> httnMap;
+  // We use the presShell-ignoring map because when we do a lookup in the map
+  // for the scrollbar, we don't have (or care about) the presShellId.
+  std::unordered_map<ScrollableLayerGuid,
+                     HitTestingTreeNode*,
+                     ScrollableLayerGuid::HashIgnoringPresShellFn,
+                     ScrollableLayerGuid::EqualIgnoringPresShellFn> httnMap;
 
   bool activeAnimations = false;
   LayersId lastLayersId{(uint64_t)-1};
   wr::WrPipelineId lastPipelineId;
 
   // We iterate backwards here because the HitTestingTreeNode is optimized
   // for backwards iteration. The equivalent code in AsyncCompositionManager
   // iterates forwards, but the direction shouldn't really matter in practice
@@ -577,20 +584,17 @@ APZCTreeManager::PushStateToWR(wr::Trans
             // that has already been torn down. In that case just skip over
             // those layers.
             return;
           }
           lastPipelineId = wrBridge->PipelineId();
           lastLayersId = aNode->GetLayersId();
         }
 
-        // Use a 0 presShellId because when we do a lookup in this map for the
-        // scrollbar below we don't have (or care about) the presShellId.
-        ScrollableLayerGuid guid(lastLayersId, 0, apzc->GetGuid().mScrollId);
-        httnMap.emplace(guid, aNode);
+        httnMap.emplace(apzc->GetGuid(), aNode);
 
         ParentLayerPoint layerTranslation = apzc->GetCurrentAsyncTransform(
             AsyncPanZoomController::eForCompositing).mTranslation;
         // The positive translation means the painted content is supposed to
         // move down (or to the right), and that corresponds to a reduction in
         // the scroll offset. Since we are effectively giving WR the async
         // scroll delta here, we want to negate the translation.
         ParentLayerPoint asyncScrollDelta = -layerTranslation;
--- a/gfx/layers/apz/src/APZCTreeManager.h
+++ b/gfx/layers/apz/src/APZCTreeManager.h
@@ -47,17 +47,16 @@ class AsyncPanZoomController;
 class APZCTreeManagerParent;
 class APZSampler;
 class APZUpdater;
 class CompositorBridgeParent;
 class OverscrollHandoffChain;
 struct OverscrollHandoffState;
 class FocusTarget;
 struct FlingHandoffState;
-struct ScrollableLayerGuidHash;
 class LayerMetricsWrapper;
 class InputQueue;
 class GeckoContentController;
 class HitTestingTreeNode;
 class WebRenderScrollDataWrapper;
 struct AncestorTransform;
 struct ScrollThumbData;
 
@@ -731,17 +730,19 @@ private:
    * 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;
 
   /* 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, ScrollableLayerGuidHash> mZoomConstraints;
+  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.
    * This must only be accessed on the controller thread.
    */
   KeyboardMap mKeyboardMap;
   /* This tracks the focus targets of chrome and content and whether we have
    * a current focus target or whether we are waiting for a new confirmation.
    */