[M106] Add per-user-state metrics for send-tab-to-self UI

Before this CL, metrics were only recorded when a device was selected for sending. Now also record when: device list shown, sign-in promo shown and no target device message shown. Also deprecate the Sharing.SendTabToSelf.NotificationStatus::Sent bucket since it's redundant with the existing metric above. https://source.chromium.org/chromium/chromium/src/+/main:components/send_tab_to_self/metrics_util.cc;l=67;drc=9a4aa67fccec134875880bfd893f0ed731c0e51b (cherry picked from commit 39a7381) Bug: 1295204 Change-Id: Ie29e8cf1ee2def4127c3e778bfe1fbcddc81a42a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3867811 Reviewed-by: Weilun Shi <sweilun@chromium.org> Auto-Submit: Victor Vianna <victorvianna@google.com> Commit-Queue: Weilun Shi <sweilun@chromium.org> Reviewed-by: Jeffrey Cohen <jeffreycohen@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1042396} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3876427 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Commit-Queue: Victor Vianna <victorvianna@google.com> Cr-Commit-Position: refs/branch-heads/5249@{#328} Cr-Branched-From: 4f7bea5-refs/heads/main@{#1036826}
chromium · Sep 6, 2022 · e9ef251 · e9ef251
1 parent 6c05b7e
commit e9ef251
Show file tree

Hide file tree

