cc: Plumbing for BeginFrameSource based on Surfaces

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 69618ba8b4ca4507f77705db7fa2834e2a07e452..d60db1d1e3ca33f26f9e7c667270027ede843fd1 100644
--- a/cc/surfaces/surface_aggregator.cc
+++ b/cc/surfaces/surface_aggregator.cc
@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/containers/hash_tables.h"
 #include "base/logging.h"
+#include "base/stl_util.h"
 #include "base/trace_event/trace_event.h"
 #include "cc/base/math_util.h"
 #include "cc/output/compositor_frame.h"
@@ -40,17 +41,26 @@ void MoveMatchingRequests(
 
 }  // namespace
 
-SurfaceAggregator::SurfaceAggregator(SurfaceManager* manager,
+SurfaceAggregator::SurfaceAggregator(SurfaceAggregatorClient* client,
+                                     SurfaceManager* manager,
                                      ResourceProvider* provider,
                                      bool aggregate_only_damaged)
-    : manager_(manager),
+    : client_(client),
+      manager_(manager),
       provider_(provider),
       next_render_pass_id_(1),
       aggregate_only_damaged_(aggregate_only_damaged) {
   DCHECK(manager_);
 }
 
-SurfaceAggregator::~SurfaceAggregator() {}
+SurfaceAggregator::~SurfaceAggregator() {

[mithro-old, 2015/10/01 03:00:23]

I feel like this should be calling ProcessAddedAndRemovedSurfaces() after
resetting contained_surfaces to null? Would that work?

[brianderson, 2015/10/07 20:54:48]

+jbauman: wdyt? Doing that sounds like a good idea, but it would also release
resources of the previously_contained_surfaces. Is there already code elsewhere
that releases resources before we get to this destructor?

[jbauman, 2015/10/13 20:23:20]

I think that should work. I just checked and the destructor order happening with
the cc::Display makes sense.

+  // Notify client of all surfaces being removed.
+  for (const auto& surface : previous_contained_surfaces_) {
+    Surface* surface_ptr = manager_->GetSurfaceForId(surface.first);
+    if (surface_ptr)
+      client_->RemoveSurface(surface_ptr);
+  }
+}
 
 // Create a clip rect for an aggregated quad from the original clip rect and
 // the clip rect from the surface it's on.
@@ -135,14 +145,18 @@ int SurfaceAggregator::ChildIdForSurface(Surface* surface) {
   }
 }
 
+gfx::Rect SurfaceAggregator::DamageRectForSurface(
+    const Surface* surface,
+    const RenderPass& source,
+    const gfx::Rect& full_rect) const {
+  auto it = previous_contained_surfaces_.find(surface->surface_id());
+  if (it == previous_contained_surfaces_.end())
+    return full_rect;
 
-gfx::Rect SurfaceAggregator::DamageRectForSurface(const Surface* surface,
-                                                  const RenderPass& source,
-                                                  const gfx::Rect& full_rect) {
-  int previous_index = previous_contained_surfaces_[surface->surface_id()];
+  int previous_index = it->second;
   if (previous_index == surface->frame_index())
     return gfx::Rect();
-  else if (previous_index == surface->frame_index() - 1)
+  if (previous_index == surface->frame_index() - 1)
     return source.damage_rect;
   return full_rect;
 }
@@ -450,9 +464,10 @@ void SurfaceAggregator::CopyPasses(const DelegatedFrameData* frame_data,
   }
 }
 
-void SurfaceAggregator::RemoveUnreferencedChildren() {
+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);
       if (it != surface_id_to_resource_child_id_.end()) {
@@ -460,9 +475,21 @@ void SurfaceAggregator::RemoveUnreferencedChildren() {
         surface_id_to_resource_child_id_.erase(it);
       }
 
+      // Notify client of removed surface.
       Surface* surface_ptr = manager_->GetSurfaceForId(surface.first);
-      if (surface_ptr)
+      if (surface_ptr) {
         surface_ptr->RunDrawCallbacks(SurfaceDrawStatus::DRAW_SKIPPED);
+        client_->RemoveSurface(surface_ptr);
+      }
+    }
+  }
+
+  for (const auto& surface : contained_surfaces_) {
+    if (!previous_contained_surfaces_.count(surface.first)) {
+      // Notify client of added surface.
+      Surface* surface_ptr = manager_->GetSurfaceForId(surface.first);
+      if (surface_ptr)
+        client_->AddSurface(surface_ptr);
     }
   }
 }
@@ -602,7 +629,7 @@ scoped_ptr<CompositorFrame> SurfaceAggregator::Aggregate(SurfaceId surface_id) {
   dest_pass_list_->back()->damage_rect = root_damage_rect_;
 
   dest_pass_list_ = NULL;
-  RemoveUnreferencedChildren();
+  ProcessAddedAndRemovedSurfaces();
   contained_surfaces_.swap(previous_contained_surfaces_);
   contained_surfaces_.clear();