Implementing GATT connection/disconnect on OS X.

device/bluetooth/bluetooth_adapter_mac.mm

Index: device/bluetooth/bluetooth_adapter_mac.mm
diff --git a/device/bluetooth/bluetooth_adapter_mac.mm b/device/bluetooth/bluetooth_adapter_mac.mm
index 72554ed21b458129818ec0f12243a6be8a58cd7a..875a567c9ac8a645d2fa663bd492a6016e583c4a 100644
--- a/device/bluetooth/bluetooth_adapter_mac.mm
+++ b/device/bluetooth/bluetooth_adapter_mac.mm
@@ -93,6 +93,9 @@ BluetoothAdapterMac::BluetoothAdapterMac()
 }
 
 BluetoothAdapterMac::~BluetoothAdapterMac() {
+  // Need to make sure the devices are destroyed before CBCentralManage is

[fbeaufortchromium, 2016/02/11 10:26:02]

s/CBCentralManage/CBCentralManager

[msarda, 2016/02/11 10:55:38]

I would kill this comment and just do what the comment says:
devices_clear();
low_energy_central_manager_.release();

[jlebel, 2016/02/19 11:02:35]

This is really important to do it explicitly. We could think this line is
useless since it will be done anyway once the destroy method is done. But there
is an issue for the disconnect.
Doing the 2 lines that you suggested looks like it is useless too.  But it is
important.
I don't mind adding low_energy_central_manager_.release();

+  // released.
+  devices_.clear();
 }
 
 std::string BluetoothAdapterMac::GetAddress() const {
@@ -222,18 +225,21 @@ bool BluetoothAdapterMac::IsLowEnergyAvailable() {
   return base::mac::IsOSYosemiteOrLater();
 }
 
+void BluetoothAdapterMac::RemovePairingDelegateInternal(
+    BluetoothDevice::PairingDelegate* pairing_delegate) {}
+
 void BluetoothAdapterMac::SetCentralManagerForTesting(
     CBCentralManager* central_manager) {
   CHECK(BluetoothAdapterMac::IsLowEnergyAvailable());
-  [central_manager performSelector:@selector(setDelegate:)
-                        withObject:low_energy_central_manager_delegate_];
-  low_energy_central_manager_.reset(central_manager);
+  [central_manager setDelegate:low_energy_central_manager_delegate_];
+  low_energy_central_manager_.reset(central_manager,
+                                    base::scoped_policy::RETAIN);
   low_energy_discovery_manager_->SetCentralManager(
       low_energy_central_manager_.get());
 }
 
-void BluetoothAdapterMac::RemovePairingDelegateInternal(
-    BluetoothDevice::PairingDelegate* pairing_delegate) {
+CBCentralManager* BluetoothAdapterMac::GetCentralManagerForTesting() {
+  return low_energy_central_manager_;
 }
 
 void BluetoothAdapterMac::AddDiscoverySession(
@@ -456,25 +462,23 @@ void BluetoothAdapterMac::LowEnergyDeviceUpdated(
     CBPeripheral* peripheral,
     NSDictionary* advertisement_data,
     int rssi) {
-  std::string device_address =
-      BluetoothLowEnergyDeviceMac::GetPeripheralHashAddress(peripheral);
-  // Try to find device from |devices_| with key |device_address|,
+  BluetoothLowEnergyDeviceMac* device_mac =
+      GetBluetoothLowEnergyDeviceMac(peripheral);

[fbeaufortchromium, 2016/02/11 10:26:02]

Was the comment line below removed by accident?

// Try to find device from |devices_| with key |device_address|,

Or should we simply remove the comment?

[jlebel, 2016/02/19 11:02:35]

Done.

[msarda, 2016/02/22 14:01:06]

This was not Done. Please put back the line.

   // if has no entry in the map, create new device and insert into |devices_|,
   // otherwise update the existing device.
-  DevicesMap::const_iterator iter = devices_.find(device_address);
-  if (iter == devices_.end()) {
+  if (!device_mac) {
     VLOG(1) << "LowEnergyDeviceUpdated new device";
     // A new device has been found.
-    BluetoothLowEnergyDeviceMac* device_mac = new BluetoothLowEnergyDeviceMac(
-        this, peripheral, advertisement_data, rssi);
+    device_mac = new BluetoothLowEnergyDeviceMac(this, peripheral,
+                                                 advertisement_data, rssi);
+    std::string device_address =
+        BluetoothLowEnergyDeviceMac::GetPeripheralHashAddress(peripheral);
     devices_.add(device_address, scoped_ptr<BluetoothDevice>(device_mac));
     FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
                       DeviceAdded(this, device_mac));
     return;
   }
 
-  BluetoothLowEnergyDeviceMac* device_mac =
-      static_cast<BluetoothLowEnergyDeviceMac*>(iter->second);
   std::string stored_device_id = device_mac->GetIdentifier();
   std::string updated_device_id =
       BluetoothLowEnergyDeviceMac::GetPeripheralIdentifier(peripheral);
@@ -537,4 +541,65 @@ void BluetoothAdapterMac::AddPairedDevices() {
   }
 }
 
+void BluetoothAdapterMac::CreateGattConnection(
+    BluetoothLowEnergyDeviceMac* device_mac) {
+  [low_energy_central_manager_ connectPeripheral:device_mac->peripheral_
+                                         options:nil];
+}
+
+void BluetoothAdapterMac::DisconnectGatt(
+    BluetoothLowEnergyDeviceMac* device_mac) {
+  [low_energy_central_manager_
+      cancelPeripheralConnection:device_mac->peripheral_];
+}
+
+void BluetoothAdapterMac::DidConnectPeripheral(CBPeripheral* peripheral) {
+  BluetoothLowEnergyDeviceMac* device_mac =
+      GetBluetoothLowEnergyDeviceMac(peripheral);
+  // Try to find device from |devices_| with key |device_address|,

[msarda, 2016/02/11 10:55:38]

s/find device/find the device
s/,/.
s/if has no entry in the map, disconnect the peripheral./If there is no entry in
the map then disconnect the peripheral.

[jlebel, 2016/02/19 11:02:35]

Done.

+  // if has no entry in the map, disconnect the peripheral.
+  if (!device_mac) {
+    [low_energy_central_manager_ cancelPeripheralConnection:peripheral];
+    return;
+  }
+  device_mac->GattConnected();
+}
+
+void BluetoothAdapterMac::DidFailToConnectPeripheral(CBPeripheral* peripheral,
+                                                     NSError* error) {
+  BluetoothLowEnergyDeviceMac* device_mac =
+      GetBluetoothLowEnergyDeviceMac(peripheral);
+  if (!device_mac) {

[msarda, 2016/02/11 10:55:38]

This code is duplicated in 3 functions. Consider refactoring it to a helper
method.

[jlebel, 2016/02/19 11:02:35]

I'm not sure we will gain a lot of lines by adding an helper. I will have
problems to find the right name, and the |if| will still be needed.

+    [low_energy_central_manager_ cancelPeripheralConnection:peripheral];
+    return;
+  }
+  // TODO(http://crbug.com/585894): Need to convert the error.

[fbeaufortchromium, 2016/02/11 10:26:02]

Can we VLOG the error there?

[jlebel, 2016/02/19 11:02:35]

Done.

+  device_mac->DidFailToConnectGatt(BluetoothClassicDeviceMac::ERROR_UNKNOWN);
+}
+
+void BluetoothAdapterMac::DidDisconnectPeripheral(CBPeripheral* peripheral,
+                                                  NSError* error) {
+  BluetoothLowEnergyDeviceMac* device_mac =
+      GetBluetoothLowEnergyDeviceMac(peripheral);
+  if (!device_mac) {
+    [low_energy_central_manager_ cancelPeripheralConnection:peripheral];
+    return;
+  }
+  // TODO(http://crbug.com/585897): Need to pass the error.
+  device_mac->DidDisconnectPeripheral();
+}
+
+BluetoothLowEnergyDeviceMac*
+BluetoothAdapterMac::GetBluetoothLowEnergyDeviceMac(CBPeripheral* peripheral) {
+  std::string device_address =
+      BluetoothLowEnergyDeviceMac::GetPeripheralHashAddress(peripheral);
+  // Try to find device from |devices_| with key |device_address|,

[msarda, 2016/02/11 10:55:38]

I do not understand the comment. Where do you disconnect the peripheral?

[jlebel, 2016/02/19 11:02:35]

Done.

+  // if has no entry in the map, disconnect the peripheral.
+  DevicesMap::const_iterator iter = devices_.find(device_address);
+  if (iter == devices_.end()) {
+    return nil;
+  }
+  return static_cast<BluetoothLowEnergyDeviceMac*>(iter->second);
+}
+
 }  // namespace device