[FedCM] Update Android UI to align with latest mocks

This patch also makes a minor update on the title from "Signing in" to "Signing you in" on desktop. Demo is available on https://bugs.chromium.org/p/chromium/issues/detail?id=1408538#c5 Bug: 1408538 Change-Id: I638c8a51171671ee482b84f85a4cc07adbb700d5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4235068 Reviewed-by: Yi Gu <yigu@chromium.org> Commit-Queue: Zachary Tan <tanzachary@chromium.org> Cr-Commit-Position: refs/heads/main@{#1103873}
chromium · Feb 10, 2023 · 2ba8bd4 · 2ba8bd4
1 parent 4170eea
commit 2ba8bd4
Show file tree

Hide file tree

Showing 16 changed files with 57 additions and 208 deletions.
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
@@ -14143,7 +14143,7 @@ Please help our engineers fix this problem. Tell us what happened right before y
       Verifying…
     </message>
     <message name="IDS_VERIFY_SHEET_TITLE_AUTO_SIGNIN" desc="Header for auto sign-in sheet. Sheet is shown to notify the user that they are signing in to a website using an account from an identity provider." translateable="false">
-      Signing in…
+      Signing you in…
     </message>
     <message name="IDS_AUTO_SIGNIN_OPTOUT_CHECKBOX" desc="Allows users to opt out of FedCM auto sign-in by unchecking the checkbox" translateable="false">
       Automatically sign me in to this website

diff --git a/chrome/browser/ui/android/strings/android_chrome_strings.grd b/chrome/browser/ui/android/strings/android_chrome_strings.grd
@@ -5547,9 +5547,6 @@ To change this setting, <ph name="BEGIN_LINK">&lt;resetlink&gt;</ph>reset sync<p
       </message>
 
       <!-- WebID Account Selection strings -->
-      <message name="IDS_ACCOUNT_SELECTION_SHEET_TITLE_AUTO" desc="Header for sign in sheet. Sheet is shown to inform user of sign in. Sign in occurs automatically. (User can cancel)." translateable="false">
-        Signing in to <ph name="SITE_ETLD_PLUS_ONE">%1$s<ex>rp.example</ex></ph> with <ph name="IDENTITY_PROVIDER_ETLD_PLUS_ONE">%2$s<ex>idp.com</ex></ph>
-      </message>
       <message name="IDS_ACCOUNT_SELECTION_SHEET_TITLE_EXPLICIT" desc="Header for sign in sheet. Sheet is shown to prompt user for sign in consent.">
         Sign in to <ph name="SITE_ETLD_PLUS_ONE">%1$s<ex>rp.example</ex></ph> with <ph name="IDENTITY_PROVIDER_ETLD_PLUS_ONE">%2$s<ex>idp.com</ex></ph>
       </message>
@@ -5581,7 +5578,10 @@ To change this setting, <ph name="BEGIN_LINK">&lt;resetlink&gt;</ph>reset sync<p
       <message name="IDS_ACCOUNT_SELECTION_SHEET_CLOSED" desc="Accessibility string read when the Account Selection bottom sheet showing a list of the user's accounts is closed." is_accessibility_with_no_ui="true">
         Sign in bottom sheet is closed.
       </message>
-      <message name="IDS_VERIFY_SHEET_TITLE" desc="Header for verify sheet.">
+      <message name="IDS_VERIFY_SHEET_TITLE_AUTO_SIGNIN" desc="Header for verify sheet for auto sign-in." translateable="false">
+        Signing you in…
+      </message>
+      <message name="IDS_VERIFY_SHEET_TITLE" desc="Header for verify sheet for explicit sign-in.">
         Verifying…
       </message>
 

