diff options
-rw-r--r-- | device/bluetooth/bluetooth_device.cc | 13 | ||||
-rw-r--r-- | device/bluetooth/bluetooth_device_android.cc | 52 | ||||
-rw-r--r-- | device/bluetooth/bluetooth_device_unittest.cc | 6 | ||||
-rw-r--r-- | device/bluetooth/test/bluetooth_test_android.cc | 48 | ||||
-rw-r--r-- | tools/metrics/histograms/histograms.xml | 118 |
5 files changed, 161 insertions, 76 deletions
diff --git a/device/bluetooth/bluetooth_device.cc b/device/bluetooth/bluetooth_device.cc index 05532cc..c85af11 100644 --- a/device/bluetooth/bluetooth_device.cc +++ b/device/bluetooth/bluetooth_device.cc @@ -303,6 +303,10 @@ void BluetoothDevice::DidConnectGatt() { } void BluetoothDevice::DidFailToConnectGatt(ConnectErrorCode error) { + // Connection request should only be made if there are no active + // connections. + DCHECK(gatt_connections_.empty()); + for (const auto& error_callback : create_gatt_connection_error_callbacks_) error_callback.Run(error); create_gatt_connection_success_callbacks_.clear(); @@ -311,13 +315,8 @@ void BluetoothDevice::DidFailToConnectGatt(ConnectErrorCode error) { void BluetoothDevice::DidDisconnectGatt() { // Pending calls to connect GATT are not expected, if they were then - // DidFailToConnectGatt should be called. But in case callbacks exist - // flush them to ensure a consistent state. - if (create_gatt_connection_error_callbacks_.size() > 0) { - VLOG(1) << "Unexpected / unexplained DidDisconnectGatt call while " - "create_gatt_connection_error_callbacks_ are pending."; - } - DidFailToConnectGatt(ERROR_FAILED); + // DidFailToConnectGatt should have been called. + DCHECK(create_gatt_connection_error_callbacks_.empty()); // Invalidate all BluetoothGattConnection objects. for (BluetoothGattConnection* connection : gatt_connections_) { diff --git a/device/bluetooth/bluetooth_device_android.cc b/device/bluetooth/bluetooth_device_android.cc index 71ceb66..253680b 100644 --- a/device/bluetooth/bluetooth_device_android.cc +++ b/device/bluetooth/bluetooth_device_android.cc @@ -8,6 +8,7 @@ #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" @@ -17,6 +18,20 @@ 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, @@ -208,36 +223,21 @@ void BluetoothDeviceAndroid::OnConnectionStateChange( 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); } 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(); } } diff --git a/device/bluetooth/bluetooth_device_unittest.cc b/device/bluetooth/bluetooth_device_unittest.cc index 69410b5..4fc30e4 100644 --- a/device/bluetooth/bluetooth_device_unittest.cc +++ b/device/bluetooth/bluetooth_device_unittest.cc @@ -457,7 +457,11 @@ TEST_F(BluetoothTest, BluetoothGattConnection_ErrorAfterConnection) { EXPECT_EQ(1, gatt_connection_attempts_); SimulateGattConnectionError(device, BluetoothDevice::ERROR_AUTH_FAILED); SimulateGattConnectionError(device, BluetoothDevice::ERROR_FAILED); - EXPECT_EQ(BluetoothDevice::ERROR_AUTH_FAILED, last_connect_error_code_); + // TODO: Change to ERROR_AUTH_FAILED. We should be getting a callback + // only with the first error, but our android framework doesn't yet + // support sending different errors. + // http://crbug.com/578191 + EXPECT_EQ(BluetoothDevice::ERROR_FAILED, last_connect_error_code_); for (BluetoothGattConnection* connection : gatt_connections_) EXPECT_FALSE(connection->IsConnected()); } diff --git a/device/bluetooth/test/bluetooth_test_android.cc b/device/bluetooth/test/bluetooth_test_android.cc index 353822d..e23a835 100644 --- a/device/bluetooth/test/bluetooth_test_android.cc +++ b/device/bluetooth/test/bluetooth_test_android.cc @@ -84,52 +84,16 @@ void BluetoothTestAndroid::SimulateGattConnection(BluetoothDevice* device) { void BluetoothTestAndroid::SimulateGattConnectionError( BluetoothDevice* device, - BluetoothDevice::ConnectErrorCode error) { - int android_error_value = 0; - switch (error) { // Constants are from android.bluetooth.BluetoothGatt. - case BluetoothDevice::ERROR_ATTRIBUTE_LENGTH_INVALID: - android_error_value = 0x0000000d; // GATT_INVALID_ATTRIBUTE_LENGTH - break; - case BluetoothDevice::ERROR_AUTH_FAILED: - android_error_value = 0x00000005; // GATT_INSUFFICIENT_AUTHENTICATION - break; - case BluetoothDevice::ERROR_CONNECTION_CONGESTED: - android_error_value = 0x0000008f; // GATT_CONNECTION_CONGESTED - break; - case BluetoothDevice::ERROR_FAILED: - android_error_value = 0x00000101; // GATT_FAILURE - break; - case BluetoothDevice::ERROR_INSUFFICIENT_ENCRYPTION: - android_error_value = 0x0000000f; // GATT_INSUFFICIENT_ENCRYPTION - break; - case BluetoothDevice::ERROR_OFFSET_INVALID: - android_error_value = 0x00000007; // GATT_INVALID_OFFSET - break; - case BluetoothDevice::ERROR_READ_NOT_PERMITTED: - android_error_value = 0x00000002; // GATT_READ_NOT_PERMITTED - break; - case BluetoothDevice::ERROR_REQUEST_NOT_SUPPORTED: - android_error_value = 0x00000006; // GATT_REQUEST_NOT_SUPPORTED - break; - case BluetoothDevice::ERROR_WRITE_NOT_PERMITTED: - android_error_value = 0x00000003; // GATT_WRITE_NOT_PERMITTED - break; - case BluetoothDevice::ERROR_AUTH_CANCELED: - case BluetoothDevice::ERROR_AUTH_REJECTED: - case BluetoothDevice::ERROR_AUTH_TIMEOUT: - case BluetoothDevice::ERROR_INPROGRESS: - case BluetoothDevice::ERROR_UNKNOWN: - case BluetoothDevice::ERROR_UNSUPPORTED_DEVICE: - case BluetoothDevice::NUM_CONNECT_ERROR_CODES: - NOTREACHED() << "No translation for error code: " << error; - } - + BluetoothDevice::ConnectErrorCode) { BluetoothDeviceAndroid* device_android = static_cast<BluetoothDeviceAndroid*>(device); Java_FakeBluetoothDevice_connectionStateChange( AttachCurrentThread(), device_android->GetJavaObject().obj(), - android_error_value, + // TODO(ortuno): Add all types of errors Android can produce. For now we + // just return a timeout error. + // http://crbug.com/578191 + 0x08, // Connection Timeout from Bluetooth Spec. false); // connected } @@ -139,7 +103,7 @@ void BluetoothTestAndroid::SimulateGattDisconnection(BluetoothDevice* device) { Java_FakeBluetoothDevice_connectionStateChange( AttachCurrentThread(), device_android->GetJavaObject().obj(), - 0, // android.bluetooth.BluetoothGatt.GATT_SUCCESS + 0x13, // Connection terminate by peer user from Bluetooth Spec. false); // disconnected } diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 7db2113..77fa2ac 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -3149,6 +3149,39 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. </summary> </histogram> +<histogram name="Bluetooth.Android.GATTConnection.Disconnected.Result" + enum="AndroidGATTConnectionErrorCodes"> + <owner>jyasskin@chromium.org</owner> + <owner>ortuno@chromium.org</owner> + <owner>scheib@chromium.org</owner> + <summary> + Disconnected GATT connection status codes. Used to better understand errors + seen in Android. + </summary> +</histogram> + +<histogram name="Bluetooth.Android.GATTConnection.Failure.Result" + enum="AndroidGATTConnectionErrorCodes"> + <owner>jyasskin@chromium.org</owner> + <owner>ortuno@chromium.org</owner> + <owner>scheib@chromium.org</owner> + <summary> + Failed GATT connection error codes. Used to better understand errors seen in + Android. + </summary> +</histogram> + +<histogram name="Bluetooth.Android.GATTConnection.Success.Result" + enum="AndroidGATTConnectionErrorCodes"> + <owner>jyasskin@chromium.org</owner> + <owner>ortuno@chromium.org</owner> + <owner>scheib@chromium.org</owner> + <summary> + Successful GATT connection result codes. Used to better understand Android + results. + </summary> +</histogram> + <histogram name="Bluetooth.ConnectedDeviceCount" units="devices"> <owner>keybuk@chromium.org</owner> <summary> @@ -56366,6 +56399,91 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. <int value="4" label="EvictAll"/> </enum> +<enum name="AndroidGATTConnectionErrorCodes" type="int"> + <summary> + This list includes all errors from the Bluetooth Specification Version 4.2 + [Vol 2, Part D] as well as an error from Android's BluetoothGatt (0x101 + GATT_FAILURE) and an error from Bluedroid's gatt_api.h (0x100 L2CAP + connection cancelled). + </summary> + <int value="0" label="0x00 Success"/> + <int value="1" label="0x01 Unknown HCI Command"/> + <int value="2" label="0x02 Unknown Connection Identifier"/> + <int value="3" label="0x03 Hardware Failure"/> + <int value="4" label="0x04 Page Timeout"/> + <int value="5" label="0x05 Authentication Failure"/> + <int value="6" label="0x06 PIN or Key Missing"/> + <int value="7" label="0x07 Memory Capacity Exceeded"/> + <int value="8" label="0x08 Connection Timeout"/> + <int value="9" label="0x09 Connection Limit Exceeded"/> + <int value="10" + label="0x0A Synchronous Connection Limit To A Device Exceeded"/> + <int value="11" label="0x0B ACL Connection Already Exists"/> + <int value="12" label="0x0C Command Disallowed"/> + <int value="13" label="0x0D Connection Rejected due to Limited Resources"/> + <int value="14" label="0x0E Connection Rejected Due To Security Reasons"/> + <int value="15" label="0x0F Connection Rejected due to Unacceptable BD_ADDR"/> + <int value="16" label="0x10 Connection Accept Timeout Exceeded"/> + <int value="17" label="0x11 Unsupported Feature or Parameter Value"/> + <int value="18" label="0x12 Invalid HCI Command Parameters"/> + <int value="19" label="0x13 Remote User Terminated Connection"/> + <int value="20" + label="0x14 Remote Device Terminated Connection due to Low Resources"/> + <int value="21" + label="0x15 Remote Device Terminated Connection due to Power Off"/> + <int value="22" label="0x16 Connection Terminated By Local Host"/> + <int value="23" label="0x17 Repeated Attempts"/> + <int value="24" label="0x18 Pairing Not Allowed"/> + <int value="25" label="0x19 Unknown LMP PDU"/> + <int value="26" + label="0x1A Unsupported Remote Feature / Unsupported LMP Feature"/> + <int value="27" label="0x1B SCO Offset Rejected"/> + <int value="28" label="0x1C SCO Interval Rejected"/> + <int value="29" label="0x1D SCO Air Mode Rejected"/> + <int value="30" label="0x1E Invalid LMP Parameters / Invalid LL Parameters"/> + <int value="31" label="0x1F Unspecified Error"/> + <int value="32" + label="0x20 Unsupported LMP Parameter Value / Unsupported LL Parameter + Value"/> + <int value="33" label="0x21 Role Change Not Allowed"/> + <int value="34" label="0x22 LMP Response Timeout / LL Response Timeout"/> + <int value="35" label="0x23 LMP Error Transaction Collision"/> + <int value="36" label="0x24 LMP PDU Not Allowed"/> + <int value="37" label="0x25 Encryption Mode Not Acceptable"/> + <int value="38" label="0x26 Link Key cannot be Changed"/> + <int value="39" label="0x27 Requested QoS Not Supported"/> + <int value="40" label="0x28 Instant Passed"/> + <int value="41" label="0x29 Pairing With Unit Key Not Supported"/> + <int value="42" label="0x2A Different Transaction Collision"/> + <int value="43" label="0x2B Reserved"/> + <int value="44" label="0x2C QoS Unacceptable Parameter"/> + <int value="45" label="0x2D QoS Rejected"/> + <int value="46" label="0x2E Channel Classification Not Supported"/> + <int value="47" label="0x2F Insufficient Security"/> + <int value="48" label="0x30 Parameter Out Of Mandatory Range"/> + <int value="49" label="0x31 Reserved"/> + <int value="50" label="0x32 Role Switch Pending"/> + <int value="51" label="0x33 Reserved"/> + <int value="52" label="0x34 Reserved Slot Violation"/> + <int value="53" label="0x35 Role Switch Failed"/> + <int value="54" label="0x36 Extended Inquiry Response Too Large"/> + <int value="55" label="0x37 Secure Simple Pairing Not Supported By Host"/> + <int value="56" label="0x38 Host Busy - Pairing"/> + <int value="57" + label="0x39 Connection Rejected due to No Suitable Channel Found"/> + <int value="58" label="0x3A Controller Busy"/> + <int value="59" label="0x3B Unacceptable Connection Parameters"/> + <int value="60" label="0x3C Directed Advertising Timeout"/> + <int value="61" label="0x3D Connection Terminated due to MIC Failure"/> + <int value="62" label="0x3E Connection Failed to be Established"/> + <int value="63" label="0x3F MAC Connection Failed"/> + <int value="64" + label="0x40 Coarse Clock Adjustment Rejected but Will Try to Adjust + Using Clock Dragging"/> + <int value="256" label="0x100 Bluedroid L2CAP connection cancelled"/> + <int value="257" label="0x101 Android GATT Failure"/> +</enum> + <enum name="AndroidKernelVersion" type="int"> <int value="131078" label="2.6"/> <int value="196608" label="3.0"/> |