CopyOutputResult must have a working release callback when received over mojo

cc/ipc/single_release_callback_struct_traits.cc

Index: cc/ipc/single_release_callback_struct_traits.cc
diff --git a/cc/ipc/single_release_callback_struct_traits.cc b/cc/ipc/single_release_callback_struct_traits.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c47ff4c9da4308ecf8f262e02220a114ac20c14b
--- /dev/null
+++ b/cc/ipc/single_release_callback_struct_traits.cc
@@ -0,0 +1,30 @@
+// Copyright 2016 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 "cc/ipc/single_release_callback_struct_traits.h"
+#include "cc/ipc/texture_mailbox_releaser.mojom.h"
+
+namespace mojo {
+
+// static
+cc::mojom::TextureMailboxReleaserPtr
+StructTraits<cc::mojom::SingleReleaseCallbackDataView,
+             std::unique_ptr<cc::SingleReleaseCallback>>::
+    releaser(const std::unique_ptr<cc::SingleReleaseCallback>& callback) {
+  auto result = callback->ptr_holder_->TakePtr();

[danakj, 2017/02/08 23:24:17]

It seems like we're being tied to the c++ types too strongly here. When we
serialize we've already made a mojo type and hid it inside the
SingleReleaseCallback just so that we have the right type here. Maybe we can
loosen these requirements, and use a different type than SingleReleaseCallback
here as a member of the mojo'd CopyOutputResult. Or maybe we need a different
mojo CopyOutputResult type or something.

But I think at the end we shouldn't have SingleResultCallback knowing about
mojo, we should have a distinct type entirely, or it has an interface that
happens to have a mojo-based impl without it knowing. Which means TakePtr() here
doesn't work. So that says to me the serialization code shouldn't be making a
mojo-backed SingleReleaseCallback, only the reciever side should be doing that.
And the sender should be sending something else..

[Saman Sami, 2017/02/09 16:30:01]

In struct traits, when serializing, a mojo pointer has to be somehow obtained.
One way is to have already stored the pointer in the copy output result (either
directly, or indirectly in SingleReleaseCallback), which you do not seem to be a
huge fan of. An alternative is that CopyOutputResult can have a PtrProvider or
something that has a virtual method that GpuCompositorFrameSink implements which
takes in a SingleReleaseCallback and returns a mojo pointer to be used in struct
traits while serializing. When deseriazing, we turn the mojo pointer into a
SingleReleaseCallback using base::Bind. What do you think? This doesn't sound
too terrible.

[Saman Sami, 2017/02/09 16:52:25]

However, note that having a method like TakePtr that returns a
TextureMailboxReleaserPtr is unavoidable. In my new proposal, the PtrProvider
class will have such a method. But it would be a virtual method with no
implementation in cc/, which is I guess the best we could hope for.

+  callback->ptr_holder_.reset();
+  return result;
+}
+
+// static
+bool StructTraits<cc::mojom::SingleReleaseCallbackDataView,
+                  std::unique_ptr<cc::SingleReleaseCallback>>::
+    Read(cc::mojom::SingleReleaseCallbackDataView data,
+         std::unique_ptr<cc::SingleReleaseCallback>* out) {
+  auto releaser = data.TakeReleaser<cc::mojom::TextureMailboxReleaserPtr>();
+  *out = cc::SingleReleaseCallback::Create(std::move(releaser));
+  return true;
+}
+
+}  // namespace mojo