Mojo C++ Bindings: Introduce mojo::SupportsStrongBinding

mojo/public/cpp/bindings/tests/binding_unittest.cc

Index: mojo/public/cpp/bindings/tests/binding_unittest.cc
diff --git a/mojo/public/cpp/bindings/tests/binding_unittest.cc b/mojo/public/cpp/bindings/tests/binding_unittest.cc
index a2a26c64226220f81a266cc3b4101ddb206a0a20..e9a334c28c672a80ece7a1e576d10e1d5e8e4031 100644
--- a/mojo/public/cpp/bindings/tests/binding_unittest.cc
+++ b/mojo/public/cpp/bindings/tests/binding_unittest.cc
@@ -36,13 +36,18 @@ class BindingTestBase : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(BindingTestBase);
 };
 
-class ServiceImpl : public sample::Service {
+class ServiceImpl : public SupportsStrongBinding<sample::Service> {
  public:
-  explicit ServiceImpl(bool* was_deleted = nullptr)
-      : was_deleted_(was_deleted) {}
+  explicit ServiceImpl(
+      bool* was_deleted = nullptr,
+      const base::Closure& destruction_callback = base::Closure())
+      : was_deleted_(was_deleted),
+        destruction_callback_(destruction_callback) {}
   ~ServiceImpl() override {
     if (was_deleted_)
       *was_deleted_ = true;
+    if (!destruction_callback_.is_null())
+      destruction_callback_.Run();
   }
 
  private:
@@ -56,6 +61,7 @@ class ServiceImpl : public sample::Service {
   void GetPort(InterfaceRequest<sample::Port> port) override {}
 
   bool* const was_deleted_;
+  const base::Closure destruction_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(ServiceImpl);
 };
@@ -173,25 +179,19 @@ TEST_F(BindingTest, CloseDoesntCallConnectionErrorHandler) {
 class ServiceImplWithBinding : public ServiceImpl {
  public:
   ServiceImplWithBinding(bool* was_deleted,
-                         const base::Closure& closure,
+                         const base::Closure& destruction_callback,
                          InterfaceRequest<sample::Service> request)
-      : ServiceImpl(was_deleted),
-        binding_(this, std::move(request)),
-        closure_(closure) {
+      : ServiceImpl(was_deleted, destruction_callback),
+        binding_(this, std::move(request)) {
     binding_.set_connection_error_handler(
         base::Bind(&ServiceImplWithBinding::OnConnectionError,
                    base::Unretained(this)));
   }
 
  private:
-  ~ServiceImplWithBinding() override{
-    closure_.Run();
-  }
-
   void OnConnectionError() { delete this; }
 
   Binding<sample::Service> binding_;
-  base::Closure closure_;
 
   DISALLOW_COPY_AND_ASSIGN(ServiceImplWithBinding);
 };
@@ -488,55 +488,46 @@ TEST_F(BindingTest, CustomImplPointerType) {
 
 using StrongBindingTest = BindingTestBase;
 
-// Tests that destroying a mojo::StrongBinding closes the bound message pipe
-// handle but does *not* destroy the implementation object.
-TEST_F(StrongBindingTest, DestroyClosesMessagePipe) {
-  base::RunLoop run_loop;
-  bool encountered_error = false;
-  bool was_deleted = false;
+TEST_F(StrongBindingTest, ConnectionErrorDestroysImpl) {
   sample::ServicePtr ptr;
-  auto request = GetProxy(&ptr);
-  ptr.set_connection_error_handler(
-      SetFlagAndRunClosure(&encountered_error, run_loop.QuitClosure()));
-  bool called = false;
-  base::RunLoop run_loop2;
-
-  auto binding = MakeStrongBinding(base::MakeUnique<ServiceImpl>(&was_deleted),
-                                   std::move(request));
-  ptr->Frobinate(
-      nullptr, sample::Service::BazOptions::REGULAR, nullptr,
-      SetFlagAndRunClosure<int32_t>(&called, run_loop2.QuitClosure()));
-  run_loop2.Run();
-  EXPECT_TRUE(called);
-  EXPECT_FALSE(encountered_error);
-  binding->Close();
+  bool was_deleted = false;
 
-  // Now that the StrongBinding is closed we should detect an error on the other
-  // end of the pipe.
+  base::RunLoop run_loop;
+  StrongBindingPtr<sample::Service> weak_binding = mojo::MakeStrongBinding(
+      base::MakeUnique<ServiceImpl>(&was_deleted, run_loop.QuitClosure()),
+      GetProxy(&ptr));
+  ptr.reset();
+  EXPECT_FALSE(was_deleted);
   run_loop.Run();
-  EXPECT_TRUE(encountered_error);
-
-  // Destroying the StrongBinding also destroys the impl.
-  ASSERT_TRUE(was_deleted);
+  EXPECT_TRUE(was_deleted);
+  EXPECT_FALSE(weak_binding);
 }
 
-// Tests the typical case, where the implementation object owns the
-// StrongBinding (and should be destroyed on connection error).
-TEST_F(StrongBindingTest, ConnectionErrorDestroysImpl) {
+TEST_F(StrongBindingTest, ExplicitImplDeletionIsSafe) {
   sample::ServicePtr ptr;
   bool was_deleted = false;
-  // Will delete itself.
-  base::RunLoop run_loop;
-  new ServiceImplWithBinding(&was_deleted, run_loop.QuitClosure(),
-                             GetProxy(&ptr));
 
-  base::RunLoop().RunUntilIdle();
-  EXPECT_FALSE(was_deleted);
+  base::Closure close_callback;
+
+  std::unique_ptr<ServiceImpl> impl =
+      base::MakeUnique<ServiceImpl>(&was_deleted);
+  ServiceImpl* raw_impl = impl.get();
+  StrongBindingPtr<sample::Service> weak_binding =
+      mojo::MakeStrongBinding(std::move(impl), GetProxy(&ptr));
 
-  ptr.reset();
   EXPECT_FALSE(was_deleted);
-  run_loop.Run();
+
+  base::RunLoop run_loop;
+  ptr.set_connection_error_handler(run_loop.QuitClosure());
+
+  EXPECT_TRUE(weak_binding);
+
+  delete raw_impl;
+
   EXPECT_TRUE(was_deleted);
+  EXPECT_FALSE(weak_binding);
+
+  run_loop.Run();
 }
 
 TEST_F(StrongBindingTest, FlushForTesting) {