CopyOutputRequest must have a working result_callback_ when received over mojo

cc/ipc/copy_output_request_struct_traits.cc

Index: cc/ipc/copy_output_request_struct_traits.cc
diff --git a/cc/ipc/copy_output_request_struct_traits.cc b/cc/ipc/copy_output_request_struct_traits.cc
index 3985ba41eb744a7c114458b4bc94e1c8b26ce9b8..59890a99a51fe44ca1b8dec5ab77f7143f353b87 100644
--- a/cc/ipc/copy_output_request_struct_traits.cc
+++ b/cc/ipc/copy_output_request_struct_traits.cc
@@ -4,9 +4,62 @@
 
 #include "cc/ipc/copy_output_request_struct_traits.h"
 
+#include "base/callback_helpers.h"
+#include "mojo/public/cpp/bindings/strong_binding.h"
+
+namespace {
+
+// When we're sending a CopyOutputRequest, we keep the result_callback_ in a
+// CopyOutputResultSenderImpl and send a CopyOutputResultSenderPtr to the other
+// process. When SendResult is called, we run the stored result_callback_.
+class CopyOutputResultSenderImpl : public cc::mojom::CopyOutputResultSender {
+ public:
+  CopyOutputResultSenderImpl(
+      cc::CopyOutputRequest::CopyOutputRequestCallback result_callback)
+      : result_callback_(result_callback) {
+    DCHECK(result_callback_);
+  }
+
+  ~CopyOutputResultSenderImpl() override {
+    if (result_callback_)
+      result_callback_.Run(cc::CopyOutputResult::CreateEmptyResult());
+  }
+
+  // mojom::TextureMailboxReleaser implementation:
+  void SendResult(std::unique_ptr<cc::CopyOutputResult> result) override {
+    if (!result_callback_)
+      return;
+    base::ResetAndReturn(&result_callback_).Run(std::move(result));
+  }
+
+ private:
+  cc::CopyOutputRequest::CopyOutputRequestCallback result_callback_;
+};
+
+void SendResult(cc::mojom::CopyOutputResultSenderPtr ptr,
+                std::unique_ptr<cc::CopyOutputResult> result) {
+  ptr->SendResult(std::move(result));
+}
+
+}  // namespace
+
 namespace mojo {
 
 // static
+cc::mojom::CopyOutputResultSenderPtr
+StructTraits<cc::mojom::CopyOutputRequestDataView,
+             std::unique_ptr<cc::CopyOutputRequest>>::
+    result_sender(const std::unique_ptr<cc::CopyOutputRequest>& request) {
+  cc::mojom::CopyOutputResultSenderPtr result_sender;
+  if (request->result_callback_) {

[danakj, 2017/02/15 19:48:19]

This one shouldn't be needed. As you said cc::CopyOutputRequest always has a
callback.
(https://cs.chromium.org/chromium/src/cc/output/copy_output_request.cc?rcl=064...)
But the mojo interface may be used badly. That's why we need the checks above.

[danakj, 2017/02/15 19:50:26]

Oh. Uh, I guess there is CreateEmptyRequest() which some unit tests use. I don't
think we need to support that with mojo right now. I was just worried about the
IPC interface being maliciously used and guarding the receiving side.

[Saman Sami, 2017/02/15 22:34:21]

Right. We won't support empty requests.

+    auto impl = base::MakeUnique<CopyOutputResultSenderImpl>(
+        std::move(request->result_callback_));
+    MakeStrongBinding(std::move(impl), MakeRequest(&result_sender));
+  }
+  return result_sender;
+}
+
+// static
 bool StructTraits<cc::mojom::CopyOutputRequestDataView,
                   std::unique_ptr<cc::CopyOutputRequest>>::
     Read(cc::mojom::CopyOutputRequestDataView data,
@@ -24,6 +77,13 @@ bool StructTraits<cc::mojom::CopyOutputRequestDataView,
   if (!data.ReadTextureMailbox(&request->texture_mailbox_))
     return false;
 
+  auto result_sender =
+      data.TakeResultSender<cc::mojom::CopyOutputResultSenderPtr>();
+  if (result_sender) {
+    request->result_callback_ =
+        base::Bind(SendResult, base::Passed(&result_sender));
+  }
+
   *out_p = std::move(request);
 
   return true;