[signin] Add a histogram for user actions in reauth dialog

This CL adds a new Signin.TransactionalReauthUserAction* histogram.

The histogram records what actions the user performed in the reauth
dialog. It will be useful to understand how the user interacts
with this new UI.

The histogram is also split by an access point to see whether the dialog
perceived differently depending on context.

Bug: 1045515
Change-Id: Ie5d7b65153395e9739c89e9defcd15b9b4c32d51
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2264368
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782362}

chrome/browser/signin/signin_ui_util.cc

@@ -11,6 +11,8 @@
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
+#include "base/notreached.h"
+#include "base/strings/strcat.h"
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
@@ -28,6 +30,7 @@
 #include "chrome/browser/ui/ui_features.h"
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_service.h"
+#include "components/signin/public/base/signin_metrics.h"
 #include "components/signin/public/base/signin_pref_names.h"
 #include "components/signin/public/identity_manager/account_info.h"
 #include "components/signin/public/identity_manager/accounts_in_cookie_jar_info.h"
@@ -136,6 +139,26 @@ void CreateDiceTurnSyncOnHelper(
 }
 #endif  // BUILDFLAG(ENABLE_DICE_SUPPORT)
 
+std::string GetReauthAccessPointHistogramSuffix(
+    signin_metrics::ReauthAccessPoint access_point) {
+  switch (access_point) {
+    case signin_metrics::ReauthAccessPoint::kUnknown:
+      NOTREACHED();
+      return std::string();
+    case signin_metrics::ReauthAccessPoint::kAutofillDropdown:
+      return "ToFillPassword";
+    case signin_metrics::ReauthAccessPoint::kPasswordSaveBubble:
+      return "ToSaveOrUpdatePassword";
+    case signin_metrics::ReauthAccessPoint::kPasswordSettings:
+      return "ToManageInSettings";
+    case signin_metrics::ReauthAccessPoint::kGeneratePasswordDropdown:
+    case signin_metrics::ReauthAccessPoint::kGeneratePasswordContextMenu:
+      return "ToGeneratePassword";
+    case signin_metrics::ReauthAccessPoint::kPasswordMoveBubble:
+      return "ToMovePassword";
+  }
+}
+
 }  // namespace
 
 namespace signin_ui_util {
@@ -421,31 +444,30 @@ void RecordProfileMenuClick(Profile* profile) {
 void RecordTransactionalReauthResult(
     signin_metrics::ReauthAccessPoint access_point,
     signin::ReauthResult result) {
-  base::UmaHistogramEnumeration("Signin.TransactionalReauthResult", result);
-  switch (access_point) {
-    case signin_metrics::ReauthAccessPoint::kAutofillDropdown:
-      base::UmaHistogramEnumeration(
-          "Signin.TransactionalReauthResult.ToFillPassword", result);
-      break;
-    case signin_metrics::ReauthAccessPoint::kPasswordSaveBubble:
-      base::UmaHistogramEnumeration(
-          "Signin.TransactionalReauthResult.ToSaveOrUpdatePassword", result);
-      break;
-    case signin_metrics::ReauthAccessPoint::kPasswordSettings:
-      base::UmaHistogramEnumeration(
-          "Signin.TransactionalReauthResult.ToManageInSettings", result);
-      break;
-    case signin_metrics::ReauthAccessPoint::kGeneratePasswordDropdown:
-    case signin_metrics::ReauthAccessPoint::kGeneratePasswordContextMenu:
-      base::UmaHistogramEnumeration(
-          "Signin.TransactionalReauthResult.ToGeneratePassword", result);
-      break;
-    case signin_metrics::ReauthAccessPoint::kPasswordMoveBubble:
-      base::UmaHistogramEnumeration(
-          "Signin.TransactionalReauthResult.ToMovePassword", result);
-      break;
-    default:
-      NOTREACHED();
+  const char kHistogramName[] = "Signin.TransactionalReauthResult";
+  base::UmaHistogramEnumeration(kHistogramName, result);
+
+  std::string access_point_suffix =
+      GetReauthAccessPointHistogramSuffix(access_point);
+  if (!access_point_suffix.empty()) {
+    std::string suffixed_histogram_name =
+        base::StrCat({kHistogramName, ".", access_point_suffix});
+    base::UmaHistogramEnumeration(suffixed_histogram_name, result);
+  }
+}
+
+void RecordTransactionalReauthUserAction(
+    signin_metrics::ReauthAccessPoint access_point,
+    SigninReauthViewController::UserAction user_action) {
+  const char kHistogramName[] = "Signin.TransactionalReauthUserAction";
+  base::UmaHistogramEnumeration(kHistogramName, user_action);
+
+  std::string access_point_suffix =
+      GetReauthAccessPointHistogramSuffix(access_point);
+  if (!access_point_suffix.empty()) {
+    std::string suffixed_histogram_name =
+        base::StrCat({kHistogramName, ".", access_point_suffix});
+    base::UmaHistogramEnumeration(suffixed_histogram_name, user_action);
   }
 }
 

chrome/browser/signin/signin_ui_util.h

@@ -12,6 +12,7 @@
 #include "base/strings/string16.h"
 #include "build/buildflag.h"
 #include "chrome/browser/signin/reauth_result.h"
+#include "chrome/browser/ui/signin_reauth_view_controller.h"
 #include "chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.h"
 #include "components/signin/public/base/signin_buildflags.h"
 #include "components/signin/public/base/signin_metrics.h"
@@ -126,6 +127,11 @@ void RecordTransactionalReauthResult(
     signin_metrics::ReauthAccessPoint access_point,
     signin::ReauthResult result);
 
+// Records user action performed in a transactional reauth dialog/tab.
+void RecordTransactionalReauthUserAction(
+    signin_metrics::ReauthAccessPoint access_point,
+    SigninReauthViewController::UserAction user_action);
+
 }  // namespace signin_ui_util
 
 #endif  // CHROME_BROWSER_SIGNIN_SIGNIN_UI_UTIL_H_

chrome/browser/ui/signin_reauth_view_controller.cc

@@ -9,6 +9,7 @@
 
 #include "base/feature_list.h"
 #include "base/notreached.h"
+#include "base/optional.h"
 #include "base/task_runner.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/reauth_result.h"
@@ -115,6 +116,14 @@ void SigninReauthViewController::SetWebContents(
 void SigninReauthViewController::OnModalSigninClosed() {
   dialog_delegate_observer_.Remove(dialog_delegate_);
   dialog_delegate_ = nullptr;
+
+  DCHECK(ui_state == UIState::kConfirmationDialog ||
+         ui_state == UIState::kGaiaReauthDialog);
+  UserAction action = ui_state == UIState::kConfirmationDialog
+                          ? UserAction::kCloseConfirmationDialog
+                          : UserAction::kCloseGaiaReauthDialog;
+  signin_ui_util::RecordTransactionalReauthUserAction(access_point_, action);
+
   CompleteReauth(signin::ReauthResult::kDismissedByUser);
 }
 
@@ -127,6 +136,7 @@ void SigninReauthViewController::OnReauthConfirmed() {
 }
 
 void SigninReauthViewController::OnReauthDismissed() {
+  RecordClickOnce(UserAction::kClickCancelButton);
   CompleteReauth(signin::ReauthResult::kDismissedByUser);
 }
 
@@ -145,6 +155,25 @@ void SigninReauthViewController::OnGaiaReauthPageComplete(
   DCHECK(!gaia_reauth_page_result_);
   gaia_reauth_page_state_ = GaiaReauthPageState::kDone;
   gaia_reauth_page_result_ = result;
+
+  if (ui_state == UIState::kGaiaReauthDialog ||
+      ui_state == UIState::kGaiaReauthTab) {
+    base::Optional<UserAction> action;
+    if (gaia_reauth_page_result_ == signin::ReauthResult::kSuccess) {
+      action = UserAction::kPassGaiaReauth;
+    }
+    if (gaia_reauth_page_result_ == signin::ReauthResult::kDismissedByUser) {
+      action = ui_state == UIState::kGaiaReauthDialog
+                   ? UserAction::kCloseGaiaReauthDialog
+                   : UserAction::kCloseGaiaReauthTab;
+    }
+
+    if (action) {
+      signin_ui_util::RecordTransactionalReauthUserAction(access_point_,
+                                                          *action);
+    }
+  }
+
   OnStateChanged();
 }
 
@@ -181,19 +210,32 @@ void SigninReauthViewController::CompleteReauth(signin::ReauthResult result) {
 void SigninReauthViewController::OnStateChanged() {
   if (user_confirmed_reauth_ &&
       gaia_reauth_page_state_ == GaiaReauthPageState::kNavigated) {
+    RecordClickOnce(UserAction::kClickNextButton);
     ShowGaiaReauthPage();
     return;
   }
 
   if (user_confirmed_reauth_ &&
       gaia_reauth_page_state_ == GaiaReauthPageState::kDone) {
     DCHECK(gaia_reauth_page_result_);
+    RecordClickOnce(UserAction::kClickConfirmButton);
     CompleteReauth(*gaia_reauth_page_result_);
     return;
   }
 }
 
+void SigninReauthViewController::RecordClickOnce(UserAction click_action) {
+  if (has_recorded_click)
+    return;
+
+  signin_ui_util::RecordTransactionalReauthUserAction(access_point_,
+                                                      click_action);
+  has_recorded_click = true;
+}
+
 void SigninReauthViewController::ShowReauthConfirmationDialog() {
+  DCHECK_EQ(ui_state, UIState::kNone);
+  ui_state = UIState::kConfirmationDialog;
   dialog_delegate_ =
       SigninViewControllerDelegate::CreateReauthConfirmationDelegate(
           browser_, account_id_, access_point_);
@@ -217,10 +259,14 @@ void SigninReauthViewController::ShowGaiaReauthPage() {
 }
 
 void SigninReauthViewController::ShowGaiaReauthPageInDialog() {
+  DCHECK_EQ(ui_state, UIState::kConfirmationDialog);
+  ui_state = UIState::kGaiaReauthDialog;
   dialog_delegate_->SetWebContents(reauth_web_contents_.get());
 }
 
 void SigninReauthViewController::ShowGaiaReauthPageInNewTab() {
+  DCHECK_EQ(ui_state, UIState::kConfirmationDialog);
+  ui_state = UIState::kGaiaReauthTab;
   // Remove the observer to not trigger OnModalSigninClosed() that will abort
   // the reauth flow.
   dialog_delegate_observer_.Remove(dialog_delegate_);

chrome/browser/ui/signin_reauth_view_controller.h

@@ -51,9 +51,46 @@ class SigninReauthViewController
   };
 
   enum class GaiaReauthPageState {
-    kStarted = 0,    // The Gaia Reauth page is loading in background.
-    kNavigated = 1,  // The first navigation has been committed.
-    kDone = 2  // The reauth has been completed and the result is available.
+    // The Gaia Reauth page is loading in background.
+    kStarted = 0,
+    // The first navigation has been committed in background.
+    kNavigated = 1,
+    // The reauth has been completed and the result is available.
+    kDone = 2
+  };
+
+  enum class UIState {
+    // Nothing is being displayed.
+    kNone = 0,
+    // The Reauth confirmation webUI page is being displayed in a modal dialog.
+    kConfirmationDialog = 1,
+    // The Gaia Reauth page is being displayed in a modal dialog.
+    kGaiaReauthDialog = 2,
+    // The Gaia Reauth page is being displayed in a tab.
+    kGaiaReauthTab = 3
+  };
+
+  // These values are persisted to logs. Entries should not be renumbered and
+  // numeric values should never be reused.
+  enum class UserAction {
+    // The user clicked on the confirm button in the Reauth confirmation dialog.
+    // The Gaia Reauth was auto-approved and did not show up as a next step.
+    kClickConfirmButton = 0,
+    // The user clicked on the next button in the Reauth confirmation dialog.
+    // The Gaia Reauth showed up as a next step.
+    kClickNextButton = 1,
+    // The user clicked on the cancel button in the Reauth confirmation dialog.
+    kClickCancelButton = 2,
+    // The user closed the Reauth confirmation dialog without clicking on the
+    // cancel button.
+    kCloseConfirmationDialog = 3,
+    // The user closed the Gaia Reauth page displayed in a dialog.
+    kCloseGaiaReauthDialog = 4,
+    // The user closed the Gaia Reauth page displayed in a tab.
+    kCloseGaiaReauthTab = 5,
+    // The user successfully authenticated on the Gaia Reauth page.
+    kPassGaiaReauth = 6,
+    kMaxValue = kPassGaiaReauth
   };
 
   SigninReauthViewController(
@@ -98,19 +135,27 @@ class SigninReauthViewController
   // Calls |reauth_callback_| with |result| and closes all Reauth UIs.
   void CompleteReauth(signin::ReauthResult result);
 
-  // Notifies about a change in the reauth flow state.
+  // Notifies about a change in the reauth flow state. Must be called whenever
+  // |user_confirmed_reauth_| or |gaia_reauth_page_state_| has changed.
   void OnStateChanged();
 
+  void RecordClickOnce(UserAction click_action);
+
   void ShowReauthConfirmationDialog();
   void ShowGaiaReauthPage();
   void ShowGaiaReauthPageInDialog();
   void ShowGaiaReauthPageInNewTab();
 
+  // Controller inputs.
   Browser* const browser_;
   const CoreAccountId account_id_;
   const signin_metrics::ReauthAccessPoint access_point_;
   base::OnceCallback<void(signin::ReauthResult)> reauth_callback_;
 
+  // Dialog state useful for recording metrics.
+  UIState ui_state = UIState::kNone;
+  bool has_recorded_click = false;
+
   // Delegate displaying the dialog.
   SigninViewControllerDelegate* dialog_delegate_ = nullptr;
   ScopedObserver<SigninViewControllerDelegate,