Delete old RenderPassId mappings in SurfaceAggregator.

Mappings need to stay the same across frames so textures aren't
unnecessarily reallocated, but they should be deleted once they're not
used to avoid leaking memory.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2544203003
Cr-Commit-Position: refs/heads/master@{#436134}

cc/surfaces/surface_aggregator.cc

@@ -93,29 +93,6 @@ SurfaceAggregator::ClipData SurfaceAggregator::CalculateClipRect(
   return out_clip;
 }
 
-class SurfaceAggregator::RenderPassIdAllocator {
- public:
-  explicit RenderPassIdAllocator(int* next_index) : next_index_(next_index) {}
-  ~RenderPassIdAllocator() {}
-
-  void AddKnownPass(RenderPassId id) {
-    if (id_to_index_map_.find(id) != id_to_index_map_.end())
-      return;
-    id_to_index_map_[id] = (*next_index_)++;
-  }
-
-  RenderPassId Remap(RenderPassId id) {
-    DCHECK(id_to_index_map_.find(id) != id_to_index_map_.end());
-    return RenderPassId(1, id_to_index_map_[id]);
-  }
-
- private:
-  std::unordered_map<RenderPassId, int, RenderPassIdHash> id_to_index_map_;
-  int* next_index_;
-
-  DISALLOW_COPY_AND_ASSIGN(RenderPassIdAllocator);
-};
-
 static void UnrefHelper(base::WeakPtr<SurfaceFactory> surface_factory,
                         const ReturnedResourceArray& resources,
                         BlockingTaskRunner* main_thread_task_runner) {
@@ -125,12 +102,17 @@ static void UnrefHelper(base::WeakPtr<SurfaceFactory> surface_factory,
 
 RenderPassId SurfaceAggregator::RemapPassId(RenderPassId surface_local_pass_id,
                                             const SurfaceId& surface_id) {
-  std::unique_ptr<RenderPassIdAllocator>& allocator =
-      render_pass_allocator_map_[surface_id];
-  if (!allocator)
-    allocator.reset(new RenderPassIdAllocator(&next_render_pass_id_));
-  allocator->AddKnownPass(surface_local_pass_id);
-  return allocator->Remap(surface_local_pass_id);
+  auto key = std::make_pair(surface_id, surface_local_pass_id);
+  auto it = render_pass_allocator_map_.find(key);
+  if (it != render_pass_allocator_map_.end()) {
+    it->second.in_use = true;
+    return it->second.id;
+  }
+
+  RenderPassInfo render_pass_info;
+  render_pass_info.id = RenderPassId(1, next_render_pass_id_++);
+  render_pass_allocator_map_[key] = render_pass_info;
+  return render_pass_info.id;
 }
 
 int SurfaceAggregator::ChildIdForSurface(Surface* surface) {
@@ -792,6 +774,17 @@ CompositorFrame SurfaceAggregator::Aggregate(const SurfaceId& surface_id) {
   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())

cc/surfaces/surface_aggregator.h

@@ -5,6 +5,7 @@
 #ifndef CC_SURFACES_SURFACE_AGGREGATOR_H_
 #define CC_SURFACES_SURFACE_AGGREGATOR_H_
 
+#include <map>
 #include <memory>
 #include <set>
 #include <unordered_map>
@@ -62,6 +63,13 @@ class CC_SURFACES_EXPORT SurfaceAggregator {
     bool may_contain_video = false;
   };
 
+  struct RenderPassInfo {
+    // This is the id the pass is mapped to.
+    RenderPassId id;
+    // This is true if the pass was used in the last aggregated frame.
+    bool in_use = true;
+  };
+
   ClipData CalculateClipRect(const ClipData& surface_clip,
                              const ClipData& quad_clip,
                              const gfx::Transform& target_transform);
@@ -108,11 +116,12 @@ class CC_SURFACES_EXPORT SurfaceAggregator {
   SurfaceManager* manager_;
   ResourceProvider* provider_;
 
-  class RenderPassIdAllocator;
+  // Every Surface has its own RenderPass ID namespace. This structure maps
+  // each source RenderPassID to a unified ID namespace that's used in the
+  // aggregated frame. An entry is removed from the map if it's not used
+  // for one output frame.
   using RenderPassIdAllocatorMap =
-      std::unordered_map<SurfaceId,
-                         std::unique_ptr<RenderPassIdAllocator>,
-                         SurfaceIdHash>;
+      std::map<std::pair<SurfaceId, RenderPassId>, RenderPassInfo>;
   RenderPassIdAllocatorMap render_pass_allocator_map_;
   int next_render_pass_id_;
   const bool aggregate_only_damaged_;

cc/surfaces/surface_aggregator_unittest.cc

@@ -260,6 +260,59 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, MultiPassSimpleFrame) {
   AggregateAndVerify(passes, arraysize(passes), ids, arraysize(ids));
 }
 
+// Ensure that the render pass ID map properly keeps and deletes entries.
+TEST_F(SurfaceAggregatorValidSurfaceTest, MultiPassDeallocation) {
+  test::Quad quads[][2] = {{test::Quad::SolidColorQuad(SK_ColorWHITE),
+                            test::Quad::SolidColorQuad(SK_ColorLTGRAY)},
+                           {test::Quad::SolidColorQuad(SK_ColorGRAY),
+                            test::Quad::SolidColorQuad(SK_ColorDKGRAY)}};
+  test::Pass passes[] = {
+      test::Pass(quads[0], arraysize(quads[0]), RenderPassId(1, 2)),
+      test::Pass(quads[1], arraysize(quads[1]), RenderPassId(1, 1))};
+
+  SubmitCompositorFrame(&factory_, passes, arraysize(passes),
+                        root_local_frame_id_);
+
+  SurfaceId surface_id(factory_.frame_sink_id(), root_local_frame_id_);
+
+  CompositorFrame aggregated_frame;
+  aggregated_frame = aggregator_.Aggregate(surface_id);
+  RenderPassId id0 = aggregated_frame.render_pass_list[0]->id;
+  RenderPassId id1 = aggregated_frame.render_pass_list[1]->id;
+  EXPECT_NE(id1, id0);
+
+  // Aggregated RenderPassIds should remain the same between frames.
+  aggregated_frame = aggregator_.Aggregate(surface_id);
+  EXPECT_EQ(id0, aggregated_frame.render_pass_list[0]->id);
+  EXPECT_EQ(id1, aggregated_frame.render_pass_list[1]->id);
+
+  test::Pass passes2[] = {
+      test::Pass(quads[0], arraysize(quads[0]), RenderPassId(1, 3)),
+      test::Pass(quads[1], arraysize(quads[1]), RenderPassId(1, 1))};
+
+  SubmitCompositorFrame(&factory_, passes2, arraysize(passes2),
+                        root_local_frame_id_);
+
+  // The RenderPass that still exists should keep the same ID.
+  aggregated_frame = aggregator_.Aggregate(surface_id);
+  RenderPassId id2 = aggregated_frame.render_pass_list[0]->id;
+  EXPECT_NE(id2, id1);
+  EXPECT_NE(id2, id0);
+  EXPECT_EQ(id1, aggregated_frame.render_pass_list[1]->id);
+
+  SubmitCompositorFrame(&factory_, passes, arraysize(passes),
+                        root_local_frame_id_);
+
+  // RenderPassId(1, 2) didn't exist in the previous frame, so it should be
+  // mapped to a new ID.
+  aggregated_frame = aggregator_.Aggregate(surface_id);
+  RenderPassId id3 = aggregated_frame.render_pass_list[0]->id;
+  EXPECT_NE(id3, id2);
+  EXPECT_NE(id3, id1);
+  EXPECT_NE(id3, id0);
+  EXPECT_EQ(id1, aggregated_frame.render_pass_list[1]->id);
+}
+
 // This tests very simple embedding. root_surface has a frame containing a few
 // solid color quads and a surface quad referencing embedded_surface.
 // embedded_surface has a frame containing only a solid color quad. The solid