bluetooth: macOS: Adding histograms for NSError values

device/bluetooth/bluetooth_adapter_mac_metrics.mm

Index: device/bluetooth/bluetooth_adapter_mac_metrics.mm
diff --git a/device/bluetooth/bluetooth_adapter_mac_metrics.mm b/device/bluetooth/bluetooth_adapter_mac_metrics.mm
new file mode 100644
index 0000000000000000000000000000000000000000..ebecf4543ec1511c03036391e91f30dfa0eb4667
--- /dev/null
+++ b/device/bluetooth/bluetooth_adapter_mac_metrics.mm
@@ -0,0 +1,216 @@
+// Copyright 2017 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_metrics.h"
+
+#import <CoreBluetooth/CoreBluetooth.h>
+#import <Foundation/Foundation.h>
+
+#include "base/metrics/histogram_macros.h"
+
+namespace {
+
+#if !defined(MAC_OS_X_VERSION_10_11)
+// Available with macOS 10.11 SDK or after.
+const NSInteger CBErrorMaxConnection = 11;
+#endif  // !defined(MAC_OS_X_VERSION_10_11)
+
+WebBluetoothMacOSErrors GetWebBluetoothMacOSErrorsFromNSError(NSError* error) {
+  NSString* error_domain = [error domain];
+  NSInteger error_code = [error code];
+  if ([error_domain isEqualToString:CBErrorDomain]) {

[ortuno, 2017/05/31 06:59:52]

We don't need all these enums. We can just histogram the integer value of the
error like we do for android[1] 

[1]
https://cs.chromium.org/chromium/src/device/bluetooth/bluetooth_device_androi...

[jlebel, 2017/06/13 22:31:37]

So I would have to do:
if ([error_domain isEqualToString:CBErrorDomain])
  return [error code];
if ([error_domain isEqualToString:CBATTErrorDomain])
  return [error code] + 1000;
return -1;

[ortuno, 2017/06/14 03:46:29]

Yup, what's the difference between CBErrorDomain and CBATTErrorDomain? Why do we
+1000 the CBATTErrorDomain and not CBErrorDomain?

[jlebel, 2017/06/19 07:07:49]

Both errors from CBErrorDomain and CBATTErrorDomain starts at 0. I choose 1000,
just in case Apple adds error codes.

CBErrorDomain error codes can be:
https://developer.apple.com/documentation/corebluetooth/cberror?language=objc

CBATTErrorDomain error codes can be:
https://developer.apple.com/documentation/corebluetooth/cbatterror?language=objc

See (not the official version, but it is close enough):
https://github.com/mstg/iOS-full-sdk/blob/master/iPhoneOS9.3.sdk/System/Libra...

[ortuno, 2017/06/20 05:59:40]

I see. Let's do what you propose instead of using all these enums:

+    switch (static_cast<CBError>(error_code)) {
+      case CBErrorUnknown:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBERROR_UNKNOWN;
+      case CBErrorInvalidParameters:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBERROR_INVALID_PARAMETERS;
+      case CBErrorInvalidHandle:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBERROR_INVALID_HANDLE;
+      case CBErrorNotConnected:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBERROR_NOT_CONNECTED;
+      case CBErrorOutOfSpace:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBERROR_OUT_OF_SPACE;
+      case CBErrorOperationCancelled:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBERROR_OPERATION_CANCELLED;
+      case CBErrorConnectionTimeout:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBERROR_CONNECTION_TIMEOUT;
+      case CBErrorPeripheralDisconnected:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBERROR_PERIPHERAL_DISCONNECTED;
+      case CBErrorUUIDNotAllowed:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBERROR_UUID_NOT_ALLOWED;
+      case CBErrorAlreadyAdvertising:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBERROR_ALREADY_ADVERTISING;
+#if defined(MAC_OS_X_VERSION_10_11)
+      case CBErrorMaxConnection:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBERROR_MAX_CONNECTION;
+#endif  // defined(MAC_OS_X_VERSION_10_11)
+    }
+#if !defined(MAC_OS_X_VERSION_10_11)
+    if (error_code == CBErrorMaxConnection) {
+      return WebBluetoothMacOSErrors::
+          WEB_BLUETOOTH_MACOS_ERRORS_CBERROR_MAX_CONNECTION;
+    }
+#endif  // !defined(MAC_OS_X_VERSION_10_11)
+    return WebBluetoothMacOSErrors::WEB_BLUETOOTH_MACOS_ERRORS_CBERROR_MAX;
+  } else if ([error_domain isEqualToString:CBATTErrorDomain]) {
+    switch (static_cast<CBATTError>(error_code)) {
+      case CBATTErrorSuccess:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_SUCCESS;
+      case CBATTErrorInvalidHandle:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_INVALID_HANDLE;
+      case CBATTErrorReadNotPermitted:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_READ_NOT_PERMITTED;
+      case CBATTErrorWriteNotPermitted:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_WRITE_NOT_PERMITTED;
+      case CBATTErrorInvalidPdu:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_INVALID_PDU;
+      case CBATTErrorInsufficientAuthentication:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_INSUFFICIENT_AUTHENTICATION;
+      case CBATTErrorRequestNotSupported:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_REQUEST_NOT_SUPPORTED;
+      case CBATTErrorInvalidOffset:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_INVALID_OFFSET;
+      case CBATTErrorInsufficientAuthorization:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_INSUFFICIENT_AUTHORIZATION;
+      case CBATTErrorPrepareQueueFull:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_PREPARE_QUEUE_FULL;
+      case CBATTErrorAttributeNotFound:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_ATTRIBUTE_NOT_FOUND;
+      case CBATTErrorAttributeNotLong:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_ATTRIBUTE_NOT_LONG;
+      case CBATTErrorInsufficientEncryptionKeySize:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_INSUFFICIENT_ENCRYPTION_KEY_SIZE;
+      case CBATTErrorInvalidAttributeValueLength:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_INVALID_ATTRIBUTE_VALUE_LENGTH;
+      case CBATTErrorUnlikelyError:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_UNLIKELY_ERROR;
+      case CBATTErrorInsufficientEncryption:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_INSUFFICIENT_ENCRYPTION;
+      case CBATTErrorUnsupportedGroupType:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_UNSUPPORTED_GROUP_TYPE;
+      case CBATTErrorInsufficientResources:
+        return WebBluetoothMacOSErrors::
+            WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_INSUFFICIENT_RESOURCES;
+    }
+    return WebBluetoothMacOSErrors::WEB_BLUETOOTH_MACOS_ERRORS_CBATT_ERROR_MAX;
+  }
+  return WebBluetoothMacOSErrors::
+      WEB_BLUETOOTH_MACOS_ERRORS_UNKNOWN_ERROR_DOMAIN;
+}
+
+}  // namespace
+
+void LogNSErrorToHistogram(NSError* error,

[ortuno, 2017/05/31 06:59:52]

Add

if (error == nil) {
  return;
}

That way we don't have to check before calling this function.

optional: I'm not sure which one is better: a big switch statement like we have
here or separate functions. What do you think?

[jlebel, 2017/06/13 22:31:37]

Done.

+                           WebBluetoothMacOSAPIs web_bluetooth_macos_apis) {
+  WebBluetoothMacOSErrors histogram_macos_error =
+      GetWebBluetoothMacOSErrorsFromNSError(error);
+  switch (web_bluetooth_macos_apis) {
+    case WebBluetoothMacOSAPIs::
+        WEB_BLUETOOTH_MACOS_APIS_DID_FAIL_TO_CONNECT_TO_PERIPHERAL:
+      UMA_HISTOGRAM_ENUMERATION(
+          "Bluetooth.Web.MacOS.Errors.DidFailToConnectToPeripheral",
+          static_cast<int>(histogram_macos_error),
+          static_cast<int>(
+              WebBluetoothMacOSErrors::WEB_BLUETOOTH_MACOS_ERRORS_MAX));
+      break;
+    case WebBluetoothMacOSAPIs::
+        WEB_BLUETOOTH_MACOS_APIS_DID_DISCONNECT_PERIPHERAL:
+      UMA_HISTOGRAM_ENUMERATION(
+          "Bluetooth.Web.MacOS.Errors.DidDisconnectPeripheral",
+          static_cast<int>(histogram_macos_error),
+          static_cast<int>(
+              WebBluetoothMacOSErrors::WEB_BLUETOOTH_MACOS_ERRORS_MAX));
+      break;
+    case WebBluetoothMacOSAPIs::
+        WEB_BLUETOOTH_MACOS_APIS_DID_DISCOVER_PRIMARY_SERVICES:
+      UMA_HISTOGRAM_ENUMERATION(
+          "Bluetooth.Web.MacOS.Errors.DidDiscoverPrimaryServices",
+          static_cast<int>(histogram_macos_error),
+          static_cast<int>(
+              WebBluetoothMacOSErrors::WEB_BLUETOOTH_MACOS_ERRORS_MAX));
+      break;
+    case WebBluetoothMacOSAPIs::
+        WEB_BLUETOOTH_MACOS_APIS_DID_DISCOVER_CHARACTERISTICS:
+      UMA_HISTOGRAM_ENUMERATION(
+          "Bluetooth.Web.MacOS.Errors.DidDiscoverCharacteristics",
+          static_cast<int>(histogram_macos_error),
+          static_cast<int>(
+              WebBluetoothMacOSErrors::WEB_BLUETOOTH_MACOS_ERRORS_MAX));
+      break;
+    case WebBluetoothMacOSAPIs::WEB_BLUETOOTH_MACOS_APIS_DID_UPDATE_VALUE:
+      UMA_HISTOGRAM_ENUMERATION(
+          "Bluetooth.Web.MacOS.Errors.DidUpdateValue",
+          static_cast<int>(histogram_macos_error),
+          static_cast<int>(
+              WebBluetoothMacOSErrors::WEB_BLUETOOTH_MACOS_ERRORS_MAX));
+      break;
+    case WebBluetoothMacOSAPIs::WEB_BLUETOOTH_MACOS_APIS_DID_WRITE_VALUE:
+      UMA_HISTOGRAM_ENUMERATION(
+          "Bluetooth.Web.MacOS.Errors.DidWriteValue",
+          static_cast<int>(histogram_macos_error),
+          static_cast<int>(
+              WebBluetoothMacOSErrors::WEB_BLUETOOTH_MACOS_ERRORS_MAX));
+      break;
+    case WebBluetoothMacOSAPIs::
+        WEB_BLUETOOTH_MACOS_APIS_DID_UPDATE_NOTIFICATION_STATE:
+      UMA_HISTOGRAM_ENUMERATION(
+          "Bluetooth.Web.MacOS.Errors.DidUpdateNotificationState",
+          static_cast<int>(histogram_macos_error),
+          static_cast<int>(
+              WebBluetoothMacOSErrors::WEB_BLUETOOTH_MACOS_ERRORS_MAX));
+      break;
+    case WebBluetoothMacOSAPIs::
+        WEB_BLUETOOTH_MACOS_APIS_DID_DISCOVER_DESCRIPTORS:
+      UMA_HISTOGRAM_ENUMERATION(
+          "Bluetooth.Web.MacOS.Errors.DidDiscoverDescriptors",
+          static_cast<int>(histogram_macos_error),
+          static_cast<int>(
+              WebBluetoothMacOSErrors::WEB_BLUETOOTH_MACOS_ERRORS_MAX));
+      break;
+    case WebBluetoothMacOSAPIs::
+        WEB_BLUETOOTH_MACOS_APIS_DID_UPDATE_VALUE_FOR_DESCRIPTOR:
+      UMA_HISTOGRAM_ENUMERATION(
+          "Bluetooth.Web.MacOS.Errors.DidUpdateValueForDescriptor",
+          static_cast<int>(histogram_macos_error),
+          static_cast<int>(
+              WebBluetoothMacOSErrors::WEB_BLUETOOTH_MACOS_ERRORS_MAX));
+      break;
+    case WebBluetoothMacOSAPIs::
+        WEB_BLUETOOTH_MACOS_APIS_DID_WRITE_VALUE_FOR_DESCRIPTOR:
+      UMA_HISTOGRAM_ENUMERATION(
+          "Bluetooth.Web.MacOS.Errors.DidWriteValueForDescriptor",
+          static_cast<int>(histogram_macos_error),
+          static_cast<int>(
+              WebBluetoothMacOSErrors::WEB_BLUETOOTH_MACOS_ERRORS_MAX));
+      break;
+  }
+}