Refactor scrollbar maps from multimap & std::set to simpler flat_{map, set}

cc/trees/layer_tree_impl.cc

 // Copyright 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/trees/layer_tree_impl.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 1648 matching lines @@
       std::find(picture_layers_.begin(), picture_layers_.end(), layer);
   DCHECK(it != picture_layers_.end());
   picture_layers_.erase(it);
 }
 
 void LayerTreeImpl::RegisterScrollbar(ScrollbarLayerImplBase* scrollbar_layer) {
   ElementId scroll_element_id = scrollbar_layer->scroll_element_id();
   if (!scroll_element_id)
     return;
 
-  element_id_to_scrollbar_layer_ids_.insert(
-      std::pair<ElementId, int>(scroll_element_id, scrollbar_layer->id()));
+  if (scrollbar_layer->orientation() == HORIZONTAL) {
+    element_id_to_scrollbar_layer_ids_[scroll_element_id].horizontal =

[enne (OOO), 2017/05/22 17:05:52]

If there was a previous scrollbar, does that need to be unregistered? I think
previously you could have two scrollbars for the same element id and it wouldn't
clobber.

Maybe this just needs to DCHECK instead?

[pdr., 2017/05/22 17:30:20]

Good point, added a DCHECK.

+        scrollbar_layer->id();
+  } else {
+    element_id_to_scrollbar_layer_ids_[scroll_element_id].vertical =
+        scrollbar_layer->id();
+  }
+
   if (IsActiveTree() && scrollbar_layer->is_overlay_scrollbar()) {
     layer_tree_host_impl_->RegisterScrollbarAnimationController(
         scroll_element_id, scrollbar_layer->Opacity());
   }
 
   // TODO(pdr): Refactor DidUpdateScrollState to use ElementIds instead of
   // layer ids and remove this use of LayerIdByElementId.
   DidUpdateScrollState(LayerIdByElementId(scroll_element_id));
 }
 
 void LayerTreeImpl::UnregisterScrollbar(
     ScrollbarLayerImplBase* scrollbar_layer) {
   ElementId scroll_element_id = scrollbar_layer->scroll_element_id();
   if (!scroll_element_id)
     return;
 
-  auto scrollbar_range =
-      element_id_to_scrollbar_layer_ids_.equal_range(scroll_element_id);
-  for (auto i = scrollbar_range.first; i != scrollbar_range.second; ++i)
-    if (i->second == scrollbar_layer->id()) {
-      element_id_to_scrollbar_layer_ids_.erase(i);
-      break;
+  auto& scrollbar_ids = element_id_to_scrollbar_layer_ids_[scroll_element_id];
+  if (scrollbar_layer->orientation() == HORIZONTAL)
+    scrollbar_ids.horizontal = Layer::INVALID_ID;
+  else
+    scrollbar_ids.vertical = Layer::INVALID_ID;
+
+  if (scrollbar_ids.horizontal == Layer::INVALID_ID &&
+      scrollbar_ids.vertical == Layer::INVALID_ID) {
+    element_id_to_scrollbar_layer_ids_.erase(scroll_element_id);
+    if (IsActiveTree()) {
+      layer_tree_host_impl_->UnregisterScrollbarAnimationController(
+          scroll_element_id);
     }
-
-  if (IsActiveTree() &&
-      element_id_to_scrollbar_layer_ids_.count(scroll_element_id) == 0) {
-    layer_tree_host_impl_->UnregisterScrollbarAnimationController(
-        scroll_element_id);
   }
 }
 
 ScrollbarSet LayerTreeImpl::ScrollbarsFor(ElementId scroll_element_id) const {
   ScrollbarSet scrollbars;
-  auto scrollbar_range =
-      element_id_to_scrollbar_layer_ids_.equal_range(scroll_element_id);
-  for (auto i = scrollbar_range.first; i != scrollbar_range.second; ++i)
-    scrollbars.insert(LayerById(i->second)->ToScrollbarLayer());
+  auto it = element_id_to_scrollbar_layer_ids_.find(scroll_element_id);
+  if (it != element_id_to_scrollbar_layer_ids_.end()) {
+    const ScrollbarLayerIds& layer_ids = it->second;
+    if (layer_ids.horizontal != Layer::INVALID_ID)
+      scrollbars.insert(LayerById(layer_ids.horizontal)->ToScrollbarLayer());
+    if (layer_ids.vertical != Layer::INVALID_ID)
+      scrollbars.insert(LayerById(layer_ids.vertical)->ToScrollbarLayer());
+  }
   return scrollbars;
 }
 
 void LayerTreeImpl::RegisterScrollLayer(LayerImpl* layer) {
   if (layer->scroll_clip_layer_id() == Layer::INVALID_ID)
     return;
 
   clip_scroll_map_.insert(
       std::pair<int, int>(layer->scroll_clip_layer_id(), layer->id()));
 
@@ skipping 390 matching lines @@
 
 void LayerTreeImpl::ResetAllChangeTracking() {
   layers_that_should_push_properties_.clear();
   // Iterate over all layers, including masks.
   for (auto& layer : *layers_)
     layer->ResetChangeTracking();
   property_trees_.ResetAllChangeTracking();
 }
 
 }  // namespace cc