Fix BluetoothAdapterProfileChromeOS lifecycle management

Have BluetoothAdapterProfileChromeOS objects managed through
BluetoothAdapterChromeOS, avoiding problems where references were being
left behind in BluetoothSocketChromeOS after deletion.

Adds some unit tests to confirm lifecycle works correctly even when
multiple profiles are requested simultaneously.

This is a resubmit of https://codereview.chromium.org/935383003/ after a revert
for LSan test failures https://codereview.chromium.org/975323002/, a patch with
just changes from the old CL can be found there.

BUG=457978
R=armansito, isherman@chromium.org, keybuk

Review URL: https://codereview.chromium.org/982593002

Cr-Commit-Position: refs/heads/master@{#319721}

chromeos/dbus/fake_bluetooth_profile_manager_client.cc

@@ -19,6 +19,8 @@ const char FakeBluetoothProfileManagerClient::kL2capUuid[] =
     "4d995052-33cc-4fdf-b446-75f32942a076";
 const char FakeBluetoothProfileManagerClient::kRfcommUuid[] =
     "3f6d6dbf-a6ad-45fc-9653-47dc912ef70e";
+const char FakeBluetoothProfileManagerClient::kUnregisterableUuid[] =
+    "00000000-0000-0000-0000-000000000000";
 
 FakeBluetoothProfileManagerClient::FakeBluetoothProfileManagerClient() {
 }
@@ -37,6 +39,14 @@ void FakeBluetoothProfileManagerClient::RegisterProfile(
     const ErrorCallback& error_callback) {
   VLOG(1) << "RegisterProfile: " << profile_path.value() << ": " << uuid;
 
+  if (uuid == kUnregisterableUuid) {
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE, base::Bind(error_callback,
+                              bluetooth_profile_manager::kErrorInvalidArguments,
+                              "Can't register this UUID"));
+    return;
+  }
+
   // check options for channel & psm
 
   ServiceProviderMap::iterator iter = service_provider_map_.find(profile_path);

chromeos/dbus/fake_bluetooth_profile_manager_client.h

@@ -51,6 +51,7 @@ class CHROMEOS_EXPORT FakeBluetoothProfileManagerClient
   // UUIDs recognised for testing.
   static const char kL2capUuid[];
   static const char kRfcommUuid[];
+  static const char kUnregisterableUuid[];
 
  private:
   // Map of a D-Bus object path to the FakeBluetoothProfileServiceProvider

device/bluetooth/bluetooth_adapter_chromeos.cc

@@ -85,6 +85,13 @@ void BluetoothAdapterChromeOS::Shutdown() {
   if (IsPresent())
     RemoveAdapter();  // Also deletes devices_.
   DCHECK(devices_.empty());
+  // profiles_ should be empty because all BluetoothSockets have been signaled
+  // that this adapter is disappearing.
+  DCHECK(profiles_.empty());
+
+  for (auto& it : profile_queues_)
+    delete it.second;
+  profile_queues_.clear();
 
   DBusThreadManager::Get()->GetBluetoothAdapterClient()->RemoveObserver(this);
   DBusThreadManager::Get()->GetBluetoothDeviceClient()->RemoveObserver(this);
@@ -957,6 +964,12 @@ void BluetoothAdapterChromeOS::UseProfile(
     const ErrorCompletionCallback& error_callback) {
   DCHECK(delegate);
 
+  if (!IsPresent()) {
+    VLOG(2) << "Adapter not present, erroring out";
+    error_callback.Run("Adapter not present");
+    return;
+  }
+
   if (profiles_.find(uuid) != profiles_.end()) {
     // TODO(jamuraa) check that the options are the same and error when they are
     // not.
@@ -965,15 +978,35 @@ void BluetoothAdapterChromeOS::UseProfile(
     return;
   }
 
-  profiles_[uuid] = BluetoothAdapterProfileChromeOS::Register(
-      this, uuid, options,
-      base::Bind(&BluetoothAdapterChromeOS::OnRegisterProfile, this, uuid,
+  if (profile_queues_.find(uuid) == profile_queues_.end()) {
+    BluetoothAdapterProfileChromeOS::Register(
+        uuid, options,
+        base::Bind(&BluetoothAdapterChromeOS::OnRegisterProfile, this, uuid),
+        base::Bind(&BluetoothAdapterChromeOS::OnRegisterProfileError, this,
+                   uuid));
+
+    profile_queues_[uuid] = new std::vector<RegisterProfileCompletionPair>();
+  }
+
+  profile_queues_[uuid]->push_back(std::make_pair(
+      base::Bind(&BluetoothAdapterChromeOS::SetProfileDelegate, this, uuid,
                  device_path, delegate, success_callback, error_callback),
-      base::Bind(&BluetoothAdapterChromeOS::OnRegisterProfileError, this, uuid,
-                 error_callback));
+      error_callback));
+}
+
+void BluetoothAdapterChromeOS::ReleaseProfile(
+    const dbus::ObjectPath& device_path,
+    BluetoothAdapterProfileChromeOS* profile) {
+  VLOG(2) << "Releasing Profile: " << profile->uuid().canonical_value()
+          << " from " << device_path.value();
+  profile->RemoveDelegate(
+      device_path, base::Bind(&BluetoothAdapterChromeOS::RemoveProfile,
+                              weak_ptr_factory_.GetWeakPtr(), profile->uuid()));
 }
 
-void BluetoothAdapterChromeOS::ReleaseProfile(const BluetoothUUID& uuid) {
+void BluetoothAdapterChromeOS::RemoveProfile(const BluetoothUUID& uuid) {
+  VLOG(2) << "Remove Profile: " << uuid.canonical_value();
+
   if (profiles_.find(uuid) != profiles_.end()) {
     delete profiles_[uuid];
     profiles_.erase(uuid);
@@ -982,39 +1015,51 @@ void BluetoothAdapterChromeOS::ReleaseProfile(const BluetoothUUID& uuid) {
 
 void BluetoothAdapterChromeOS::OnRegisterProfile(
     const BluetoothUUID& uuid,
-    const dbus::ObjectPath& device_path,
-    BluetoothProfileServiceProvider::Delegate* delegate,
-    const ProfileRegisteredCallback& success_callback,
-    const ErrorCompletionCallback& error_callback) {
-  SetProfileDelegate(uuid, device_path, delegate, success_callback,
-                     error_callback);
+    scoped_ptr<BluetoothAdapterProfileChromeOS> profile) {
+  profiles_[uuid] = profile.release();
+
+  if (profile_queues_.find(uuid) == profile_queues_.end())
+    return;
+
+  for (auto& it : *profile_queues_[uuid])
+    it.first.Run();
+  delete profile_queues_[uuid];
+  profile_queues_.erase(uuid);
 }
 
-bool BluetoothAdapterChromeOS::SetProfileDelegate(
+void BluetoothAdapterChromeOS::SetProfileDelegate(
     const BluetoothUUID& uuid,
     const dbus::ObjectPath& device_path,
     BluetoothProfileServiceProvider::Delegate* delegate,
     const ProfileRegisteredCallback& success_callback,
     const ErrorCompletionCallback& error_callback) {
+  if (profiles_.find(uuid) == profiles_.end()) {
+    error_callback.Run("Cannot find profile!");
+    return;
+  }
+
   if (profiles_[uuid]->SetDelegate(device_path, delegate)) {
     success_callback.Run(profiles_[uuid]);
-    return true;
+    return;
   }
   // Already set
   error_callback.Run(bluetooth_agent_manager::kErrorAlreadyExists);
-  return false;
 }
 
 void BluetoothAdapterChromeOS::OnRegisterProfileError(
     const BluetoothUUID& uuid,
-    const ErrorCompletionCallback& error_callback,
     const std::string& error_name,
     const std::string& error_message) {
-  LOG(WARNING) << object_path_.value()
-               << ": Failed to register profile: " << error_name << ": "
-               << error_message;
-  error_callback.Run(error_message);
-  ReleaseProfile(uuid);
+  VLOG(2) << object_path_.value() << ": Failed to register profile: "
+          << error_name << ": " << error_message;
+  if (profile_queues_.find(uuid) == profile_queues_.end())
+    return;
+
+  for (auto& it : *profile_queues_[uuid])
+    it.second.Run(error_message);
+
+  delete profile_queues_[uuid];
+  profile_queues_.erase(uuid);
 }
 
 void BluetoothAdapterChromeOS::OnSetDiscoverable(

device/bluetooth/bluetooth_adapter_chromeos.h

@@ -145,8 +145,9 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothAdapterChromeOS
                   const ProfileRegisteredCallback& success_callback,
                   const ErrorCompletionCallback& error_callback);
 
-  // Releases the profile associated with |uuid|
-  void ReleaseProfile(const device::BluetoothUUID& uuid);
+  // Release use of a profile by a device.
+  void ReleaseProfile(const dbus::ObjectPath& device_path,
+                      BluetoothAdapterProfileChromeOS* profile);
 
  protected:
   // BluetoothAdapter:
@@ -283,20 +284,21 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothAdapterChromeOS
 
   // Called by dbus:: on completion of the D-Bus method to register a profile.
   void OnRegisterProfile(const device::BluetoothUUID& uuid,
-                         const dbus::ObjectPath& device_path,
-                         BluetoothProfileServiceProvider::Delegate* delegate,
-                         const ProfileRegisteredCallback& success_callback,
-                         const ErrorCompletionCallback& error_callback);
-  bool SetProfileDelegate(const device::BluetoothUUID& uuid,
+                         scoped_ptr<BluetoothAdapterProfileChromeOS> profile);
+
+  void SetProfileDelegate(const device::BluetoothUUID& uuid,
                           const dbus::ObjectPath& device_path,
                           BluetoothProfileServiceProvider::Delegate* delegate,
                           const ProfileRegisteredCallback& success_callback,
                           const ErrorCompletionCallback& error_callback);
   void OnRegisterProfileError(const device::BluetoothUUID& uuid,
-                              const ErrorCompletionCallback& error_callback,
                               const std::string& error_name,
                               const std::string& error_message);
 
+  // Called by BluetoothAdapterProfileChromeOS when no users of a profile
+  // remain.
+  void RemoveProfile(const device::BluetoothUUID& uuid);
+
   // Processes the queued discovery requests. For each DiscoveryCallbackPair in
   // the queue, this method will try to add a new discovery session. This method
   // is called whenever a pending D-Bus call to start or stop discovery has
@@ -340,6 +342,14 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothAdapterChromeOS
   // The profiles we have registered with the bluetooth daemon.
   std::map<device::BluetoothUUID, BluetoothAdapterProfileChromeOS*> profiles_;
 
+  // Callback pair for the profile registration queue.
+  typedef std::pair<base::Closure, ErrorCompletionCallback>
+      RegisterProfileCompletionPair;
+
+  // Queue of delegates waiting for a profile to register.
+  std::map<device::BluetoothUUID, std::vector<RegisterProfileCompletionPair>*>
+      profile_queues_;
+
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate its weak pointers before any other members are destroyed.
   base::WeakPtrFactory<BluetoothAdapterChromeOS> weak_ptr_factory_;

device/bluetooth/bluetooth_adapter_profile_chromeos.cc

@@ -19,29 +19,26 @@
 namespace chromeos {
 
 // static
-BluetoothAdapterProfileChromeOS* BluetoothAdapterProfileChromeOS::Register(
-    BluetoothAdapterChromeOS* adapter,
+void BluetoothAdapterProfileChromeOS::Register(
     const device::BluetoothUUID& uuid,
     const BluetoothProfileManagerClient::Options& options,
-    const base::Closure& success_callback,
+    const ProfileRegisteredCallback& success_callback,
     const BluetoothProfileManagerClient::ErrorCallback& error_callback) {
-  DCHECK(adapter);
-
-  BluetoothAdapterProfileChromeOS* profile =
-      new BluetoothAdapterProfileChromeOS(adapter, uuid);
+  scoped_ptr<BluetoothAdapterProfileChromeOS> profile(
+      new BluetoothAdapterProfileChromeOS(uuid));
 
   VLOG(1) << "Registering profile: " << profile->object_path().value();
   DBusThreadManager::Get()->GetBluetoothProfileManagerClient()->RegisterProfile(
-      profile->object_path(), uuid.canonical_value(), options, success_callback,
+      profile->object_path(),
+      uuid.canonical_value(),
+      options,
+      base::Bind(success_callback, base::Passed(&profile)),
       error_callback);
-
-  return profile;
 }
 
 BluetoothAdapterProfileChromeOS::BluetoothAdapterProfileChromeOS(
-    BluetoothAdapterChromeOS* adapter,
     const device::BluetoothUUID& uuid)
-    : uuid_(uuid), adapter_(adapter), weak_ptr_factory_(this) {
+    : uuid_(uuid), weak_ptr_factory_(this) {
   std::string uuid_path;
   base::ReplaceChars(uuid.canonical_value(), ":-", "_", &uuid_path);
   object_path_ =
@@ -54,7 +51,6 @@ BluetoothAdapterProfileChromeOS::BluetoothAdapterProfileChromeOS(
 }
 
 BluetoothAdapterProfileChromeOS::~BluetoothAdapterProfileChromeOS() {
-  profile_.reset();
 }
 
 bool BluetoothAdapterProfileChromeOS::SetDelegate(

device/bluetooth/bluetooth_adapter_profile_chromeos.h

@@ -21,17 +21,23 @@ namespace chromeos {
 // profile for custom Bluetooth services managed by a BluetoothAdapter.
 // Maintains a list of delegates which may serve the profile.
 // One delegate is allowed for each device.
+//
+// BluetoothAdapterProfileChromeOS objects are owned by the
+// BluetoothAdapterChromeOS and allocated through Register()
 class DEVICE_BLUETOOTH_EXPORT BluetoothAdapterProfileChromeOS
     : public chromeos::BluetoothProfileServiceProvider::Delegate {
  public:
+  typedef base::Callback<void(scoped_ptr<BluetoothAdapterProfileChromeOS>
+                                  profile)> ProfileRegisteredCallback;
+
   // Registers a profile with the BlueZ server for |uuid| with the
-  // options |options|.  Returns a newly allocated pointer, or nullptr
-  // if there was a problem.
-  static BluetoothAdapterProfileChromeOS* Register(
-      BluetoothAdapterChromeOS* adapter,
+  // options |options|.  |success_callback| is provided with a newly
+  // allocated profile if registration is successful, otherwise |error_callback|
+  // will be called.
+  static void Register(
       const device::BluetoothUUID& uuid,
       const BluetoothProfileManagerClient::Options& options,
-      const base::Closure& success_callback,
+      const ProfileRegisteredCallback& success_callback,
       const BluetoothProfileManagerClient::ErrorCallback& error_callback);
 
   ~BluetoothAdapterProfileChromeOS() override;
@@ -39,6 +45,9 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothAdapterProfileChromeOS
   // The object path of the profile.
   const dbus::ObjectPath& object_path() const { return object_path_; }
 
+  // Returns the UUID of the profile
+  const device::BluetoothUUID& uuid() const { return uuid_; }
+
   // Add a delegate for a device associated with this profile.
   // An empty |device_path| indicates a local listening service.
   // Returns true if the delegate was set, and false if the |device_path|
@@ -55,8 +64,7 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothAdapterProfileChromeOS
   size_t DelegateCount() const { return delegates_.size(); }
 
  private:
-  BluetoothAdapterProfileChromeOS(BluetoothAdapterChromeOS* adapter,
-                                  const device::BluetoothUUID& uuid);
+  BluetoothAdapterProfileChromeOS(const device::BluetoothUUID& uuid);
 
   // BluetoothProfileServiceProvider::Delegate:
   void Released() override;
@@ -86,9 +94,6 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothAdapterProfileChromeOS
   // Profile dbus object for receiving profile method calls from BlueZ
   scoped_ptr<BluetoothProfileServiceProvider> profile_;
 
-  // Adapter that owns this profile.
-  BluetoothAdapterChromeOS* adapter_;
-
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate its weak pointers before any other members are destroyed.
   base::WeakPtrFactory<BluetoothAdapterProfileChromeOS> weak_ptr_factory_;

device/bluetooth/bluetooth_adapter_profile_chromeos_unittest.cc

@@ -20,25 +20,17 @@
 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),
+      : success_callback_count_(0),
+        error_callback_count_(0),
+        fake_delegate_paired_(FakeBluetoothDeviceClient::kPairedDevicePath),
         fake_delegate_autopair_(FakeBluetoothDeviceClient::kLegacyAutopairPath),
         fake_delegate_listen_(""),
-        profile_(nullptr),
-        success_callback_count_(0),
-        error_callback_count_(0) {}
+        profile_user_ptr_(nullptr) {}
 
   void SetUp() override {
     scoped_ptr<DBusThreadManagerSetter> dbus_setter =
@@ -70,6 +62,7 @@ class BluetoothAdapterProfileChromeOSTest : public testing::Test {
   }
 
   void TearDown() override {
+    profile_.reset();
     adapter_ = nullptr;
     DBusThreadManager::Shutdown();
   }
@@ -95,24 +88,20 @@ class BluetoothAdapterProfileChromeOSTest : public testing::Test {
         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
     }
 
@@ -121,17 +110,30 @@ class BluetoothAdapterProfileChromeOSTest : public testing::Test {
     dbus::ObjectPath device_path_;
   };
 
-  FakeDelegate fake_delegate_paired_;
-  FakeDelegate fake_delegate_autopair_;
-  FakeDelegate fake_delegate_listen_;
+  void ProfileSuccessCallback(
+      scoped_ptr<BluetoothAdapterProfileChromeOS> profile) {
+    profile_.swap(profile);
+    ++success_callback_count_;
+  }
 
-  BluetoothAdapterProfileChromeOS* profile_;
+  void ProfileUserSuccessCallback(BluetoothAdapterProfileChromeOS* profile) {
+    profile_user_ptr_ = profile;
+    ++success_callback_count_;
+  }
+
+  void MatchedProfileCallback(BluetoothAdapterProfileChromeOS* profile) {
+    ASSERT_EQ(profile_user_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) {
     ++error_callback_count_;
   }
 
@@ -142,6 +144,16 @@ class BluetoothAdapterProfileChromeOSTest : public testing::Test {
 
   unsigned int success_callback_count_;
   unsigned int error_callback_count_;
+
+  FakeDelegate fake_delegate_paired_;
+  FakeDelegate fake_delegate_autopair_;
+  FakeDelegate fake_delegate_listen_;
+
+  scoped_ptr<BluetoothAdapterProfileChromeOS> profile_;
+
+  // unowned pointer as expected to be used by clients of
+  // BluetoothAdapterChromeOS::UseProfile like BluetoothSocketChromeOS
+  BluetoothAdapterProfileChromeOS* profile_user_ptr_;
 };
 
 TEST_F(BluetoothAdapterProfileChromeOSTest, DelegateCount) {
@@ -150,13 +162,18 @@ TEST_F(BluetoothAdapterProfileChromeOSTest, DelegateCount) {
 
   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());
 
@@ -174,8 +191,6 @@ TEST_F(BluetoothAdapterProfileChromeOSTest, DelegateCount) {
                            base::Bind(&base::DoNothing));
 
   EXPECT_EQ(0U, profile_->DelegateCount());
-
-  delete profile_;
 };
 
 TEST_F(BluetoothAdapterProfileChromeOSTest, BlackHole) {
@@ -184,11 +199,10 @@ TEST_F(BluetoothAdapterProfileChromeOSTest, BlackHole) {
 
   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)));
 
@@ -213,8 +227,6 @@ TEST_F(BluetoothAdapterProfileChromeOSTest, BlackHole) {
   EXPECT_EQ(1U, error_callback_count_);
 
   EXPECT_EQ(0U, fake_delegate_paired_.connections_);
-
-  delete profile_;
 };
 
 TEST_F(BluetoothAdapterProfileChromeOSTest, Routing) {
@@ -223,17 +235,18 @@ TEST_F(BluetoothAdapterProfileChromeOSTest, Routing) {
 
   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_);
@@ -290,8 +303,81 @@ TEST_F(BluetoothAdapterProfileChromeOSTest, Routing) {
   EXPECT_EQ(0U, error_callback_count_);
 
   EXPECT_EQ(1U, fake_delegate_listen_.connections_);
+};
+
+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_user_ptr_);
+  EXPECT_EQ(2U, success_callback_count_);
+  EXPECT_EQ(0U, error_callback_count_);
+
+  adapter->ReleaseProfile(fake_delegate_paired_.device_path_,
+                          profile_user_ptr_);
+  adapter->ReleaseProfile(fake_delegate_autopair_.device_path_,
+                          profile_user_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();
 
-  delete profile_;
+  EXPECT_FALSE(profile_user_ptr_);
+  EXPECT_EQ(0U, success_callback_count_);
+  EXPECT_EQ(2U, error_callback_count_);
 };
 
 }  // namespace chromeos

device/bluetooth/bluetooth_chromeos_unittest.cc

@@ -324,7 +324,8 @@ class BluetoothChromeOSTest : public testing::Test {
     QuitMessageLoop();
   }
 
-  void ProfileRegisteredCallback(BluetoothAdapterProfileChromeOS*) {
+  void ProfileRegisteredCallback(BluetoothAdapterProfileChromeOS* profile) {
+    adapter_profile_ = profile;
     ++callback_count_;
     QuitMessageLoop();
   }
@@ -426,6 +427,7 @@ class BluetoothChromeOSTest : public testing::Test {
   enum BluetoothDevice::ConnectErrorCode last_connect_error_;
   std::string last_client_error_;
   ScopedVector<BluetoothDiscoverySession> discovery_sessions_;
+  BluetoothAdapterProfileChromeOS* adapter_profile_;
 
  private:
   // Some tests use a message loop since background processing is simulated;
@@ -3188,6 +3190,8 @@ TEST_F(BluetoothChromeOSTest, Shutdown) {
 
   EXPECT_EQ(dbus::ObjectPath(""), adapter_chrome_os->object_path());
 
+  adapter_profile_ = NULL;
+
   FakeBluetoothProfileServiceProviderDelegate profile_delegate;
   adapter_chrome_os->UseProfile(
       BluetoothUUID(), dbus::ObjectPath(""),
@@ -3196,11 +3200,10 @@ TEST_F(BluetoothChromeOSTest, Shutdown) {
                  base::Unretained(this)),
       base::Bind(&BluetoothChromeOSTest::ErrorCompletionCallback,
                  base::Unretained(this)));
-  base::MessageLoop::current()->Run();
-  EXPECT_EQ(1, callback_count_--) << "UseProfile error";
-  EXPECT_EQ(0, error_callback_count_) << "UseProfile error";
 
-  adapter_chrome_os->ReleaseProfile(BluetoothUUID());
+  EXPECT_FALSE(adapter_profile_) << "UseProfile error";
+  EXPECT_EQ(0, callback_count_) << "UseProfile error";
+  EXPECT_EQ(1, error_callback_count_--) << "UseProfile error";
 
   // Protected and private methods:
 
@@ -3258,6 +3261,8 @@ TEST_F(BluetoothChromeOSTest, Shutdown) {
   // OnStopDiscovery tested in Shutdown_OnStopDiscovery
   // OnStopDiscoveryError tested in Shutdown_OnStopDiscoveryError
 
+  adapter_profile_ = NULL;
+
   // OnRegisterProfile SetProfileDelegate, OnRegisterProfileError, require
   // UseProfile to be set first, do so again here just before calling them.
   adapter_chrome_os->UseProfile(
@@ -3268,14 +3273,9 @@ TEST_F(BluetoothChromeOSTest, Shutdown) {
       base::Bind(&BluetoothChromeOSTest::ErrorCompletionCallback,
                  base::Unretained(this)));
 
-  adapter_chrome_os->OnRegisterProfile(
-      BluetoothUUID(), dbus::ObjectPath(""), &profile_delegate,
-      base::Bind(&BluetoothChromeOSTest::ProfileRegisteredCallback,
-                 base::Unretained(this)),
-      base::Bind(&BluetoothChromeOSTest::ErrorCompletionCallback,
-                 base::Unretained(this)));
-  EXPECT_EQ(1, callback_count_--) << "OnRegisterProfile error";
-  EXPECT_EQ(1, error_callback_count_--) << "OnRegisterProfile error";
+  EXPECT_FALSE(adapter_profile_) << "UseProfile error";
+  EXPECT_EQ(0, callback_count_) << "UseProfile error";
+  EXPECT_EQ(1, error_callback_count_--) << "UseProfile error";
 
   adapter_chrome_os->SetProfileDelegate(
       BluetoothUUID(), dbus::ObjectPath(""), &profile_delegate,
@@ -3286,13 +3286,9 @@ TEST_F(BluetoothChromeOSTest, Shutdown) {
   EXPECT_EQ(0, callback_count_) << "SetProfileDelegate error";
   EXPECT_EQ(1, error_callback_count_--) << "SetProfileDelegate error";
 
-  adapter_chrome_os->OnRegisterProfileError(
-      BluetoothUUID(),
-      base::Bind(&BluetoothChromeOSTest::ErrorCompletionCallback,
-                 base::Unretained(this)),
-      "", "");
+  adapter_chrome_os->OnRegisterProfileError(BluetoothUUID(), "", "");
   EXPECT_EQ(0, callback_count_) << "OnRegisterProfileError error";
-  EXPECT_EQ(1, error_callback_count_--) << "OnRegisterProfileError error";
+  EXPECT_EQ(0, error_callback_count_) << "OnRegisterProfileError error";
 
   adapter_chrome_os->ProcessQueuedDiscoveryRequests();
 

device/bluetooth/bluetooth_socket_chromeos.cc

@@ -110,6 +110,8 @@ void BluetoothSocketChromeOS::Connect(
   if (security_level == SECURITY_LEVEL_LOW)
     options_->require_authentication.reset(new bool(false));
 
+  adapter_ = device->adapter();
+
   RegisterProfile(device->adapter(), success_callback, error_callback);
 }
 
@@ -128,7 +130,8 @@ void BluetoothSocketChromeOS::Listen(
     return;
   }
 
-  adapter->AddObserver(this);
+  adapter_ = adapter;
+  adapter_->AddObserver(this);
 
   uuid_ = uuid;
   options_.reset(new BluetoothProfileManagerClient::Options());
@@ -221,12 +224,10 @@ void BluetoothSocketChromeOS::RegisterProfile(
   DCHECK(!profile_);
   DCHECK(adapter);
 
-  adapter_ = adapter;
-
   // If the adapter is not present, this is a listening socket and the
   // adapter isn't running yet.  Report success and carry on;
   // the profile will be registered when the daemon becomes available.
-  if (!adapter_->IsPresent()) {
+  if (!adapter->IsPresent()) {
     VLOG(1) << uuid_.canonical_value() << " on " << device_path_.value()
             << ": Delaying profile registration.";
     base::MessageLoop::current()->PostTask(FROM_HERE, success_callback);
@@ -309,7 +310,7 @@ void BluetoothSocketChromeOS::AdapterPresentChanged(BluetoothAdapter* adapter,
   DCHECK(ui_task_runner()->RunsTasksOnCurrentThread());
 
   if (!present) {
-    // Adapter removed, the profile is now invalid.
+    // Adapter removed, we can't use the profile anymore.
     UnregisterProfile();
     return;
   }
@@ -549,20 +550,9 @@ void BluetoothSocketChromeOS::UnregisterProfile() {
 
   VLOG(1) << profile_->object_path().value() << ": Release profile";
 
-  profile_->RemoveDelegate(
-      device_path_,
-      base::Bind(&BluetoothSocketChromeOS::ReleaseProfile, this, profile_));
-
+  static_cast<BluetoothAdapterChromeOS*>(adapter_.get())
+      ->ReleaseProfile(device_path_, profile_);
   profile_ = nullptr;
 }
 
-void BluetoothSocketChromeOS::ReleaseProfile(
-    BluetoothAdapterProfileChromeOS* profile) {
-  if (adapter_)
-    static_cast<BluetoothAdapterChromeOS*>(adapter_.get())
-        ->ReleaseProfile(uuid_);
-  else
-    delete profile;
-}
-
 }  // namespace chromeos

device/bluetooth/bluetooth_socket_chromeos.h

@@ -148,12 +148,10 @@ class CHROMEOS_EXPORT BluetoothSocketChromeOS
   // Method run to clean-up a listening socket.
   void DoCloseListening();
 
-  // Unregister the underlying profile client object from the Bluetooth Daemon.
+  // Unregisters this socket's usage of the Bluetooth profile which cleans up
+  // the profile if no one is using it.
   void UnregisterProfile();
 
-  // Releases the profile after the delegate is gone.
-  void ReleaseProfile(BluetoothAdapterProfileChromeOS* profile);
-
   // Adapter the profile is registered against
   scoped_refptr<device::BluetoothAdapter> adapter_;
 

device/bluetooth/bluetooth_socket_chromeos_unittest.cc

@@ -107,7 +107,8 @@ class BluetoothSocketChromeOSTest : public testing::Test {
     ++error_callback_count_;
     last_message_ = message;
 
-    message_loop_.Quit();
+    if (message_loop_.is_running())
+      message_loop_.Quit();
   }
 
   void ConnectToServiceSuccessCallback(scoped_refptr<BluetoothSocket> socket) {
@@ -558,4 +559,71 @@ TEST_F(BluetoothSocketChromeOSTest, PairedConnectFails) {
   EXPECT_TRUE(last_socket_.get() == NULL);
 }
 
+TEST_F(BluetoothSocketChromeOSTest, SocketListenTwice) {
+  adapter_->CreateRfcommService(
+      BluetoothUUID(FakeBluetoothProfileManagerClient::kRfcommUuid),
+      BluetoothAdapter::ServiceOptions(),
+      base::Bind(&BluetoothSocketChromeOSTest::CreateServiceSuccessCallback,
+                 base::Unretained(this)),
+      base::Bind(&BluetoothSocketChromeOSTest::ErrorCallback,
+                 base::Unretained(this)));
+
+  message_loop_.Run();
+
+  EXPECT_EQ(1U, success_callback_count_);
+  EXPECT_EQ(0U, error_callback_count_);
+  EXPECT_TRUE(last_socket_.get() != NULL);
+
+  // Take control of this socket.
+  scoped_refptr<BluetoothSocket> server_socket;
+  server_socket.swap(last_socket_);
+
+  server_socket->Accept(
+      base::Bind(&BluetoothSocketChromeOSTest::AcceptSuccessCallback,
+                 base::Unretained(this)),
+      base::Bind(&BluetoothSocketChromeOSTest::ErrorCallback,
+                 base::Unretained(this)));
+
+  server_socket->Close();
+
+  server_socket = NULL;
+
+  message_loop_.RunUntilIdle();
+
+  EXPECT_EQ(1U, success_callback_count_);
+  EXPECT_EQ(1U, error_callback_count_);
+
+  adapter_->CreateRfcommService(
+      BluetoothUUID(FakeBluetoothProfileManagerClient::kRfcommUuid),
+      BluetoothAdapter::ServiceOptions(),
+      base::Bind(&BluetoothSocketChromeOSTest::CreateServiceSuccessCallback,
+                 base::Unretained(this)),
+      base::Bind(&BluetoothSocketChromeOSTest::ErrorCallback,
+                 base::Unretained(this)));
+
+  message_loop_.Run();
+
+  EXPECT_EQ(2U, success_callback_count_);
+  EXPECT_EQ(1U, error_callback_count_);
+  EXPECT_TRUE(last_socket_.get() != NULL);
+
+  // Take control of this socket.
+  server_socket.swap(last_socket_);
+
+  server_socket->Accept(
+      base::Bind(&BluetoothSocketChromeOSTest::AcceptSuccessCallback,
+                 base::Unretained(this)),
+      base::Bind(&BluetoothSocketChromeOSTest::ErrorCallback,
+                 base::Unretained(this)));
+
+  server_socket->Close();
+
+  server_socket = NULL;
+
+  message_loop_.RunUntilIdle();
+
+  EXPECT_EQ(2U, success_callback_count_);
+  EXPECT_EQ(2U, error_callback_count_);
+}
+
 }  // namespace chromeos