cc: refcount resources as we send them to the parent

cc/trees/layer_tree_host_unittest_delegated.cc

Index: cc/trees/layer_tree_host_unittest_delegated.cc
diff --git a/cc/trees/layer_tree_host_unittest_delegated.cc b/cc/trees/layer_tree_host_unittest_delegated.cc
index d48931328afb8f4af02a324966b609af66d2031f..efc65b430f3058085d92e7e1e293a5a98ba7b6c1 100644
--- a/cc/trees/layer_tree_host_unittest_delegated.cc
+++ b/cc/trees/layer_tree_host_unittest_delegated.cc
@@ -488,14 +488,18 @@ class LayerTreeHostDelegatedTestMergeResources
     AddTransferableResource(frame1.get(), 999);
     delegated_->SetFrameData(frame1.Pass());
 
-    // The second frame uses resource 999 still, but also adds 555.
+    // The second frame doesn't use resource 999 anymore, but adds 555.
     scoped_ptr<DelegatedFrameData> frame2 =
         CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
-    AddTextureQuad(frame2.get(), 999);
     AddTextureQuad(frame2.get(), 555);
     AddTransferableResource(frame2.get(), 555);
     delegated_->SetFrameData(frame2.Pass());
 
+    TransferableResourceArray returned_resources;
+    delegated_->TakeUnusedResourcesForChildCompositor(&returned_resources);
+    EXPECT_EQ(1u, returned_resources.size());
+    EXPECT_EQ(999u, returned_resources[0].id);
+
     PostSetNeedsCommitToMainThread();
   }
 
@@ -509,12 +513,11 @@ class LayerTreeHostDelegatedTestMergeResources
             delegated_impl->ChildId());
 
     // Both frames' resources should be in the parent's resource provider.

[danakj, 2013/08/15 22:20:19]

Comment's wrong now, but why'd you choose to remove 999 in the second frame? Why
not just send both resources? I think this test can be useful testing that
sending a resource in two consecutive frames it doesn't get lost (kinda what it
was testing before)?

[piman, 2013/08/16 04:29:55]

TBH I don't remember (this CL has been pending for a few weeks now). I put it
back. I think it was that we don't merge the resources anymore so this test
didn't make sense anymore and I wanted to repurpose.... But whatever was tested
here is tested below too in LayerTreeHostDelegatedTestDontLeakResource

-    EXPECT_EQ(2u, map.size());
-    EXPECT_EQ(1u, map.count(999));
+    EXPECT_EQ(1u, map.size());
+    EXPECT_EQ(0u, map.count(999));
     EXPECT_EQ(1u, map.count(555));
 
-    EXPECT_EQ(2u, delegated_impl->Resources().size());
-    EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(999)->second));
+    EXPECT_EQ(1u, delegated_impl->Resources().size());
     EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(555)->second));
 
     EndTest();
@@ -890,6 +893,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.
@@ -909,13 +913,17 @@ 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());
+        EXPECT_EQ(2u, resources.size());

[danakj, 2013/08/15 22:20:19]

Verify which resources are in the array.

[piman, 2013/08/16 04:29:55]

Done.

         break;
       case 6:
         // Retrieve unused resources to the main thread.
@@ -1227,14 +1235,30 @@ 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(2u, resources.size());
         EXPECT_EQ(999u, resources[0].id);
+        EXPECT_EQ(555u, resources[1].id);
+        frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));

[danakj, 2013/08/15 22:20:19]

// Send a frame with no resources in it.

[piman, 2013/08/16 04:29:55]

Done.

+        delegated_->SetFrameData(frame.Pass());
+        break;
 
+      case 3:
+        // The impl side will get back the resource at some point.
+        // TODO(danakj): The test should work without this.

[danakj, 2013/08/15 22:20:19]

I see what you did there :)

[piman, 2013/08/16 04:29:55]

copy & paste :)

+        layer_tree_host()->SetNeedsCommit();
+        break;
+      case 4:
+        resources.clear();

[danakj, 2013/08/15 22:20:19]

// The unused resource 555 should be returned.

[piman, 2013/08/16 04:29:55]

Done.

+        delegated_->TakeUnusedResourcesForChildCompositor(&resources);
+        EXPECT_EQ(1u, resources.size());
+        EXPECT_EQ(555u, resources[0].id);
         EndTest();
         break;
     }
@@ -1260,6 +1284,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 {}
 };
 
@@ -1404,20 +1433,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);
-        EXPECT_EQ(1u, resources.size());
-        EXPECT_EQ(444u, resources[0].id);
+        EXPECT_EQ(2u, resources.size());
+        EXPECT_EQ(999u, resources[0].id);
+        EXPECT_EQ(444u, resources[1].id);
+
+        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(4u, resources.size());
+        EXPECT_EQ(999u, resources[0].id);
+        EXPECT_EQ(555u, resources[1].id);
+        EXPECT_EQ(555u, resources[2].id);
+        EXPECT_EQ(555u, resources[3].id);
 
         EndTest();
         break;