Refactor LayerTreeImpl's scrollbar map to be keyed on element ids

cc/trees/layer_tree_impl.h

Index: cc/trees/layer_tree_impl.h
diff --git a/cc/trees/layer_tree_impl.h b/cc/trees/layer_tree_impl.h
index 1d785f86e4e06ad3072424ea5352207ac8521fb6..6b4f6ab07ffbf7f2c5095beed73978b9098904fe 100644
--- a/cc/trees/layer_tree_impl.h
+++ b/cc/trees/layer_tree_impl.h
@@ -95,7 +95,7 @@ class CC_EXPORT LayerTreeImpl {
   gfx::Size DrawViewportSize() const;
   const gfx::Rect ViewportRectForTilePriority() const;
   std::unique_ptr<ScrollbarAnimationController>
-  CreateScrollbarAnimationController(int scroll_layer_id);
+  CreateScrollbarAnimationController(ElementId scroll_element_id);
   void DidAnimateScrollOffset();
   bool use_gpu_rasterization() const;
   GpuRasterizationStatus GetGpuRasterizationStatus() const;
@@ -395,7 +395,7 @@ class CC_EXPORT LayerTreeImpl {
 
   void RegisterScrollbar(ScrollbarLayerImplBase* scrollbar_layer);
   void UnregisterScrollbar(ScrollbarLayerImplBase* scrollbar_layer);
-  ScrollbarSet ScrollbarsFor(int scroll_layer_id) const;
+  ScrollbarSet ScrollbarsFor(ElementId scroll_element_id) const;
 
   void RegisterScrollLayer(LayerImpl* layer);
   void UnregisterScrollLayer(LayerImpl* layer);
@@ -532,11 +532,11 @@ class CC_EXPORT LayerTreeImpl {
   // derived from LayerImpl::scroll_clip_layer_ and exists to avoid O(n) walks.)
   std::unordered_map<int, int> clip_scroll_map_;
 
-  // Maps scroll layer ids to scrollbar layer ids.  For each scroll layer, there
-  // may be 1 or 2 scrollbar layers (for vertical and horizontal).  (This is
-  // derived from ScrollbarLayerImplBase::scroll_layer_id_ and exists to avoid
-  // O(n) walks.)
-  std::multimap<int, int> scrollbar_map_;
+  // Maps scroll element ids to scrollbar layer ids. For each scroll layer,
+  // there may be 1 or 2 scrollbar layers (for vertical and horizontal). (This
+  // is derived from ScrollbarLayerImplBase::scroll_element_id_ and exists to
+  // avoid O(n) walks.)
+  std::multimap<ElementId, int> scrollbar_map_;

[wkorman, 2017/04/17 17:44:35]

Suggestion, consider rename to something like element_id_to_scroll_layer_ids,
which makes code referencing more self documenting. We've been using this for
newer named maps in cc.

[wkorman, 2017/04/17 17:44:35]

Future optimization potential -- similar to recent brettw@ change, maybe we
should look at internal impl of multimap, since we know there will never be more
than two, I expect this has inefficient memory overhead.

Could switch to analogue of flat_map for multimap, or a map whose value is a
tuple/vector/array-of-two.

See https://codereview.chromium.org/2801603010

[pdr., 2017/04/19 18:02:18]

Switched name from scrollbar_map_ to element_id_to_scrollbar_layer_ids_.

Yeah I think a flat_map and/or map whose value is a pair would be ideal here.
Mind if I pursue this in a followup? I'd like to keep this patch as just the
layer id -> element id change. I've added a TODO above
element_id_to_scrollbar_layer_ids_ to update to a better datastructure.

[wkorman, 2017/04/19 18:07:09]

Yeah, definitely better as separate change.

 
   std::vector<PictureLayerImpl*> picture_layers_;
   LayerImplList surface_layers_;