[Merge 105] [FedCM] Add metric for reason account picker was dismissed

This CL adds a UMA histogram which tracks the reason that the FedCM
account picker was dismissed.

BUG=1343857

(cherry picked from commit 48eaa14)

Change-Id: Ia2f1d27d3c811149b8d1ad98cf8ac2f740191c70
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3774541
Reviewed-by: Yi Gu <yigu@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1027117}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3788737
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Gary Kacmarcik <garykac@chromium.org>
Cr-Commit-Position: refs/branch-heads/5195@{#75}
Cr-Branched-From: 7aa3f07-refs/heads/main@{#1027018}

Author: pkotwicz

chrome/browser/ui/android/webid/BUILD.gn

@@ -13,6 +13,8 @@ android_library("public_java") {
     "//url:gurl_java",
   ]
 
+  srcjar_deps = [ ":java_enums_srcjar" ]
+
   sources = [
     "java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionComponent.java",
     "java/src/org/chromium/chrome/browser/ui/android/webid/data/Account.java",
@@ -29,3 +31,7 @@ generate_jni("jni_headers") {
     "java/src/org/chromium/chrome/browser/ui/android/webid/data/IdentityProviderMetadata.java",
   ]
 }
+
+java_cpp_enum("java_enums_srcjar") {
+  sources = [ "//content/public/browser/identity_request_dialog_controller.h" ]
+}

chrome/browser/ui/android/webid/account_selection_view_android.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/ui/android/webid/jni_headers/ClientIdMetadata_jni.h"
 #include "chrome/browser/ui/android/webid/jni_headers/IdentityProviderMetadata_jni.h"
 #include "chrome/browser/ui/webid/account_selection_view.h"
+#include "content/public/browser/identity_request_dialog_controller.h"
 #include "ui/android/color_utils_android.h"
 #include "ui/android/view_android.h"
 #include "ui/android/window_android.h"
@@ -28,6 +29,7 @@ using base::android::ConvertJavaStringToUTF8;
 using base::android::ConvertUTF8ToJavaString;
 using base::android::JavaParamRef;
 using base::android::ScopedJavaLocalRef;
+using DismissReason = content::IdentityRequestDialogController::DismissReason;
 
 namespace {
 
@@ -123,7 +125,7 @@ void AccountSelectionViewAndroid::Show(
     // It's possible that the constructor cannot access the bottom sheet clank
     // component. That case may be temporary but we can't let users in a
     // waiting state so report that AccountSelectionView is dismissed instead.
-    delegate_->OnDismiss(/* should_embargo=*/false);
+    delegate_->OnDismiss(DismissReason::OTHER);
     return;
   }
 
@@ -152,13 +154,13 @@ void AccountSelectionViewAndroid::OnAccountSelected(
       env, account_string_fields, account_picture_url, is_sign_in));
 }
 
-void AccountSelectionViewAndroid::OnDismiss(JNIEnv* env, bool should_embargo) {
-  delegate_->OnDismiss(should_embargo);
+void AccountSelectionViewAndroid::OnDismiss(JNIEnv* env, jint dismiss_reason) {
+  delegate_->OnDismiss(static_cast<DismissReason>(dismiss_reason));
 }
 
 void AccountSelectionViewAndroid::OnAutoSignInCancelled(JNIEnv* env) {
   // TODO(yigu): Alternatively we could fall back to manual sign in flow.
-  delegate_->OnDismiss(/*should_embargo=*/false);
+  delegate_->OnDismiss(DismissReason::OTHER);
 }
 
 bool AccountSelectionViewAndroid::RecreateJavaObject() {

chrome/browser/ui/android/webid/account_selection_view_android.h

@@ -32,7 +32,7 @@ class AccountSelectionViewAndroid : public AccountSelectionView {
       const base::android::JavaParamRef<jobjectArray>& account_string_fields,
       const base::android::JavaParamRef<jobject>& account_picture_url,
       bool is_sign_in);
-  void OnDismiss(JNIEnv* env, bool should_embargo);
+  void OnDismiss(JNIEnv* env, jint dismiss_reason);
   void OnAutoSignInCancelled(JNIEnv* env);
 
  private:

chrome/browser/ui/android/webid/internal/java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionBridge.java

@@ -16,6 +16,7 @@
 import org.chromium.chrome.browser.ui.android.webid.data.IdentityProviderMetadata;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetControllerProvider;
+import org.chromium.content.webid.IdentityRequestDialogDismissReason;
 import org.chromium.ui.base.WindowAndroid;
 import org.chromium.url.GURL;
 
@@ -91,9 +92,9 @@ private void showAccounts(String rpForDisplay, String idpForDisplay, Account[] a
     }
 
     @Override
-    public void onDismissed(boolean shouldEmbargo) {
+    public void onDismissed(@IdentityRequestDialogDismissReason int dismissReason) {
         if (mNativeView != 0) {
-            AccountSelectionBridgeJni.get().onDismiss(mNativeView, shouldEmbargo);
+            AccountSelectionBridgeJni.get().onDismiss(mNativeView, dismissReason);
         }
     }
 
@@ -122,7 +123,8 @@ public void onAutoSignInCancelled() {
     interface Natives {
         void onAccountSelected(long nativeAccountSelectionViewAndroid, String[] accountFields,
                 GURL accountPictureUrl, boolean isSignedIn);
-        void onDismiss(long nativeAccountSelectionViewAndroid, boolean shouldEmbargo);
+        void onDismiss(long nativeAccountSelectionViewAndroid,
+                @IdentityRequestDialogDismissReason int dismissReason);
         void onAutoSignInCancelled(long nativeAccountSelectionViewAndroid);
     }
 }

chrome/browser/ui/android/webid/internal/java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionControllerTest.java

@@ -59,6 +59,7 @@
 import org.chromium.chrome.browser.ui.android.webid.data.IdentityProviderMetadata;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
 import org.chromium.components.image_fetcher.ImageFetcher;
+import org.chromium.content.webid.IdentityRequestDialogDismissReason;
 import org.chromium.ui.modelutil.MVCListAdapter.ListItem;
 import org.chromium.ui.modelutil.MVCListAdapter.ModelList;
 import org.chromium.ui.modelutil.PropertyKey;
@@ -341,7 +342,7 @@ public void testCallsDelegateAndHidesOnSingleAccountDismiss() {
         mMediator.showAccounts(TEST_ETLD_PLUS_ONE, TEST_ETLD_PLUS_ONE_1, Arrays.asList(ANA),
                 IDP_METADATA, CLIENT_ID_METADATA, false);
         pressBack();
-        verify(mMockDelegate).onDismissed(/*shouldEmbargo=*/false);
+        verify(mMockDelegate).onDismissed(IdentityRequestDialogDismissReason.OTHER);
         assertTrue(mMediator.wasDismissed());
     }
 
@@ -351,7 +352,7 @@ public void testCallsDelegateAndHidesOnAccountPickerDismiss() {
         mMediator.showAccounts(TEST_ETLD_PLUS_ONE, TEST_ETLD_PLUS_ONE_1, Arrays.asList(ANA, BOB),
                 IDP_METADATA, CLIENT_ID_METADATA, false);
         pressBack();
-        verify(mMockDelegate).onDismissed(/*shouldEmbargo=*/false);
+        verify(mMockDelegate).onDismissed(IdentityRequestDialogDismissReason.OTHER);
         assertTrue(mMediator.wasDismissed());
     }
 
@@ -444,7 +445,7 @@ public void testCallsDelegateAndHidesOnlyOnceWithAutoSignIn() {
         mMediator.showAccounts(TEST_ETLD_PLUS_ONE, TEST_ETLD_PLUS_ONE_1, Arrays.asList(ANA),
                 IDP_METADATA, CLIENT_ID_METADATA, true);
         pressBack();
-        verify(mMockDelegate).onDismissed(/*shouldEmbargo=*/false);
+        verify(mMockDelegate).onDismissed(IdentityRequestDialogDismissReason.OTHER);
         verifyNoMoreInteractions(mMockDelegate);
         assertTrue(mMediator.wasDismissed());
         // The delayed task should not call delegate after user dismissing.
@@ -486,7 +487,7 @@ public void testShowVerifySheet() {
     private void pressBack() {
         if (mBottomSheetContent.handleBackPress()) return;
 
-        mMediator.onDismissed(/*shouldEmbargo=*/false);
+        mMediator.onDismissed(IdentityRequestDialogDismissReason.OTHER);
     }
 
     private int countAllItems() {

chrome/browser/ui/android/webid/internal/java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionIntegrationTest.java

@@ -58,6 +58,7 @@
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetController.SheetState;
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetControllerProvider;
+import org.chromium.content.webid.IdentityRequestDialogDismissReason;
 import org.chromium.url.GURL;
 import org.chromium.url.JUnitTestGURLs;
 
@@ -143,7 +144,7 @@ public void testBackDismissesAndCallsCallback() {
 
         Espresso.pressBack();
 
-        waitForEvent(mMockBridge).onDismissed(/*shouldEmbargo=*/false);
+        waitForEvent(mMockBridge).onDismissed(IdentityRequestDialogDismissReason.OTHER);
         verify(mMockBridge, never()).onAccountSelected(any());
     }
 
@@ -209,7 +210,7 @@ public void testDismissedIfUnableToShow() throws Exception {
             mAccountSelection.showAccounts(EXAMPLE_ETLD_PLUS_ONE, TEST_ETLD_PLUS_ONE_1,
                     Arrays.asList(ANA, BOB), IDP_METADATA, mClientIdMetadata, false);
         });
-        waitForEvent(mMockBridge).onDismissed(/*shouldEmbargo=*/false);
+        waitForEvent(mMockBridge).onDismissed(IdentityRequestDialogDismissReason.OTHER);
         verify(mMockBridge, never()).onAccountSelected(any());
         Espresso.onView(withText("Another bottom sheet content")).check(matches(isDisplayed()));
 

chrome/browser/ui/android/webid/internal/java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionMediator.java

@@ -27,6 +27,7 @@
 import org.chromium.components.browser_ui.bottomsheet.BottomSheetObserver;
 import org.chromium.components.browser_ui.bottomsheet.EmptyBottomSheetObserver;
 import org.chromium.components.image_fetcher.ImageFetcher;
+import org.chromium.content.webid.IdentityRequestDialogDismissReason;
 import org.chromium.ui.KeyboardVisibilityDelegate;
 import org.chromium.ui.KeyboardVisibilityDelegate.KeyboardVisibilityListener;
 import org.chromium.ui.modelutil.MVCListAdapter.ListItem;
@@ -77,7 +78,7 @@ class AccountSelectionMediator {
                 @Override
                 public void keyboardVisibilityChanged(boolean isShowing) {
                     if (isShowing) {
-                        onDismissed(/*shouldEmbargo=*/false);
+                        onDismissed(IdentityRequestDialogDismissReason.VIRTUAL_KEYBOARD_SHOWN);
                     }
                 }
             };
@@ -106,10 +107,11 @@ public void onSheetClosed(@BottomSheetController.StateChangeReason int reason) {
 
                 if (mWasDismissed) return;
 
-                // Dismissing the FedCM bottom sheet via {@link StateChangeReason#SWIPE} is more
-                // intentional than other methods such as {@link StateChangeReason#OMNIBOX_FOCUS}.
-                onDismissed(/*shouldEmbargo=*/(
-                        reason == BottomSheetController.StateChangeReason.SWIPE));
+                @IdentityRequestDialogDismissReason
+                int dismissReason = (reason == BottomSheetController.StateChangeReason.SWIPE)
+                        ? IdentityRequestDialogDismissReason.SWIPE
+                        : IdentityRequestDialogDismissReason.OTHER;
+                onDismissed(dismissReason);
             }
         };
     }
@@ -130,7 +132,7 @@ private void handleBackPress() {
     private PropertyModel createHeaderItem(HeaderType headerType, String rpForDisplay,
             String idpForDisplay, IdentityProviderMetadata idpMetadata) {
         Runnable closeOnClickRunnable = () -> {
-            onDismissed(/*shouldEmbargo=*/true);
+            onDismissed(IdentityRequestDialogDismissReason.CLOSE_BUTTON);
 
             RecordHistogram.recordBooleanHistogram(
                     "Blink.FedCm.CloseVerifySheet.Android", mHeaderType == HeaderType.VERIFY);
@@ -273,7 +275,7 @@ private void showContent() {
             KeyboardVisibilityDelegate.getInstance().addKeyboardVisibilityListener(
                     mKeyboardVisibilityListener);
         } else {
-            onDismissed(/*shouldEmbargo=*/false);
+            onDismissed(IdentityRequestDialogDismissReason.OTHER);
         }
     }
 
@@ -329,9 +331,9 @@ void onAccountSelected(Account selectedAccount) {
         updateBackPressBehavior();
     }
 
-    void onDismissed(boolean shouldEmbargo) {
+    void onDismissed(@IdentityRequestDialogDismissReason int dismissReason) {
         hideContent();
-        mDelegate.onDismissed(shouldEmbargo);
+        mDelegate.onDismissed(dismissReason);
     }
 
     void onAutoSignInCancelled() {

chrome/browser/ui/android/webid/java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionComponent.java

@@ -7,6 +7,7 @@
 import org.chromium.chrome.browser.ui.android.webid.data.Account;
 import org.chromium.chrome.browser.ui.android.webid.data.ClientIdMetadata;
 import org.chromium.chrome.browser.ui.android.webid.data.IdentityProviderMetadata;
+import org.chromium.content.webid.IdentityRequestDialogDismissReason;
 
 import java.util.List;
 
@@ -30,7 +31,7 @@ interface Delegate {
          * Called when the user dismisses the AccountSelectionComponent. Not called if a suggestion
          * was selected.
          */
-        void onDismissed(boolean shouldEmbargo);
+        void onDismissed(@IdentityRequestDialogDismissReason int dismissReason);
 
         /**
          * Called when the user cancels auto sign in.

chrome/browser/ui/views/webid/fake_delegate.h

@@ -18,7 +18,8 @@ class FakeDelegate : public AccountSelectionView::Delegate {
 
   void OnAccountSelected(const Account& account) override {}
 
-  void OnDismiss(bool should_embargo) override {}
+  void OnDismiss(content::IdentityRequestDialogController::DismissReason
+                     dismiss_reason) override {}
 
   // AccountSelectionView::Delegate
   gfx::NativeView GetNativeView() override;

chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop.cc

@@ -11,6 +11,8 @@
 #include "chrome/browser/ui/views/frame/top_container_view.h"
 #include "ui/views/bubble/bubble_dialog_delegate_view.h"
 
+using DismissReason = content::IdentityRequestDialogController::DismissReason;
+
 // static
 std::unique_ptr<AccountSelectionView> AccountSelectionView::Create(
     AccountSelectionView::Delegate* delegate) {
@@ -106,9 +108,12 @@ void FedCmAccountSelectionView::OnTabStripModelChanged(
 }
 
 void FedCmAccountSelectionView::OnWidgetDestroying(views::Widget* widget) {
-  bool should_embargo = (bubble_widget_->closed_reason() ==
-                         views::Widget::ClosedReason::kCloseButtonClicked);
-  OnDismiss(should_embargo);
+  DismissReason dismiss_reason =
+      (bubble_widget_->closed_reason() ==
+       views::Widget::ClosedReason::kCloseButtonClicked)
+          ? DismissReason::CLOSE_BUTTON
+          : DismissReason::OTHER;
+  OnDismiss(dismiss_reason);
 }
 
 void FedCmAccountSelectionView::OnAccountSelected(
@@ -122,16 +127,16 @@ void FedCmAccountSelectionView::Close() {
     return;
 
   bubble_widget_->Close();
-  OnDismiss(/*should_embargo=*/false);
+  OnDismiss(DismissReason::OTHER);
 }
 
-void FedCmAccountSelectionView::OnDismiss(bool should_embargo) {
+void FedCmAccountSelectionView::OnDismiss(DismissReason dismiss_reason) {
   if (!bubble_widget_)
     return;
 
   bubble_widget_->RemoveObserver(this);
   bubble_widget_.reset();
 
   if (notify_delegate_of_dismiss_)
-    delegate_->OnDismiss(should_embargo);
+    delegate_->OnDismiss(dismiss_reason);
 }

chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop.h

@@ -58,10 +58,10 @@ class FedCmAccountSelectionView : public AccountSelectionView,
   // Closes the widget and notifies the delegate.
   void Close();
 
-  // Notify the delegate that the widget was closed.
-  // |should_embargo| indicates whether the FedCM API should be embargoed due
-  // to the user explicitly dismissing the dialog.
-  void OnDismiss(bool should_embargo);
+  // Notify the delegate that the widget was closed with reason
+  // `dismiss_reason`.
+  void OnDismiss(
+      content::IdentityRequestDialogController::DismissReason dismiss_reason);
 
   // Whether to notify the delegate when the widget is closed.
   bool notify_delegate_of_dismiss_{true};

chrome/browser/ui/webid/account_selection_view.h

@@ -28,10 +28,11 @@ class AccountSelectionView {
     virtual ~Delegate() = default;
     // Informs the controller that the user has made a selection.
     virtual void OnAccountSelected(const Account& account) = 0;
-    // Informs the controller that the user has dismissed the sheet.
-    // |should_embargo| indicates whether the FedCM API should be embargoed due
-    // to the user explicitly dismissing the FedCM account picker.
-    virtual void OnDismiss(bool should_embargo) = 0;
+    // Informs the controller that the user has dismissed the sheet with reason
+    // `dismiss_reason`.
+    virtual void OnDismiss(
+        content::IdentityRequestDialogController::DismissReason
+            dismiss_reason) = 0;
     // The web page view containing the focused field.
     virtual gfx::NativeView GetNativeView() = 0;
     // The WebContents for the page.

chrome/browser/ui/webid/identity_dialog_controller.cc

@@ -52,11 +52,13 @@ void IdentityDialogController::ShowAccountsDialog(
     const content::IdentityProviderMetadata& idp_metadata,
     const content::ClientIdData& client_data,
     content::IdentityRequestAccount::SignInMode sign_in_mode,
-    AccountSelectionCallback on_selected) {
+    AccountSelectionCallback on_selected,
+    DismissCallback dismiss_callback) {
   // IDP scheme is expected to always be `https://`.
   CHECK(idp_url.SchemeIs(url::kHttpsScheme));
   rp_web_contents_ = rp_web_contents;
   on_account_selection_ = std::move(on_selected);
+  on_dismiss_ = std::move(dismiss_callback);
   std::string rp_for_display =
       FormatUrlForDisplay(rp_web_contents_->GetLastCommittedURL());
   std::string idp_for_display = FormatUrlForDisplay(idp_url);
@@ -68,18 +70,20 @@ void IdentityDialogController::ShowAccountsDialog(
 }
 
 void IdentityDialogController::OnAccountSelected(const Account& account) {
+  on_dismiss_.Reset();
   std::move(on_account_selection_)
       .Run(account.id,
            account.login_state ==
-               content::IdentityRequestAccount::LoginState::kSignIn,
-           /* should_embargo=*/false);
+               content::IdentityRequestAccount::LoginState::kSignIn);
 }
 
-void IdentityDialogController::OnDismiss(bool should_embargo) {
+void IdentityDialogController::OnDismiss(DismissReason dismiss_reason) {
   // |OnDismiss| can be called after |OnAccountSelected| which sets the callback
   // to null.
-  if (on_account_selection_)
-    std::move(on_account_selection_).Run(std::string(), false, should_embargo);
+  if (on_dismiss_) {
+    on_account_selection_.Reset();
+    std::move(on_dismiss_).Run(dismiss_reason);
+  }
 }
 
 gfx::NativeView IdentityDialogController::GetNativeView() {