Showing 11 changed files with 71 additions and 45 deletions.
diff --git a/chrome/browser/android/send_tab_to_self/metrics_recorder.cc b/chrome/browser/android/send_tab_to_self/metrics_recorder.cc
@@ -11,8 +11,10 @@
 namespace send_tab_to_self {
 
 // Static free function declared and called directly from Java.
-static void JNI_MetricsRecorder_RecordDeviceClickedInShareSheet(JNIEnv* env) {
-  RecordDeviceClicked(ShareEntryPoint::kShareSheet);
+static void JNI_MetricsRecorder_RecordSendingEvent(JNIEnv* env,
+                                                   int sending_event) {
+  RecordSendingEvent(ShareEntryPoint::kShareSheet,
+                     static_cast<SendingEvent>(sending_event));
 }
 
 // Static free function declared and called directly from Java.

diff --git a/...rc/org/chromium/chrome/browser/share/send_tab_to_self/DevicePickerBottomSheetContent.java b/...rc/org/chromium/chrome/browser/share/send_tab_to_self/DevicePickerBottomSheetContent.java
@@ -137,7 +137,7 @@ public int getSheetClosedAccessibilityStringId() {
 
     @Override
     public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
-        MetricsRecorder.recordDeviceClickedInShareSheet();
+        MetricsRecorder.recordSendingEvent(SendingEvent.CLICK_ITEM);
         TargetDeviceInfo targetDeviceInfo = mAdapter.getItem(position);
 
         SendTabToSelfAndroidBridge.addEntry(mProfile, mUrl, mTitle, targetDeviceInfo.cacheGuid);

diff --git a/.../android/java/src/org/chromium/chrome/browser/share/send_tab_to_self/MetricsRecorder.java b/.../android/java/src/org/chromium/chrome/browser/share/send_tab_to_self/MetricsRecorder.java
@@ -12,8 +12,8 @@
  */
 @JNINamespace("send_tab_to_self")
 class MetricsRecorder {
-    public static void recordDeviceClickedInShareSheet() {
-        MetricsRecorderJni.get().recordDeviceClickedInShareSheet();
+    public static void recordSendingEvent(@SendingEvent int sendingEvent) {
+        MetricsRecorderJni.get().recordSendingEvent(sendingEvent);
     }
 
     public static void recordNotificationShown() {
@@ -34,7 +34,7 @@ public static void recordNotificationTimedOut() {
 
     @NativeMethods
     interface Natives {
-        void recordDeviceClickedInShareSheet();
+        void recordSendingEvent(int sendingEvent);
         void recordNotificationShown();
         void recordNotificationOpened();
         void recordNotificationDismissed();

diff --git a/...java/src/org/chromium/chrome/browser/share/send_tab_to_self/SendTabToSelfCoordinator.java b/...java/src/org/chromium/chrome/browser/share/send_tab_to_self/SendTabToSelfCoordinator.java
@@ -156,16 +156,19 @@ public void show() {
             // This must be the old behavior where the entry point is shown even in states where
             // no promo is shown.
             assert !ChromeFeatureList.isEnabled(ChromeFeatureList.SEND_TAB_TO_SELF_SIGNIN_PROMO);
+            MetricsRecorder.recordSendingEvent(SendingEvent.SHOW_NO_TARGET_DEVICE_MESSAGE);
             mController.requestShowContent(new NoTargetDeviceBottomSheetContent(mContext), true);
             return;
         }
 
         switch (displayReason.get()) {
             case EntryPointDisplayReason.INFORM_NO_TARGET_DEVICE:
+                MetricsRecorder.recordSendingEvent(SendingEvent.SHOW_NO_TARGET_DEVICE_MESSAGE);
                 mController.requestShowContent(
                         new NoTargetDeviceBottomSheetContent(mContext), true);
                 return;
             case EntryPointDisplayReason.OFFER_FEATURE:
+                MetricsRecorder.recordSendingEvent(SendingEvent.SHOW_DEVICE_LIST);
                 // TODO(crbug.com/1219434): Merge with INFORM_NO_TARGET_DEVICE, just let the UI
                 // differentiate between the 2 by checking the device list size.
                 List<TargetDeviceInfo> targetDevices =
@@ -176,6 +179,7 @@ public void show() {
                         true);
                 return;
             case EntryPointDisplayReason.OFFER_SIGN_IN: {
+                MetricsRecorder.recordSendingEvent(SendingEvent.SHOW_SIGNIN_PROMO);
                 new AccountPickerBottomSheetCoordinator(mWindowAndroid, mController,
                         new SendTabToSelfAccountPickerDelegate(this::onSignInComplete, mProfile));
                 return;

diff --git a/chrome/browser/ui/views/send_tab_to_self/send_tab_to_self_bubble_controller.cc b/chrome/browser/ui/views/send_tab_to_self/send_tab_to_self_bubble_controller.cc
@@ -68,16 +68,23 @@ void SendTabToSelfBubbleController::ShowBubble(bool show_back_button) {
   DCHECK(reason);
   switch (*reason) {
     case send_tab_to_self::EntryPointDisplayReason::kOfferFeature:
+      send_tab_to_self::RecordSendingEvent(ShareEntryPoint::kOmniboxIcon,
+                                           SendingEvent::kShowDeviceList);
       send_tab_to_self_bubble_view_ =
           browser->window()->ShowSendTabToSelfDevicePickerBubble(
               &GetWebContents());
       break;
     case send_tab_to_self::EntryPointDisplayReason::kOfferSignIn:
+      send_tab_to_self::RecordSendingEvent(ShareEntryPoint::kOmniboxIcon,
+                                           SendingEvent::kShowSigninPromo);
       send_tab_to_self_bubble_view_ =
           browser->window()->ShowSendTabToSelfPromoBubble(
               &GetWebContents(), /*show_signin_button=*/true);
       break;
     case send_tab_to_self::EntryPointDisplayReason::kInformNoTargetDevice:
+      send_tab_to_self::RecordSendingEvent(
+          ShareEntryPoint::kOmniboxIcon,
+          SendingEvent::kShowNoTargetDeviceMessage);
       send_tab_to_self_bubble_view_ =
           browser->window()->ShowSendTabToSelfPromoBubble(
               &GetWebContents(), /*show_signin_button=*/false);
@@ -119,7 +126,8 @@ void SendTabToSelfBubbleController::OnDeviceSelected(
     const std::string& target_device_guid) {
   // TODO(crbug.com/1288843): This is being recorded for entry points other
   // than the omnibox. Make the entry point a ShowBubble() argument.
-  send_tab_to_self::RecordDeviceClicked(ShareEntryPoint::kOmniboxIcon);
+  send_tab_to_self::RecordSendingEvent(ShareEntryPoint::kOmniboxIcon,
+                                       SendingEvent::kClickItem);
 
   SendTabToSelfModel* model =
       SendTabToSelfSyncServiceFactory::GetForProfile(GetProfile())

diff --git a/components/send_tab_to_self/BUILD.gn b/components/send_tab_to_self/BUILD.gn
@@ -61,7 +61,10 @@ static_library("send_tab_to_self") {
 if (is_android) {
   java_cpp_enum("java_enum_srcjar") {
     visibility = [ ":*" ]
-    sources = [ "entry_point_display_reason.h" ]
+    sources = [
+      "entry_point_display_reason.h",
+      "metrics_util.h",
+    ]
   }
 
   android_library("send_tab_to_self_java") {

diff --git a/components/send_tab_to_self/metrics_util.cc b/components/send_tab_to_self/metrics_util.cc
@@ -11,16 +11,6 @@ namespace send_tab_to_self {
 
 namespace {
 
-// State of the send tab to self option in the UI.
-// These values are persisted to logs. Entries should not be renumbered and
-// numeric values should never be reused.
-enum class ClickResult {
-  // kShowItem = 0,
-  kClickItem = 1,
-  // kShowDeviceList = 2,
-  kMaxValue = kClickItem,
-};
-
 // Status of received STTS notifications.
 // These values are persisted to logs. Entries should not be renumbered and
 // numeric values should never be reused.
@@ -30,7 +20,7 @@ enum class NotificationStatus {
   kDismissed = 1,
   kOpened = 2,
   kTimedOut = 3,
-  kSent = 4,
+  // kSent = 4,
   kDismissReasonUnknown = 5,
   kThrottled = 6,
   kMaxValue = kThrottled,
@@ -57,14 +47,11 @@ std::string GetEntryPointHistogramString(ShareEntryPoint entry_point) {
 
 }  // namespace
 
-void RecordDeviceClicked(ShareEntryPoint entry_point) {
-  // TODO(crbug.com/956722): Only kClickItem is used today, so we should replace
-  // this with a histogram which doesn't use the ClickResult enum.
+void RecordSendingEvent(ShareEntryPoint entry_point, SendingEvent event) {
   base::UmaHistogramEnumeration(
       base::StrCat({"SendTabToSelf.", GetEntryPointHistogramString(entry_point),
                     ".ClickResult"}),
-      ClickResult::kClickItem);
-  RecordNotificationSent();
+      event);
 }
 
 void RecordNotificationShown() {
@@ -87,11 +74,6 @@ void RecordNotificationTimedOut() {
                                 NotificationStatus::kTimedOut);
 }
 
-void RecordNotificationSent() {
-  base::UmaHistogramEnumeration("Sharing.SendTabToSelf.NotificationStatus",
-                                NotificationStatus::kSent);
-}
-
 void RecordNotificationDismissReasonUnknown() {
   base::UmaHistogramEnumeration("Sharing.SendTabToSelf.NotificationStatus",
                                 NotificationStatus::kDismissReasonUnknown);

diff --git a/components/send_tab_to_self/metrics_util.h b/components/send_tab_to_self/metrics_util.h
@@ -17,11 +17,23 @@ enum class ShareEntryPoint {
   kTabMenu,
 };
 
-// Records whether the user clicked to send a tab to a device.
-void RecordDeviceClicked(ShareEntryPoint entry_point);
+// State of the send tab to self option in the UI.
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused.
+// For historical reasons, this maps to SendTabToSelfClickResult in enums.xml.
+// A Java counterpart will be generated for this enum.
+// GENERATED_JAVA_ENUM_PACKAGE: (
+//   org.chromium.chrome.browser.share.send_tab_to_self)
+enum class SendingEvent {
+  // kShowItem = 0,
+  kClickItem = 1,
+  kShowDeviceList = 2,
+  kShowNoTargetDeviceMessage = 3,
+  kShowSigninPromo = 4,
+  kMaxValue = kShowSigninPromo,
+};
 
-// Records when a tab is sent to a device.
-void RecordNotificationSent();
+void RecordSendingEvent(ShareEntryPoint entry_point, SendingEvent event);
 
 // Records when a received STTS notification is shown.
 void RecordNotificationShown();

diff --git a/ios/chrome/browser/ui/send_tab_to_self/send_tab_to_self_coordinator.mm b/ios/chrome/browser/ui/send_tab_to_self/send_tab_to_self_coordinator.mm
@@ -253,8 +253,9 @@ - (void)dismissViewControllerAnimated {
 
 - (void)sendTabToTargetDeviceCacheGUID:(NSString*)cacheGUID
                       targetDeviceName:(NSString*)deviceName {
-  send_tab_to_self::RecordDeviceClicked(
-      send_tab_to_self::ShareEntryPoint::kShareMenu);
+  send_tab_to_self::RecordSendingEvent(
+      send_tab_to_self::ShareEntryPoint::kShareMenu,
+      send_tab_to_self::SendingEvent::kClickItem);
 
   SendTabToSelfSyncServiceFactory::GetForBrowserState(
       self.browser->GetBrowserState())
@@ -293,6 +294,14 @@ - (void)show {
   switch (*displayReason) {
     case send_tab_to_self::EntryPointDisplayReason::kInformNoTargetDevice:
     case send_tab_to_self::EntryPointDisplayReason::kOfferFeature: {
+      const auto sending_event =
+          *displayReason ==
+                  send_tab_to_self::EntryPointDisplayReason::kOfferFeature
+              ? send_tab_to_self::SendingEvent::kShowDeviceList
+              : send_tab_to_self::SendingEvent::kShowNoTargetDeviceMessage;
+      send_tab_to_self::RecordSendingEvent(
+          send_tab_to_self::ShareEntryPoint::kShareMenu, sending_event);
+
       ChromeBrowserState* browserState = self.browser->GetBrowserState();
       send_tab_to_self::SendTabToSelfSyncService* syncService =
           SendTabToSelfSyncServiceFactory::GetForBrowserState(browserState);
@@ -328,6 +337,10 @@ - (void)show {
       break;
     }
     case send_tab_to_self::EntryPointDisplayReason::kOfferSignIn: {
+      send_tab_to_self::RecordSendingEvent(
+          send_tab_to_self::ShareEntryPoint::kShareMenu,
+          send_tab_to_self::SendingEvent::kShowSigninPromo);
+
       __weak __typeof(self) weakSelf = self;
       ShowSigninCommandCompletionCallback callback = ^(BOOL succeeded) {
         [weakSelf onSigninComplete:succeeded];

diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
@@ -87193,17 +87193,17 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf
 </enum>
 
 <enum name="SendTabToSelfClickResult">
-  <int value="0" label="SendTabToSelf entry point is shown.">
+  <int value="0" label="(obsolete) SendTabToSelf entry point is shown.">
     <obsolete>
       Removed on 06/2020.
     </obsolete>
   </int>
   <int value="1" label="SendTabToSelf target device is clicked."/>
   <int value="2" label="SendTabToSelf device list is shown.">
-    <obsolete>
-      Removed on 06/2020.
-    </obsolete>
+    Removed on 06/2020 and brought back 09/2022.
   </int>
+  <int value="3" label="No target device message shown."/>
+  <int value="4" label="Sign-in promo shown."/>
 </enum>
 
 <enum name="SendTabToSelfNotification">
@@ -87226,7 +87226,11 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf
   <int value="1" label="Dismissed"/>
   <int value="2" label="Opened"/>
   <int value="3" label="TimedOut"/>
-  <int value="4" label="Sent"/>
+  <int value="4" label="(obsolete) Sent">
+    <obsolete>
+      Removed 09/2022.
+    </obsolete>
+  </int>
   <int value="5" label="DismissReasonUnknown"/>
 </enum>
 

diff --git a/tools/metrics/histograms/metadata/others/histograms.xml b/tools/metrics/histograms/metadata/others/histograms.xml
@@ -12090,11 +12090,9 @@ should be kept until we remove incident reporting. -->
   <summary>
     Records user flow for SendTabToSelf feature.
 
-    Only the &quot;SendTabToSelf target device is clicked&quot; bucket is
-    recorded today, causing sharing of a tab with the tapped device. Until M84
-    other buckets were recorded. Because each bucket represents a different
-    aspect of the flow, it's important to look at &quot;bucket count&quot;, not
-    &quot;bucket proportion&quot;.
+    Because each bucket represents a different aspect of the flow, it's
+    important to look at &quot;bucket count&quot;, not &quot;bucket
+    proportion&quot;.
 
     On iOS this was not recorded between M85 and M90, inclusive (see
     crbug.com/1204944).