Bluetooth: mac: Crash while loging nil error.

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 0db7131318d218b8003124231e7d7c87357608b6..0522306ff0e7f81788d2d057af2953223ac3d292 100644
--- a/device/bluetooth/bluetooth_adapter_mac.mm
+++ b/device/bluetooth/bluetooth_adapter_mac.mm
@@ -561,13 +561,15 @@ void BluetoothAdapterMac::DidFailToConnectPeripheral(CBPeripheral* peripheral,
     [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);
 }
 
@@ -579,11 +581,16 @@ void BluetoothAdapterMac::DidDisconnectPeripheral(CBPeripheral* peripheral,
     [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*