Privatize StrongBinding lifetime management

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 33398af9d7c88c4e36df447cb75a7ff0f8bcdf55..c2ec238ca832c0ee73d0c473a433c4c9e5ace9b2 100644
--- a/mojo/public/cpp/bindings/tests/binding_unittest.cc
+++ b/mojo/public/cpp/bindings/tests/binding_unittest.cc
@@ -434,43 +434,31 @@ TEST_F(StrongBindingTest, DestroyClosesMessagePipe) {
   base::RunLoop run_loop;
   bool encountered_error = false;
   bool was_deleted = false;
-  ServiceImpl impl(&was_deleted);
   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;
-  {
-    StrongBinding<sample::Service> binding(&impl, 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);
-  }
-  // Now that the StrongBinding is out of scope we should detect an error on the
-  // other end of the pipe.
-  run_loop.Run();
-  EXPECT_TRUE(encountered_error);
-  // But destroying the StrongBinding doesn't destroy the object.
-  ASSERT_FALSE(was_deleted);
-}
-
-class ServiceImplWithStrongBinding : public ServiceImpl {
- public:
-  ServiceImplWithStrongBinding(bool* was_deleted,
-                               InterfaceRequest<sample::Service> request)
-      : ServiceImpl(was_deleted), binding_(this, std::move(request)) {}
 
-  StrongBinding<sample::Service>& binding() { return binding_; }
+  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();
 
- private:
-  StrongBinding<sample::Service> binding_;
+  // Now that the StrongBinding is closed we should detect an error on the other
+  // end of the pipe.
+  run_loop.Run();
+  EXPECT_TRUE(encountered_error);
 
-  DISALLOW_COPY_AND_ASSIGN(ServiceImplWithStrongBinding);
-};
+  // Destroying the StrongBinding also destroys the impl.
+  ASSERT_TRUE(was_deleted);
+}
 
 // Tests the typical case, where the implementation object owns the
 // StrongBinding (and should be destroyed on connection error).
@@ -491,47 +479,14 @@ TEST_F(StrongBindingTest, ConnectionErrorDestroysImpl) {
   EXPECT_TRUE(was_deleted);
 }
 
-// Tests that even when the implementation object owns the StrongBinding, that

[yzshen1, 2016/09/09 16:21:30]

This pattern seems pretty weird. +1 that StrongBinding no longer supports this!

If some code needs this, I think it should using Binding and does the lifetime
management explicitly by itself. 

-// the implementation can still be deleted (which should result in the message
-// pipe being closed). Also checks that the connection error handler doesn't get
-// called.
-TEST_F(StrongBindingTest, ExplicitDeleteImpl) {
-  bool ptr_error_handler_called = false;
-  sample::ServicePtr ptr;
-  auto request = GetProxy(&ptr);
-  base::RunLoop run_loop;
-  ptr.set_connection_error_handler(
-      SetFlagAndRunClosure(&ptr_error_handler_called, run_loop.QuitClosure()));
-  bool was_deleted = false;
-  ServiceImplWithStrongBinding* impl =
-      new ServiceImplWithStrongBinding(&was_deleted, std::move(request));
-  bool binding_error_handler_called = false;
-  impl->binding().set_connection_error_handler(
-      SetFlagAndRunClosure(&binding_error_handler_called));
-
-  base::RunLoop().RunUntilIdle();
-  EXPECT_FALSE(ptr_error_handler_called);
-  EXPECT_FALSE(was_deleted);
-
-  delete impl;
-  EXPECT_FALSE(ptr_error_handler_called);
-  EXPECT_TRUE(was_deleted);
-  was_deleted = false;  // It shouldn't be double-deleted!
-  run_loop.Run();
-  EXPECT_TRUE(ptr_error_handler_called);
-  EXPECT_FALSE(was_deleted);
-
-  EXPECT_FALSE(binding_error_handler_called);
-}
-
 TEST_F(StrongBindingTest, FlushForTesting) {
   bool called = false;
   bool was_deleted = false;
   sample::ServicePtr ptr;
   auto request = GetProxy(&ptr);
-  StrongBinding<sample::Service> binding(new ServiceImpl(&was_deleted),
-                                         std::move(request));
-  binding.set_connection_error_handler(base::Bind(&Fail));
+  auto binding = MakeStrongBinding(base::MakeUnique<ServiceImpl>(&was_deleted),
+                                   std::move(request));
+  binding->set_connection_error_handler(base::Bind(&Fail));
 
   ptr->Frobinate(nullptr, sample::Service::BazOptions::REGULAR, nullptr,
                  SetFlagAndRunClosure<int32_t>(&called));
@@ -539,13 +494,16 @@ TEST_F(StrongBindingTest, FlushForTesting) {
   // Because the flush is sent from the binding, it only guarantees that the
   // request has been received, not the response. The second flush waits for the
   // response to be received.
-  binding.FlushForTesting();
-  binding.FlushForTesting();
+  ASSERT_TRUE(binding);
+  binding->FlushForTesting();
+  ASSERT_TRUE(binding);
+  binding->FlushForTesting();
   EXPECT_TRUE(called);
   EXPECT_FALSE(was_deleted);
   ptr.reset();
-  binding.set_connection_error_handler(base::Closure());
-  binding.FlushForTesting();
+  ASSERT_TRUE(binding);
+  binding->set_connection_error_handler(base::Closure());
+  binding->FlushForTesting();
   EXPECT_TRUE(was_deleted);
 }
 
@@ -554,18 +512,18 @@ TEST_F(StrongBindingTest, FlushForTestingWithClosedPeer) {
   bool was_deleted = false;
   sample::ServicePtr ptr;
   auto request = GetProxy(&ptr);
-  StrongBinding<sample::Service> binding(new ServiceImpl(&was_deleted),
-                                         std::move(request));
-  binding.set_connection_error_handler(SetFlagAndRunClosure(&called));
+  auto binding = MakeStrongBinding(base::MakeUnique<ServiceImpl>(&was_deleted),
+                                   std::move(request));
+  binding->set_connection_error_handler(SetFlagAndRunClosure(&called));
   ptr.reset();
 
   EXPECT_FALSE(called);
   EXPECT_FALSE(was_deleted);
-  binding.FlushForTesting();
+  ASSERT_TRUE(binding);
+  binding->FlushForTesting();
   EXPECT_TRUE(called);
   EXPECT_TRUE(was_deleted);
-  binding.FlushForTesting();
-  EXPECT_TRUE(was_deleted);
+  ASSERT_FALSE(binding);
 }
 
 }  // namespace