[Android][Mfill] Add metrics recorder for AllPasswordsBottomSheet

This CL records user actions on the warning dialog and the bottom sheet
for the AllPasswordsBottomSheet.
Adds a test for recording histogram.

Bug: 1104132
Change-Id: I88afa34eb9d1562c138e4b967dca4da9d09e0b8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2429072
Commit-Queue: Reda Tawfik <redatawfik@google.com>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: Ioana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812926}

chrome/android/features/keyboard_accessory/internal/BUILD.gn

@@ -52,6 +52,7 @@ android_library("internal_java") {
     "java/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/AllPasswordsBottomSheetBridge.java",
     "java/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/AllPasswordsBottomSheetCoordinator.java",
     "java/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/AllPasswordsBottomSheetMediator.java",
+    "java/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/AllPasswordsBottomSheetMetricsRecorder.java",
     "java/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/AllPasswordsBottomSheetProperties.java",
     "java/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/AllPasswordsBottomSheetView.java",
     "java/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/AllPasswordsBottomSheetViewBinder.java",

chrome/android/features/keyboard_accessory/internal/java/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/AllPasswordsBottomSheetMediator.java

@@ -4,9 +4,15 @@
 
 package org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet;
 
+import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetMetricsRecorder.UMA_ALL_PASSWORDS_BOTTOM_SHEET_ACTIONS;
+import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetMetricsRecorder.UMA_WARNING_ACTIONS;
+import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetMetricsRecorder.recordHistogram;
 import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.SHEET_ITEMS;
 import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.VISIBLE;
 
+import org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetMetricsRecorder.AllPasswordsBottomSheetActions;
+import org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetMetricsRecorder.AllPasswordsWarningActions;
+import org.chromium.components.browser_ui.bottomsheet.BottomSheetController.StateChangeReason;
 import org.chromium.components.embedder_support.util.UrlUtilities;
 import org.chromium.ui.modaldialog.DialogDismissalCause;
 import org.chromium.ui.modaldialog.ModalDialogManager;
@@ -29,11 +35,14 @@ class AllPasswordsBottomSheetMediator implements ModalDialogProperties.Controlle
     private boolean mIsPasswordField;
     private ModalDialogManager mModalDialogManager;
     private PropertyModel mDialogModel;
+    private boolean mSearchUsed;
 
     @Override
     public void onClick(PropertyModel model, int buttonType) {
         int dismissalCause = DialogDismissalCause.NEGATIVE_BUTTON_CLICKED;
         if (buttonType == ModalDialogProperties.ButtonType.POSITIVE) {
+            recordHistogram(UMA_WARNING_ACTIONS, AllPasswordsWarningActions.WARNING_ACCEPTED,
+                    AllPasswordsWarningActions.COUNT);
             showCredentials();
             dismissalCause = DialogDismissalCause.POSITIVE_BUTTON_CLICKED;
         }
@@ -42,7 +51,9 @@ class AllPasswordsBottomSheetMediator implements ModalDialogProperties.Controlle
 
     @Override
     public void onDismiss(PropertyModel model, int dismissalCause) {
-        if (dismissalCause == DialogDismissalCause.NEGATIVE_BUTTON_CLICKED) {
+        if (dismissalCause != DialogDismissalCause.POSITIVE_BUTTON_CLICKED) {
+            recordHistogram(UMA_WARNING_ACTIONS, AllPasswordsWarningActions.WARNING_CANCELED,
+                    AllPasswordsWarningActions.COUNT);
             mDelegate.onDismissed();
         }
     }
@@ -81,6 +92,8 @@ class AllPasswordsBottomSheetMediator implements ModalDialogProperties.Controlle
         // Shows the warning dialog only if the user is about to fill a password field.
         if (mIsPasswordField) {
             mModalDialogManager.showDialog(mDialogModel, ModalDialogManager.ModalDialogType.APP);
+            recordHistogram(UMA_WARNING_ACTIONS, AllPasswordsWarningActions.WARNING_DIALOG_SHOWN,
+                    AllPasswordsWarningActions.COUNT);
         } else {
             showCredentials();
         }
@@ -96,6 +109,7 @@ class AllPasswordsBottomSheetMediator implements ModalDialogProperties.Controlle
      * @param newText the text used to filter the credentials.
      */
     void onQueryTextChange(String newText) {
+        mSearchUsed = true;
         ListModel<ListItem> sheetItems = mModel.get(SHEET_ITEMS);
         sheetItems.clear();
 
@@ -130,12 +144,23 @@ class AllPasswordsBottomSheetMediator implements ModalDialogProperties.Controlle
     }
 
     void onCredentialSelected(Credential credential) {
+        recordHistogram(UMA_ALL_PASSWORDS_BOTTOM_SHEET_ACTIONS,
+                AllPasswordsBottomSheetActions.CREDENTIAL_SELECTED,
+                AllPasswordsBottomSheetActions.COUNT);
+        if (mSearchUsed) {
+            recordHistogram(UMA_ALL_PASSWORDS_BOTTOM_SHEET_ACTIONS,
+                    AllPasswordsBottomSheetActions.SEARCH_USED,
+                    AllPasswordsBottomSheetActions.COUNT);
+        }
         mModel.set(VISIBLE, false);
         mDelegate.onCredentialSelected(credential);
     }
 
-    void onDismissed(Integer integer) {
+    void onDismissed(@StateChangeReason Integer reason) {
         if (!mModel.get(VISIBLE)) return; // Dismiss only if not dismissed yet.
+        recordHistogram(UMA_ALL_PASSWORDS_BOTTOM_SHEET_ACTIONS,
+                AllPasswordsBottomSheetActions.SHEET_DISMISSED,
+                AllPasswordsBottomSheetActions.COUNT);
         mModel.set(VISIBLE, false);
         mDelegate.onDismissed();
     }

chrome/android/features/keyboard_accessory/internal/java/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/AllPasswordsBottomSheetMetricsRecorder.java

+// Copyright 2020 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.
+
+package org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet;
+
+import androidx.annotation.IntDef;
+
+import org.chromium.base.metrics.RecordHistogram;
+
+/**
+ * This class provides helpers to record metrics related to the AllPasswordsBottomSheet.
+ */
+class AllPasswordsBottomSheetMetricsRecorder {
+    static final String UMA_ALL_PASSWORDS_BOTTOM_SHEET_ACTIONS =
+            "PasswordManager.AllPasswordsBottomSheet.UserAction";
+    static final String UMA_WARNING_ACTIONS =
+            "PasswordManager.AllPasswordsBottomSheet.WarningActions";
+
+    // Used to record metrics for the AllPasswordsBottomSheet actions. Entries should
+    // not be renumbered and numeric values should never be reused. Must be kept in
+    // sync with the enum in enums.xml.
+    @IntDef({AllPasswordsBottomSheetActions.CREDENTIAL_SELECTED,
+            AllPasswordsBottomSheetActions.SHEET_DISMISSED,
+            AllPasswordsBottomSheetActions.SEARCH_USED})
+    @interface AllPasswordsBottomSheetActions {
+        int CREDENTIAL_SELECTED = 0;
+        int SHEET_DISMISSED = 1;
+        int SEARCH_USED = 2;
+        int COUNT = 3;
+    }
+
+    // Used to record metrics for the AllPasswordsBottomSheet warning actions. Entries should
+    // not be renumbered and numeric values should never be reused. Must be kept in
+    // sync with the enum in enums.xml.
+    @IntDef({AllPasswordsWarningActions.WARNING_ACCEPTED,
+            AllPasswordsWarningActions.WARNING_CANCELED,
+            AllPasswordsWarningActions.WARNING_DIALOG_SHOWN})
+    @interface AllPasswordsWarningActions {
+        int WARNING_DIALOG_SHOWN = 0;
+        int WARNING_ACCEPTED = 1;
+        int WARNING_CANCELED = 2;
+        int COUNT = 3;
+    }
+
+    static void recordHistogram(String name, int action, int max) {
+        RecordHistogram.recordEnumeratedHistogram(name, action, max);
+    }
+}

chrome/android/features/keyboard_accessory/junit/src/org/chromium/chrome/browser/keyboard_accessory/all_passwords_bottom_sheet/AllPasswordsBottomSheetControllerTest.java

@@ -12,6 +12,11 @@ import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetMetricsRecorder.AllPasswordsBottomSheetActions.CREDENTIAL_SELECTED;
+import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetMetricsRecorder.AllPasswordsBottomSheetActions.SEARCH_USED;
+import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetMetricsRecorder.AllPasswordsBottomSheetActions.SHEET_DISMISSED;
+import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetMetricsRecorder.UMA_ALL_PASSWORDS_BOTTOM_SHEET_ACTIONS;
+import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetMetricsRecorder.UMA_WARNING_ACTIONS;
 import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.CredentialProperties.CREDENTIAL;
 import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.DISMISS_HANDLER;
 import static org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.SHEET_ITEMS;
@@ -26,14 +31,17 @@ import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.robolectric.annotation.Config;
 
+import org.chromium.base.metrics.test.ShadowRecordHistogram;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.base.test.util.JniMocker;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
+import org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetMetricsRecorder.AllPasswordsWarningActions;
 import org.chromium.chrome.browser.keyboard_accessory.all_passwords_bottom_sheet.AllPasswordsBottomSheetProperties.ItemType;
 import org.chromium.chrome.test.util.browser.Features;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
 import org.chromium.components.embedder_support.util.UrlUtilities;
 import org.chromium.components.embedder_support.util.UrlUtilitiesJni;
+import org.chromium.ui.modaldialog.DialogDismissalCause;
 import org.chromium.ui.modaldialog.ModalDialogManager;
 import org.chromium.ui.modaldialog.ModalDialogProperties;
 import org.chromium.ui.modaldialog.ModalDialogProperties.ButtonType;
@@ -45,7 +53,7 @@ import org.chromium.ui.modelutil.PropertyModel;
  * Controller tests for the all passwords bottom sheet.
  */
 @RunWith(BaseRobolectricTestRunner.class)
-@Config(manifest = Config.NONE)
+@Config(manifest = Config.NONE, shadows = {ShadowRecordHistogram.class})
 @Features.EnableFeatures(ChromeFeatureList.FILLING_PASSWORDS_FROM_ANY_ORIGIN)
 public class AllPasswordsBottomSheetControllerTest {
     private static final Credential ANA =
@@ -65,7 +73,6 @@ public class AllPasswordsBottomSheetControllerTest {
     private UrlUtilities.Natives mUrlUtilitiesJniMock;
     @Mock
     private AllPasswordsBottomSheetCoordinator.Delegate mMockDelegate;
-
     @Mock
     private ModalDialogManager mModalDialogManager;
 
@@ -75,6 +82,7 @@ public class AllPasswordsBottomSheetControllerTest {
 
     @Before
     public void setUp() {
+        ShadowRecordHistogram.reset();
         MockitoAnnotations.initMocks(this);
         mJniMocker.mock(UrlUtilitiesJni.TEST_HOOKS, mUrlUtilitiesJniMock);
         mMediator = new AllPasswordsBottomSheetMediator();
@@ -100,6 +108,9 @@ public class AllPasswordsBottomSheetControllerTest {
         mMediator.warnAndShow();
         verify(mModalDialogManager)
                 .showDialog(mModalDialogModel, ModalDialogManager.ModalDialogType.APP);
+        assertThat(ShadowRecordHistogram.getHistogramValueCountForTesting(
+                           UMA_WARNING_ACTIONS, AllPasswordsWarningActions.WARNING_DIALOG_SHOWN),
+                is(1));
     }
 
     @Test
@@ -107,13 +118,20 @@ public class AllPasswordsBottomSheetControllerTest {
         mMediator.setCredentials(TEST_CREDENTIALS, IS_PASSWORD_FIELD);
         mMediator.onClick(mModalDialogModel, ModalDialogProperties.ButtonType.POSITIVE);
         assertThat(mModel.get(VISIBLE), is(true));
+        assertThat(ShadowRecordHistogram.getHistogramValueCountForTesting(
+                           UMA_WARNING_ACTIONS, AllPasswordsWarningActions.WARNING_ACCEPTED),
+                is(1));
     }
 
     @Test
     public void testBottomSheetNotShowingIfWarningDismissed() {
         mMediator.setCredentials(TEST_CREDENTIALS, IS_PASSWORD_FIELD);
         mMediator.onClick(mModalDialogModel, ButtonType.NEGATIVE);
+        mMediator.onDismiss(mModalDialogModel, DialogDismissalCause.NEGATIVE_BUTTON_CLICKED);
         assertThat(mModel.get(VISIBLE), is(false));
+        assertThat(ShadowRecordHistogram.getHistogramValueCountForTesting(
+                           UMA_WARNING_ACTIONS, AllPasswordsWarningActions.WARNING_CANCELED),
+                is(1));
     }
 
     @Test
@@ -139,6 +157,20 @@ public class AllPasswordsBottomSheetControllerTest {
         mMediator.onCredentialSelected(TEST_CREDENTIALS[1]);
         assertThat(mModel.get(VISIBLE), is(false));
         verify(mMockDelegate).onCredentialSelected(TEST_CREDENTIALS[1]);
+        assertThat(ShadowRecordHistogram.getHistogramValueCountForTesting(
+                           UMA_ALL_PASSWORDS_BOTTOM_SHEET_ACTIONS, CREDENTIAL_SELECTED),
+                is(1));
+    }
+
+    @Test
+    public void testRecordingSearchMetrics() {
+        mMediator.setCredentials(TEST_CREDENTIALS, IS_PASSWORD_FIELD);
+        mMediator.onClick(mModalDialogModel, ModalDialogProperties.ButtonType.POSITIVE);
+        mMediator.onQueryTextChange("Bob");
+        mMediator.onCredentialSelected(TEST_CREDENTIALS[1]);
+        assertThat(ShadowRecordHistogram.getHistogramValueCountForTesting(
+                           UMA_ALL_PASSWORDS_BOTTOM_SHEET_ACTIONS, SEARCH_USED),
+                is(1));
     }
 
     @Test
@@ -148,6 +180,9 @@ public class AllPasswordsBottomSheetControllerTest {
         mMediator.onDismissed(BottomSheetController.StateChangeReason.BACK_PRESS);
         assertThat(mModel.get(VISIBLE), is(false));
         verify(mMockDelegate).onDismissed();
+        assertThat(ShadowRecordHistogram.getHistogramValueCountForTesting(
+                           UMA_ALL_PASSWORDS_BOTTOM_SHEET_ACTIONS, SHEET_DISMISSED),
+                is(1));
     }
 
     @Test

tools/metrics/histograms/enums.xml

@@ -1262,6 +1262,18 @@ Unknown properties are collapsed to zero. -->
   <int value="5" label="InferenceExecutionFailed"/>
 </enum>
 
+<enum name="AllPasswordsBottomSheetActions">
+  <int value="0" label="Credential selected"/>
+  <int value="1" label="Sheet dismissed"/>
+  <int value="2" label="Search is used before selecting a credential"/>
+</enum>
+
+<enum name="AllPasswordsWarningActions">
+  <int value="0" label="Warning shown"/>
+  <int value="1" label="Warning accepted"/>
+  <int value="2" label="Warning canceled"/>
+</enum>
+
 <enum name="AlsaSampleFormatType">
   <int value="-1" label="SND_PCM_FORMAT_UNKNOWN"/>
   <int value="0" label="SND_PCM_FORMAT_S8"/>

tools/metrics/histograms/histograms_xml/password/histograms.xml

@@ -518,6 +518,25 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="PasswordManager.AllPasswordsBottomSheet.UserAction"
+    enum="AllPasswordsBottomSheetActions" expires_after="M90">
+  <owner>redatawfik@google.com</owner>
+  <owner>fhorschig@chromium.org</owner>
+  <summary>
+    Android only. Records the user action on the all passwords bottom sheet.
+  </summary>
+</histogram>
+
+<histogram name="PasswordManager.AllPasswordsBottomSheet.WarningActions"
+    enum="AllPasswordsWarningActions" expires_after="M90">
+  <owner>redatawfik@google.com</owner>
+  <owner>fhorschig@chromium.org</owner>
+  <summary>
+    Android only. Records the number of times the all passwords warning dialog
+    is shown and user action on it.
+  </summary>
+</histogram>
+
 <histogram name="PasswordManager.Android.PasswordCredentialEntry"
     enum="PasswordManagerAndroidPasswordEntryActions" expires_after="M88">
   <owner>fhorschig@chromium.org</owner>