cc: Propagate read lock fences constraints to parent compositors.

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 087f0fe7751364bed6f3b37281b1a4962d53cde9..1b7ad00aa3cf61b8f837c36ad7e82dc0b7d23362 100644
--- a/cc/resources/resource_provider.cc
+++ b/cc/resources/resource_provider.cc
@@ -587,7 +587,8 @@ void ResourceProvider::DeleteResource(ResourceId id) {
   DCHECK_EQ(resource->imported_count, 0);
   DCHECK(resource->pending_set_pixels || !resource->locked_for_write);
 
-  if (resource->exported_count > 0 || resource->lock_for_read_count > 0) {
+  if (resource->exported_count > 0 || resource->lock_for_read_count > 0 ||
+      !ReadLockFenceHasPassed(resource)) {
     resource->marked_for_deletion = true;
     return;
   } else {
@@ -947,6 +948,12 @@ void ResourceProvider::UnlockForWrite(ResourceProvider::Resource* resource) {
   resource->locked_for_write = false;
 }
 
+void ResourceProvider::EnableReadLockFencesForTesting(ResourceId id) {
+  Resource* resource = GetResource(id);
+  DCHECK(resource);
+  resource->read_lock_fences_enabled = true;
+}
+
 ResourceProvider::ScopedReadLockGL::ScopedReadLockGL(
     ResourceProvider* resource_provider,
     ResourceId resource_id)
@@ -1375,6 +1382,7 @@ void ResourceProvider::ReceiveFromChild(
       resource->mailbox = TextureMailbox(it->mailbox_holder.mailbox,
                                          it->mailbox_holder.texture_target,
                                          it->mailbox_holder.sync_point);
+      resource->read_lock_fences_enabled = it->read_lock_fences_enabled;
     }
     resource->child_id = child;
     // Don't allocate a texture for a child.
@@ -1430,14 +1438,6 @@ void ResourceProvider::ReceiveReturnsFromParent(
     if (resource->exported_count)
       continue;
 
-    // Need to wait for the current read lock fence to pass before we can
-    // recycle this resource.
-    if (resource->read_lock_fences_enabled) {
-      if (current_read_lock_fence_.get())
-        current_read_lock_fence_->Set();
-      resource->read_lock_fence = current_read_lock_fence_;
-    }
-
     if (returned.sync_point) {
       DCHECK(!resource->has_shared_bitmap_id);
       if (resource->origin == Resource::INTERNAL) {
@@ -1482,6 +1482,7 @@ void ResourceProvider::TransferResource(GLES2Interface* gl,
   resource->mailbox_holder.texture_target = source->target;
   resource->filter = source->filter;
   resource->size = source->size;
+  resource->read_lock_fences_enabled = source->read_lock_fences_enabled;
   resource->is_repeated = (source->wrap_mode == GL_REPEAT);
 
   if (source->type == RESOURCE_TYPE_BITMAP) {
@@ -1548,19 +1549,22 @@ void ResourceProvider::DeleteAndReturnUnusedResourcesToChild(
     ResourceId child_id = child_info->parent_to_child_map[local_id];
     DCHECK(child_info->child_to_parent_map.count(child_id));
 
+    bool fence_passed = ReadLockFenceHasPassed(&resource);

[reveman, 2015/06/16 01:35:10]

Checking the fence is an atomic op at best. Should we avoid this when possible?

[Daniele Castagna, 2015/06/17 15:20:41]

Done.

     bool is_lost =
         resource.lost ||
         (resource.type == RESOURCE_TYPE_GL_TEXTURE && lost_output_surface_);
-    if (resource.exported_count > 0 || resource.lock_for_read_count > 0) {

[piman, 2015/06/16 02:02:47]

I'm really sorry if our out-of-band discussion was misleading - I'm happy as
long as the fence logic and the exported logic are consistent - which you had in
PS1. All I wanted was a test that exercises the logic with unpassed fences!

[Daniele Castagna, 2015/06/16 15:39:20]

I'm not sure the fence logic can be consistent with the exported logic.
When a resource is still exported we can delay the deletion since we know that
the resource will eventually be returned from the parent, and we'll delete it at
that time.
Same thing for locked resources, we know that they will be unlocked and we can
release it when that happens.
For resources that haven't passed the fence while we're destroying the child,
this is our last chance to return them. There is no method called referring to
that resource after the fence has passed.
Marking them as lost should make sure that when the child RP receives them back
they're not going to reuse them, since they might be still in use for drawing.

Does this make sense?

-      if (style != FOR_SHUTDOWN) {
-        // Defer this until we receive the resource back from the parent or
-        // the read lock is released.
+
+    if (resource.exported_count > 0 || resource.lock_for_read_count > 0 ||
+        !fence_passed) {
+      if (style == FOR_SHUTDOWN ||
+          (!fence_passed && child_info->marked_for_deletion)) {
+        // We can't postpone the deletion , so we'll have to lose it.

[reveman, 2015/06/16 01:35:10]

I think this comment would be more useful if it explained why.

nit: " , " ->  ", "

[Daniele Castagna, 2015/06/17 15:20:41]

Done.

+        is_lost = true;

[reveman, 2015/06/16 01:35:10]

Is this necessary when "resource.exported_count > 0 ||
resource.lock_for_read_count > 0" and style != FOR_SHUTDOWN?

[Daniele Castagna, 2015/06/17 15:20:41]

Changed this as discussed.

+      } else {
+        // Defer this resource deletion.
         resource.marked_for_deletion = true;
         continue;
       }
-
-      // We still have an exported_count, so we'll have to lose it.
-      is_lost = true;
     }
 
     if (gl && resource.filter != resource.original_filter) {