bluetooth: Invalidate connection objects if a connection fails and add histograms

device/bluetooth/bluetooth_device_android.cc

 // Copyright 2015 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_device_android.h"
 
 #include "base/android/context_utils.h"
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
+#include "base/metrics/sparse_histogram.h"
 #include "base/strings/stringprintf.h"
 #include "device/bluetooth/bluetooth_adapter_android.h"
 #include "device/bluetooth/bluetooth_remote_gatt_service_android.h"
 #include "jni/ChromeBluetoothDevice_jni.h"
 
 using base::android::AttachCurrentThread;
 using base::android::AppendJavaStringArrayToStringVector;
 
 namespace device {
+namespace {
+void RecordConnectionSuccessResult(int32_t status) {
+  UMA_HISTOGRAM_SPARSE_SLOWLY("Bluetooth.Android.GATTConnection.Success.Result",
+                              status);
+}
+void RecordConnectionFailureResult(int32_t status) {
+  UMA_HISTOGRAM_SPARSE_SLOWLY("Bluetooth.Android.GATTConnection.Failure.Result",
+                              status);
+}
+void RecordConnectionTerminatedResult(int32_t status) {
+  UMA_HISTOGRAM_SPARSE_SLOWLY(
+      "Bluetooth.Android.GATTConnection.Disconnected.Result", status);
+}
+}  // namespace
 
 BluetoothDeviceAndroid* BluetoothDeviceAndroid::Create(
     BluetoothAdapterAndroid* adapter,
     jobject bluetooth_device_wrapper) {  // Java Type: bluetoothDeviceWrapper
   BluetoothDeviceAndroid* device = new BluetoothDeviceAndroid(adapter);
 
   device->j_device_.Reset(Java_ChromeBluetoothDevice_create(
       AttachCurrentThread(), reinterpret_cast<intptr_t>(device),
       bluetooth_device_wrapper));
 
@@ skipping 171 matching lines @@
   NOTIMPLEMENTED();
 }
 
 void BluetoothDeviceAndroid::OnConnectionStateChange(
     JNIEnv* env,
     const JavaParamRef<jobject>& jcaller,
     int32_t status,
     bool connected) {
   gatt_connected_ = connected;
   if (gatt_connected_) {
+    RecordConnectionSuccessResult(status);
     DidConnectGatt();
+  } else if (!create_gatt_connection_error_callbacks_.empty()) {
+    // We assume that if there are any pending connection callbacks there
+    // was a failed connection attempt.
+    RecordConnectionFailureResult(status);
+    // TODO(ortuno): Return an error code based on |status|
+    // http://crbug.com/578191
+    DidFailToConnectGatt(ERROR_FAILED);

[scheib, 2016/01/16 23:59:52]

Add a LOG for now to help us understand codes in bugreports. TODO it to be
removed. https://code.google.com/p/chromium/issues/detail?id=529510

[ortuno, 2016/01/19 20:40:08]

I didn't add log because Android already logs it. Do you think I should still
log?

[scheib, 2016/01/19 20:58:22]

No, the BluetoothGattCallbackImpl log is sufficient.

   } else {
+    // Otherwise an existing connection was terminated.
+    RecordConnectionTerminatedResult(status);
     gatt_services_.clear();
     SetGattServicesDiscoveryComplete(false);
-
-    switch (status) {   // Constants are from android.bluetooth.BluetoothGatt.
-      case 0x0000008f:  // GATT_CONNECTION_CONGESTED
-        return DidFailToConnectGatt(ERROR_CONNECTION_CONGESTED);
-      case 0x00000101:  // GATT_FAILURE
-        return DidFailToConnectGatt(ERROR_FAILED);
-      case 0x00000005:  // GATT_INSUFFICIENT_AUTHENTICATION
-        return DidFailToConnectGatt(ERROR_AUTH_FAILED);
-      case 0x0000000f:  // GATT_INSUFFICIENT_ENCRYPTION
-        return DidFailToConnectGatt(ERROR_INSUFFICIENT_ENCRYPTION);
-      case 0x0000000d:  // GATT_INVALID_ATTRIBUTE_LENGTH
-        return DidFailToConnectGatt(ERROR_ATTRIBUTE_LENGTH_INVALID);
-      case 0x00000007:  // GATT_INVALID_OFFSET
-        return DidFailToConnectGatt(ERROR_OFFSET_INVALID);
-      case 0x00000002:  // GATT_READ_NOT_PERMITTED
-        return DidFailToConnectGatt(ERROR_READ_NOT_PERMITTED);
-      case 0x00000006:  // GATT_REQUEST_NOT_SUPPORTED
-        return DidFailToConnectGatt(ERROR_REQUEST_NOT_SUPPORTED);
-      case 0x00000000:  // GATT_SUCCESS
-        return DidDisconnectGatt();
-      case 0x00000003:  // GATT_WRITE_NOT_PERMITTED
-        return DidFailToConnectGatt(ERROR_WRITE_NOT_PERMITTED);
-      default:
-        VLOG(1) << "Unhandled status: " << status;
-        return DidFailToConnectGatt(ERROR_UNKNOWN);
-    }
+    DidDisconnectGatt();
   }
 }
 
 void BluetoothDeviceAndroid::OnGattServicesDiscovered(
     JNIEnv* env,
     const JavaParamRef<jobject>& jcaller) {
   SetGattServicesDiscoveryComplete(true);
   FOR_EACH_OBSERVER(BluetoothAdapter::Observer, GetAdapter()->GetObservers(),
                     GattServicesDiscovered(GetAdapter(), this));
 }
@@ skipping 33 matching lines @@
       AttachCurrentThread(), j_device_.obj(),
       base::android::GetApplicationContext());
 }
 
 void BluetoothDeviceAndroid::DisconnectGatt() {
   Java_ChromeBluetoothDevice_disconnectGatt(AttachCurrentThread(),
                                             j_device_.obj());
 }
 
 }  // namespace device