Privatize StrongBinding lifetime management

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

 // Copyright 2015 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.
 
 // Note: This file tests both binding.h (mojo::Binding) and strong_binding.h
 // (mojo::StrongBinding).
 
 #include "mojo/public/cpp/bindings/binding.h"
 
 #include <stdint.h>
@@ skipping 416 matching lines @@
 // StrongBindingTest -----------------------------------------------------------
 
 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;
-  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.
+
+  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();
+
+  // 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);
-  // But destroying the StrongBinding doesn't destroy the object.
-  ASSERT_FALSE(was_deleted);
+
+  // Destroying the StrongBinding also destroys the impl.
+  ASSERT_TRUE(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_; }
-
- private:
-  StrongBinding<sample::Service> binding_;
-
-  DISALLOW_COPY_AND_ASSIGN(ServiceImplWithStrongBinding);
-};
-
 // Tests the typical case, where the implementation object owns the
 // StrongBinding (and should be destroyed on connection error).
 TEST_F(StrongBindingTest, ConnectionErrorDestroysImpl) {
   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);
 
   ptr.reset();
   EXPECT_FALSE(was_deleted);
   run_loop.Run();
   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));
   EXPECT_FALSE(called);
   // 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);
 }
 
 TEST_F(StrongBindingTest, FlushForTestingWithClosedPeer) {
   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(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
 }  // mojo