cc: Propagate read lock fences constraints to parent compositors.

cc/resources/resource_provider_unittest.cc

Index: cc/resources/resource_provider_unittest.cc
diff --git a/cc/resources/resource_provider_unittest.cc b/cc/resources/resource_provider_unittest.cc
index 2982a3d462d440faa169ec75c1260c31e3d8cb72..4d5a101d82a44b972d9bde413c6d3b679ad063b9 100644
--- a/cc/resources/resource_provider_unittest.cc
+++ b/cc/resources/resource_provider_unittest.cc
@@ -1045,6 +1045,71 @@ TEST_P(ResourceProviderTest, ReadLockCountStopsReturnToChildOrDelete) {
   resource_provider_->DestroyChild(child_id);
 }
 
+TEST_P(ResourceProviderTest, ReadLockFenceStopsReturnToChildOrDelete) {
+  if (GetParam() != ResourceProvider::RESOURCE_TYPE_GL_TEXTURE)
+    return;
+  gfx::Size size(1, 1);
+  ResourceFormat format = RGBA_8888;
+
+  ResourceId id1 = child_resource_provider_->CreateResource(
+      size, GL_CLAMP_TO_EDGE, ResourceProvider::TEXTURE_HINT_IMMUTABLE, format);
+  uint8_t data1[4] = {1, 2, 3, 4};
+  child_resource_provider_->CopyToResource(id1, data1, size);
+  child_resource_provider_->EnableReadLockFences(id1);
+  ReturnedResourceArray returned_to_child;
+  int child_id =
+      resource_provider_->CreateChild(GetReturnCallback(&returned_to_child));
+
+  // Transfer some resources to the parent.
+  ResourceProvider::ResourceIdArray resource_ids_to_transfer;
+  resource_ids_to_transfer.push_back(id1);
+  TransferableResourceArray list;
+  child_resource_provider_->PrepareSendToParent(resource_ids_to_transfer,
+                                                &list);
+  ASSERT_EQ(1u, list.size());
+  EXPECT_TRUE(child_resource_provider_->InUseByConsumer(id1));
+
+  resource_provider_->ReceiveFromChild(child_id, list);
+  class TestFence : public ResourceProvider::Fence {
+   public:
+    TestFence() {}
+
+    void Set() override { NOTIMPLEMENTED(); }
+    bool HasPassed() override { return passed; }
+    void Wait() override { NOTIMPLEMENTED(); }
+
+    bool passed = false;
+
+   private:
+    ~TestFence() override {}
+  };
+
+  scoped_refptr<TestFence> fence(new TestFence);
+  resource_provider_->SetReadLockFence(fence.get());
+  {
+    unsigned parent_id = list.front().id;
+    resource_provider_->WaitSyncPointIfNeeded(parent_id);
+    ResourceProvider::ScopedReadLockGL lock(resource_provider_.get(),
+                                            parent_id);
+  }
+  resource_provider_->DeclareUsedResourcesFromChild(
+      child_id, ResourceProvider::ResourceIdSet());
+  EXPECT_EQ(0u, returned_to_child.size());
+
+  resource_provider_->DeclareUsedResourcesFromChild(
+      child_id, ResourceProvider::ResourceIdSet());
+  EXPECT_EQ(0u, returned_to_child.size());
+  fence->passed = true;
+
+  resource_provider_->DeclareUsedResourcesFromChild(
+      child_id, ResourceProvider::ResourceIdSet());
+  EXPECT_EQ(1u, returned_to_child.size());
+
+  child_resource_provider_->ReceiveReturnsFromParent(returned_to_child);
+  child_resource_provider_->DeleteResource(id1);
+  EXPECT_EQ(0u, child_resource_provider_->num_resources());
+}

[piman, 2015/06/12 01:53:52]

Could you add another test for the lost resource case? If you DestroyChild in
the parent before the fence has passed, all resources should be returned as
lost.

[Daniele Castagna, 2015/06/12 23:36:42]

If DestroyChild is called after a context lost Fence::HasPassed is always going
to return true:
https://code.google.com/p/chromium/codesearch#chromium/src/gpu/command_buffer...

The test wouldn't be testing a lot since it'd need to set TestFence::passed to
true to simulate that.

Do you think it's still worth adding it?

+
 TEST_P(ResourceProviderTest, TransferSoftwareResources) {
   if (GetParam() != ResourceProvider::RESOURCE_TYPE_BITMAP)
     return;