exo::CompositorFrameSinkHolder's release callbacks hold ref

components/exo/buffer.cc

Index: components/exo/buffer.cc
diff --git a/components/exo/buffer.cc b/components/exo/buffer.cc
index 9b1778d989d6c0250efff832d6372f401e15e129..216b5051a8dda36fab7d09121b1047f513accaae 100644
--- a/components/exo/buffer.cc
+++ b/components/exo/buffer.cc
@@ -24,6 +24,7 @@
 #include "cc/output/context_provider.h"
 #include "cc/resources/single_release_callback.h"
 #include "cc/resources/texture_mailbox.h"
+#include "components/exo/compositor_frame_sink_holder.h"
 #include "gpu/command_buffer/client/context_support.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
 #include "ui/aura/env.h"
@@ -401,7 +402,8 @@ Buffer::~Buffer() {}
 std::unique_ptr<cc::SingleReleaseCallback> Buffer::ProduceTextureMailbox(

[reveman, 2017/01/02 19:57:56]

It doesn't make sense for this function to return a release callback anymore.
Also doesn't make much sense to be producing a TextureMailbox anymore. Please
have this return a bool and a cc::TransferableResource instead. The buffer is
the instance that cares about the release callback and not the surface so this
buffer code is also where a release callback should be set for the sink holder.
Please change the signature to:

bool Buffer::ProduceTransferableResource(CompositorFrameSinkHolder*
compositor_frame_sink_holder,
                                         int resource_id,
                                         bool secure_output_only,
                                         bool client_usage,
                                         cc::TransferableResource* resource);

     cc::TextureMailbox* texture_mailbox,
     bool secure_output_only,
-    bool client_usage) {
+    bool client_usage,
+    CompositorFrameSinkHolder* compositor_frame_sink_holder) {
   DCHECK(attach_count_);
   DLOG_IF(WARNING, use_count_ && client_usage)
       << "Producing a texture mailbox for a buffer that has not been released";
@@ -436,6 +438,8 @@ std::unique_ptr<cc::SingleReleaseCallback> Buffer::ProduceTextureMailbox(
     return nullptr;
   }
 
+  compositor_frame_sink_holder_ = compositor_frame_sink_holder;

[reveman, 2017/01/02 19:57:56]

nit: short comment here saying something like: "We need to keep a reference to
the sink holder to receive a release callback.

+
   // 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.
@@ -534,6 +538,8 @@ void Buffer::CheckReleaseCallback() {
   // 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;

[reveman, 2017/01/02 19:57:56]

Do we really need this here? Removing it would solve the sink holder destruction
problems afaict.

 }
 
 void Buffer::ReleaseTexture(std::unique_ptr<Texture> texture) {