[Fast Pair] Add success funnel to Subsequent pair

- Adds a new histogram and associated metrics logging
for the Subsequent pairing flow.
- Exposes new hooks for pairing started, handshake
complete, and pairing complete. These new hooks will
also be used for future metrics.

TEST=Verified via chrome://histograms that all four metrics
are properly logged.
TEST=Added unit tests to verify new hook calls.

Bug: b/259596469
Low-Coverage-Reason: Metrics logger switch cases for follow-up CLs.
Change-Id: I1c712d4c0a6e93596e1818645d55d50e4064e2b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4035032
Commit-Queue: Jack Shira <jackshira@google.com>
Reviewed-by: Daniel Classon <dclasson@google.com>
Reviewed-by: Juliet Lévesque <julietlevesque@google.com>
Cr-Commit-Position: refs/heads/main@{#1073574}

ash/quick_pair/common/fast_pair/fast_pair_metrics.cc

@@ -532,6 +532,8 @@ const char kSavedDevicesTotalUxLoadTime[] =
     "Bluetooth.ChromeOS.FastPair.SavedDevices.TotalUxLoadTime";
 const char kSavedDevicesCount[] =
     "Bluetooth.ChromeOS.FastPair.SavedDevices.DeviceCount";
+constexpr char kSubsequentSuccessFunnelMetric[] =
+    "ChromeOS.FastPair.SubsequentPairing";
 
 const std::string GetEngagementFlowInitialModelIdMetric(
     const ash::quick_pair::Device& device) {
@@ -587,6 +589,11 @@ void AttemptRecordingFastPairEngagementFlow(const Device& device,
   }
 }
 
+void RecordSubsequentSuccessFunnelFlow(
+    FastPairSubsequentSuccessFunnelEvent event) {
+  base::UmaHistogramEnumeration(kSubsequentSuccessFunnelMetric, event);
+}
+
 void AttemptRecordingTotalUxPairTime(const Device& device,
                                      base::TimeDelta total_pair_time) {
   switch (device.protocol) {

ash/quick_pair/common/fast_pair/fast_pair_metrics.h

@@ -57,6 +57,20 @@ enum COMPONENT_EXPORT(QUICK_PAIR_COMMON)
       kAssociateAccountDismissedAfterLearnMorePressed = 133,
     };
 
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused. The numbers here correspond to the
+// ordering of the flow. This enum should be kept in sync with the
+// FastPairSubsequentSuccessFunnelEvent enum in
+// src/tools/metrics/histograms/enums.xml.
+enum class COMPONENT_EXPORT(QUICK_PAIR_COMMON)
+    FastPairSubsequentSuccessFunnelEvent {
+      kNotificationsClicked = 0,
+      kInitializationStarted = 1,
+      kPairingStarted = 2,
+      kProcessComplete = 3,
+      kMaxValue = kProcessComplete,
+    };
+
 // These values are persisted to logs. Entries should not be renumbered and
 // numeric values should never be reused. This enum should be kept in sync
 // with the FastPairPairingMethod enum in
@@ -105,6 +119,10 @@ COMPONENT_EXPORT(QUICK_PAIR_COMMON)
 void AttemptRecordingFastPairEngagementFlow(const Device& device,
                                             FastPairEngagementFlowEvent event);
 
+COMPONENT_EXPORT(QUICK_PAIR_COMMON)
+void RecordSubsequentSuccessFunnelFlow(
+    FastPairSubsequentSuccessFunnelEvent event);
+
 COMPONENT_EXPORT(QUICK_PAIR_COMMON)
 void AttemptRecordingTotalUxPairTime(const Device& device,
                                      base::TimeDelta total_pair_time);

ash/quick_pair/keyed_service/quick_pair_metrics_logger.cc

@@ -99,6 +99,16 @@ void QuickPairMetricsLogger::OnDiscoveryAction(scoped_refptr<Device> device,
                                                DiscoveryAction action) {
   switch (action) {
     case DiscoveryAction::kPairToDevice:
+      switch (device->protocol) {
+        case Protocol::kFastPairSubsequent:
+          RecordSubsequentSuccessFunnelFlow(
+              FastPairSubsequentSuccessFunnelEvent::kNotificationsClicked);
+          break;
+        case Protocol::kFastPairInitial:
+        case Protocol::kFastPairRetroactive:
+          break;
+      }
+
       if (base::Contains(discovery_learn_more_devices_, device)) {
         AttemptRecordingFastPairEngagementFlow(
             *device, FastPairEngagementFlowEvent::
@@ -177,6 +187,42 @@ void QuickPairMetricsLogger::OnDeviceFound(scoped_refptr<Device> device) {
       *device, FastPairEngagementFlowEvent::kDiscoveryUiShown);
 }
 
+void QuickPairMetricsLogger::OnPairingStart(scoped_refptr<Device> device) {
+  switch (device->protocol) {
+    case Protocol::kFastPairSubsequent:
+      RecordSubsequentSuccessFunnelFlow(
+          FastPairSubsequentSuccessFunnelEvent::kInitializationStarted);
+      break;
+    case Protocol::kFastPairInitial:
+    case Protocol::kFastPairRetroactive:
+      break;
+  }
+}
+
+void QuickPairMetricsLogger::OnHandshakeComplete(scoped_refptr<Device> device) {
+  switch (device->protocol) {
+    case Protocol::kFastPairSubsequent:
+      RecordSubsequentSuccessFunnelFlow(
+          FastPairSubsequentSuccessFunnelEvent::kPairingStarted);
+      break;
+    case Protocol::kFastPairInitial:
+    case Protocol::kFastPairRetroactive:
+      break;
+  }
+}
+
+void QuickPairMetricsLogger::OnPairingComplete(scoped_refptr<Device> device) {
+  switch (device->protocol) {
+    case Protocol::kFastPairSubsequent:
+      RecordSubsequentSuccessFunnelFlow(
+          FastPairSubsequentSuccessFunnelEvent::kProcessComplete);
+      break;
+    case Protocol::kFastPairInitial:
+    case Protocol::kFastPairRetroactive:
+      break;
+  }
+}
+
 void QuickPairMetricsLogger::OnRetroactivePairFound(
     scoped_refptr<Device> device) {
   AttemptRecordingFastPairRetroactiveEngagementFlow(

ash/quick_pair/keyed_service/quick_pair_metrics_logger.h

@@ -41,11 +41,14 @@ class QuickPairMetricsLogger : public PairerBroker::Observer,
 
  private:
   // PairerBroker::Observer
+  void OnPairingStart(scoped_refptr<Device> device) override;
+  void OnHandshakeComplete(scoped_refptr<Device> device) override;
   void OnDevicePaired(scoped_refptr<Device> device) override;
-  void OnPairFailure(scoped_refptr<Device> device,
-                     PairFailure failure) override;
   void OnAccountKeyWrite(scoped_refptr<Device> device,
                          absl::optional<AccountKeyFailure> error) override;
+  void OnPairingComplete(scoped_refptr<Device> device) override;
+  void OnPairFailure(scoped_refptr<Device> device,
+                     PairFailure failure) override;
 
   // UIBroker::Observer
   void OnDiscoveryAction(scoped_refptr<Device> device,

ash/quick_pair/keyed_service/quick_pair_metrics_logger_unittest.cc

@@ -46,6 +46,8 @@ constexpr char kFastPairEngagementFlowMetricInitial[] =
 constexpr char kFastPairEngagementFlowMetricSubsequent[] =
     "Bluetooth.ChromeOS.FastPair.EngagementFunnel.Steps."
     "SubsequentPairingProtocol";
+constexpr char kSubsequentSuccessFunnelMetric[] =
+    "ChromeOS.FastPair.SubsequentPairing";
 const char kFastPairRetroactiveEngagementFlowMetric[] =
     "Bluetooth.ChromeOS.FastPair.RetroactiveEngagementFunnel.Steps";
 constexpr char kFastPairPairTimeMetricInitial[] =
@@ -274,6 +276,21 @@ class QuickPairMetricsLoggerTest : public testing::Test {
     }
   }
 
+  void SimulatePairingFlow(Protocol protocol) {
+    switch (protocol) {
+      case Protocol::kFastPairSubsequent:
+        mock_ui_broker_->NotifyDiscoveryAction(subsequent_device_,
+                                               DiscoveryAction::kPairToDevice);
+        mock_pairer_broker_->NotifyPairingStart(subsequent_device_);
+        mock_pairer_broker_->NotifyHandshakeComplete(subsequent_device_);
+        mock_pairer_broker_->NotifyPairComplete(subsequent_device_);
+        break;
+      case Protocol::kFastPairInitial:
+      case Protocol::kFastPairRetroactive:
+        break;
+    }
+  }
+
   void SimulateErrorUiDismissedByUser(Protocol protocol) {
     switch (protocol) {
       case Protocol::kFastPairInitial:
@@ -909,6 +926,28 @@ TEST_F(QuickPairMetricsLoggerTest, LogPairingSucceeded_Subsequent) {
             0);
 }
 
+TEST_F(QuickPairMetricsLoggerTest, LogSuccessFunnel_Subseqent) {
+  SimulatePairingFlow(Protocol::kFastPairSubsequent);
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(histogram_tester().GetBucketCount(
+                kSubsequentSuccessFunnelMetric,
+                FastPairSubsequentSuccessFunnelEvent::kNotificationsClicked),
+            1);
+  EXPECT_EQ(histogram_tester().GetBucketCount(
+                kSubsequentSuccessFunnelMetric,
+                FastPairSubsequentSuccessFunnelEvent::kInitializationStarted),
+            1);
+  EXPECT_EQ(histogram_tester().GetBucketCount(
+                kSubsequentSuccessFunnelMetric,
+                FastPairSubsequentSuccessFunnelEvent::kPairingStarted),
+            1);
+  EXPECT_EQ(histogram_tester().GetBucketCount(
+                kSubsequentSuccessFunnelMetric,
+                FastPairSubsequentSuccessFunnelEvent::kProcessComplete),
+            1);
+}
+
 TEST_F(QuickPairMetricsLoggerTest, LogErrorUiDismissed_Initial) {
   SimulateErrorUiDismissed(Protocol::kFastPairInitial);
   base::RunLoop().RunUntilIdle();

ash/quick_pair/pairing/fast_pair/fast_pair_pairer_impl.cc

@@ -100,6 +100,7 @@ FastPairPairerImpl::Factory* FastPairPairerImpl::Factory::g_test_factory_ =
 std::unique_ptr<FastPairPairer> FastPairPairerImpl::Factory::Create(
     scoped_refptr<device::BluetoothAdapter> adapter,
     scoped_refptr<Device> device,
+    base::OnceCallback<void(scoped_refptr<Device>)> handshake_complete_callback,
     base::OnceCallback<void(scoped_refptr<Device>)> paired_callback,
     base::OnceCallback<void(scoped_refptr<Device>, PairFailure)>
         pair_failed_callback,
@@ -109,13 +110,15 @@ std::unique_ptr<FastPairPairer> FastPairPairerImpl::Factory::Create(
         pairing_procedure_complete) {
   if (g_test_factory_)
     return g_test_factory_->CreateInstance(
-        std::move(adapter), std::move(device), std::move(paired_callback),
+        std::move(adapter), std::move(device),
+        std::move(handshake_complete_callback), std::move(paired_callback),
         std::move(pair_failed_callback),
         std::move(account_key_failure_callback),
         std::move(pairing_procedure_complete));
 
   return base::WrapUnique(new FastPairPairerImpl(
-      std::move(adapter), std::move(device), std::move(paired_callback),
+      std::move(adapter), std::move(device),
+      std::move(handshake_complete_callback), std::move(paired_callback),
       std::move(pair_failed_callback), std::move(account_key_failure_callback),
       std::move(pairing_procedure_complete)));
 }
@@ -131,6 +134,7 @@ FastPairPairerImpl::Factory::~Factory() = default;
 FastPairPairerImpl::FastPairPairerImpl(
     scoped_refptr<device::BluetoothAdapter> adapter,
     scoped_refptr<Device> device,
+    base::OnceCallback<void(scoped_refptr<Device>)> handshake_complete_callback,
     base::OnceCallback<void(scoped_refptr<Device>)> paired_callback,
     base::OnceCallback<void(scoped_refptr<Device>, PairFailure)>
         pair_failed_callback,
@@ -139,6 +143,7 @@ FastPairPairerImpl::FastPairPairerImpl(
     base::OnceCallback<void(scoped_refptr<Device>)> pairing_procedure_complete)
     : adapter_(std::move(adapter)),
       device_(std::move(device)),
+      handshake_complete_callback_(std::move(handshake_complete_callback)),
       paired_callback_(std::move(paired_callback)),
       pair_failed_callback_(std::move(pair_failed_callback)),
       account_key_failure_callback_(std::move(account_key_failure_callback)),
@@ -207,6 +212,8 @@ void FastPairPairerImpl::OnHandshakeComplete(
   DCHECK(fast_pair_handshake_);
   DCHECK(fast_pair_handshake_->completed_successfully());
 
+  std::move(handshake_complete_callback_).Run(device_);
+
   fast_pair_gatt_service_client_ =
       fast_pair_handshake_->fast_pair_gatt_service_client();
 

ash/quick_pair/pairing/fast_pair/fast_pair_pairer_impl.h

@@ -43,6 +43,8 @@ class FastPairPairerImpl : public FastPairPairer,
     static std::unique_ptr<FastPairPairer> Create(
         scoped_refptr<device::BluetoothAdapter> adapter,
         scoped_refptr<Device> device,
+        base::OnceCallback<void(scoped_refptr<Device>)>
+            handshake_complete_callback,
         base::OnceCallback<void(scoped_refptr<Device>)> paired_callback,
         base::OnceCallback<void(scoped_refptr<Device>, PairFailure)>
             pair_failed_callback,
@@ -59,6 +61,8 @@ class FastPairPairerImpl : public FastPairPairer,
     virtual std::unique_ptr<FastPairPairer> CreateInstance(
         scoped_refptr<device::BluetoothAdapter> adapter,
         scoped_refptr<Device> device,
+        base::OnceCallback<void(scoped_refptr<Device>)>
+            handshake_complete_callback,
         base::OnceCallback<void(scoped_refptr<Device>)> paired_callback,
         base::OnceCallback<void(scoped_refptr<Device>, PairFailure)>
             pair_failed_callback,
@@ -74,6 +78,8 @@ class FastPairPairerImpl : public FastPairPairer,
   FastPairPairerImpl(
       scoped_refptr<device::BluetoothAdapter> adapter,
       scoped_refptr<Device> device,
+      base::OnceCallback<void(scoped_refptr<Device>)>
+          handshake_complete_callback,
       base::OnceCallback<void(scoped_refptr<Device>)> paired_callback,
       base::OnceCallback<void(scoped_refptr<Device>, PairFailure)>
           pair_failed_callback,
@@ -156,6 +162,7 @@ class FastPairPairerImpl : public FastPairPairer,
   scoped_refptr<Device> device_;
   FastPairGattServiceClient* fast_pair_gatt_service_client_;
   std::string pairing_device_address_;
+  base::OnceCallback<void(scoped_refptr<Device>)> handshake_complete_callback_;
   base::OnceCallback<void(scoped_refptr<Device>)> paired_callback_;
   base::OnceCallback<void(scoped_refptr<Device>, PairFailure)>
       pair_failed_callback_;

ash/quick_pair/pairing/fast_pair/fast_pair_pairer_impl_unittest.cc

@@ -391,15 +391,17 @@ class FastPairPairerImplTest : public AshTestBase {
   // This is done on-demand to enable setting up mock expectations first.
   void CreatePairer() {
     pairer_ = std::make_unique<FastPairPairerImpl>(
-        adapter_, device_, paired_callback_.Get(),
+        adapter_, device_, handshake_complete_callback_.Get(),
+        paired_callback_.Get(),
         base::BindOnce(&FastPairPairerImplTest::PairFailedCallback,
                        weak_ptr_factory_.GetWeakPtr()),
         account_key_failure_callback_.Get(), pairing_procedure_complete_.Get());
   }
 
   void CreatePairerAsFactory() {
     pairer_ = FastPairPairerImpl::Factory::Create(
-        adapter_, device_, paired_callback_.Get(),
+        adapter_, device_, handshake_complete_callback_.Get(),
+        paired_callback_.Get(),
         base::BindOnce(&FastPairPairerImplTest::PairFailedCallback,
                        weak_ptr_factory_.GetWeakPtr()),
         account_key_failure_callback_.Get(), pairing_procedure_complete_.Get());
@@ -438,6 +440,8 @@ class FastPairPairerImplTest : public AshTestBase {
   FakeBluetoothDevice* fake_bluetooth_device_ptr_ = nullptr;
   scoped_refptr<FakeBluetoothAdapter> adapter_;
   scoped_refptr<Device> device_;
+  base::MockCallback<base::OnceCallback<void(scoped_refptr<Device>)>>
+      handshake_complete_callback_;
   base::MockCallback<base::OnceCallback<void(scoped_refptr<Device>)>>
       paired_callback_;
   base::MockCallback<

ash/quick_pair/pairing/mock_pairer_broker.cc

@@ -39,5 +39,20 @@ void MockPairerBroker::NotifyAccountKeyWrite(
     obs.OnAccountKeyWrite(device, failure);
 }
 
+void MockPairerBroker::NotifyPairingStart(scoped_refptr<Device> device) {
+  for (auto& obs : observers_)
+    obs.OnPairingStart(device);
+}
+
+void MockPairerBroker::NotifyHandshakeComplete(scoped_refptr<Device> device) {
+  for (auto& obs : observers_)
+    obs.OnHandshakeComplete(device);
+}
+
+void MockPairerBroker::NotifyPairComplete(scoped_refptr<Device> device) {
+  for (auto& obs : observers_)
+    obs.OnPairingComplete(device);
+}
+
 }  // namespace quick_pair
 }  // namespace ash

ash/quick_pair/pairing/mock_pairer_broker.h

@@ -31,8 +31,11 @@ class MockPairerBroker : public PairerBroker {
 
   void AddObserver(Observer* observer) override;
   void RemoveObserver(Observer* observer) override;
+  void NotifyPairingStart(scoped_refptr<Device> device);
+  void NotifyHandshakeComplete(scoped_refptr<Device> device);
   void NotifyDevicePaired(scoped_refptr<Device> device);
   void NotifyPairFailure(scoped_refptr<Device> device, PairFailure failure);
+  void NotifyPairComplete(scoped_refptr<Device> device);
   void NotifyAccountKeyWrite(scoped_refptr<Device> device,
                              absl::optional<AccountKeyFailure> error);
 

ash/quick_pair/pairing/pairer_broker.h

@@ -5,6 +5,7 @@
 #ifndef ASH_QUICK_PAIR_PAIRING_PAIRER_BROKER_H_
 #define ASH_QUICK_PAIR_PAIRING_PAIRER_BROKER_H_
 
+#include "ash/quick_pair/common/device.h"
 #include "ash/quick_pair/common/pair_failure.h"
 #include "ash/quick_pair/common/protocol.h"
 #include "base/observer_list_types.h"
@@ -13,7 +14,6 @@
 namespace ash {
 namespace quick_pair {
 
-struct Device;
 enum class AccountKeyFailure;
 
 // The PairerBroker is the entry point for the Pairing component in the Quick
@@ -24,11 +24,14 @@ class PairerBroker {
  public:
   class Observer : public base::CheckedObserver {
    public:
-    virtual void OnDevicePaired(scoped_refptr<Device> device) = 0;
-    virtual void OnPairFailure(scoped_refptr<Device> device,
-                               PairFailure failure) = 0;
+    virtual void OnPairingStart(scoped_refptr<Device> device) {}
+    virtual void OnHandshakeComplete(scoped_refptr<Device> device) {}
+    virtual void OnDevicePaired(scoped_refptr<Device> device) {}
     virtual void OnAccountKeyWrite(scoped_refptr<Device> device,
-                                   absl::optional<AccountKeyFailure> error) = 0;
+                                   absl::optional<AccountKeyFailure> error) {}
+    virtual void OnPairingComplete(scoped_refptr<Device> device) {}
+    virtual void OnPairFailure(scoped_refptr<Device> device,
+                               PairFailure failure) {}
   };
 
   virtual ~PairerBroker() = default;