Use flat_set in SurfaceAggregator.

cc/surfaces/surface_aggregator.cc

 // Copyright 2014 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/surfaces/surface_aggregator.h"
 
 #include <stddef.h>
 
 #include <map>
 
@@ skipping 126 matching lines @@
     return it->second.id;
   }
 
   RenderPassInfo render_pass_info;
   render_pass_info.id = next_render_pass_id_++;
   render_pass_allocator_map_[key] = render_pass_info;
   return render_pass_info.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()));
     if (surface->factory()) {
       provider_->SetChildNeedsSyncTokens(
           child_id, surface->factory()->needs_sync_points());
     }
     surface_id_to_resource_child_id_[surface->surface_id()] = child_id;
     return child_id;
   } else {
@@ skipping 73 matching lines @@
   std::multimap<int, std::unique_ptr<CopyOutputRequest>> copy_requests;
   surface->TakeCopyOutputRequests(&copy_requests);
 
   const RenderPassList& render_pass_list = frame.render_pass_list;
   if (!valid_surfaces_.count(surface->surface_id())) {
     for (auto& request : copy_requests)
       request.second->SendEmptyResult();
     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;
   const ResourceProvider::ResourceIdMap& child_to_parent_map =
       provider_ ? provider_->GetChildToParentMap(ChildIdForSurface(surface))
                 : empty_map;
   bool merge_pass =
       surface_quad->shared_quad_state->opacity == 1.f && copy_requests.empty();
 
   const RenderPassList& referenced_passes = render_pass_list;
@@ skipping 77 matching lines @@
                             clip_rect, dest_pass);
 
     RenderPassDrawQuad* quad =
         dest_pass->CreateAndAppendDrawQuad<RenderPassDrawQuad>();
     quad->SetNew(shared_quad_state, surface_quad->rect,
                  surface_quad->visible_rect, remapped_pass_id, 0, gfx::RectF(),
                  gfx::Size(), gfx::Vector2dF(), gfx::PointF(),
                  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() {
   if (dest_pass_list_->empty())
     return;
 
   RenderPass* root_render_pass = dest_pass_list_->back().get();
   if (root_render_pass->color_space == output_color_space_)
     return;
 
@@ skipping 210 matching lines @@
     }
 
     dest_pass_list_->push_back(std::move(copy_pass));
   }
 }
 
 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);
       }
 
       // Notify client of removed surface.
       Surface* surface_ptr = manager_->GetSurfaceForId(surface.first);
       if (surface_ptr) {
         surface_ptr->RunDrawCallbacks();
       }
