Manage profiles in BluetoothAdapter on ChromeOS

device/bluetooth/bluetooth_adapter_chromeos.cc

Index: device/bluetooth/bluetooth_adapter_chromeos.cc
diff --git a/device/bluetooth/bluetooth_adapter_chromeos.cc b/device/bluetooth/bluetooth_adapter_chromeos.cc
index e69c479a77fe63c0a96faa5085b1efd95935c11e..f4952a1d50fd481a3a6f01f162c6e69ce9242602 100644
--- a/device/bluetooth/bluetooth_adapter_chromeos.cc
+++ b/device/bluetooth/bluetooth_adapter_chromeos.cc
@@ -20,6 +20,7 @@
 #include "chromeos/dbus/bluetooth_device_client.h"
 #include "chromeos/dbus/bluetooth_input_client.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
+#include "device/bluetooth/bluetooth_adapter_profile_chromeos.h"
 #include "device/bluetooth/bluetooth_device.h"
 #include "device/bluetooth/bluetooth_device_chromeos.h"
 #include "device/bluetooth/bluetooth_pairing_chromeos.h"
@@ -877,6 +878,77 @@ void BluetoothAdapterChromeOS::NotifyGattDescriptorValueChanged(
                     GattDescriptorValueChanged(this, descriptor, value));
 }
 
+BluetoothAdapterProfileChromeOS* BluetoothAdapterChromeOS::UseProfile(
+    const BluetoothUUID& uuid,
+    const dbus::ObjectPath& device_path,
+    const BluetoothProfileManagerClient::Options* options,
+    BluetoothProfileServiceProvider::Delegate* delegate,
+    const base::Closure& success_callback,
+    const ErrorCompletionCallback& error_callback) {

[armansito, 2015/01/21 01:44:47]

Add a DCHECK for delegate.

[Marie Janssen, 2015/01/22 21:55:32]

Done.

+  if (profiles_.find(uuid) != profiles_.end()) {
+    // TODO(jamuraa) check that the options are the same and error when they are
+    // not.
+    if (SetProfileDelegate(uuid, device_path, delegate, success_callback,
+                           error_callback)) {
+      return profiles_[uuid];
+    } else {

[armansito, 2015/01/21 01:44:47]

No need for else since you have an early return already.

[Marie Janssen, 2015/01/22 21:55:32]

Done.

+      return NULL;
+    }
+  }
+
+  profiles_[uuid] = BluetoothAdapterProfileChromeOS::Register(
+      this, uuid, options,
+      base::Bind(&BluetoothAdapterChromeOS::OnRegisterProfile, this, uuid,
+                 device_path, delegate, success_callback, error_callback),

[armansito, 2015/01/21 01:44:47]

I don't think it's safe to pass a raw pointer to |delegate| to the callback,
since it might be freed before the callback executes. I guess we're leaving
ownership of delegates to the upper-layer here, so I don't know what the best
solution is.

[Marie Janssen, 2015/01/22 21:55:32]

This is consistent with the passing of |delegate| to the previously used
callback in the BluetoothSocketChromeOS implementation - it's just as safe as it
was before.  I can switch it over to a weak_ptr with a little effort if that is
preferred.

+      base::Bind(&BluetoothAdapterChromeOS::OnRegisterProfileError, this,
+                 error_callback));
+
+  return profiles_[uuid];
+}
+
+void BluetoothAdapterChromeOS::ReleaseProfile(const BluetoothUUID& uuid) {
+  if (profiles_.find(uuid) != profiles_.end()) {
+    delete profiles_[uuid];
+    profiles_.erase(uuid);
+  }
+}
+
+void BluetoothAdapterChromeOS::OnRegisterProfile(
+    const BluetoothUUID& uuid,
+    const dbus::ObjectPath& device_path,
+    BluetoothProfileServiceProvider::Delegate* delegate,
+    const base::Closure& success_callback,
+    const ErrorCompletionCallback& error_callback) {
+  SetProfileDelegate(uuid, device_path, delegate, success_callback,
+                     error_callback);
+}
+
+bool BluetoothAdapterChromeOS::SetProfileDelegate(
+    const BluetoothUUID& uuid,
+    const dbus::ObjectPath& device_path,
+    BluetoothProfileServiceProvider::Delegate* delegate,
+    const base::Closure& success_callback,
+    const ErrorCompletionCallback& error_callback) {
+  if (profiles_[uuid]->SetDelegate(device_path, delegate)) {
+    success_callback.Run();
+    return true;
+  } else {

[armansito, 2015/01/21 01:44:47]

No need for else, since you have an early return already.

[Marie Janssen, 2015/01/22 21:55:32]

Done.

+    // Already set
+    error_callback.Run(bluetooth_agent_manager::kErrorAlreadyExists);
+    return false;
+  }
+}
+
+void BluetoothAdapterChromeOS::OnRegisterProfileError(
+    const ErrorCompletionCallback& error_callback,
+    const std::string& error_name,
+    const std::string& error_message) {

[armansito, 2015/01/21 01:44:47]

Since registering the profile failed, should it be cleaned up?

[Marie Janssen, 2015/01/22 21:55:32]

Done.

+  LOG(WARNING) << object_path_.value()
+               << ": Failed to register profile: " << error_name << ": "
+               << error_message;
+  error_callback.Run(error_message);
+}
+
 void BluetoothAdapterChromeOS::OnSetDiscoverable(
     const base::Closure& callback,
     const ErrorCallback& error_callback,