Manage profiles in BluetoothAdapter on ChromeOS

device/bluetooth/bluetooth_adapter_chromeos.cc

 // Copyright 2013 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 "device/bluetooth/bluetooth_adapter_chromeos.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
 #include "base/sys_info.h"
 #include "base/thread_task_runner_handle.h"
 #include "chromeos/dbus/bluetooth_adapter_client.h"
 #include "chromeos/dbus/bluetooth_agent_manager_client.h"
 #include "chromeos/dbus/bluetooth_agent_service_provider.h"
 #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"
 #include "device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.h"
 #include "device/bluetooth/bluetooth_remote_gatt_descriptor_chromeos.h"
 #include "device/bluetooth/bluetooth_remote_gatt_service_chromeos.h"
 #include "device/bluetooth/bluetooth_socket_chromeos.h"
 #include "device/bluetooth/bluetooth_socket_thread.h"
 #include "device/bluetooth/bluetooth_uuid.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
@@ skipping 837 matching lines @@
     const std::vector<uint8>& value) {
   DCHECK_EQ(static_cast<BluetoothRemoteGattServiceChromeOS*>(
                 descriptor->GetCharacteristic()->GetService())->GetAdapter(),
             this);
 
   FOR_EACH_OBSERVER(BluetoothAdapter::Observer,
                     observers_,
                     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,
     bool success) {
   // Set the discoverable_timeout property to zero so the adapter remains
   // discoverable forever.
   DBusThreadManager::Get()->GetBluetoothAdapterClient()->
       GetProperties(object_path_)->discoverable_timeout.Set(
           0,
           base::Bind(&BluetoothAdapterChromeOS::OnPropertyChangeCompleted,
@@ skipping 178 matching lines @@
 
     // If the queued request resulted in a pending call, then let it
     // asynchonously process the remaining queued requests once the pending
     // call returns.
     if (discovery_request_pending_)
       return;
   }
 }
 
 }  // namespace chromeos