Bluetooth: mac: Crash while loging nil error.

device/bluetooth/bluetooth_adapter_mac.mm

 // 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_mac.h"
 
 #import <IOBluetooth/objc/IOBluetoothDevice.h>
 #import <IOBluetooth/objc/IOBluetoothHostController.h>
 #include <stddef.h>
 
@@ skipping 543 matching lines @@
 }
 
 void BluetoothAdapterMac::DidFailToConnectPeripheral(CBPeripheral* peripheral,
                                                      NSError* error) {
   BluetoothLowEnergyDeviceMac* device_mac =
       GetBluetoothLowEnergyDeviceMac(peripheral);
   if (!device_mac) {
     [low_energy_central_manager_ cancelPeripheralConnection:peripheral];
     return;
   }
-  VLOG(1) << "Bluetooth error, domain: " << error.domain.UTF8String
-          << ", error code: " << error.code;
   BluetoothDevice::ConnectErrorCode error_code =
-      BluetoothDeviceMac::GetConnectErrorCodeFromNSError(error);
-  VLOG(1) << "Bluetooth error, domain: " << error.domain.UTF8String
-          << ", error code: " << error.code
-          << ", converted into: " << error_code;
+      BluetoothDevice::ConnectErrorCode::ERROR_FAILED;
+  VLOG(1) << "Fail to connect to peripheral";
+  if (error) {

[ortuno, 2016/08/09 15:40:25]

DidFailToConnectPeripheral should always be called with an error right? Maybe
just DCHECK.

[jlebel, 2016/08/09 16:23:57]

From the documentation it is not obvious, but from CBCentralManager.h it is very
clear:
- (void)centralManager:(CBCentralManager *)central
didFailToConnectPeripheral:(CBPeripheral *)peripheral error:(nullable NSError
*)error;

I guess if there is no answer from the peripheral, the connection fails without
an error.

+    BluetoothDeviceMac::GetConnectErrorCodeFromNSError(error);

[ortuno, 2016/08/09 15:40:25]

Did you mean to save the result of this function in the error_code variable?
Otherwise you are always logging ERROR_FAILED.

[jlebel, 2016/08/09 16:23:57]

Done.

+    VLOG(1) << "Bluetooth error, domain: " << error.domain.UTF8String
+            << ", error code: " << error.code
+            << ", converted into: " << error_code;
+  }
   device_mac->DidFailToConnectGatt(error_code);
 }
 
 void BluetoothAdapterMac::DidDisconnectPeripheral(CBPeripheral* peripheral,
                                                   NSError* error) {
   BluetoothLowEnergyDeviceMac* device_mac =
       GetBluetoothLowEnergyDeviceMac(peripheral);
   if (!device_mac) {
     [low_energy_central_manager_ cancelPeripheralConnection:peripheral];
     return;
   }
-  VLOG(1) << "Bluetooth error, domain: " << error.domain.UTF8String
-          << ", error code: " << error.code;
-  BluetoothDevice::ConnectErrorCode error_code =
-      BluetoothDeviceMac::GetConnectErrorCodeFromNSError(error);
-  device_mac->DidDisconnectPeripheral(error_code);
+  VLOG(1) << "Peripheral disconnected";
+  if (error) {
+    VLOG(1) << "Bluetooth error, domain: " << error.domain.UTF8String
+            << ", error code: " << error.code;
+    BluetoothDevice::ConnectErrorCode error_code =
+        BluetoothDeviceMac::GetConnectErrorCodeFromNSError(error);
+    device_mac->DidDisconnectPeripheral(error_code);
+  } else {
+    device_mac->DidDisconnectPeripheral();

[ortuno, 2016/08/09 15:40:25]

Maybe just pass in BluetoothDevice::ConnectErrorCode::ERROR_UNKNOWN here. That
way you don't need to overload the function.

[jlebel, 2016/08/09 16:23:58]

But if we disconnect explicitly, we should not generate an error, no?

+  }
 }
 
 BluetoothLowEnergyDeviceMac*
 BluetoothAdapterMac::GetBluetoothLowEnergyDeviceMac(CBPeripheral* peripheral) {
   std::string device_address =
       BluetoothLowEnergyDeviceMac::GetPeripheralHashAddress(peripheral);
   DevicesMap::const_iterator iter = devices_.find(device_address);
   if (iter == devices_.end()) {
     return nil;
   }
   return static_cast<BluetoothLowEnergyDeviceMac*>(iter->second);
 }
 
 }  // namespace device