cc: Return resources to child compositor via a Callback.

cc/resources/resource_provider.cc

Index: cc/resources/resource_provider.cc
diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc
index 017c2e9cacebe3501bf24af8e4552d7086f12a97..6e7beff473fc306dc17287937b335c2eb4d76684 100644
--- a/cc/resources/resource_provider.cc
+++ b/cc/resources/resource_provider.cc
@@ -205,6 +205,8 @@ scoped_ptr<ResourceProvider> ResourceProvider::Create(
 }
 
 ResourceProvider::~ResourceProvider() {
+  while (!children_.empty())
+    DestroyChild(children_.begin()->first);
   while (!resources_.empty())
     DeleteResourceInternal(resources_.begin(), ForShutdown);
 
@@ -779,9 +781,12 @@ void ResourceProvider::CleanUpGLIfNeeded() {
   Finish();
 }
 
-int ResourceProvider::CreateChild() {
+int ResourceProvider::CreateChild(const ReturnCallback& return_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
+
   Child child_info;
+  child_info.return_callback = return_callback;
+
   int child = next_child_++;
   children_[child] = child_info;
   return child;
@@ -789,20 +794,26 @@ int ResourceProvider::CreateChild() {
 
 void ResourceProvider::DestroyChild(int child_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
+
   ChildMap::iterator it = children_.find(child_id);
   DCHECK(it != children_.end());
   Child& child = it->second;
+
+  ResourceIdArray resources_for_child;
+
   for (ResourceIdMap::iterator child_it = child.child_to_parent_map.begin();
        child_it != child.child_to_parent_map.end();
        ++child_it) {
     ResourceId id = child_it->second;
-    // We're abandoning this resource, it will not get recycled.
-    // crbug.com/224062
-    ResourceMap::iterator resource_it = resources_.find(id);
-    CHECK(resource_it != resources_.end());
-    resource_it->second.imported_count = 0;
-    DeleteResource(id);
+    resources_for_child.push_back(id);
   }
+
+  // If the child is going away, don't consider any resources in use.
+  child.in_use_resources.clear();
+
+  DeleteAndReturnUnusedResourcesToChild(
+      &child, ForShutdown, resources_for_child);
+
   children_.erase(it);
 }
 
@@ -844,62 +855,6 @@ void ResourceProvider::PrepareSendToParent(const ResourceIdArray& resources,
   }
 }
 
-void ResourceProvider::PrepareSendReturnsToChild(
-    int child,
-    const ResourceIdArray& resources,
-    ReturnedResourceArray* list) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  WebGraphicsContext3D* context3d = Context3d();
-  if (!context3d || !context3d->makeContextCurrent()) {
-    // TODO(skaslev): Implement this path for software compositing.
-    return;
-  }
-  Child& child_info = children_.find(child)->second;
-  bool need_sync_point = false;
-  for (ResourceIdArray::const_iterator it = resources.begin();
-       it != resources.end(); ++it) {
-    Resource* resource = GetResource(*it);
-    DCHECK(!resource->locked_for_write);
-    DCHECK(!resource->lock_for_read_count);
-    DCHECK(child_info.parent_to_child_map.find(*it) !=
-           child_info.parent_to_child_map.end());
-
-    if (resource->filter != resource->original_filter) {
-      DCHECK(resource->target);
-      DCHECK(resource->gl_id);
-
-      GLC(context3d, context3d->bindTexture(resource->target, resource->gl_id));
-      GLC(context3d, context3d->texParameteri(resource->target,
-                                              GL_TEXTURE_MIN_FILTER,
-                                              resource->original_filter));
-      GLC(context3d, context3d->texParameteri(resource->target,
-                                              GL_TEXTURE_MAG_FILTER,
-                                              resource->original_filter));
-    }
-
-    ReturnedResource returned;
-    returned.id = child_info.parent_to_child_map[*it];
-    returned.sync_point = resource->mailbox.sync_point();
-    if (!returned.sync_point)
-      need_sync_point = true;
-    returned.count = resource->imported_count;
-    list->push_back(returned);
-
-    child_info.parent_to_child_map.erase(*it);
-    child_info.child_to_parent_map.erase(returned.id);
-    resources_[*it].imported_count = 0;
-    DeleteResource(*it);
-  }
-  if (need_sync_point) {
-    unsigned int sync_point = context3d->insertSyncPoint();
-    for (ReturnedResourceArray::iterator it = list->begin();
-         it != list->end(); ++it) {
-      if (!it->sync_point)
-        it->sync_point = sync_point;
-    }
-  }
-}
-
 void ResourceProvider::ReceiveFromChild(
     int child, const TransferableResourceArray& resources) {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ -931,20 +886,54 @@ void ResourceProvider::ReceiveFromChild(
     GLC(context3d, context3d->bindTexture(GL_TEXTURE_2D, texture_id));
     GLC(context3d, context3d->consumeTextureCHROMIUM(GL_TEXTURE_2D,
                                                      it->mailbox.name));
-    ResourceId id = next_id_++;
-    Resource resource(
-        texture_id, it->size, it->format, it->filter, 0, GL_CLAMP_TO_EDGE,
-        TextureUsageAny);
+    ResourceId local_id = next_id_++;
+    Resource resource(texture_id,
+                      it->size,
+                      it->format,
+                      it->filter,
+                      0,
+                      GL_CLAMP_TO_EDGE,
+                      TextureUsageAny);
     resource.mailbox.SetName(it->mailbox);
+    resource.child_id = child;
     // Don't allocate a texture for a child.
     resource.allocated = true;
     resource.imported_count = 1;
-    resources_[id] = resource;
-    child_info.parent_to_child_map[id] = it->id;
-    child_info.child_to_parent_map[it->id] = id;
+    resources_[local_id] = resource;
+    child_info.parent_to_child_map[local_id] = it->id;
+    child_info.child_to_parent_map[it->id] = local_id;
   }
 }
 
+void ResourceProvider::DeclareUsedResourcesFromChild(
+    int child,
+    const ResourceIdArray& resources_from_child) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  Child& child_info = children_.find(child)->second;
+  child_info.in_use_resources.clear();
+
+  for (size_t i = 0; i < resources_from_child.size(); ++i) {
+    ResourceIdMap::iterator it =
+        child_info.child_to_parent_map.find(resources_from_child[i]);
+    DCHECK(it != child_info.child_to_parent_map.end());
+
+    ResourceId local_id = it->second;
+    child_info.in_use_resources.insert(local_id);
+  }
+
+  ResourceIdArray unused;
+  for (ResourceIdMap::iterator it = child_info.child_to_parent_map.begin();
+       it != child_info.child_to_parent_map.end();
+       ++it) {
+    ResourceId local_id = it->second;
+    bool resource_is_in_use = child_info.in_use_resources.count(local_id);
+    if (!resource_is_in_use)
+      unused.push_back(local_id);
+  }
+  DeleteAndReturnUnusedResourcesToChild(&child_info, Normal, unused);
+}
+
 void ResourceProvider::ReceiveReturnsFromParent(
     const ReturnedResourceArray& resources) {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ -953,16 +942,29 @@ void ResourceProvider::ReceiveReturnsFromParent(
     // TODO(skaslev): Implement this path for software compositing.
     return;
   }
+
+  int child_id = 0;
+  Child* child_info = NULL;
+  ResourceIdArray resources_for_child;
+
   for (ReturnedResourceArray::const_iterator it = resources.begin();
        it != resources.end();
        ++it) {
-    ResourceMap::iterator map_iterator = resources_.find(it->id);
-    DCHECK(map_iterator != resources_.end());
+    ResourceId local_id = it->id;
+    ResourceMap::iterator map_iterator = resources_.find(local_id);
+
+    // Resource was already lost (e.g. it belonged to a child that was
+    // destroyed).
+    if (map_iterator == resources_.end())
+      continue;
+
     Resource* resource = &map_iterator->second;
+
     CHECK_GE(resource->exported_count, it->count);
     resource->exported_count -= it->count;
     if (resource->exported_count)
       continue;
+
     if (resource->gl_id) {
       if (it->sync_point)
         GLC(context3d, context3d->waitSyncPoint(it->sync_point));
@@ -970,8 +972,38 @@ void ResourceProvider::ReceiveReturnsFromParent(
       resource->mailbox =
           TextureMailbox(resource->mailbox.name(), it->sync_point);
     }
-    if (resource->marked_for_deletion)
-      DeleteResourceInternal(map_iterator, Normal);
+
+    if (!resource->marked_for_deletion)
+      continue;
+    if (!resource->child_id)
+      continue;
+
+    // Delete the resource and return it to a child if it came from one.
+    if (resource->child_id != child_id) {
+      ChildMap::iterator child_it = children_.find(resource->child_id);
+      if (child_it == children_.end()) {
+        // The child is gone and has abandoned the resource.

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

How can this happen? If the child is gone, we removed the resource (and you test
for it on l.958), right?

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

Hm.. let me double check.. maybe this condition is wrong in the final form of
this CL.

[danakj, 2013/09/20 17:35:12]

DCHECKing here pointed out that Resource::child_id was not being initialized to
0. Woops. And initializing it to 0 failed some tests, because I wasn't deleting
resources returned from the parent with a child_id = 0. Fixed both of these and
left in the DCHECK.

+        DeleteResourceInternal(map_iterator, Normal);
+        continue;
+      }
+
+      if (child_id) {
+        DCHECK_NE(resources_for_child.size(), 0u);
+        DeleteAndReturnUnusedResourcesToChild(
+            child_info, Normal, resources_for_child);
+        resources_for_child.clear();

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

Not sure how "coherent" the child ids will be in practice during the returns,
and if they're not, what the cost will be to call the callback many times. Maybe
a TODO to investigate? (options are to sort the |resources| by child_id, or to
keep a per-child id map of resources to delete).

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

I thought about a per-child id map but I thought it was kinda gross. Sorting is
a cool idea though!

[danakj, 2013/09/20 17:35:12]

Implementing sorting required some fun with pairs, but should work.

+      }
+
+      child_info = &child_it->second;
+      child_id = resource->child_id;
+    }
+    resources_for_child.push_back(local_id);
+  }
+
+  if (child_id) {
+    DCHECK_NE(resources_for_child.size(), 0u);
+    DeleteAndReturnUnusedResourcesToChild(
+        child_info, Normal, resources_for_child);
   }
 }
 
@@ -1009,6 +1041,93 @@ void ResourceProvider::TransferResource(WebGraphicsContext3D* context,
   }
 }
 
+void ResourceProvider::DeleteAndReturnUnusedResourcesToChild(
+    Child* child_info,
+    DeleteStyle style,
+    const ResourceIdArray& unused) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(child_info);
+
+  if (unused.empty())
+    return;
+
+  WebGraphicsContext3D* context3d = Context3d();
+  if (!context3d || !context3d->makeContextCurrent()) {
+    // TODO(skaslev): Implement this path for software compositing.
+    return;
+  }
+
+  ReturnedResourceArray to_return;
+
+  bool need_sync_point = false;
+  for (size_t i = 0; i < unused.size(); ++i) {
+    ResourceId local_id = unused[i];
+
+    ResourceMap::iterator it = resources_.find(local_id);
+    CHECK(it != resources_.end());
+    Resource& resource = it->second;
+
+    DCHECK(!resource.locked_for_write);
+    DCHECK(!resource.lock_for_read_count);
+    DCHECK_EQ(0u, child_info->in_use_resources.count(local_id));
+    DCHECK(child_info->parent_to_child_map.count(local_id));
+
+    ResourceId child_id = child_info->parent_to_child_map[local_id];
+    DCHECK(child_info->child_to_parent_map.count(child_id));
+
+    bool is_lost = false;
+    if (resource.exported_count > 0) {
+      if (style != ForShutdown) {
+        // Defer this until we receive the resource back from the parent.
+        resource.marked_for_deletion = true;
+        continue;
+      }
+
+      // We still have an exported_count, so we'll have to lose it.
+      is_lost = true;
+    }
+
+    if (resource.filter != resource.original_filter) {
+      DCHECK(resource.target);
+      DCHECK(resource.gl_id);
+
+      GLC(context3d, context3d->bindTexture(resource.target, resource.gl_id));
+      GLC(context3d,
+          context3d->texParameteri(resource.target,
+                                   GL_TEXTURE_MIN_FILTER,
+                                   resource.original_filter));
+      GLC(context3d,
+          context3d->texParameteri(resource.target,
+                                   GL_TEXTURE_MAG_FILTER,
+                                   resource.original_filter));
+    }
+
+    ReturnedResource returned;
+    returned.id = child_id;
+    returned.sync_point = resource.mailbox.sync_point();
+    if (!returned.sync_point)
+      need_sync_point = true;
+    returned.count = resource.imported_count;
+    // TODO(danakj): Save the |is_lost| bit.
+    to_return.push_back(returned);
+
+    child_info->parent_to_child_map.erase(local_id);
+    child_info->child_to_parent_map.erase(child_id);
+    resource.imported_count = 0;
+    DeleteResourceInternal(it, style);
+  }
+  if (need_sync_point) {
+    unsigned int sync_point = context3d->insertSyncPoint();
+    for (size_t i = 0; i < to_return.size(); ++i) {
+      if (!to_return[i].sync_point)
+        to_return[i].sync_point = sync_point;
+    }
+  }
+
+  if (!to_return.empty())
+    child_info->return_callback.Run(to_return);
+}
+
 void ResourceProvider::AcquirePixelBuffer(ResourceId id) {
   Resource* resource = GetResource(id);
   DCHECK(!resource->external);