Fix BluetoothAdapterProfileChromeOS lifecycle management

device/bluetooth/bluetooth_adapter_profile_chromeos_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.
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "chromeos/dbus/bluetooth_profile_service_provider.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/fake_bluetooth_adapter_client.h"
 #include "chromeos/dbus/fake_bluetooth_agent_manager_client.h"
 #include "chromeos/dbus/fake_bluetooth_device_client.h"
 #include "chromeos/dbus/fake_bluetooth_profile_manager_client.h"
 #include "device/bluetooth/bluetooth_adapter.h"
 #include "device/bluetooth/bluetooth_adapter_chromeos.h"
 #include "device/bluetooth/bluetooth_adapter_factory.h"
 #include "device/bluetooth/bluetooth_adapter_profile_chromeos.h"
 #include "device/bluetooth/bluetooth_uuid.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using device::BluetoothAdapter;
 using device::BluetoothUUID;
 
-namespace {
-
-void DoNothingDBusErrorCallback(const std::string& error_name,
-                                const std::string& error_message) {
-}
-
-}  // namespace
-
 namespace chromeos {
 
 class BluetoothAdapterProfileChromeOSTest : public testing::Test {
  public:
   BluetoothAdapterProfileChromeOSTest()
       : fake_delegate_paired_(FakeBluetoothDeviceClient::kPairedDevicePath),
         fake_delegate_autopair_(FakeBluetoothDeviceClient::kLegacyAutopairPath),
         fake_delegate_listen_(""),
-        profile_(nullptr),
+        profile_ptr_(nullptr),
         success_callback_count_(0),
         error_callback_count_(0) {}
 
   void SetUp() override {
     scoped_ptr<DBusThreadManagerSetter> dbus_setter =
         DBusThreadManager::GetSetterForTesting();
 
     dbus_setter->SetBluetoothAdapterClient(
         scoped_ptr<BluetoothAdapterClient>(new FakeBluetoothAdapterClient));
     dbus_setter->SetBluetoothAgentManagerClient(
@@ skipping 13 matching lines @@
     ASSERT_TRUE(adapter_->IsInitialized());
     ASSERT_TRUE(adapter_->IsPresent());
 
     // Turn on the adapter.
     adapter_->SetPowered(true, base::Bind(&base::DoNothing),
                          base::Bind(&base::DoNothing));
     ASSERT_TRUE(adapter_->IsPowered());
   }
 
   void TearDown() override {
+    profile_.reset();
     adapter_ = nullptr;
     DBusThreadManager::Shutdown();
   }
 
   void AdapterCallback(scoped_refptr<BluetoothAdapter> adapter) {
     adapter_ = adapter;
   }
 
   class FakeDelegate
       : public chromeos::BluetoothProfileServiceProvider::Delegate {
@@ skipping 12 matching lines @@
         scoped_ptr<dbus::FileDescriptor> fd,
         const BluetoothProfileServiceProvider::Delegate::Options& options,
         const ConfirmationCallback& callback) override {
       VLOG(1) << "connection for " << device_path.value() << " on "
               << device_path_.value();
       ++connections_;
       fd->CheckValidity();
       close(fd->TakeValue());
       callback.Run(SUCCESS);
       if (device_path_.value() != "")
-        ASSERT_TRUE(device_path == device_path_);
+        ASSERT_EQ(device_path_, device_path);
     }
 
     void RequestDisconnection(const dbus::ObjectPath& device_path,
                               const ConfirmationCallback& callback) override {
       VLOG(1) << "disconnect " << device_path.value();
       ++disconnections_;
     }
 
     void Cancel() override {
       VLOG(1) << "cancel";
       // noop
     }
 
     unsigned int connections_;
     unsigned int disconnections_;
     dbus::ObjectPath device_path_;
   };
 
   FakeDelegate fake_delegate_paired_;
   FakeDelegate fake_delegate_autopair_;
   FakeDelegate fake_delegate_listen_;
 
-  BluetoothAdapterProfileChromeOS* profile_;
+  scoped_ptr<BluetoothAdapterProfileChromeOS> profile_;
+
+  void ProfileSuccessCallback(
+      scoped_ptr<BluetoothAdapterProfileChromeOS> profile) {
+    profile_.swap(profile);
+    ++success_callback_count_;
+  }
+
+  BluetoothAdapterProfileChromeOS* profile_ptr_;

[Ilya Sherman, 2015/03/05 04:33:54]

It's kind of confusing to have both profile_ and profile_ptr_.  It looks like
tests use either one or the other.  Maybe it makes sense to separate these sets
of tests somehow?  Or, maybe the two different profile pointer variables could
have names that hint more directly at how they are used?

[Ilya Sherman, 2015/03/05 04:33:54]

FWIW, it's also surprising to me that this test class has public member
variables.  The typical style is to list all member variables at the bottom of a
visibility section.  In test fixture classes, the member variables are typically
protected.

[Marie Janssen, 2015/03/09 16:22:49]

Done.

+
+  void ProfileUserSuccessCallback(BluetoothAdapterProfileChromeOS* profile) {
+    profile_ptr_ = profile;
+    ++success_callback_count_;
+  }
+
+  void MatchedProfileCallback(BluetoothAdapterProfileChromeOS* profile) {
+    VLOG(1) << "Matched Profile Callback";
+    ASSERT_EQ(profile_ptr_, profile);
+    ++success_callback_count_;
+  }
 
   void DBusConnectSuccessCallback() { ++success_callback_count_; }
 
   void DBusErrorCallback(const std::string& error_name,
                          const std::string& error_message) {
     VLOG(1) << "DBus Connect Error: " << error_name << " - " << error_message;
     ++error_callback_count_;
   }
 
+  void BasicErrorCallback(const std::string& error_message) {
+    VLOG(1) << "Error: " << error_message;
+    ++error_callback_count_;
+  }
+
  protected:
   base::MessageLoop message_loop_;
 
   scoped_refptr<BluetoothAdapter> adapter_;
 
   unsigned int success_callback_count_;
   unsigned int error_callback_count_;
 };
 
 TEST_F(BluetoothAdapterProfileChromeOSTest, DelegateCount) {
   BluetoothUUID uuid(FakeBluetoothProfileManagerClient::kRfcommUuid);
   BluetoothProfileManagerClient::Options options;
 
   options.require_authentication.reset(new bool(false));
 
-  profile_ = BluetoothAdapterProfileChromeOS::Register(
-      static_cast<BluetoothAdapterChromeOS*>(adapter_.get()), uuid, options,
-      base::Bind(&base::DoNothing), base::Bind(&DoNothingDBusErrorCallback));
+  BluetoothAdapterProfileChromeOS::Register(
+      uuid, options,
+      base::Bind(&BluetoothAdapterProfileChromeOSTest::ProfileSuccessCallback,
+                 base::Unretained(this)),
+      base::Bind(&BluetoothAdapterProfileChromeOSTest::DBusErrorCallback,
+                 base::Unretained(this)));
 
   message_loop_.RunUntilIdle();
 
   EXPECT_TRUE(profile_);
+  EXPECT_EQ(1U, success_callback_count_);
+  EXPECT_EQ(0U, error_callback_count_);
 
   EXPECT_EQ(0U, profile_->DelegateCount());
 
   profile_->SetDelegate(fake_delegate_paired_.device_path_,
                         &fake_delegate_paired_);
 
   EXPECT_EQ(1U, profile_->DelegateCount());
 
   profile_->RemoveDelegate(fake_delegate_autopair_.device_path_,
                            base::Bind(&base::DoNothing));
 
   EXPECT_EQ(1U, profile_->DelegateCount());
 
   profile_->RemoveDelegate(fake_delegate_paired_.device_path_,
                            base::Bind(&base::DoNothing));
 
   EXPECT_EQ(0U, profile_->DelegateCount());
-
-  delete profile_;
 };
 
 TEST_F(BluetoothAdapterProfileChromeOSTest, BlackHole) {
   BluetoothUUID uuid(FakeBluetoothProfileManagerClient::kRfcommUuid);
   BluetoothProfileManagerClient::Options options;
 
   options.require_authentication.reset(new bool(false));
 
-  profile_ = BluetoothAdapterProfileChromeOS::Register(
-      static_cast<BluetoothAdapterChromeOS*>(adapter_.get()), uuid, options,
-      base::Bind(
-          &BluetoothAdapterProfileChromeOSTest::DBusConnectSuccessCallback,
-          base::Unretained(this)),
+  BluetoothAdapterProfileChromeOS::Register(
+      uuid, options,
+      base::Bind(&BluetoothAdapterProfileChromeOSTest::ProfileSuccessCallback,
+                 base::Unretained(this)),
       base::Bind(&BluetoothAdapterProfileChromeOSTest::DBusErrorCallback,
                  base::Unretained(this)));
 
   message_loop_.RunUntilIdle();
 
   EXPECT_TRUE(profile_);
   EXPECT_EQ(1U, success_callback_count_);
   EXPECT_EQ(0U, error_callback_count_);
 
   DBusThreadManager::Get()->GetBluetoothDeviceClient()->ConnectProfile(
       dbus::ObjectPath(FakeBluetoothDeviceClient::kPairedDevicePath),
       FakeBluetoothProfileManagerClient::kRfcommUuid,
       base::Bind(
           &BluetoothAdapterProfileChromeOSTest::DBusConnectSuccessCallback,
           base::Unretained(this)),
       base::Bind(&BluetoothAdapterProfileChromeOSTest::DBusErrorCallback,
                  base::Unretained(this)));
 
   message_loop_.RunUntilIdle();
 
   EXPECT_EQ(1U, success_callback_count_);
   EXPECT_EQ(1U, error_callback_count_);
 
   EXPECT_EQ(0U, fake_delegate_paired_.connections_);
-
-  delete profile_;
 };
 
 TEST_F(BluetoothAdapterProfileChromeOSTest, Routing) {
   BluetoothUUID uuid(FakeBluetoothProfileManagerClient::kRfcommUuid);
   BluetoothProfileManagerClient::Options options;
 
   options.require_authentication.reset(new bool(false));
 
-  profile_ = BluetoothAdapterProfileChromeOS::Register(
-      static_cast<BluetoothAdapterChromeOS*>(adapter_.get()), uuid, options,
-      base::Bind(
-          &BluetoothAdapterProfileChromeOSTest::DBusConnectSuccessCallback,
-          base::Unretained(this)),
+  BluetoothAdapterProfileChromeOS::Register(
+      uuid, options,
+      base::Bind(&BluetoothAdapterProfileChromeOSTest::ProfileSuccessCallback,
+                 base::Unretained(this)),
       base::Bind(&BluetoothAdapterProfileChromeOSTest::DBusErrorCallback,
                  base::Unretained(this)));
 
   message_loop_.RunUntilIdle();
 
   ASSERT_TRUE(profile_);
+  ASSERT_EQ(1U, success_callback_count_);
+  ASSERT_EQ(0U, error_callback_count_);
 
   profile_->SetDelegate(fake_delegate_paired_.device_path_,
                         &fake_delegate_paired_);
   profile_->SetDelegate(fake_delegate_autopair_.device_path_,
                         &fake_delegate_autopair_);
   profile_->SetDelegate(fake_delegate_listen_.device_path_,
                         &fake_delegate_listen_);
 
   DBusThreadManager::Get()->GetBluetoothDeviceClient()->ConnectProfile(
       dbus::ObjectPath(FakeBluetoothDeviceClient::kPairedDevicePath),
@@ skipping 36 matching lines @@
           base::Unretained(this)),
       base::Bind(&BluetoothAdapterProfileChromeOSTest::DBusErrorCallback,
                  base::Unretained(this)));
 
   message_loop_.RunUntilIdle();
 
   EXPECT_EQ(4U, success_callback_count_);
   EXPECT_EQ(0U, error_callback_count_);
 
   EXPECT_EQ(1U, fake_delegate_listen_.connections_);
+};
 
-  delete profile_;
+TEST_F(BluetoothAdapterProfileChromeOSTest, SimultaneousRegister) {
+  BluetoothUUID uuid(FakeBluetoothProfileManagerClient::kRfcommUuid);
+  BluetoothProfileManagerClient::Options options;
+  BluetoothAdapterChromeOS* adapter =
+      static_cast<BluetoothAdapterChromeOS*>(adapter_.get());
+
+  options.require_authentication.reset(new bool(false));
+
+  success_callback_count_ = 0;
+  error_callback_count_ = 0;
+
+  adapter->UseProfile(
+      uuid, fake_delegate_paired_.device_path_, options, &fake_delegate_paired_,
+      base::Bind(
+          &BluetoothAdapterProfileChromeOSTest::ProfileUserSuccessCallback,
+          base::Unretained(this)),
+      base::Bind(&BluetoothAdapterProfileChromeOSTest::BasicErrorCallback,
+                 base::Unretained(this)));
+
+  adapter->UseProfile(
+      uuid, fake_delegate_autopair_.device_path_, options,
+      &fake_delegate_autopair_,
+      base::Bind(&BluetoothAdapterProfileChromeOSTest::MatchedProfileCallback,
+                 base::Unretained(this)),
+      base::Bind(&BluetoothAdapterProfileChromeOSTest::BasicErrorCallback,
+                 base::Unretained(this)));
+
+  message_loop_.RunUntilIdle();
+
+  EXPECT_TRUE(profile_ptr_);
+  EXPECT_EQ(2U, success_callback_count_);
+  EXPECT_EQ(0U, error_callback_count_);
+
+  adapter->ReleaseProfile(fake_delegate_paired_.device_path_, profile_ptr_);
+  adapter->ReleaseProfile(fake_delegate_autopair_.device_path_, profile_ptr_);
+
+  message_loop_.RunUntilIdle();
+};
+
+TEST_F(BluetoothAdapterProfileChromeOSTest, SimultaneousRegisterFail) {
+  BluetoothUUID uuid(FakeBluetoothProfileManagerClient::kUnregisterableUuid);
+  BluetoothProfileManagerClient::Options options;
+  BluetoothAdapterChromeOS* adapter =
+      static_cast<BluetoothAdapterChromeOS*>(adapter_.get());
+
+  options.require_authentication.reset(new bool(false));
+
+  success_callback_count_ = 0;
+  error_callback_count_ = 0;
+
+  adapter->UseProfile(
+      uuid, fake_delegate_paired_.device_path_, options, &fake_delegate_paired_,
+      base::Bind(
+          &BluetoothAdapterProfileChromeOSTest::ProfileUserSuccessCallback,
+          base::Unretained(this)),
+      base::Bind(&BluetoothAdapterProfileChromeOSTest::BasicErrorCallback,
+                 base::Unretained(this)));
+
+  adapter->UseProfile(
+      uuid, fake_delegate_autopair_.device_path_, options,
+      &fake_delegate_autopair_,
+      base::Bind(&BluetoothAdapterProfileChromeOSTest::MatchedProfileCallback,
+                 base::Unretained(this)),
+      base::Bind(&BluetoothAdapterProfileChromeOSTest::BasicErrorCallback,
+                 base::Unretained(this)));
+
+  message_loop_.RunUntilIdle();
+
+  EXPECT_FALSE(profile_ptr_);
+  EXPECT_EQ(0U, success_callback_count_);
+  EXPECT_EQ(2U, error_callback_count_);
 };
 
 }  // namespace chromeos