Use flat_set in SurfaceAggregator.

cc/surfaces/surface_aggregator.cc

Index: cc/surfaces/surface_aggregator.cc
diff --git a/cc/surfaces/surface_aggregator.cc b/cc/surfaces/surface_aggregator.cc
index a5e31ddc480f8a6f040c0b4cb7c8a3ac25e0ce35..ceb5e74eb79518fe7d3adffe25d51171167727be 100644
--- a/cc/surfaces/surface_aggregator.cc
+++ b/cc/surfaces/surface_aggregator.cc
@@ -144,8 +144,7 @@ int SurfaceAggregator::RemapPassId(int surface_local_pass_id,
 }
 
 int SurfaceAggregator::ChildIdForSurface(Surface* surface) {
-  SurfaceToResourceChildIdMap::iterator it =
-      surface_id_to_resource_child_id_.find(surface->surface_id());
+  auto it = surface_id_to_resource_child_id_.find(surface->surface_id());
   if (it == surface_id_to_resource_child_id_.end()) {
     int child_id =
         provider_->CreateChild(base::Bind(&UnrefHelper, surface->factory()));
@@ -239,7 +238,7 @@ void SurfaceAggregator::HandleSurfaceQuad(
     return;
   }
 
-  SurfaceSet::iterator it = referenced_surfaces_.insert(surface_id).first;
+  referenced_surfaces_.insert(surface_id);
   // TODO(vmpstr): provider check is a hack for unittests that don't set up a
   // resource provider.
   ResourceProvider::ResourceIdMap empty_map;
@@ -337,7 +336,8 @@ void SurfaceAggregator::HandleSurfaceQuad(
                  gfx::RectF(surface_quad->rect));
   }
 
-  referenced_surfaces_.erase(it);
+  // Need to re-query since referenced_surfaces_ iterators are not stable.
+  referenced_surfaces_.erase(referenced_surfaces_.find(surface_id));

[danakj, 2017/04/18 17:48:15]

Shouldn't this be base::Erase?

[danakj, 2017/04/18 17:49:12]

Oh, or nvm I guess we only have EraseIf for flat_tree.

[brettw, 2017/04/18 18:26:05]

I thought base::Erase is really a better version of std::remove/remove_if. If
you want to find one thing in a map and delete it, the iterator calls are still
the right thing.

 }
 
 void SurfaceAggregator::AddColorConversionPass() {
@@ -568,8 +568,7 @@ void SurfaceAggregator::ProcessAddedAndRemovedSurfaces() {
   for (const auto& surface : previous_contained_surfaces_) {
     if (!contained_surfaces_.count(surface.first)) {
       // Release resources of removed surface.
-      SurfaceToResourceChildIdMap::iterator it =
-          surface_id_to_resource_child_id_.find(surface.first);
+      auto it = surface_id_to_resource_child_id_.find(surface.first);
       if (it != surface_id_to_resource_child_id_.end()) {
         provider_->DestroyChild(it->second);
         surface_id_to_resource_child_id_.erase(it);
@@ -653,13 +652,18 @@ gfx::Rect SurfaceAggregator::PrewalkTree(const SurfaceId& surface_id,
   };
   std::vector<SurfaceInfo> child_surfaces;
 
-  std::unordered_set<int> pixel_moving_background_filter_passes;
+  // This data is created once and typically small or empty. Collect all items
+  // and pass to a flat_vector to sort once.
+  std::vector<int> pixel_moving_background_filter_passes_data;
   for (const auto& render_pass : frame.render_pass_list) {
     if (render_pass->background_filters.HasFilterThatMovesPixels()) {
-      pixel_moving_background_filter_passes.insert(
+      pixel_moving_background_filter_passes_data.push_back(
           RemapPassId(render_pass->id, surface_id));
     }
   }
+  base::flat_set<int> pixel_moving_background_filter_passes(
+      std::move(pixel_moving_background_filter_passes_data),
+      base::KEEP_FIRST_OF_DUPES);
 
   for (const auto& render_pass : base::Reversed(frame.render_pass_list)) {
     int remapped_pass_id = RemapPassId(render_pass->id, surface_id);
@@ -728,8 +732,7 @@ gfx::Rect SurfaceAggregator::PrewalkTree(const SurfaceId& surface_id,
 
   // Avoid infinite recursion by adding current surface to
   // referenced_surfaces_.
-  SurfaceSet::iterator it =
-      referenced_surfaces_.insert(surface->surface_id()).first;
+  referenced_surfaces_.insert(surface->surface_id());
   for (const auto& surface_info : child_surfaces) {
     gfx::Rect surface_damage =
         PrewalkTree(surface_info.id, surface_info.has_moved_pixels,
@@ -769,7 +772,7 @@ gfx::Rect SurfaceAggregator::PrewalkTree(const SurfaceId& surface_id,
     }
   }
 
-  referenced_surfaces_.erase(it);
+  referenced_surfaces_.erase(referenced_surfaces_.find(surface->surface_id()));
   if (!damage_rect.IsEmpty() && frame.metadata.may_contain_video)
     result->may_contain_video = true;
   return damage_rect;
@@ -810,7 +813,7 @@ void SurfaceAggregator::CopyUndrawnSurfaces(PrewalkResult* prewalk_result) {
         }
       }
     } else {
-      SurfaceSet::iterator it = referenced_surfaces_.insert(surface_id).first;
+      auto it = referenced_surfaces_.insert(surface_id).first;
       CopyPasses(frame, surface);
       referenced_surfaces_.erase(it);
     }
@@ -860,7 +863,7 @@ CompositorFrame SurfaceAggregator::Aggregate(const SurfaceId& surface_id) {
   frame.metadata.may_contain_video = prewalk_result.may_contain_video;
 
   CopyUndrawnSurfaces(&prewalk_result);
-  SurfaceSet::iterator it = referenced_surfaces_.insert(surface_id).first;
+  auto it = referenced_surfaces_.insert(surface_id).first;
   CopyPasses(root_surface_frame, surface);
   referenced_surfaces_.erase(it);
   AddColorConversionPass();
@@ -890,9 +893,7 @@ CompositorFrame SurfaceAggregator::Aggregate(const SurfaceId& surface_id) {
   contained_surfaces_.swap(previous_contained_surfaces_);
   contained_surfaces_.clear();
 
-  for (SurfaceIndexMap::iterator it = previous_contained_surfaces_.begin();
-       it != previous_contained_surfaces_.end();
-       ++it) {
+  for (auto it : previous_contained_surfaces_) {
     Surface* surface = manager_->GetSurfaceForId(it->first);
     if (surface)
       surface->TakeLatencyInfo(&frame.metadata.latency_info);
@@ -914,8 +915,7 @@ CompositorFrame SurfaceAggregator::Aggregate(const SurfaceId& surface_id) {
 }
 
 void SurfaceAggregator::ReleaseResources(const SurfaceId& surface_id) {
-  SurfaceToResourceChildIdMap::iterator it =
-      surface_id_to_resource_child_id_.find(surface_id);
+  auto it = surface_id_to_resource_child_id_.find(surface_id);
   if (it != surface_id_to_resource_child_id_.end()) {
     provider_->DestroyChild(it->second);
     surface_id_to_resource_child_id_.erase(it);