List all child surfaces (including occluded) in CompositorFrame

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 a0108b7d09c99a6e135a0049d80ef881f8fedb21..98aac33823adc64fc99423bbd8c87624f2ad754f 100644
--- a/cc/surfaces/surface_aggregator.cc
+++ b/cc/surfaces/surface_aggregator.cc
@@ -59,6 +59,10 @@ SurfaceAggregator::~SurfaceAggregator() {
   ProcessAddedAndRemovedSurfaces();
 }
 
+SurfaceAggregator::PrewalkResult::PrewalkResult() : has_copy_requests(false) {}
+
+SurfaceAggregator::PrewalkResult::~PrewalkResult() {}
+
 // Create a clip rect for an aggregated quad from the original clip rect and
 // the clip rect from the surface it's on.
 SurfaceAggregator::ClipData SurfaceAggregator::CalculateClipRect(
@@ -494,7 +498,8 @@ void SurfaceAggregator::ProcessAddedAndRemovedSurfaces() {
 // Walk the Surface tree from surface_id. Validate the resources of the current
 // surface and its descendants, check if there are any copy requests, and
 // return the combined damage rect.
-gfx::Rect SurfaceAggregator::PrewalkTree(SurfaceId surface_id) {
+gfx::Rect SurfaceAggregator::PrewalkTree(SurfaceId surface_id,
+                                         PrewalkResult* result) {
   if (referenced_surfaces_.count(surface_id))
     return gfx::Rect();
   Surface* surface = manager_->GetSurfaceForId(surface_id);
@@ -572,7 +577,7 @@ gfx::Rect SurfaceAggregator::PrewalkTree(SurfaceId surface_id) {
     provider_->DeclareUsedResourcesFromChild(child_id, referenced_resources);
 
   for (const auto& render_pass : frame_data->render_pass_list)
-    has_copy_requests_ |= !render_pass->copy_requests.empty();
+    result->has_copy_requests |= !render_pass->copy_requests.empty();
 
   gfx::Rect damage_rect;
   if (!frame_data->render_pass_list.empty()) {
@@ -586,14 +591,57 @@ gfx::Rect SurfaceAggregator::PrewalkTree(SurfaceId surface_id) {
   SurfaceSet::iterator it =
       referenced_surfaces_.insert(surface->surface_id()).first;
   for (const auto& surface_info : child_surfaces) {
-    gfx::Rect surface_damage = PrewalkTree(surface_info.first);
+    gfx::Rect surface_damage = PrewalkTree(surface_info.first, result);
     damage_rect.Union(
         MathUtil::MapEnclosingClippedRect(surface_info.second, surface_damage));
   }
+
+  for (const SurfaceId surface_id :

[danakj, 2015/11/16 21:45:46]

did you mean const&?

or maybe just auto&?

+       surface_frame->metadata.referenced_surfaces) {
+    if (!contained_surfaces_.count(surface_id)) {
+      result->undrawn_surfaces.insert(surface_id);
+      PrewalkTree(surface_id, result);
+    }
+  }
+
   referenced_surfaces_.erase(it);
   return damage_rect;
 }
 
+void SurfaceAggregator::CopyUnreferencedSurfaces(

[danakj, 2015/11/16 21:45:45]

CopyUndrawnSurfaces?

+    PrewalkResult* prewalk_result) {
+  std::vector<SurfaceId> surfaces_to_copy(

[danakj, 2015/11/16 21:45:45]

Can you leave a comment here to the effect of how the Prewalk step may have
skipped surfaces that would not be drawn, and any such surface would have been
added to this surfaces_to_copy list. But that each of those surfaces may have
children that it would have skipped implicitly, so they won't appear in this
list and we'll have to look for them here?

+      prewalk_result->undrawn_surfaces.begin(),
+      prewalk_result->undrawn_surfaces.end());
+
+  for (size_t i = 0; i < surfaces_to_copy.size(); i++) {
+    SurfaceId surface_id = surfaces_to_copy[i];
+    Surface* surface = manager_->GetSurfaceForId(surface_id);
+    const CompositorFrame* surface_frame = surface->GetEligibleFrame();
+    if (!surface_frame)
+      continue;
+    bool has_copy_requests = false;

[danakj, 2015/11/16 21:45:45]

can you name this somehow differently to not almost-collide with
has_copy_requests_? surface_has_copy_requests?

+    for (const auto& render_pass :
+         surface_frame->delegated_frame_data->render_pass_list) {
+      has_copy_requests |= !render_pass->copy_requests.empty();
+    }
+    if (!has_copy_requests) {
+      // Children may have copy requests, so make sure to check them as well.
+      for (const SurfaceId child_id :

[danakj, 2015/11/16 21:45:45]

ditto?

+           surface_frame->metadata.referenced_surfaces) {
+        if (!prewalk_result->undrawn_surfaces.count(child_id)) {

[danakj, 2015/11/16 21:45:45]

Why this if? How could a child of an undrawn surface be drawn itself?

+          surfaces_to_copy.push_back(child_id);
+          prewalk_result->undrawn_surfaces.insert(child_id);
+        }
+      }
+    } else {
+      SurfaceSet::iterator it = referenced_surfaces_.insert(surface_id).first;
+      CopyPasses(surface_frame->delegated_frame_data.get(), surface);
+      referenced_surfaces_.erase(it);
+    }
+  }
+}
+
 scoped_ptr<CompositorFrame> SurfaceAggregator::Aggregate(SurfaceId surface_id) {
   Surface* surface = manager_->GetSurfaceForId(surface_id);
   DCHECK(surface);
@@ -612,9 +660,11 @@ scoped_ptr<CompositorFrame> SurfaceAggregator::Aggregate(SurfaceId surface_id) {
   dest_pass_list_ = &frame->delegated_frame_data->render_pass_list;
 
   valid_surfaces_.clear();
-  has_copy_requests_ = false;
-  root_damage_rect_ = PrewalkTree(surface_id);
+  PrewalkResult prewalk_result;
+  root_damage_rect_ = PrewalkTree(surface_id, &prewalk_result);
+  has_copy_requests_ = prewalk_result.has_copy_requests;
 
+  CopyUnreferencedSurfaces(&prewalk_result);
   SurfaceSet::iterator it = referenced_surfaces_.insert(surface_id).first;
   CopyPasses(root_surface_frame->delegated_frame_data.get(), surface);
   referenced_surfaces_.erase(it);