CopyOutputResult must have a working release callback when received over mojo

cc/ipc/struct_traits_unittest.cc

 // 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 "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"
 #include "cc/quads/debug_border_draw_quad.h"
 #include "cc/quads/render_pass.h"
 #include "cc/quads/render_pass_draw_quad.h"
 #include "cc/quads/solid_color_draw_quad.h"
 #include "cc/quads/stream_video_draw_quad.h"
 #include "cc/quads/surface_draw_quad.h"
 #include "cc/quads/texture_draw_quad.h"
 #include "cc/quads/yuv_video_draw_quad.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkString.h"
 #include "third_party/skia/include/effects/SkDropShadowImageFilter.h"
 
 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() {}
 
  protected:
   mojom::TraitsTestServicePtr GetTraitsTestProxy() {
     return traits_test_bindings_.CreateInterfacePtrAndBind(this);
   }
 
+  TestTextureMailboxReleaserProvider ptr_provider_;
+
  private:
   // TraitsTestService:
   void EchoBeginFrameArgs(const BeginFrameArgs& b,
                           const EchoBeginFrameArgsCallback& callback) override {
     callback.Run(b);
   }
 
   void EchoCompositorFrame(
       CompositorFrame c,
       const EchoCompositorFrameCallback& callback) override {
     callback.Run(std::move(c));
   }
 
   void EchoCompositorFrameMetadata(
       const CompositorFrameMetadata& c,
       const EchoCompositorFrameMetadataCallback& callback) override {
     callback.Run(c);
   }
 
   void EchoCopyOutputRequest(
       std::unique_ptr<CopyOutputRequest> c,
       const EchoCopyOutputRequestCallback& callback) override {
     callback.Run(std::move(c));
   }
 
   void EchoCopyOutputResult(
       std::unique_ptr<CopyOutputResult> c,
       const EchoCopyOutputResultCallback& callback) override {
+    c->set_ptr_provider(&ptr_provider_);
     callback.Run(std::move(c));
   }
 
   void EchoFilterOperation(
       const FilterOperation& f,
       const EchoFilterOperationCallback& callback) override {
     callback.Run(f);
   }
 
   void EchoFilterOperations(
@@ skipping 51 matching lines @@
       const EchoTransferableResourceCallback& callback) override {
     callback.Run(t);
   }
 
   mojo::BindingSet<TraitsTestService> traits_test_bindings_;
   DISALLOW_COPY_AND_ASSIGN(StructTraitsTest);
 };
 
 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
 
 TEST_F(StructTraitsTest, BeginFrameArgs) {
   const base::TimeTicks frame_time = base::TimeTicks::Now();
   const base::TimeTicks deadline = base::TimeTicks::Now();
   const base::TimeDelta interval = base::TimeDelta::FromMilliseconds(1337);
   const BeginFrameArgs::BeginFrameArgsType type = BeginFrameArgs::NORMAL;
   const bool on_critical_path = true;
   BeginFrameArgs input;
@@ skipping 278 matching lines @@
   EXPECT_EQ(in_bitmap->getSize(), out_bitmap->getSize());
   EXPECT_EQ(0, std::memcmp(in_bitmap->getPixels(), out_bitmap->getPixels(),
                            in_bitmap->getSize()));
 }
 
 TEST_F(StructTraitsTest, CopyOutputResult_Texture) {
   const gfx::Size size(1234, 5678);
   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;
   proxy->EchoCopyOutputResult(std::move(input), &output);
 
   EXPECT_FALSE(output->HasBitmap());
   EXPECT_TRUE(output->HasTexture());
   EXPECT_EQ(size, output->size());
 
   TextureMailbox out_mailbox;
   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) {
   const FilterOperation inputs[] = {
       FilterOperation::CreateBlurFilter(20),
       FilterOperation::CreateDropShadowFilter(gfx::Point(4, 4), 4.0f,
                                               SkColorSetARGB(255, 40, 0, 0)),
       FilterOperation::CreateReferenceFilter(SkDropShadowImageFilter::Make(
           SkIntToScalar(3), SkIntToScalar(8), SkIntToScalar(4),
           SkIntToScalar(9), SK_ColorBLACK,
@@ skipping 617 matching lines @@
   EXPECT_EQ(u_plane_resource_id, out_quad->u_plane_resource_id());
   EXPECT_EQ(v_plane_resource_id, out_quad->v_plane_resource_id());
   EXPECT_EQ(a_plane_resource_id, out_quad->a_plane_resource_id());
   EXPECT_EQ(color_space, out_quad->color_space);
   EXPECT_EQ(resource_offset, out_quad->resource_offset);
   EXPECT_EQ(resource_multiplier, out_quad->resource_multiplier);
   EXPECT_EQ(bits_per_channel, out_quad->bits_per_channel);
 }
 
 }  // namespace cc