diff --git a/chrome/browser/ui/android/webid/account_selection_view_android.cc b/chrome/browser/ui/android/webid/account_selection_view_android.cc
@@ -170,11 +170,6 @@ 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(DismissReason::OTHER);
-}
-
 bool AccountSelectionViewAndroid::RecreateJavaObject() {
   if (delegate_->GetNativeView() == nullptr ||
       delegate_->GetNativeView()->GetWindowAndroid() == nullptr) {

diff --git a/chrome/browser/ui/android/webid/account_selection_view_android.h b/chrome/browser/ui/android/webid/account_selection_view_android.h
@@ -34,7 +34,6 @@ class AccountSelectionViewAndroid : public AccountSelectionView {
       const base::android::JavaParamRef<jobject>& account_picture_url,
       bool is_sign_in);
   void OnDismiss(JNIEnv* env, jint dismiss_reason);
-  void OnAutoSignInCancelled(JNIEnv* env);
 
  private:
   // Returns either true if the java counterpart of this bridge is initialized

diff --git a/chrome/browser/ui/android/webid/internal/BUILD.gn b/chrome/browser/ui/android/webid/internal/BUILD.gn
@@ -60,7 +60,6 @@ android_resources("java_resources") {
     "java/res/layout/account_selection_data_sharing_consent_item.xml",
     "java/res/layout/account_selection_header_item.xml",
     "java/res/layout/account_selection_sheet.xml",
-    "java/res/layout/auto_sign_in_cancel_button.xml",
     "java/res/values/dimens.xml",
   ]
 }

diff --git a/chrome/browser/ui/android/webid/internal/java/res/layout/account_selection_sheet.xml b/chrome/browser/ui/android/webid/internal/java/res/layout/account_selection_sheet.xml
@@ -32,9 +32,6 @@ found in the LICENSE file.
     <include layout="@layout/account_selection_continue_button"
         android:id="@+id/account_selection_continue_btn"
         android:visibility="gone"/>
-    <include layout="@layout/auto_sign_in_cancel_button"
-        android:id="@+id/auto_sign_in_cancel_btn"
-        android:visibility="gone"/>
     <include layout="@layout/account_selection_data_sharing_consent_item"
         android:id="@+id/user_data_sharing_consent"
         android:visibility="gone"/>

diff --git a/chrome/browser/ui/android/webid/internal/java/res/layout/auto_sign_in_cancel_button.xml b/chrome/browser/ui/android/webid/internal/java/res/layout/auto_sign_in_cancel_button.xml
diff --git a/...nternal/java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionBridge.java b/...nternal/java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionBridge.java
@@ -109,22 +109,11 @@ public void onAccountSelected(GURL idpConfigUrl, Account account) {
         }
     }
 
-    @Override
-    public void onAutoSignInCancelled() {
-        if (mNativeView != 0) {
-            // This call passes the account fields directly as String and GURL parameters as an
-            // optimization to avoid needing multiple JNI getters on the Account class on for each
-            // field.
-            AccountSelectionBridgeJni.get().onAutoSignInCancelled(mNativeView);
-        }
-    }
-
     @NativeMethods
     interface Natives {
         void onAccountSelected(long nativeAccountSelectionViewAndroid, GURL idpConfigUrl,
                 String[] accountFields, GURL accountPictureUrl, boolean isSignedIn);
         void onDismiss(long nativeAccountSelectionViewAndroid,
                 @IdentityRequestDialogDismissReason int dismissReason);
-        void onAutoSignInCancelled(long nativeAccountSelectionViewAndroid);
     }
 }
diff --git a/...java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionControllerTest.java b/...java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionControllerTest.java
@@ -49,7 +49,6 @@
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.chrome.browser.ui.android.webid.AccountSelectionProperties.AccountProperties;
 import org.chromium.chrome.browser.ui.android.webid.AccountSelectionProperties.AccountProperties.Avatar;
-import org.chromium.chrome.browser.ui.android.webid.AccountSelectionProperties.AutoSignInCancelButtonProperties;
 import org.chromium.chrome.browser.ui.android.webid.AccountSelectionProperties.ContinueButtonProperties;
 import org.chromium.chrome.browser.ui.android.webid.AccountSelectionProperties.DataSharingConsentProperties;
 import org.chromium.chrome.browser.ui.android.webid.AccountSelectionProperties.HeaderProperties.HeaderType;
@@ -221,14 +220,6 @@ public void testShowAccountSignUpHeader() {
         assertEquals(HeaderType.SIGN_IN, headerModel.get(TYPE));
     }
 
