CopyOutputResult must have a working release callback when received over mojo

cc/ipc/copy_output_result_struct_traits.cc

Index: cc/ipc/copy_output_result_struct_traits.cc
diff --git a/cc/ipc/copy_output_result_struct_traits.cc b/cc/ipc/copy_output_result_struct_traits.cc
index 0398fe115cd52463b56fbf2a1a1540d3d96b2935..008635cc510d5eecdd68669ac05fdd698f9c10af 100644
--- a/cc/ipc/copy_output_result_struct_traits.cc
+++ b/cc/ipc/copy_output_result_struct_traits.cc
@@ -4,30 +4,53 @@
 
 #include "cc/ipc/copy_output_result_struct_traits.h"
 
-namespace mojo {
+#include "mojo/public/cpp/bindings/strong_binding.h"
 
-// static
-bool StructTraits<cc::mojom::CopyOutputResultDataView,
-                  std::unique_ptr<cc::CopyOutputResult>>::
-    Read(cc::mojom::CopyOutputResultDataView data,
-         std::unique_ptr<cc::CopyOutputResult>* out_p) {
-  auto result = cc::CopyOutputResult::CreateEmptyResult();
+namespace {
 
-  if (!data.ReadSize(&result->size_))
-    return false;
+// This class retains the release_callback_ of the CopyOutputResult that is
+// being sent over mojo. A TextureMailboxReleaserPtr that talks to this impl
+// object will be sent over mojo instead of the release_callback_ (which is not
+// serializable). Once the client calls Release, the release_callback_ will be
+// called. An object of this class will remain alive until the MessagePipe
+// attached to it goes away (i.e. StrongBinding is used).
+class TextureMailboxReleaserImpl : public cc::mojom::TextureMailboxReleaser {
+ public:
+  TextureMailboxReleaserImpl(
+      std::unique_ptr<cc::SingleReleaseCallback> release_callback)
+      : release_callback_(std::move(release_callback)) {
+    DCHECK(release_callback_);
+  }
 
-  result->bitmap_ = base::MakeUnique<SkBitmap>();
-  if (!data.ReadBitmap(result->bitmap_.get()))
-    return false;
+  ~TextureMailboxReleaserImpl() override {
+    // If the client fails to call Release, we should do it ourselves because
+    // release_callback_ will fail if it's never called.
+    if (release_callback_)
+      release_callback_->Run(gpu::SyncToken(), true);
+  }
 
-  if (!data.ReadTextureMailbox(&result->texture_mailbox_))
-    return false;
+  // mojom::TextureMailboxReleaser implementation:
+  void Release(const gpu::SyncToken& sync_token, bool is_lost) override {
+    if (!release_callback_)
+      return;
+    release_callback_->Run(sync_token, is_lost);
+    release_callback_.reset();
+  }
 
-  *out_p = std::move(result);
+ private:
+  std::unique_ptr<cc::SingleReleaseCallback> release_callback_;
+};
 
-  return true;
+void Release(cc::mojom::TextureMailboxReleaserPtr ptr,
+             const gpu::SyncToken& sync_token,
+             bool is_lost) {
+  ptr->Release(sync_token, is_lost);
 }
 
+}  // namespace
+
+namespace mojo {
+
 // static
 const SkBitmap& StructTraits<cc::mojom::CopyOutputResultDataView,
                              std::unique_ptr<cc::CopyOutputResult>>::
@@ -38,4 +61,74 @@ const SkBitmap& StructTraits<cc::mojom::CopyOutputResultDataView,
   return *result->bitmap_;
 }
 
+// static
+cc::mojom::TextureMailboxReleaserPtr
+StructTraits<cc::mojom::CopyOutputResultDataView,
+             std::unique_ptr<cc::CopyOutputResult>>::
+    releaser(const std::unique_ptr<cc::CopyOutputResult>& result) {
+  if (!result->release_callback_)
+    return {};
+  cc::mojom::TextureMailboxReleaserPtr releaser;
+  auto impl = base::MakeUnique<TextureMailboxReleaserImpl>(
+      std::move(result->release_callback_));
+  MakeStrongBinding(std::move(impl), MakeRequest(&releaser));
+  return releaser;
+}
+
+// static
+bool StructTraits<cc::mojom::CopyOutputResultDataView,
+                  std::unique_ptr<cc::CopyOutputResult>>::
+    Read(cc::mojom::CopyOutputResultDataView data,
+         std::unique_ptr<cc::CopyOutputResult>* out_p) {
+  // We first read into local variables and then call the appropriate
+  // constructor of cc::CopyOutputResult.
+  gfx::Size size;
+  auto bitmap = base::MakeUnique<SkBitmap>();
+  cc::TextureMailbox texture_mailbox;
+  std::unique_ptr<cc::SingleReleaseCallback> release_callback;
+
+  if (!data.ReadSize(&size))
+    return false;
+
+  if (!data.ReadBitmap(bitmap.get()))
+    return false;
+
+  if (!data.ReadTextureMailbox(&texture_mailbox))
+    return false;
+
+  auto releaser = data.TakeReleaser<cc::mojom::TextureMailboxReleaserPtr>();
+  if (releaser) {
+    // CopyOutputResult does not have a TextureMailboxReleaserPtr member.
+    // We use base::Bind to turn TextureMailboxReleaser::Release into a
+    // ReleaseCallback.
+    release_callback = cc::SingleReleaseCallback::Create(
+        base::Bind(Release, base::Passed(&releaser)));
+  }
+
+  // Empty result

[danakj, 2017/02/14 23:49:34]

comments are sentences, needs periods.

+  if (bitmap->isNull() && !texture_mailbox.IsTexture()) {
+    *out_p = cc::CopyOutputResult::CreateEmptyResult();
+    return true;
+  }
+
+  // Bitmap result
+  if (!bitmap->isNull()) {
+    // We can't have both a bitmap and a texture.
+    if (texture_mailbox.IsTexture())
+      return false;
+    *out_p = cc::CopyOutputResult::CreateBitmapResult(std::move(bitmap));
+    return true;
+  }
+
+  // Texture result
+  DCHECK(texture_mailbox.IsTexture());
+  if (size.IsEmpty())
+    return false;
+  if (!release_callback)
+    return false;
+  *out_p = cc::CopyOutputResult::CreateTextureResult(
+      size, texture_mailbox, std::move(release_callback));
+  return true;
+}
+
 }  // namespace mojo