[Bluetooth] Unify metrics recording into bluetooth_utils.

Migrate metrics recording from bluetooth_private_api.cc and
tray_bluetooth_helper_legacy.cc to bluetooth_utils.cc.
Now that all metrics recording is unified under bluetooth_utils,
subsequent CLs to flesh out these metrics in question will be
easier to understand.

Bug: 953149
Change-Id: Icf70d1e439b003f80f1250e4547c8275b77f201f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090295
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: James Vecore <vecore@google.com>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748416}

ash/system/bluetooth/tray_bluetooth_helper_legacy.cc

@@ -13,7 +13,6 @@
 #include "ash/system/model/system_tray_model.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
-#include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
@@ -39,21 +38,15 @@ namespace {
 // System tray shows a limited number of bluetooth devices.
 const int kMaximumDevicesShown = 50;
 
-void RecordUserInitiatedReconnectionAttemptResult(bool success) {
-  UMA_HISTOGRAM_BOOLEAN(
-      "Bluetooth.ChromeOS.UserInitiatedReconnectionAttempt.Result", success);
-  UMA_HISTOGRAM_BOOLEAN(
-      "Bluetooth.ChromeOS.UserInitiatedReconnectionAttempt.Result.SystemTray",
-      success);
-}
-
 void BluetoothSetDiscoveringError() {
   LOG(ERROR) << "BluetoothSetDiscovering failed.";
 }
 
 void OnBluetoothDeviceConnect(bool was_device_already_paired) {
-  if (was_device_already_paired)
-    RecordUserInitiatedReconnectionAttemptResult(true /* success */);
+  if (was_device_already_paired) {
+    device::RecordUserInitiatedReconnectionAttemptResult(
+        true /* success */, device::BluetoothUiSurface::kSystemTray);
+  }
 }
 
 void OnBluetoothDeviceConnectError(
@@ -63,8 +56,10 @@ void OnBluetoothDeviceConnectError(
              << "]. The attempted device was previously ["
              << (was_device_already_paired ? "paired" : "not paired") << "].";
 
-  if (was_device_already_paired)
-    RecordUserInitiatedReconnectionAttemptResult(false /* success */);
+  if (was_device_already_paired) {
+    device::RecordUserInitiatedReconnectionAttemptResult(
+        false /* success */, device::BluetoothUiSurface::kSystemTray);
+  }
 }
 
 std::string BluetoothAddressToStr(const BluetoothAddress& address) {
@@ -229,7 +224,8 @@ void TrayBluetoothHelperLegacy::ConnectToBluetoothDevice(
         base::UserMetricsAction("StatusArea_Bluetooth_Connect_Known"));
 
     if (!device->IsConnectable()) {
-      RecordUserInitiatedReconnectionAttemptResult(false /* success */);
+      device::RecordUserInitiatedReconnectionAttemptResult(
+          false /* success */, device::BluetoothUiSurface::kSystemTray);
       return;
     }
 

device/bluetooth/chromeos/bluetooth_utils.cc

@@ -30,6 +30,18 @@ const char kSecurityKeyServiceUUID[] = "FFFD";
 constexpr base::TimeDelta kMaxDeviceSelectionDuration =
     base::TimeDelta::FromSeconds(30);
 
+// This enum is tied directly to a UMA enum defined in
+// //tools/metrics/histograms/enums.xml, and should always reflect it (do not
+// change one without changing the other).
+enum class BluetoothTransportType {
+  kUnknown = 0,
+  kClassic = 1,
+  kLE = 2,
+  kDual = 3,
+  kInvalid = 4,
+  kMaxValue = kInvalid
+};
+
 // Get limited number of devices from |devices| and
 // prioritize paired/connecting devices over other devices.
 BluetoothAdapter::DeviceList GetLimitedNumDevices(
@@ -132,6 +144,38 @@ BluetoothAdapter::DeviceList FilterUnknownDevices(
   return result;
 }
 
+void RecordPairingDuration(const std::string& histogram_name,
+                           base::TimeDelta pairing_duration) {
+  base::UmaHistogramCustomTimes(histogram_name, pairing_duration,
+                                base::TimeDelta::FromMilliseconds(1) /* min */,
+                                base::TimeDelta::FromSeconds(30) /* max */,
+                                50 /* buckets */);
+}
+
+void RecordPairingTransport(BluetoothTransport transport) {
+  BluetoothTransportType type;
+  switch (transport) {
+    case BLUETOOTH_TRANSPORT_CLASSIC:
+      type = BluetoothTransportType::kClassic;
+      break;
+    case BLUETOOTH_TRANSPORT_LE:
+      type = BluetoothTransportType::kLE;
+      break;
+    case BLUETOOTH_TRANSPORT_DUAL:
+      type = BluetoothTransportType::kDual;
+      break;
+    case BLUETOOTH_TRANSPORT_INVALID:
+      type = BluetoothTransportType::kInvalid;
+      break;
+    default:
+      type = BluetoothTransportType::kUnknown;
+      break;
+  }
+
+  base::UmaHistogramEnumeration("Bluetooth.ChromeOS.Pairing.TransportType",
+                                type);
+}
+
 void RecordDeviceSelectionDuration(const std::string& histogram_name,
                                    base::TimeDelta duration) {
   base::UmaHistogramCustomTimes(
@@ -151,6 +195,57 @@ device::BluetoothAdapter::DeviceList FilterBluetoothDeviceList(
   return GetLimitedNumDevices(max_devices, filtered_devices);
 }
 
+void RecordPairingResult(bool success,
+                         BluetoothTransport transport,
+                         base::TimeDelta duration) {
+  RecordPairingTransport(transport);
+
+  std::string result_histogram_name_prefix =
+      "Bluetooth.ChromeOS.Pairing.Result";
+
+  std::string transport_histogram_name;
+  switch (transport) {
+    case BluetoothTransport::BLUETOOTH_TRANSPORT_CLASSIC:
+      transport_histogram_name = "Classic";
+      break;
+    case BluetoothTransport::BLUETOOTH_TRANSPORT_LE:
+      transport_histogram_name = "BLE";
+      break;
+    case BluetoothTransport::BLUETOOTH_TRANSPORT_DUAL:
+      transport_histogram_name = "Dual";
+      break;
+    default:
+      // A transport type of INVALID or other is unexpected, and no success
+      // metric for it exists.
+      return;
+  }
+
+  base::UmaHistogramBoolean(result_histogram_name_prefix, success);
+  base::UmaHistogramBoolean(
+      result_histogram_name_prefix + "." + transport_histogram_name, success);
+
+  std::string duration_histogram_name_prefix =
+      "Bluetooth.ChromeOS.Pairing.Duration";
+  std::string success_histogram_name = success ? "Success" : "Failure";
+
+  std::string base_histogram_name =
+      duration_histogram_name_prefix + "." + success_histogram_name;
+  RecordPairingDuration(base_histogram_name, duration);
+  RecordPairingDuration(base_histogram_name + "." + transport_histogram_name,
+                        duration);
+}
+
+void RecordUserInitiatedReconnectionAttemptResult(bool success,
+                                                  BluetoothUiSurface surface) {
+  std::string base_histogram_name =
+      "Bluetooth.ChromeOS.UserInitiatedReconnectionAttempt.Result";
+  base::UmaHistogramBoolean(base_histogram_name, success);
+
+  std::string surface_name =
+      (surface == BluetoothUiSurface::kSettings ? "Settings" : "SystemTray");
+  base::UmaHistogramBoolean(base_histogram_name + "." + surface_name, success);
+}
+
 void RecordDeviceSelectionDuration(base::TimeDelta duration,
                                    BluetoothUiSurface surface,
                                    bool was_paired,

device/bluetooth/chromeos/bluetooth_utils.h

@@ -30,18 +30,29 @@ enum class BluetoothUiSurface {
 };
 
 // Return filtered devices based on the filter type and max number of devices.
-device::BluetoothAdapter::DeviceList DEVICE_BLUETOOTH_EXPORT
+DEVICE_BLUETOOTH_EXPORT device::BluetoothAdapter::DeviceList
 FilterBluetoothDeviceList(const BluetoothAdapter::DeviceList& devices,
                           BluetoothFilterType filter_type,
                           int max_devices);
 
+// Record outcome of user attempting to pair to a device.
+DEVICE_BLUETOOTH_EXPORT void RecordPairingResult(bool success,
+                                                 BluetoothTransport transport,
+                                                 base::TimeDelta duration);
+
+// Record outcome of user attempting to reconnect to a previously paired device.
+DEVICE_BLUETOOTH_EXPORT void RecordUserInitiatedReconnectionAttemptResult(
+    bool success,
+    BluetoothUiSurface surface);
+
 // Record how long it took for a user to find and select the device they wished
 // to connect to.
-void DEVICE_BLUETOOTH_EXPORT
-RecordDeviceSelectionDuration(base::TimeDelta duration,
-                              BluetoothUiSurface surface,
-                              bool was_paired,
-                              BluetoothTransport transport);
+DEVICE_BLUETOOTH_EXPORT void RecordDeviceSelectionDuration(
+    base::TimeDelta duration,
+    BluetoothUiSurface surface,
+    bool was_paired,
+    BluetoothTransport transport);
+
 }  // namespace device
 
 #endif  // DEVICE_BLUETOOTH_CHROMEOS_BLUETOOTH_UTILS_H_

extensions/browser/api/bluetooth/bluetooth_private_api.cc

@@ -28,7 +28,7 @@
 
 #if defined(OS_CHROMEOS)
 #include "device/bluetooth/chromeos/bluetooth_utils.h"
-#endif
+#endif  // defined(OS_CHROMEOS)
 
 namespace bt = extensions::api::bluetooth;
 namespace bt_private = extensions::api::bluetooth_private;
@@ -41,89 +41,24 @@ static base::LazyInstance<BrowserContextKeyedAPIFactory<BluetoothPrivateAPI>>::
 
 namespace {
 
-// This enum is tied directly to a UMA enum defined in
-// //tools/metrics/histograms/enums.xml, and should always reflect it (do not
-// change one without changing the other).
-enum BluetoothTransportType {
-  kUnknown = 0,
-  kClassic = 1,
-  kLE = 2,
-  kDual = 3,
-  kInvalid = 4,
-  kMaxValue
-};
-
-std::string GetListenerId(const EventListenerInfo& details) {
-  return !details.extension_id.empty() ? details.extension_id
-                                       : details.listener_url.host();
-}
-
-void RecordPairingDuration(const std::string& histogram_name,
-                           base::TimeDelta pairing_duration) {
-  base::UmaHistogramCustomTimes(histogram_name, pairing_duration,
-                                base::TimeDelta::FromMilliseconds(1) /* min */,
-                                base::TimeDelta::FromSeconds(30) /* max */,
-                                50 /* buckets */);
-}
-
-void RecordPairingResult(bool success,
-                         bt::Transport transport,
-                         int pairing_duration_ms) {
-  std::string transport_histogram_name;
+#if defined(OS_CHROMEOS)
+device::BluetoothTransport GetBluetoothTransport(bt::Transport transport) {
   switch (transport) {
     case bt::Transport::TRANSPORT_CLASSIC:
-      transport_histogram_name = "Classic";
-      break;
+      return device::BLUETOOTH_TRANSPORT_CLASSIC;
     case bt::Transport::TRANSPORT_LE:
-      transport_histogram_name = "BLE";
-      break;
+      return device::BLUETOOTH_TRANSPORT_LE;
     case bt::Transport::TRANSPORT_DUAL:
-      transport_histogram_name = "Dual";
-      break;
+      return device::BLUETOOTH_TRANSPORT_DUAL;
     default:
-      // A transport type of INVALID or other is unexpected, and no success
-      // metric for it exists.
-      return;
+      return device::BLUETOOTH_TRANSPORT_INVALID;
   }
-
-  base::UmaHistogramBoolean("Bluetooth.ChromeOS.Pairing.Result", success);
-  base::UmaHistogramBoolean(
-      "Bluetooth.ChromeOS.Pairing.Result." + transport_histogram_name, success);
-
-  std::string duration_histogram_name_prefix =
-      "Bluetooth.ChromeOS.Pairing.Duration";
-  std::string success_histogram_name = success ? "Success" : "Failure";
-
-  std::string base_histogram_name =
-      duration_histogram_name_prefix + "." + success_histogram_name;
-  RecordPairingDuration(base_histogram_name,
-                        base::TimeDelta::FromMilliseconds(pairing_duration_ms));
-  RecordPairingDuration(base_histogram_name + "." + transport_histogram_name,
-                        base::TimeDelta::FromMilliseconds(pairing_duration_ms));
 }
+#endif  // defined(OS_CHROMEOS)
 
-void RecordPairingTransport(bt::Transport transport) {
-  BluetoothTransportType type;
-  switch (transport) {
-    case bt::Transport::TRANSPORT_CLASSIC:
-      type = BluetoothTransportType::kClassic;
-      break;
-    case bt::Transport::TRANSPORT_LE:
-      type = BluetoothTransportType::kLE;
-      break;
-    case bt::Transport::TRANSPORT_DUAL:
-      type = BluetoothTransportType::kDual;
-      break;
-    case bt::Transport::TRANSPORT_INVALID:
-      type = BluetoothTransportType::kInvalid;
-      break;
-    default:
-      type = BluetoothTransportType::kUnknown;
-      break;
-  }
-
-  base::UmaHistogramEnumeration("Bluetooth.ChromeOS.Pairing.TransportType",
-                                type);
+std::string GetListenerId(const EventListenerInfo& details) {
+  return !details.extension_id.empty() ? details.extension_id
+                                       : details.listener_url.host();
 }
 
 }  // namespace
@@ -697,9 +632,11 @@ bool BluetoothPrivateRecordPairingFunction::CreateParams() {
 
 void BluetoothPrivateRecordPairingFunction::DoWork(
     scoped_refptr<device::BluetoothAdapter> adapter) {
-  RecordPairingResult(params_->success, params_->transport,
-                      params_->pairing_duration_ms);
-  RecordPairingTransport(params_->transport);
+#if defined(OS_CHROMEOS)
+  device::RecordPairingResult(
+      params_->success, GetBluetoothTransport(params_->transport),
+      base::TimeDelta::FromMilliseconds(params_->pairing_duration_ms));
+#endif  // defined(OS_CHROMEOS)
 
   Respond(NoArguments());
 }
@@ -719,12 +656,10 @@ bool BluetoothPrivateRecordReconnectionFunction::CreateParams() {
 
 void BluetoothPrivateRecordReconnectionFunction::DoWork(
     scoped_refptr<device::BluetoothAdapter> adapter) {
-  base::UmaHistogramBoolean(
-      "Bluetooth.ChromeOS.UserInitiatedReconnectionAttempt.Result",
-      params_->success);
-  base::UmaHistogramBoolean(
-      "Bluetooth.ChromeOS.UserInitiatedReconnectionAttempt.Result.Settings",
-      params_->success);
+#if defined(OS_CHROMEOS)
+  device::RecordUserInitiatedReconnectionAttemptResult(
+      params_->success, device::BluetoothUiSurface::kSettings);
+#endif  // defined(OS_CHROMEOS)
 
   Respond(NoArguments());
 }
@@ -745,26 +680,11 @@ bool BluetoothPrivateRecordDeviceSelectionFunction::CreateParams() {
 void BluetoothPrivateRecordDeviceSelectionFunction::DoWork(
     scoped_refptr<device::BluetoothAdapter> adapter) {
 #if defined(OS_CHROMEOS)
-  device::BluetoothTransport transport;
-  switch (params_->transport) {
-    case bt::Transport::TRANSPORT_CLASSIC:
-      transport = device::BLUETOOTH_TRANSPORT_CLASSIC;
-      break;
-    case bt::Transport::TRANSPORT_LE:
-      transport = device::BLUETOOTH_TRANSPORT_LE;
-      break;
-    case bt::Transport::TRANSPORT_DUAL:
-      transport = device::BLUETOOTH_TRANSPORT_DUAL;
-      break;
-    default:
-      transport = device::BLUETOOTH_TRANSPORT_INVALID;
-      break;
-  }
-
   device::RecordDeviceSelectionDuration(
       base::TimeDelta::FromMilliseconds(params_->selection_duration_ms),
-      device::BluetoothUiSurface::kSettings, params_->was_paired, transport);
-#endif
+      device::BluetoothUiSurface::kSettings, params_->was_paired,
+      GetBluetoothTransport(params_->transport));
+#endif  // defined(OS_CHROMEOS)
 
   Respond(NoArguments());
 }