-    @Test
-    public void testShowAccountAutoSignInHeader() {
-        mMediator.showAccounts(TEST_ETLD_PLUS_ONE, TEST_ETLD_PLUS_ONE_1, Arrays.asList(ANA),
-                IDP_METADATA, CLIENT_ID_METADATA, true /* isAutoSignIn */);
-        PropertyModel headerModel = mModel.get(ItemProperties.HEADER);
-        assertEquals(HeaderType.AUTO_SIGN_IN, headerModel.get(TYPE));
-    }
-
     @Test
     public void testShowAccountsSetsAccountListAndRequestsAvatar() {
         mMediator.showAccounts(TEST_ETLD_PLUS_ONE, TEST_ETLD_PLUS_ONE_1, Arrays.asList(ANA, BOB),
@@ -418,39 +409,15 @@ public void testCallsDelegateAndHidesOnAutoSignIn() {
         assertTrue(mMediator.wasDismissed());
     }
 
-    @Test
-    public void testCallsDelegateAndHidesOnCancellingAutoSignIn() {
-        when(mMockBottomSheetController.requestShowContent(any(), anyBoolean())).thenReturn(true);
-        mMediator.showAccounts(TEST_ETLD_PLUS_ONE, TEST_ETLD_PLUS_ONE_1, Arrays.asList(ANA),
-                IDP_METADATA, CLIENT_ID_METADATA, true);
-        mMediator.onAutoSignInCancelled();
-        verify(mMockDelegate).onAutoSignInCancelled();
-        assertTrue(mMediator.wasDismissed());
-    }
-
-    @Test
-    public void testCallsCallbackAndHidesOnCancellingAutoSignIn() {
-        when(mMockBottomSheetController.requestShowContent(any(), anyBoolean())).thenReturn(true);
-        mMediator.showAccounts(TEST_ETLD_PLUS_ONE, TEST_ETLD_PLUS_ONE_1, Arrays.asList(ANA),
-                IDP_METADATA, CLIENT_ID_METADATA, true);
-        assertFalse(mMediator.wasDismissed());
-        assertNotNull(mModel.get(ItemProperties.AUTO_SIGN_IN_CANCEL_BUTTON)
-                              .get(AutoSignInCancelButtonProperties.ON_CLICK_LISTENER));
-
-        mModel.get(ItemProperties.AUTO_SIGN_IN_CANCEL_BUTTON)
-                .get(AutoSignInCancelButtonProperties.ON_CLICK_LISTENER)
-                .run();
-        verify(mMockDelegate).onAutoSignInCancelled();
-        assertTrue(mMediator.wasDismissed());
-    }
-
     @Test
     public void testCallsDelegateAndHidesOnlyOnceWithAutoSignIn() {
         when(mMockBottomSheetController.requestShowContent(any(), anyBoolean())).thenReturn(true);
         mMediator.showAccounts(TEST_ETLD_PLUS_ONE, TEST_ETLD_PLUS_ONE_1, Arrays.asList(ANA),
                 IDP_METADATA, CLIENT_ID_METADATA, true);
+        // Auto signs in even if dismissed.
         pressBack();
         verify(mMockDelegate).onDismissed(IdentityRequestDialogDismissReason.OTHER);
+        verify(mMockDelegate).onAccountSelected(TEST_CONFIG_URL, ANA);
         verifyNoMoreInteractions(mMockDelegate);
         assertTrue(mMediator.wasDismissed());
         // The delayed task should not call delegate after user dismissing.
@@ -479,16 +446,27 @@ public void testShowDataSharingConsentForSingleNewAccount() {
     }
 
     @Test
-    public void testShowVerifySheet() {
+    public void testShowVerifySheetExplicitSignin() {
         when(mMockBottomSheetController.requestShowContent(any(), anyBoolean())).thenReturn(true);
         mMediator.showAccounts(TEST_ETLD_PLUS_ONE, TEST_ETLD_PLUS_ONE_2, Arrays.asList(NEW_USER),
-                IDP_METADATA, CLIENT_ID_METADATA, false);
+                IDP_METADATA, CLIENT_ID_METADATA, false /* isAutoSignIn */);
         mMediator.showVerifySheet(ANA);
 
         assertEquals(1, mSheetAccountItems.size());
         assertEquals(HeaderType.VERIFY, mModel.get(ItemProperties.HEADER).get(TYPE));
     }
 
+    @Test
+    public void testShowVerifySheetAutoSignin() {
+        when(mMockBottomSheetController.requestShowContent(any(), anyBoolean())).thenReturn(true);
+        // showVerifySheet is called in showAccounts when isAutoSignIn is true
+        mMediator.showAccounts(TEST_ETLD_PLUS_ONE, TEST_ETLD_PLUS_ONE_1, Arrays.asList(ANA),
+                IDP_METADATA, CLIENT_ID_METADATA, true /* isAutoSignIn */);
+
+        assertEquals(1, mSheetAccountItems.size());
+        assertEquals(HeaderType.VERIFY_AUTO_SIGNIN, mModel.get(ItemProperties.HEADER).get(TYPE));
+    }
+
     private void pressBack() {
         if (mBottomSheetContent.handleBackPress()) return;
 

diff --git a/...al/java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionCoordinator.java b/...al/java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionCoordinator.java
@@ -113,11 +113,6 @@ static View buildContinueButtonView(ViewGroup parent) {
                 .inflate(R.layout.account_selection_continue_button, parent, false);
     }
 
-    static View buildAutoSignInCancelButtonView(ViewGroup parent) {
-        return LayoutInflater.from(parent.getContext())
-                .inflate(R.layout.auto_sign_in_cancel_button, parent, false);
-    }
-
     @Override
     public void showAccounts(String rpEtldPlusOne, String idpEtldPlusOne, List<Account> accounts,
             IdentityProviderMetadata idpMetadata, ClientIdMetadata clientMetadata,

diff --git a/...ernal/java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionMediator.java b/...ernal/java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionMediator.java
@@ -7,7 +7,6 @@
 import android.graphics.Bitmap;
 import android.graphics.Canvas;
 import android.graphics.Color;
-import android.os.Handler;
 import android.os.SystemClock;
 import android.text.TextUtils;
 
@@ -16,7 +15,6 @@
 
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.browser.ui.android.webid.AccountSelectionProperties.AccountProperties;
-import org.chromium.chrome.browser.ui.android.webid.AccountSelectionProperties.AutoSignInCancelButtonProperties;
 import org.chromium.chrome.browser.ui.android.webid.AccountSelectionProperties.ContinueButtonProperties;
 import org.chromium.chrome.browser.ui.android.webid.AccountSelectionProperties.DataSharingConsentProperties;
 import org.chromium.chrome.browser.ui.android.webid.AccountSelectionProperties.HeaderProperties;
@@ -57,10 +55,6 @@ class AccountSelectionMediator {
     private final BottomSheetController mBottomSheetController;
     private final AccountSelectionBottomSheetContent mBottomSheetContent;
     private final BottomSheetObserver mBottomSheetObserver;
-    private final Handler mAutoSignInTaskHandler = new Handler();
-    // TODO(yigu): Increase the time after adding a continue button for users to
-    // proceed. Eventually this should be specified by IDPs.
-    private static final int AUTO_SIGN_IN_CANCELLATION_TIMER_MS = 5000;
 
     // Amount of time during which we ignore inputs. Note that this is timed from when we invoke the
     // methods to show the accounts, so it does include any time spent animating the sheet into
@@ -180,7 +174,8 @@ private boolean shouldInputBeProcessed() {
     }
 
     void showVerifySheet(Account account) {
-        mHeaderType = HeaderType.VERIFY;
+        mHeaderType = (mHeaderType == HeaderType.AUTO_SIGN_IN) ? HeaderType.VERIFY_AUTO_SIGNIN
+                                                               : HeaderType.VERIFY;
         updateSheet(Arrays.asList(account), /*areAccountsClickable=*/false,
                 /* focusItem=*/ItemProperties.HEADER);
     }
@@ -262,11 +257,7 @@ private void updateSheet(
             assert mSelectedAccount != null;
             assert mSelectedAccount.isSignIn();
 
-            mModel.set(ItemProperties.AUTO_SIGN_IN_CANCEL_BUTTON, createAutoSignInCancelBtnItem());
-            mAutoSignInTaskHandler.postDelayed(
-                    () -> onAccountSelected(mSelectedAccount), AUTO_SIGN_IN_CANCELLATION_TIMER_MS);
-        } else {
-            mModel.set(ItemProperties.AUTO_SIGN_IN_CANCEL_BUTTON, null);
+            onAccountSelected(mSelectedAccount);
         }
 
         mModel.set(ItemProperties.CONTINUE_BUTTON,
@@ -368,11 +359,6 @@ void onDismissed(@IdentityRequestDialogDismissReason int dismissReason) {
         mDelegate.onDismissed(dismissReason);
     }
 
-    void onAutoSignInCancelled() {
-        hideContent();
-        mDelegate.onAutoSignInCancelled();
-    }
-
     private PropertyModel createAccountItem(Account account, boolean isAccountClickable) {
         return new PropertyModel.Builder(AccountProperties.ALL_KEYS)
                 .with(AccountProperties.ACCOUNT, account)
@@ -390,13 +376,6 @@ private PropertyModel createContinueBtnItem(
                 .build();
     }
 
-    private PropertyModel createAutoSignInCancelBtnItem() {
-        return new PropertyModel.Builder(AutoSignInCancelButtonProperties.ALL_KEYS)
-                .with(AutoSignInCancelButtonProperties.ON_CLICK_LISTENER,
-                        this::onAutoSignInCancelled)
-                .build();
-    }
-
     private PropertyModel createDataSharingConsentItem(
             String idpForDisplay, ClientIdMetadata metadata) {
         DataSharingConsentProperties.Properties properties =

diff --git a/...nal/java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionProperties.java b/...nal/java/src/org/chromium/chrome/browser/ui/android/webid/AccountSelectionProperties.java
@@ -57,7 +57,7 @@ private AccountProperties() {}
      * sheet.
      */
     static class HeaderProperties {
-        public enum HeaderType { AUTO_SIGN_IN, SIGN_IN, VERIFY }
+        public enum HeaderType { AUTO_SIGN_IN, SIGN_IN, VERIFY, VERIFY_AUTO_SIGNIN }
         static final ReadableObjectPropertyKey<Runnable> CLOSE_ON_CLICK_LISTENER =
                 new ReadableObjectPropertyKey<>("close_on_click_listener");
         static final ReadableObjectPropertyKey<String> IDP_FOR_DISPLAY =
@@ -113,33 +113,18 @@ static class ContinueButtonProperties {
         private ContinueButtonProperties() {}
     }
 
-    /**
-     * Properties defined here reflect the state of the cancel button used for auto sign in.
-     */
-    static class AutoSignInCancelButtonProperties {
-        static final ReadableObjectPropertyKey<Runnable> ON_CLICK_LISTENER =
-                new ReadableObjectPropertyKey<>("on_click_listener");
-
-        static final PropertyKey[] ALL_KEYS = {ON_CLICK_LISTENER};
-
-        private AutoSignInCancelButtonProperties() {}
-    }
-
     /**
      * Properties defined here reflect sections in the FedCM bottom sheet.
      */
     static class ItemProperties {
-        static final WritableObjectPropertyKey<PropertyModel> AUTO_SIGN_IN_CANCEL_BUTTON =
-                new WritableObjectPropertyKey<>("auto_sign_in_btn");
         static final WritableObjectPropertyKey<PropertyModel> CONTINUE_BUTTON =
                 new WritableObjectPropertyKey<>("continue_btn");
         static final WritableObjectPropertyKey<PropertyModel> DATA_SHARING_CONSENT =
                 new WritableObjectPropertyKey<>("data_sharing_consent");
         static final WritableObjectPropertyKey<PropertyModel> HEADER =
                 new WritableObjectPropertyKey<>("header");
 
-        static final PropertyKey[] ALL_KEYS = {
-                AUTO_SIGN_IN_CANCEL_BUTTON, CONTINUE_BUTTON, DATA_SHARING_CONSENT, HEADER};
+        static final PropertyKey[] ALL_KEYS = {CONTINUE_BUTTON, DATA_SHARING_CONSENT, HEADER};
 
         private ItemProperties() {}
     }