@@ skipping 63 matching lines @@
           parent_pass_id(parent_pass_id),
           target_to_surface_transform(target_to_surface_transform) {}
 
     SurfaceId id;
     bool has_moved_pixels;
     int parent_pass_id;
     gfx::Transform target_to_surface_transform;
   };
   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);
     bool has_pixel_moving_filter =
         render_pass->filters.HasFilterThatMovesPixels();
     if (has_pixel_moving_filter)
       moved_pixel_passes_.insert(remapped_pass_id);
     bool in_moved_pixel_pass = has_pixel_moving_filter ||
                                !!moved_pixel_passes_.count(remapped_pass_id);
     for (auto* quad : render_pass->quad_list) {
@@ skipping 48 matching lines @@
   gfx::Rect full_damage;
   if (!frame.render_pass_list.empty()) {
     RenderPass* last_pass = frame.render_pass_list.back().get();
     full_damage = last_pass->output_rect;
     damage_rect =
         DamageRectForSurface(surface, *last_pass, last_pass->output_rect);
   }
 
   // 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,
                     surface_info.parent_pass_id, result);
     if (surface_damage.IsEmpty())
       continue;
     if (surface_info.has_moved_pixels) {
       // Areas outside the rect hit by target_to_surface_transform may be
       // modified if there is a filter that moves pixels.
       damage_rect = full_damage;
@@ skipping 19 matching lines @@
   }
 
   CHECK(debug_weak_this.get());
   for (const auto& render_pass : frame.render_pass_list) {
     if (!render_pass->copy_requests.empty()) {
       int remapped_pass_id = RemapPassId(render_pass->id, surface_id);
       copy_request_passes_.insert(remapped_pass_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;
 }
 
 void SurfaceAggregator::CopyUndrawnSurfaces(PrewalkResult* prewalk_result) {
   // undrawn_surfaces are Surfaces that were identified by prewalk as being
   // referenced by a drawn Surface, but aren't contained in a SurfaceDrawQuad.
   // They need to be iterated over to ensure that any copy requests on them
   // (or on Surfaces they reference) are executed.
@@ skipping 20 matching lines @@
       for (const auto& child_id : frame.metadata.referenced_surfaces) {
         // Don't iterate over the child Surface if it was already listed as a
         // child of a different Surface, or in the case where there's infinite
         // recursion.
         if (!prewalk_result->undrawn_surfaces.count(child_id)) {
           surfaces_to_copy.push_back(child_id);
           prewalk_result->undrawn_surfaces.insert(child_id);
         }
       }
     } 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);
     }
   }
 }
 
 void SurfaceAggregator::PropagateCopyRequestPasses() {
   std::vector<int> copy_requests_to_iterate(copy_request_passes_.begin(),
                                             copy_request_passes_.end());
   while (!copy_requests_to_iterate.empty()) {
@@ skipping 29 matching lines @@
   dest_pass_list_ = &frame.render_pass_list;
 
   valid_surfaces_.clear();
   PrewalkResult prewalk_result;
   root_damage_rect_ = PrewalkTree(surface_id, false, 0, &prewalk_result);
   PropagateCopyRequestPasses();
   has_copy_requests_ = !copy_request_passes_.empty();
   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();
 
   moved_pixel_passes_.clear();
   copy_request_passes_.clear();
   render_pass_dependencies_.clear();
 
   // Remove all render pass mappings that weren't used in the current frame.
   for (auto it = render_pass_allocator_map_.begin();
        it != render_pass_allocator_map_.end();) {
     if (it->second.in_use) {
       it->second.in_use = false;
       it++;
     } else {
       it = render_pass_allocator_map_.erase(it);
     }
   }
 
   DCHECK(referenced_surfaces_.empty());
 
   if (dest_pass_list_->empty())
     return CompositorFrame();
 
   dest_pass_list_ = NULL;
   ProcessAddedAndRemovedSurfaces();
   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);
   }
 
   // TODO(jamesr): Aggregate all resource references into the returned frame's
   // resource list.
 
   // Log UMA stats for SurfaceDrawQuads on the number of surfaces that were
   // aggregated together and any failures.
   UMA_HISTOGRAM_EXACT_LINEAR(kUmaValidSurface, uma_stats_.valid_surface,
                              kUmaStatMaxSurfaces);
   UMA_HISTOGRAM_EXACT_LINEAR(kUmaMissingSurface, uma_stats_.missing_surface,
                              kUmaStatMaxSurfaces);
   UMA_HISTOGRAM_EXACT_LINEAR(kUmaNoActiveFrame, uma_stats_.no_active_frame,
                              kUmaStatMaxSurfaces);
 
   return frame;
 }
 
 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);
   }
 }
 
 void SurfaceAggregator::SetFullDamageForSurface(const SurfaceId& surface_id) {
   auto it = previous_contained_surfaces_.find(surface_id);
   if (it == previous_contained_surfaces_.end())
     return;
   // Set the last drawn index as 0 to ensure full damage next time it's drawn.
   it->second = 0;
 }
 
 void SurfaceAggregator::SetOutputColorSpace(
     const gfx::ColorSpace& blending_color_space,
     const gfx::ColorSpace& output_color_space) {
   blending_color_space_ = blending_color_space;
   output_color_space_ = output_color_space;
 }
 
 }  // namespace cc