CopyOutputResult must have a working release callback when received over mojo

cc/ipc/struct_traits_unittest.cc

Index: cc/ipc/struct_traits_unittest.cc
diff --git a/cc/ipc/struct_traits_unittest.cc b/cc/ipc/struct_traits_unittest.cc
index 5d22e492c579dd577f6d3141de592f2a9663a9d4..89529ea8c4d8e0e4c701734a21e040c303dfbdbe 100644
--- a/cc/ipc/struct_traits_unittest.cc
+++ b/cc/ipc/struct_traits_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "cc/input/selection.h"
 #include "cc/ipc/traits_test_service.mojom.h"
 #include "cc/output/copy_output_result.h"
@@ -23,6 +24,55 @@ namespace cc {
 
 namespace {
 
+class TextureMailboxReleaserImpl : public mojom::TextureMailboxReleaser {

[Ken Rockot(use gerrit already), 2017/02/10 01:17:24]

I don't understand why you need more than this impl of TextureMailboxRelease
(and therefore why you need a TextureMailboxReleaseProvider). Maybe you aren't
aware of StrongBinding! You should be able to hide this impl inside the traits
cc and have the serializer do:

  cc::mojom::TextureMailboxReleaserPtr
  StructTraits<cc::mojom::CopyOutputResultDataView,
               std::unique_ptr<cc::CopyOutputResult>>::
      releaser(const std::unique_ptr<cc::CopyOutputResult>& result) {
    if (result->release_callback_) {
      cc::mojom::TextureMailboxReleaserPtr releaser;
      mojo::MakeStrongBinding(
          base::MakeUnique<ReleaserImpl>(result->release_callback_),
          mojo::MakeRequest(&release));
      return releaser;
    } else {
      return cc::mojom::TextureMailboxReleaserPtr();
    }
  }

[Saman Sami, 2017/02/10 01:33:53]

That's a great advice! The whole point of the Provider class is that I thought
the binding has to live somewhere! I think StrongBinding should work and make
things a whole lot simpler.

+ public:
+  TextureMailboxReleaserImpl(
+      std::unique_ptr<SingleReleaseCallback> release_callback,
+      mojom::TextureMailboxReleaserRequest request,
+      base::Closure quit_closure)
+      : release_callback_(std::move(release_callback)),
+        quit_closure_(quit_closure),
+        binding_(this, std::move(request)) {}
+
+  // mojom::TextureMailboxReleaser implementation:
+  void Release(const gpu::SyncToken& sync_token, bool is_lost) override {
+    release_callback_->Run(sync_token, is_lost);
+    quit_closure_.Run();
+  }
+
+ private:
+  std::unique_ptr<SingleReleaseCallback> release_callback_;
+  base::Closure quit_closure_;
+  mojo::Binding<mojom::TextureMailboxReleaser> binding_;
+};
+
+class TestTextureMailboxReleaserProvider
+    : public TextureMailboxReleaserProvider {
+ public:
+  void RunAllRunLoops() {
+    for (const auto& run_loop : run_loops_)
+      run_loop->Run();
+    run_loops_.clear();
+  }
+
+  // TextureMailboxReleaserProvider implementation:
+  mojom::TextureMailboxReleaserPtr GetTextureMailboxReleaser(
+      std::unique_ptr<SingleReleaseCallback> release_callback) override {
+    mojom::TextureMailboxReleaserPtr ptr;
+    auto run_loop = base::MakeUnique<base::RunLoop>();
+    auto impl = base::MakeUnique<TextureMailboxReleaserImpl>(
+        std::move(release_callback), mojo::MakeRequest(&ptr),
+        run_loop->QuitClosure());
+    impls_.push_back(std::move(impl));
+    run_loops_.push_back(std::move(run_loop));
+    return ptr;
+  }
+
+ private:
+  std::vector<std::unique_ptr<base::RunLoop>> run_loops_;
+  std::vector<std::unique_ptr<TextureMailboxReleaserImpl>> impls_;
+};
+
 class StructTraitsTest : public testing::Test, public mojom::TraitsTestService {
  public:
   StructTraitsTest() {}
@@ -32,6 +82,8 @@ class StructTraitsTest : public testing::Test, public mojom::TraitsTestService {
     return traits_test_bindings_.CreateInterfacePtrAndBind(this);
   }
 
+  TestTextureMailboxReleaserProvider ptr_provider_;
+
  private:
   // TraitsTestService:
   void EchoBeginFrameArgs(const BeginFrameArgs& b,
@@ -60,6 +112,7 @@ class StructTraitsTest : public testing::Test, public mojom::TraitsTestService {
   void EchoCopyOutputResult(
       std::unique_ptr<CopyOutputResult> c,
       const EchoCopyOutputResultCallback& callback) override {
+    c->set_ptr_provider(&ptr_provider_);
     callback.Run(std::move(c));
   }
 
@@ -131,8 +184,11 @@ class StructTraitsTest : public testing::Test, public mojom::TraitsTestService {
 
 void StubCopyOutputRequestCallback(std::unique_ptr<CopyOutputResult> result) {}
 
-void StubCopyOutputResultCallback(const gpu::SyncToken& sync_token,
-                                  bool is_lost) {}
+void CopyOutputResultCallback(bool* is_called,
+                              const gpu::SyncToken& sync_token,
+                              bool is_lost) {
+  *is_called = true;
+}
 
 }  // namespace
 
@@ -431,14 +487,16 @@ TEST_F(StructTraitsTest, CopyOutputResult_Texture) {
   const int8_t mailbox_name[GL_MAILBOX_SIZE_CHROMIUM] = {
       0, 9, 8, 7, 6, 5, 4, 3, 2, 1, 9, 7, 5, 3, 1, 3};
   const uint32_t target = 3;
-  auto callback =
-      SingleReleaseCallback::Create(base::Bind(StubCopyOutputResultCallback));
+  bool is_called = false;
+  auto callback = SingleReleaseCallback::Create(
+      base::Bind(CopyOutputResultCallback, &is_called));
   gpu::Mailbox mailbox;
   mailbox.SetName(mailbox_name);
   TextureMailbox texture_mailbox(mailbox, gpu::SyncToken(), target);
 
   auto input = CopyOutputResult::CreateTextureResult(size, texture_mailbox,
                                                      std::move(callback));
+  input->set_ptr_provider(&ptr_provider_);
 
   mojom::TraitsTestServicePtr proxy = GetTraitsTestProxy();
   std::unique_ptr<CopyOutputResult> output;
@@ -452,6 +510,10 @@ TEST_F(StructTraitsTest, CopyOutputResult_Texture) {
   std::unique_ptr<SingleReleaseCallback> out_callback;
   output->TakeTexture(&out_mailbox, &out_callback);
   EXPECT_EQ(mailbox, out_mailbox.mailbox());
+  EXPECT_FALSE(is_called);
+  out_callback->Run(gpu::SyncToken(), true);
+  ptr_provider_.RunAllRunLoops();
+  EXPECT_TRUE(is_called);
 }
 
 TEST_F(StructTraitsTest, FilterOperation) {