cc: refcount resources as we send them to the parent

This allows us to fully specify the resources used by the frame, so that we can:
- allow more than one frame in flight
- allow duplicating the frame for aura::Window::RecreateLayers

For every frame we generate, we increase the "exported_count" in the child.
Every time we import the resources for a frame, we increase the "imported count"
in the parent. The contract is that we return resources exactly once for every
time it's exported.

BUG=263068

Review URL: https://chromiumcodereview.appspot.com/23023005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@218173 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 209 insertions, 98 deletions

diff --git a/cc/layers/delegated_renderer_layer.cc b/cc/layers/delegated_renderer_layer.cc
index 66064d9..45a432f 100644
--- a/cc/layers/delegated_renderer_layer.cc
+++ b/cc/layers/delegated_renderer_layer.cc
@@ -68,27 +68,18 @@ void DelegatedRendererLayer::SetDisplaySize(gfx::Size size) {
 void DelegatedRendererLayer::SetFrameData(
     scoped_ptr<DelegatedFrameData> new_frame_data) {
   if (frame_data_) {
-    // Copy the resources from the last provided frame into the new frame, as
-    // it may use resources that were transferred in the last frame.
-    new_frame_data->resource_list.insert(new_frame_data->resource_list.end(),
-                                         frame_data_->resource_list.begin(),
-                                         frame_data_->resource_list.end());
+    // Copy the resources from the last provided frame into the unused resources
+    // list, as the new frame will provide its own resources.
+    unused_resources_for_child_compositor_.insert(
+        unused_resources_for_child_compositor_.end(),
+        frame_data_->resource_list.begin(),
+        frame_data_->resource_list.end());
   }
   frame_data_ = new_frame_data.Pass();
   if (!frame_data_->render_pass_list.empty()) {
     RenderPass* root_pass = frame_data_->render_pass_list.back();
     damage_in_frame_.Union(root_pass->damage_rect);
     frame_size_ = root_pass->output_rect.size();
-
-    // TODO(danakj): This could be optimized to only add resources to the
-    // frame_data_ if they are actually used in the frame. For now, it will
-    // cause the parent (this layer) to hold onto some resources it doesn't
-    // need to for an extra frame.
-    for (size_t i = 0; i < unused_resources_for_child_compositor_.size(); ++i) {
-      frame_data_->resource_list.push_back(
-          unused_resources_for_child_compositor_[i]);
-    }
-    unused_resources_for_child_compositor_.clear();
   } else {
     frame_size_ = gfx::Size();
   }
diff --git a/cc/output/delegating_renderer_unittest.cc b/cc/output/delegating_renderer_unittest.cc
index 6cb532f..b1e7c77 100644
--- a/cc/output/delegating_renderer_unittest.cc
+++ b/cc/output/delegating_renderer_unittest.cc
@@ -129,11 +129,12 @@ class DelegatingRendererTestResources : public DelegatingRendererTest {
 
     EXPECT_EQ(2u, last_frame.delegated_frame_data->render_pass_list.size());
     // Each render pass has 10 resources in it. And the root render pass has a
-    // mask resource used when drawing the child render pass. The number 10 may
-    // change if AppendOneOfEveryQuadType() is updated, and the value here
-    // should be updated accordingly.
+    // mask resource used when drawing the child render pass, as well as its
+    // replica (it's added twice). The number 10 may change if
+    // AppendOneOfEveryQuadType() is updated, and the value here should be
+    // updated accordingly.
     EXPECT_EQ(
-        21u, last_frame.delegated_frame_data->resource_list.size());
+        22u, last_frame.delegated_frame_data->resource_list.size());
 
     EndTest();
   }
diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc
index 0296dd4..b39908a 100644
--- a/cc/resources/resource_provider.cc
+++ b/cc/resources/resource_provider.cc
@@ -73,9 +73,10 @@ ResourceProvider::Resource::Resource()
       pixels(NULL),
       pixel_buffer(NULL),
       lock_for_read_count(0),
+      imported_count(0),
+      exported_count(0),
       locked_for_write(false),
       external(false),
-      exported(false),
       marked_for_deletion(false),
       pending_set_pixels(false),
       set_pixels_completion_forced(false),
@@ -105,9 +106,10 @@ ResourceProvider::Resource::Resource(
       pixels(NULL),
       pixel_buffer(NULL),
       lock_for_read_count(0),
+      imported_count(0),
+      exported_count(0),
       locked_for_write(false),
       external(false),
-      exported(false),
       marked_for_deletion(false),
       pending_set_pixels(false),
       set_pixels_completion_forced(false),
@@ -130,9 +132,10 @@ ResourceProvider::Resource::Resource(
       pixels(pixels),
       pixel_buffer(NULL),
       lock_for_read_count(0),
+      imported_count(0),
+      exported_count(0),
       locked_for_write(false),
       external(false),
-      exported(false),
       marked_for_deletion(false),
       pending_set_pixels(false),
       set_pixels_completion_forced(false),
@@ -184,7 +187,7 @@ bool ResourceProvider::InUseByConsumer(ResourceId id) {
   ResourceMap::iterator it = resources_.find(id);
   CHECK(it != resources_.end());
   Resource* resource = &it->second;
-  return !!resource->lock_for_read_count || resource->exported;
+  return resource->lock_for_read_count > 0 || resource->exported_count > 0;
 }
 
 ResourceProvider::ResourceId ResourceProvider::CreateResource(
@@ -304,9 +307,10 @@ void ResourceProvider::DeleteResource(ResourceId id) {
   Resource* resource = &it->second;
   DCHECK(!resource->lock_for_read_count);
   DCHECK(!resource->marked_for_deletion);
+  DCHECK_EQ(resource->imported_count, 0);
   DCHECK(resource->pending_set_pixels || !resource->locked_for_write);
 
-  if (resource->exported) {
+  if (resource->exported_count > 0) {
     resource->marked_for_deletion = true;
     return;
   } else {
@@ -319,8 +323,8 @@ void ResourceProvider::DeleteResourceInternal(ResourceMap::iterator it,
   Resource* resource = &it->second;
   bool lost_resource = lost_output_surface_;
 
-  DCHECK(!resource->exported || style != Normal);
-  if (style == ForShutdown && resource->exported)
+  DCHECK(resource->exported_count == 0 || style != Normal);
+  if (style == ForShutdown && resource->exported_count > 0)
     lost_resource = true;
 
   if (resource->image_id) {
@@ -391,7 +395,7 @@ void ResourceProvider::SetPixels(ResourceId id,
   DCHECK(!resource->locked_for_write);
   DCHECK(!resource->lock_for_read_count);
   DCHECK(!resource->external);
-  DCHECK(!resource->exported);
+  DCHECK_EQ(resource->exported_count, 0);
   DCHECK(ReadLockFenceHasPassed(resource));
   LazyAllocate(resource);
 
@@ -498,7 +502,7 @@ const ResourceProvider::Resource* ResourceProvider::LockForRead(ResourceId id) {
          resource->set_pixels_completion_forced) <<
       "locked for write: " << resource->locked_for_write <<
       " pixels completion forced: " << resource->set_pixels_completion_forced;
-  DCHECK(!resource->exported);
+  DCHECK_EQ(resource->exported_count, 0);
   // Uninitialized! Call SetPixels or LockForWrite first.
   DCHECK(resource->allocated);
 
@@ -534,7 +538,7 @@ void ResourceProvider::UnlockForRead(ResourceId id) {
   CHECK(it != resources_.end());
   Resource* resource = &it->second;
   DCHECK_GT(resource->lock_for_read_count, 0);
-  DCHECK(!resource->exported);
+  DCHECK_EQ(resource->exported_count, 0);
   resource->lock_for_read_count--;
 }
 
@@ -546,7 +550,7 @@ const ResourceProvider::Resource* ResourceProvider::LockForWrite(
   Resource* resource = &it->second;
   DCHECK(!resource->locked_for_write);
   DCHECK(!resource->lock_for_read_count);
-  DCHECK(!resource->exported);
+  DCHECK_EQ(resource->exported_count, 0);
   DCHECK(!resource->external);
   DCHECK(ReadLockFenceHasPassed(resource));
   LazyAllocate(resource);
@@ -562,7 +566,7 @@ bool ResourceProvider::CanLockForWrite(ResourceId id) {
   Resource* resource = &it->second;
   return !resource->locked_for_write &&
       !resource->lock_for_read_count &&
-      !resource->exported &&
+      !resource->exported_count &&
       !resource->external &&
       ReadLockFenceHasPassed(resource);
 }
@@ -573,7 +577,7 @@ void ResourceProvider::UnlockForWrite(ResourceId id) {
   CHECK(it != resources_.end());
   Resource* resource = &it->second;
   DCHECK(resource->locked_for_write);
-  DCHECK(!resource->exported);
+  DCHECK_EQ(resource->exported_count, 0);
   DCHECK(!resource->external);
   resource->locked_for_write = false;
 }
@@ -766,8 +770,15 @@ void ResourceProvider::DestroyChild(int child_id) {
   Child& child = it->second;
   for (ResourceIdMap::iterator child_it = child.child_to_parent_map.begin();
        child_it != child.child_to_parent_map.end();
-       ++child_it)
-    DeleteResource(child_it->second);
+       ++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);
+  }
   children_.erase(it);
 }
 
@@ -792,12 +803,11 @@ void ResourceProvider::PrepareSendToParent(const ResourceIdArray& resources,
        it != resources.end();
        ++it) {
     TransferableResource resource;
-    if (TransferResource(context3d, *it, &resource)) {
-      if (!resource.sync_point)
-        need_sync_point = true;
-      resources_.find(*it)->second.exported = true;
-      list->push_back(resource);
-    }
+    TransferResource(context3d, *it, &resource);
+    if (!resource.sync_point)
+      need_sync_point = true;
+    ++resources_.find(*it)->second.exported_count;
+    list->push_back(resource);
   }
   if (need_sync_point) {
     unsigned int sync_point = context3d->insertSyncPoint();
@@ -825,8 +835,7 @@ void ResourceProvider::PrepareSendToChild(int child,
        it != resources.end();
        ++it) {
     TransferableResource resource;
-    if (!TransferResource(context3d, *it, &resource))
-      NOTREACHED();
+    TransferResource(context3d, *it, &resource);
     if (!resource.sync_point)
       need_sync_point = true;
     DCHECK(child_info.parent_to_child_map.find(*it) !=
@@ -834,7 +843,9 @@ void ResourceProvider::PrepareSendToChild(int child,
     resource.id = child_info.parent_to_child_map[*it];
     child_info.parent_to_child_map.erase(*it);
     child_info.child_to_parent_map.erase(resource.id);
-    list->push_back(resource);
+    for (int i = 0; i < resources_[*it].imported_count; ++i)
+      list->push_back(resource);
+    resources_[*it].imported_count = 0;
     DeleteResource(*it);
   }
   if (need_sync_point) {
@@ -860,6 +871,12 @@ void ResourceProvider::ReceiveFromChild(
   for (TransferableResourceArray::const_iterator it = resources.begin();
        it != resources.end();
        ++it) {
+    ResourceIdMap::iterator resource_in_map_it =
+        child_info.child_to_parent_map.find(it->id);
+    if (resource_in_map_it != child_info.child_to_parent_map.end()) {
+      resources_[resource_in_map_it->second].imported_count++;
+      continue;
+    }
     unsigned texture_id;
     // NOTE: If the parent is a browser and the child a renderer, the parent
     // is not supposed to have its context wait, because that could induce
@@ -879,6 +896,7 @@ void ResourceProvider::ReceiveFromChild(
     resource.mailbox.SetName(it->mailbox);
     // 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;
@@ -899,8 +917,10 @@ void ResourceProvider::ReceiveFromParent(
     ResourceMap::iterator map_iterator = resources_.find(it->id);
     DCHECK(map_iterator != resources_.end());
     Resource* resource = &map_iterator->second;
-    DCHECK(resource->exported);
-    resource->exported = false;
+    DCHECK_GT(resource->exported_count, 0);
+    --resource->exported_count;
+    if (resource->exported_count)
+      continue;
     resource->filter = it->filter;
     DCHECK(resource->mailbox.ContainsMailbox(it->mailbox));
     if (resource->gl_id) {
@@ -916,7 +936,7 @@ void ResourceProvider::ReceiveFromParent(
   }
 }
 
-bool ResourceProvider::TransferResource(WebGraphicsContext3D* context,
+void ResourceProvider::TransferResource(WebGraphicsContext3D* context,
                                         ResourceId id,
                                         TransferableResource* resource) {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ -927,8 +947,6 @@ bool ResourceProvider::TransferResource(WebGraphicsContext3D* context,
   DCHECK(!source->lock_for_read_count);
   DCHECK(!source->external || (source->external && source->mailbox.IsValid()));
   DCHECK(source->allocated);
-  if (source->exported)
-    return false;
   resource->id = id;
   resource->format = source->format;
   resource->filter = source->filter;
@@ -953,8 +971,6 @@ bool ResourceProvider::TransferResource(WebGraphicsContext3D* context,
     resource->sync_point = source->mailbox.sync_point();
     source->mailbox.ResetSyncPoint();
   }
-
-  return true;
 }
 
 void ResourceProvider::AcquirePixelBuffer(ResourceId id) {
@@ -963,7 +979,7 @@ void ResourceProvider::AcquirePixelBuffer(ResourceId id) {
   CHECK(it != resources_.end());
   Resource* resource = &it->second;
   DCHECK(!resource->external);
-  DCHECK(!resource->exported);
+  DCHECK_EQ(resource->exported_count, 0);
   DCHECK(!resource->image_id);
 
   if (resource->type == GLTexture) {
@@ -996,7 +1012,7 @@ void ResourceProvider::ReleasePixelBuffer(ResourceId id) {
   CHECK(it != resources_.end());
   Resource* resource = &it->second;
   DCHECK(!resource->external);
-  DCHECK(!resource->exported);
+  DCHECK_EQ(resource->exported_count, 0);
   DCHECK(!resource->image_id);
 
   // The pixel buffer can be released while there is a pending "set pixels"
@@ -1041,7 +1057,7 @@ uint8_t* ResourceProvider::MapPixelBuffer(ResourceId id) {
   CHECK(it != resources_.end());
   Resource* resource = &it->second;
   DCHECK(!resource->external);
-  DCHECK(!resource->exported);
+  DCHECK_EQ(resource->exported_count, 0);
   DCHECK(!resource->image_id);
 
   if (resource->type == GLTexture) {
@@ -1072,7 +1088,7 @@ void ResourceProvider::UnmapPixelBuffer(ResourceId id) {
   CHECK(it != resources_.end());
   Resource* resource = &it->second;
   DCHECK(!resource->external);
-  DCHECK(!resource->exported);
+  DCHECK_EQ(resource->exported_count, 0);
   DCHECK(!resource->image_id);
 
   if (resource->type == GLTexture) {
@@ -1339,7 +1355,7 @@ void ResourceProvider::AcquireImage(ResourceId id) {
   Resource* resource = &it->second;
 
   DCHECK(!resource->external);
-  DCHECK(!resource->exported);
+  DCHECK_EQ(resource->exported_count, 0);
 
   if (resource->type != GLTexture)
     return;
@@ -1363,7 +1379,7 @@ void ResourceProvider::ReleaseImage(ResourceId id) {
   Resource* resource = &it->second;
 
   DCHECK(!resource->external);
-  DCHECK(!resource->exported);
+  DCHECK_EQ(resource->exported_count, 0);
 
   if (!resource->image_id)
     return;
@@ -1383,7 +1399,7 @@ uint8_t* ResourceProvider::MapImage(ResourceId id) {
 
   DCHECK(ReadLockFenceHasPassed(resource));
   DCHECK(!resource->external);
-  DCHECK(!resource->exported);
+  DCHECK_EQ(resource->exported_count, 0);
 
   if (resource->image_id) {
     WebGraphicsContext3D* context3d = Context3d();
@@ -1405,7 +1421,7 @@ void ResourceProvider::UnmapImage(ResourceId id) {
   Resource* resource = &it->second;
 
   DCHECK(!resource->external);
-  DCHECK(!resource->exported);
+  DCHECK_EQ(resource->exported_count, 0);
 
   if (resource->image_id) {
     WebGraphicsContext3D* context3d = Context3d();
@@ -1421,7 +1437,7 @@ int ResourceProvider::GetImageStride(ResourceId id) {
   Resource* resource = &it->second;
 
   DCHECK(!resource->external);
-  DCHECK(!resource->exported);
+  DCHECK_EQ(resource->exported_count, 0);
 
   int stride = 0;
 
diff --git a/cc/resources/resource_provider.h b/cc/resources/resource_provider.h
index fa37ddd..c434b3a 100644
--- a/cc/resources/resource_provider.h
+++ b/cc/resources/resource_provider.h
@@ -353,9 +353,10 @@ class CC_EXPORT ResourceProvider {
     uint8_t* pixels;
     uint8_t* pixel_buffer;
     int lock_for_read_count;
+    int imported_count;
+    int exported_count;
     bool locked_for_write;
     bool external;
-    bool exported;
     bool marked_for_deletion;
     bool pending_set_pixels;
     bool set_pixels_completion_forced;
@@ -398,7 +399,7 @@ class CC_EXPORT ResourceProvider {
   static void PopulateSkBitmapWithResource(SkBitmap* sk_bitmap,
                                            const Resource* resource);
 
-  bool TransferResource(WebKit::WebGraphicsContext3D* context,
+  void TransferResource(WebKit::WebGraphicsContext3D* context,
                         ResourceId id,
                         TransferableResource* resource);
   enum DeleteStyle {
diff --git a/cc/resources/resource_provider_unittest.cc b/cc/resources/resource_provider_unittest.cc
index 7d6a6a4..83e90b4 100644
--- a/cc/resources/resource_provider_unittest.cc
+++ b/cc/resources/resource_provider_unittest.cc
@@ -594,13 +594,18 @@ TEST_P(ResourceProviderTest, TransferResources) {
   EXPECT_EQ(0, memcmp(data2, result, pixel_size));
   {
     // Check that transfering again the same resource from the child to the
-    // parent is a noop.
+    // parent works.
     ResourceProvider::ResourceIdArray resource_ids_to_transfer;
     resource_ids_to_transfer.push_back(id1);
     TransferableResourceArray list;
     child_resource_provider->PrepareSendToParent(resource_ids_to_transfer,
                                                  &list);
-    EXPECT_EQ(0u, list.size());
+    EXPECT_EQ(1u, list.size());
+    EXPECT_EQ(id1, list[0].id);
+    child_resource_provider->ReceiveFromParent(list);
+    // id1 was exported twice, we returned it only once, it should still be
+    // in-use.
+    EXPECT_TRUE(child_resource_provider->InUseByConsumer(id1));
   }
   {
     // Transfer resources back from the parent to the child.
diff --git a/cc/trees/layer_tree_host_unittest_delegated.cc b/cc/trees/layer_tree_host_unittest_delegated.cc
index b76184e..f15da5d 100644
--- a/cc/trees/layer_tree_host_unittest_delegated.cc
+++ b/cc/trees/layer_tree_host_unittest_delegated.cc
@@ -4,6 +4,8 @@
 
 #include "cc/trees/layer_tree_host.h"
 
+#include <algorithm>
+
 #include "base/bind.h"
 #include "cc/layers/delegated_renderer_layer.h"
 #include "cc/layers/delegated_renderer_layer_client.h"
@@ -24,6 +26,34 @@
 namespace cc {
 namespace {
 
+bool TransferableResourceLower(const TransferableResource& a,
+                               const TransferableResource& b) {
+  return a.id < b.id;
+}
+
+// Tests if the list of resources matches an expectation, modulo the order.
+bool ResourcesMatch(TransferableResourceArray actual,
+                    unsigned* expected,
+                    size_t expected_count) {
+  EXPECT_EQ(expected_count, actual.size());
+  if (expected_count != actual.size())
+    return false;
+
+  std::sort(actual.begin(), actual.end(), TransferableResourceLower);
+  std::sort(expected, expected + expected_count);
+  bool result = true;
+  for (size_t i = 0; i < expected_count; ++i) {
+    EXPECT_EQ(actual[i].id, expected[i]);
+    if (actual[i].id != expected[i])
+      result = false;
+  }
+
+  return result;
+}
+
+#define EXPECT_RESOURCES(expected, actual) \
+    EXPECT_TRUE(ResourcesMatch(actual, expected, arraysize(expected)));
+
 // These tests deal with delegated renderer layers.
 class LayerTreeHostDelegatedTest : public LayerTreeTest {
  protected:
@@ -494,10 +524,20 @@ class LayerTreeHostDelegatedTestMergeResources
     scoped_ptr<DelegatedFrameData> frame2 =
         CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
     AddTextureQuad(frame2.get(), 999);
+    AddTransferableResource(frame2.get(), 999);
     AddTextureQuad(frame2.get(), 555);
     AddTransferableResource(frame2.get(), 555);
     delegated_->SetFrameData(frame2.Pass());
 
+    // The resource 999 from frame1 is returned since it is still on the main
+    // thread.
+    TransferableResourceArray returned_resources;
+    delegated_->TakeUnusedResourcesForChildCompositor(&returned_resources);
+    {
+      unsigned expected[] = {999};
+      EXPECT_RESOURCES(expected, returned_resources);
+    }
+
     PostSetNeedsCommitToMainThread();
   }
 
@@ -625,8 +665,10 @@ class LayerTreeHostDelegatedTestReturnUnusedResources
       case 5:
         // 555 is no longer in use.
         delegated_->TakeUnusedResourcesForChildCompositor(&resources);
-        EXPECT_EQ(1u, resources.size());
-        EXPECT_EQ(555u, resources[0].id);
+        {
+          unsigned expected[] = {555};
+          EXPECT_RESOURCES(expected, resources);
+        }
 
         // Stop using any resources.
         frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
@@ -640,13 +682,9 @@ class LayerTreeHostDelegatedTestReturnUnusedResources
       case 7:
         // 444 and 999 are no longer in use.
         delegated_->TakeUnusedResourcesForChildCompositor(&resources);
-        EXPECT_EQ(2u, resources.size());
-        if (resources[0].id == 999) {
-          EXPECT_EQ(999u, resources[0].id);
-          EXPECT_EQ(444u, resources[1].id);
-        } else {
-          EXPECT_EQ(444u, resources[0].id);
-          EXPECT_EQ(999u, resources[1].id);
+        {
+          unsigned expected[] = {444, 999};
+          EXPECT_RESOURCES(expected, resources);
         }
         EndTest();
         break;
@@ -726,8 +764,10 @@ class LayerTreeHostDelegatedTestReusedResources
       case 5:
         // The 999 resource is the only unused one.
         delegated_->TakeUnusedResourcesForChildCompositor(&resources);
-        EXPECT_EQ(1u, resources.size());
-        EXPECT_EQ(999u, resources[0].id);
+        {
+          unsigned expected[] = {999};
+          EXPECT_RESOURCES(expected, resources);
+        }
         EndTest();
         break;
     }
@@ -795,13 +835,9 @@ class LayerTreeHostDelegatedTestFrameBeforeAck
         return;
       case 5:
         delegated_->TakeUnusedResourcesForChildCompositor(&resources);
-        EXPECT_EQ(2u, resources.size());
-        if (resources[0].id == 555) {
-          EXPECT_EQ(555u, resources[0].id);
-          EXPECT_EQ(444u, resources[1].id);
-        } else {
-          EXPECT_EQ(444u, resources[0].id);
-          EXPECT_EQ(555u, resources[1].id);
+        {
+          unsigned expected[] = {444, 555};
+          EXPECT_RESOURCES(expected, resources);
         }
 
         // The child compositor sends a frame before receiving an for the
@@ -892,6 +928,7 @@ class LayerTreeHostDelegatedTestFrameBeforeTakeResources
         // Keep using 999 but stop using 555 and 444.
         frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
         AddTextureQuad(frame.get(), 999);
+        AddTransferableResource(frame.get(), 999);
         delegated_->SetFrameData(frame.Pass());
 
         // Resource are not immediately released.
@@ -911,13 +948,20 @@ class LayerTreeHostDelegatedTestFrameBeforeTakeResources
         // and 444, which were just released during commit.
         frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
         AddTextureQuad(frame.get(), 999);
+        AddTransferableResource(frame.get(), 999);
         AddTextureQuad(frame.get(), 555);
+        AddTransferableResource(frame.get(), 555);
         AddTextureQuad(frame.get(), 444);
+        AddTransferableResource(frame.get(), 444);
         delegated_->SetFrameData(frame.Pass());
 
-        // The resources are used by the new frame so are not returned.
+        // The resources are used by the new frame but are returned anyway since
+        // we passed them again.
         delegated_->TakeUnusedResourcesForChildCompositor(&resources);
-        EXPECT_EQ(0u, resources.size());
+        {
+          unsigned expected[] = {444, 555};
+          EXPECT_RESOURCES(expected, resources);
+        }
         break;
       case 6:
         // Retrieve unused resources to the main thread.
@@ -1034,8 +1078,10 @@ class LayerTreeHostDelegatedTestBadFrame
       case 5:
         // The bad frame's resource is given back to the child compositor.
         delegated_->TakeUnusedResourcesForChildCompositor(&resources);
-        EXPECT_EQ(1u, resources.size());
-        EXPECT_EQ(444u, resources[0].id);
+        {
+          unsigned expected[] = {444};
+          EXPECT_RESOURCES(expected, resources);
+        }
 
         // Now send a good frame with 999 again.
         frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
@@ -1050,8 +1096,10 @@ class LayerTreeHostDelegatedTestBadFrame
       case 7:
         // The unused 555 from the last good frame is now released.
         delegated_->TakeUnusedResourcesForChildCompositor(&resources);
-        EXPECT_EQ(1u, resources.size());
-        EXPECT_EQ(555u, resources[0].id);
+        {
+          unsigned expected[] = {555};
+          EXPECT_RESOURCES(expected, resources);
+        }
 
         EndTest();
         break;
@@ -1171,8 +1219,10 @@ class LayerTreeHostDelegatedTestUnnamedResource
       case 2:
         // The unused resource should be returned.
         delegated_->TakeUnusedResourcesForChildCompositor(&resources);
-        EXPECT_EQ(1u, resources.size());
-        EXPECT_EQ(999u, resources[0].id);
+        {
+          unsigned expected[] = {999};
+          EXPECT_RESOURCES(expected, resources);
+        }
 
         EndTest();
         break;
@@ -1229,14 +1279,34 @@ class LayerTreeHostDelegatedTestDontLeakResource
         // But then we immediately stop using 999.
         frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
         AddTextureQuad(frame.get(), 555);
+        AddTransferableResource(frame.get(), 555);
         delegated_->SetFrameData(frame.Pass());
         break;
       case 2:
-        // The unused resource should be returned.
+        // The unused resources should be returned. 555 is still used, but it's
+        // returned once to account for the first frame.
         delegated_->TakeUnusedResourcesForChildCompositor(&resources);
-        EXPECT_EQ(1u, resources.size());
-        EXPECT_EQ(999u, resources[0].id);
-
+        {
+          unsigned expected[] = {555, 999};
+          EXPECT_RESOURCES(expected, resources);
+        }
+        // Send a frame with no resources in it.
+        frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
+        delegated_->SetFrameData(frame.Pass());
+        break;
+      case 3:
+        // The impl side will get back the resource at some point.
+        // TODO(piman): The test should work without this.
+        layer_tree_host()->SetNeedsCommit();
+        break;
+      case 4:
+        // The now unused resource 555 should be returned.
+        resources.clear();
+        delegated_->TakeUnusedResourcesForChildCompositor(&resources);
+        {
+          unsigned expected[] = {555};
+          EXPECT_RESOURCES(expected, resources);
+        }
         EndTest();
         break;
     }
@@ -1262,6 +1332,11 @@ class LayerTreeHostDelegatedTestDontLeakResource
     EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(555)->second));
   }
 
+  virtual void SwapBuffersOnThread(LayerTreeHostImpl* host_impl,
+                                   bool result) OVERRIDE {
+    ReturnUnusedResourcesFromParent(host_impl);
+  }
+
   virtual void AfterTest() OVERRIDE {}
 };
 
@@ -1307,8 +1382,10 @@ class LayerTreeHostDelegatedTestResourceSentToParent
       case 4:
         // 999 was returned from the grandparent and could be released.
         delegated_->TakeUnusedResourcesForChildCompositor(&resources);
-        EXPECT_EQ(1u, resources.size());
-        EXPECT_EQ(999u, resources[0].id);
+        {
+          unsigned expected[] = {999};
+          EXPECT_RESOURCES(expected, resources);
+        }
 
         EndTest();
         break;
@@ -1406,20 +1483,40 @@ class LayerTreeHostDelegatedTestCommitWithoutTake
         // Stop using 999 and 444 in this frame and commit.
         frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
         AddTextureQuad(frame.get(), 555);
+        AddTransferableResource(frame.get(), 555);
         delegated_->SetFrameData(frame.Pass());
+        // 999 and 444 will be returned for frame 1, but not 555 since it's in
+        // the current frame.
         break;
       case 3:
         // Don't take resources here, but set a new frame that uses 999 again.
         frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
         AddTextureQuad(frame.get(), 999);
+        AddTransferableResource(frame.get(), 999);
         AddTextureQuad(frame.get(), 555);
+        AddTransferableResource(frame.get(), 555);
         delegated_->SetFrameData(frame.Pass());
         break;
       case 4:
-        // 999 and 555 are in use, but 444 should be returned now.
+        // 555 from frame 1 and 2 isn't returned since it's still in use. 999
+        // from frame 1 is returned though.
+        delegated_->TakeUnusedResourcesForChildCompositor(&resources);
+        {
+          unsigned expected[] = {444, 999};
+          EXPECT_RESOURCES(expected, resources);
+        }
+
+        frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
+        delegated_->SetFrameData(frame.Pass());
+        // 555 will be returned 3 times for frames 1 2 and 3, and 999 will be
+        // returned once for frame 3.
+        break;
+      case 5:
         delegated_->TakeUnusedResourcesForChildCompositor(&resources);
-        EXPECT_EQ(1u, resources.size());
-        EXPECT_EQ(444u, resources[0].id);
+        {
+          unsigned expected[] = {555, 555, 555, 999};
+          EXPECT_RESOURCES(expected, resources);
+        }
 
         EndTest();
         break;