exo: Cleanup and make buffer release code more robust.

components/exo/buffer.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/exo/buffer.h"
 
 #include <GLES2/gl2.h>
 #include <GLES2/gl2ext.h>
 #include <GLES2/gl2extchromium.h>
 #include <stdint.h>
@@ skipping 388 matching lines @@
 
 Buffer::~Buffer() {}
 
 bool Buffer::ProduceTransferableResource(
     CompositorFrameSinkHolder* compositor_frame_sink_holder,
     cc::ResourceId resource_id,
     bool secure_output_only,
     bool client_usage,
     cc::TransferableResource* resource) {
   DCHECK(attach_count_);
-  DLOG_IF(WARNING, use_count_ && client_usage)
+  DLOG_IF(WARNING, !release_contents_callback_.IsCancelled() && client_usage)
       << "Producing a texture mailbox for a buffer that has not been released";
 
-  // Some clients think that they can reuse a buffer before it's released by
-  // performing a fast blit into the buffer. This behavior is bad as it prevents
-  // the client from knowing when the buffer is actually released (e.g. the
-  // release notification for the previous use of buffer can arrive after the
-  // buffer has been reused). We stop running the release callback when this
-  // type of behavior is detected as having the buffer always be busy will
-  // result in fewer drawing artifacts.
-  if (use_count_ && client_usage)
-    release_callback_.Reset();
-
-  // Increment the use count for this buffer.
-  ++use_count_;
-
   // If textures are lost, destroy them to ensure that we create new ones below.
   if (contents_texture_ && contents_texture_->IsLost())
     contents_texture_.reset();
   if (texture_ && texture_->IsLost())
     texture_.reset();
 
   ui::ContextFactory* context_factory =
       aura::Env::GetInstance()->context_factory();
   // Note: This can fail if GPU acceleration has been disabled.
   scoped_refptr<cc::ContextProvider> context_provider =
       context_factory->SharedMainThreadContextProvider();
   if (!context_provider) {
     DLOG(WARNING) << "Failed to acquire a context provider";
-    Release();  // Decrements the use count
     resource->id = 0;
     resource->size = gfx::Size();
     return false;
   }
 
   // The reference to the CompositorFrameSinkHolder keeps it alive until a
   // release callback is received.
   compositor_frame_sink_holder_ = compositor_frame_sink_holder;
 
   resource->id = resource_id;
   resource->format = cc::RGBA_8888;
   resource->filter = GL_LINEAR;
   resource->size = gpu_memory_buffer_->GetSize();
 
   // Create a new image texture for |gpu_memory_buffer_| with |texture_target_|
   // if one doesn't already exist. The contents of this buffer are copied to
   // |texture| using a call to CopyTexImage.
   if (!contents_texture_) {
     contents_texture_ = base::MakeUnique<Texture>(
         context_factory, context_provider.get(), gpu_memory_buffer_.get(),
         texture_target_, query_type_);
   }
+  Texture* contents_texture = contents_texture_.get();
 
+  // Cancel pending contents release callback.
+  release_contents_callback_.Reset(
+      base::Bind(&Buffer::ReleaseContents, base::Unretained(this)));
+
+  // Zero-copy means using the contents texture directly.
   if (use_zero_copy_) {
-    // Zero-copy means using the contents texture directly.
-    Texture* texture = contents_texture_.get();
-
-    // This binds the latest contents of this buffer to |texture|.
-    gpu::SyncToken sync_token = texture->BindTexImage();
-    resource->mailbox_holder =
-        gpu::MailboxHolder(texture->mailbox(), sync_token, texture_target_);
+    // This binds the latest contents of this buffer to |contents_texture|.
+    gpu::SyncToken sync_token = contents_texture->BindTexImage();
+    resource->mailbox_holder = gpu::MailboxHolder(contents_texture->mailbox(),
+                                                  sync_token, texture_target_);
     resource->is_overlay_candidate = is_overlay_candidate_;
 
     // The contents texture will be released when no longer used by the
     // compositor.
     compositor_frame_sink_holder_->SetResourceReleaseCallback(
         resource_id,
-        base::Bind(&Buffer::Texture::ReleaseTexImage, base::Unretained(texture),
+        base::Bind(&Buffer::Texture::ReleaseTexImage,
+                   base::Unretained(contents_texture),
                    base::Bind(&Buffer::ReleaseContentsTexture, AsWeakPtr(),
-                              base::Passed(&contents_texture_))));
+                              base::Passed(&contents_texture_),
+                              release_contents_callback_.callback())));
     return true;
   }
 
   // Create a mailbox texture that we copy the buffer contents to.
   if (!texture_) {
     texture_ =
         base::MakeUnique<Texture>(context_factory, context_provider.get());
   }
