cc: Return resources to child compositor via a Callback.

cc/layers/delegated_renderer_layer_impl.cc

Index: cc/layers/delegated_renderer_layer_impl.cc
diff --git a/cc/layers/delegated_renderer_layer_impl.cc b/cc/layers/delegated_renderer_layer_impl.cc
index b0a1b3ff8b3ab2afed50af65d80dc35c7e7d2975..34a1cd86755949fbe946dba7057a7fc647aba072 100644
--- a/cc/layers/delegated_renderer_layer_impl.cc
+++ b/cc/layers/delegated_renderer_layer_impl.cc
@@ -47,7 +47,7 @@ bool DelegatedRendererLayerImpl::HasContributingDelegatedRenderPasses() const {
 static ResourceProvider::ResourceId ResourceRemapHelper(
     bool* invalid_frame,
     const ResourceProvider::ResourceIdMap& child_to_parent_map,
-    ResourceProvider::ResourceIdSet *remapped_resources,
+    ResourceProvider::ResourceIdArray* resources_in_frame,
     ResourceProvider::ResourceId id) {
 
   ResourceProvider::ResourceIdMap::const_iterator it =
@@ -59,7 +59,7 @@ static ResourceProvider::ResourceId ResourceRemapHelper(
 
   DCHECK_EQ(it->first, id);
   ResourceProvider::ResourceId remapped_id = it->second;
-  remapped_resources->insert(remapped_id);
+  resources_in_frame->push_back(id);
   return remapped_id;
 }
 
@@ -85,23 +85,31 @@ void DelegatedRendererLayerImpl::PushPropertiesTo(LayerImpl* layer) {
     have_render_passes_to_push_ = false;
   }
 
-  // This is just a copy for testing since we keep the data on the pending layer
-  // for returning resources to the child for now.
+  // This is just a copy for testing, since resources are added to the
+  // ResourceProvider in the pending tree.
   delegated_layer->resources_ = resources_;
 }
 
+void DelegatedRendererLayerImpl::CreateChildIdIfNeeded(
+    const ReturnCallback& return_callback) {
+  if (child_id_)
+    return;
+
+  ResourceProvider* resource_provider = layer_tree_impl()->resource_provider();
+  child_id_ = resource_provider->CreateChild(return_callback);
+  own_child_id_ = true;
+}
+
 void DelegatedRendererLayerImpl::SetFrameData(
     scoped_ptr<DelegatedFrameData> frame_data,
     gfx::RectF damage_in_frame) {
   DCHECK(frame_data);
+  DCHECK(child_id_) << "CreateChildIdIfNeeded must be called first.";
 
   // A frame with an empty root render pass is invalid.
   DCHECK(frame_data->render_pass_list.empty() ||
          !frame_data->render_pass_list.back()->output_rect.IsEmpty());
 
-  CreateChildIdIfNeeded();
-  DCHECK(child_id_);
-
   ResourceProvider* resource_provider = layer_tree_impl()->resource_provider();
     const ResourceProvider::ResourceIdMap& resource_map =
         resource_provider->GetChildToParentMap(child_id_);
@@ -109,12 +117,12 @@ void DelegatedRendererLayerImpl::SetFrameData(
   resource_provider->ReceiveFromChild(child_id_, frame_data->resource_list);
 
   bool invalid_frame = false;
-  ResourceProvider::ResourceIdSet used_resources;
+  ResourceProvider::ResourceIdArray resources_in_frame;
   DrawQuad::ResourceIteratorCallback remap_resources_to_parent_callback =
       base::Bind(&ResourceRemapHelper,
                  &invalid_frame,
                  resource_map,
-                 &used_resources);
+                 &resources_in_frame);
   for (size_t i = 0; i < frame_data->render_pass_list.size(); ++i) {
     RenderPass* pass = frame_data->render_pass_list[i];
     for (size_t j = 0; j < pass->quad_list.size(); ++j) {
@@ -123,8 +131,15 @@ void DelegatedRendererLayerImpl::SetFrameData(
     }
   }
 
-  if (invalid_frame)
+  if (invalid_frame) {
+    // Declare we are still using the last frame's resources.
+    resource_provider->DeclareUsedResourcesFromChild(child_id_, resources_);

[piman, 2013/09/20 01:55:01]

Why do we need this? It seems like a noop. We're not changing the list of used
resources in the RP, so we won't have new unused resources. And we didn't have
unused resources before that we can return now, because we return them
immediately as soon as they're unused and back from the parent (so we would have
returned them before if we could).

[danakj, 2013/09/20 14:54:52]

We did ReceiveFromChild above, which adds any resources in the frame to the
ResourceProvider. If we're not going to use the frame, we should return the
unused ones. This does not change which resources are marked as in use, but it
will return refs on resources from the invalid frame that we are not going to
use.

(This is why the rule is ReceiveFromChild always followed by
DeclareUsedResources.)

[piman, 2013/09/20 20:14:57]

Ok, I know what I was missing: ReceiveFromChild adds unused resources, sort of
in a limbo, that are not returned right away (so that we can "attach" to them in
DeclareUsedResourcesFromChild). So if we're not going to attach to them, we
still need DeclareUsedResourcesFromChild so that we can return them immediately.

     return;
+  }
+
+  // Declare we are using the new frame's resources.
+  resources_.swap(resources_in_frame);
+  resource_provider->DeclareUsedResourcesFromChild(child_id_, resources_);
 
   // Display size is already set so we can compute what the damage rect
   // will be in layer space.
@@ -140,33 +155,9 @@ void DelegatedRendererLayerImpl::SetFrameData(
   // Save the remapped quads on the layer. This steals the quads and render
   // passes from the frame_data.
   SetRenderPasses(&frame_data->render_pass_list);
-  resources_.swap(used_resources);
   have_render_passes_to_push_ = true;
 }
 
-void DelegatedRendererLayerImpl::CollectUnusedResources(
-    ReturnedResourceArray* resources_for_ack) {
-  CreateChildIdIfNeeded();
-  DCHECK(child_id_);
-
-  ResourceProvider* resource_provider = layer_tree_impl()->resource_provider();
-    const ResourceProvider::ResourceIdMap& resource_map =
-        resource_provider->GetChildToParentMap(child_id_);
-
-  ResourceProvider::ResourceIdArray unused_resources;
-  for (ResourceProvider::ResourceIdMap::const_iterator it =
-           resource_map.begin();
-       it != resource_map.end();
-       ++it) {
-    bool resource_is_in_current_frame = resources_.count(it->second) > 0;
-    bool resource_is_in_use = resource_provider->InUseByConsumer(it->second);
-    if (!resource_is_in_current_frame && !resource_is_in_use)
-      unused_resources.push_back(it->second);
-  }
-  resource_provider->PrepareSendReturnsToChild(
-      child_id_, unused_resources, resources_for_ack);
-}
-
 void DelegatedRendererLayerImpl::SetDisplaySize(gfx::Size size) {
   if (display_size_ == size)
     return;
@@ -477,15 +468,6 @@ const char* DelegatedRendererLayerImpl::LayerTypeAsString() const {
   return "cc::DelegatedRendererLayerImpl";
 }
 
-void DelegatedRendererLayerImpl::CreateChildIdIfNeeded() {
-  if (child_id_)
-    return;
-
-  ResourceProvider* resource_provider = layer_tree_impl()->resource_provider();
-  child_id_ = resource_provider->CreateChild();
-  own_child_id_ = true;
-}
-
 void DelegatedRendererLayerImpl::ClearChildId() {
   if (!child_id_)
     return;