+  Texture* texture = texture_.get();
 
   // Copy the contents of |contents_texture| to |texture| and produce a
-  // texture mailbox from the result in |texture|.
-  Texture* contents_texture = contents_texture_.get();
-  Texture* texture = texture_.get();
-
-  // The contents texture will be released when copy has completed.
+  // texture mailbox from the result in |texture|. The contents texture will
+  // be released when copy has completed.
   gpu::SyncToken sync_token = contents_texture->CopyTexImage(
       texture, base::Bind(&Buffer::ReleaseContentsTexture, AsWeakPtr(),
-                          base::Passed(&contents_texture_)));
+                          base::Passed(&contents_texture_),
+                          release_contents_callback_.callback()));
   resource->mailbox_holder =
       gpu::MailboxHolder(texture->mailbox(), sync_token, GL_TEXTURE_2D);
   resource->is_overlay_candidate = false;
 
   // The mailbox texture will be released when no longer used by the
   // compositor.
   compositor_frame_sink_holder_->SetResourceReleaseCallback(
       resource_id,
       base::Bind(&Buffer::Texture::Release, base::Unretained(texture),
                  base::Bind(&Buffer::ReleaseTexture, AsWeakPtr(),
                             base::Passed(&texture_))));
   return true;
 }
 
 void Buffer::OnAttach() {
-  DLOG_IF(WARNING, attach_count_ > 0u)
+  DLOG_IF(WARNING, attach_count_)
       << "Reattaching a buffer that is already attached to another surface.";
-  attach_count_++;
+  ++attach_count_;
 }
 
 void Buffer::OnDetach() {
   DCHECK_GT(attach_count_, 0u);
   --attach_count_;
-  CheckReleaseCallback();
+
+  // Release buffer if no longer attached to a surface and content has been
+  // released.
+  if (!attach_count_ && release_contents_callback_.IsCancelled())
+    Release();
 }
 
 gfx::Size Buffer::GetSize() const {
   return gpu_memory_buffer_->GetSize();
 }
 
 gfx::BufferFormat Buffer::GetFormat() const {
   return gpu_memory_buffer_->GetFormat();
 }
 
 std::unique_ptr<base::trace_event::TracedValue> Buffer::AsTracedValue() const {
   std::unique_ptr<base::trace_event::TracedValue> value(
       new base::trace_event::TracedValue());
   gfx::Size size = gpu_memory_buffer_->GetSize();
   value->SetInteger("width", size.width());
   value->SetInteger("height", size.height());
   value->SetInteger("format",
                     static_cast<int>(gpu_memory_buffer_->GetFormat()));
   return value;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Buffer, private:
 
 void Buffer::Release() {
-  DCHECK_GT(use_count_, 0u);
-  --use_count_;
-  CheckReleaseCallback();
-}
-
-void Buffer::CheckReleaseCallback() {
-  if (attach_count_ || use_count_)
-    return;
-
   // Run release callback to notify the client that buffer has been released.
   if (!release_callback_.is_null())
     release_callback_.Run();
-
-  compositor_frame_sink_holder_ = nullptr;
 }
 
 void Buffer::ReleaseTexture(std::unique_ptr<Texture> texture) {
   texture_ = std::move(texture);
 }
 
-void Buffer::ReleaseContentsTexture(std::unique_ptr<Texture> texture) {
-  TRACE_EVENT0("exo", "Buffer::ReleaseContentsTexture");
+void Buffer::ReleaseContentsTexture(std::unique_ptr<Texture> texture,
+                                    const base::Closure& callback) {
   contents_texture_ = std::move(texture);
-  Release();
+  callback.Run();
+}
+
+void Buffer::ReleaseContents() {
+  TRACE_EVENT0("exo", "Buffer::ReleaseContents");
+
+  // Cancel callback to indicate that buffer has been released.
+  release_contents_callback_.Cancel();
+
+  // Release buffer if not attached to surface.
+  if (!attach_count_)
+    Release();
 }
 
 }  // namespace exo