Skip to content

Commit

Permalink
[FedCM] Implement an intermediate bottom sheet during sign-up
Browse files Browse the repository at this point in the history
IDP verifying the account and issuing token may take several seconds so
it's better to let users be aware of the process before the token is
received.

This patch implements the intermediate bottom sheet to align with the
mocks.

Bug: 1271584
Change-Id: I274891445760135397df4b6aee761c015272afb3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3304932
Commit-Queue: Yi Gu <yigu@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#948180}
  • Loading branch information
yi-gu authored and Chromium LUCI CQ committed Dec 3, 2021
1 parent c8de3b0 commit 9dd2e3d
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 6 deletions.
3 changes: 3 additions & 0 deletions chrome/browser/ui/android/strings/android_chrome_strings.grd
Original file line number Diff line number Diff line change
Expand Up @@ -5317,6 +5317,9 @@ To change this setting, <ph name="BEGIN_LINK">&lt;resetlink&gt;</ph>reset sync<p
<message name="IDS_SIGN_IN_SHEET_TITLE" desc="Header for sign in sheet." translateable="false">
Signing into <ph name="SITE_NAME">%1$s<ex>rp.example</ex></ph>
</message>
<message name="IDS_VERIFY_SHEET_TITLE" desc="Header for verify sheet." translateable="false">
Verifying…
</message>

<!-- Content Creation strings -->
<message name="IDS_CONTENT_CREATION_NOTE_TITLE_FOR_SHARE" desc="The title of the share sheet when sharing a stylized note. The date will be formatted according to the current locale.">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ private AccountSelectionBridge(long nativeView, WindowAndroid windowAndroid,

@CalledByNative
private void destroy() {
mAccountSelectionComponent.hideBottomSheet();
mNativeView = 0;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,9 @@ public void testCallsCallbackAndHidesOnSelectingItemDoesNotRecordIndexForSingleA

mSheetItems.get(2).model.get(ContinueButtonProperties.ON_CLICK_LISTENER).onResult(ANA);
verify(mMockDelegate).onAccountSelected(ANA);
assertEquals("Incorrectly visible", false, mMediator.isVisible());
assertEquals(true, mMediator.isVisible());
mMediator.hideBottomSheet();
assertEquals(false, mMediator.isVisible());
}

@Test
Expand All @@ -348,7 +350,9 @@ public void testCallsCallbackAndHidesOnSelectingItem() {

mSheetItems.get(1).model.get(AccountProperties.ON_CLICK_LISTENER).onResult(CARL);
verify(mMockDelegate).onAccountSelected(CARL);
assertEquals("Incorrectly visible", false, mMediator.isVisible());
assertEquals(true, mMediator.isVisible());
mMediator.hideBottomSheet();
assertEquals(false, mMediator.isVisible());
}

@Test
Expand Down Expand Up @@ -378,7 +382,9 @@ public void testCallsDelegateAndHidesOnAccountPickerSelectSignIn() {
CLIENT_ID_METADATA, false);
mMediator.onAccountSelected(ANA);
verify(mMockDelegate).onAccountSelected(ANA);
assertEquals("Incorrectly visible", false, mMediator.isVisible());
assertEquals(true, mMediator.isVisible());
mMediator.hideBottomSheet();
assertEquals(false, mMediator.isVisible());
}

@Test
Expand Down Expand Up @@ -421,7 +427,9 @@ public void testCallsDelegateAndHidesOnAutoSignIn() {
// Auto signs in if no action is taken.
ShadowLooper.runUiThreadTasksIncludingDelayedTasks();
verify(mMockDelegate).onAccountSelected(ANA);
assertEquals("Incorrectly visible", false, mMediator.isVisible());
assertEquals(true, mMediator.isVisible());
mMediator.hideBottomSheet();
assertEquals(false, mMediator.isVisible());
}

@Test
Expand Down Expand Up @@ -485,6 +493,17 @@ public void testShowDataSharingConsentForSingleNewAccount() {
dataSharingProperties.mFormattedIdpUrl);
}

@Test
public void testShowVerifySheet() {
when(mMockBottomSheetController.requestShowContent(any(), anyBoolean())).thenReturn(true);
mMediator.showVerifySheet(ANA);

assertEquals(2, mSheetItems.size());
assertEquals(ItemType.HEADER, mSheetItems.get(0).type);
assertEquals(HeaderType.VERIFY, mSheetItems.get(0).model.get(TYPE));
assertEquals(ItemType.ACCOUNT, mSheetItems.get(1).type);
}

private void pressBack() {
if (mBottomSheetContent.handleBackPress()) return;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,4 +138,9 @@ public void showAccounts(GURL rpUrl, GURL idpUrl, List<Account> accounts,
boolean isAutoSignIn) {
mMediator.showAccounts(rpUrl, idpUrl, accounts, idpMetadata, clientMetadata, isAutoSignIn);
}

@Override
public void hideBottomSheet() {
mMediator.hideBottomSheet();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ private boolean handleBackPress() {
return false;
}

// This method should not be used when the VERIFY header is needed.
private void addHeader(GURL rpUrl, GURL idpUrl, List<Account> accounts) {
boolean useSignInHeader = false;
for (Account account : accounts) {
Expand Down Expand Up @@ -170,6 +171,26 @@ private void addContinueButton(Account account, GURL rpUrl, GURL idpUrl,
}
}

void showVerifySheet(Account account) {
mSheetItems.clear();

Runnable closeOnClickRunnable = () -> {
onDismissed(BottomSheetController.StateChangeReason.NONE);
};
mSheetItems.add(new ListItem(ItemType.HEADER,
new PropertyModel.Builder(HeaderProperties.ALL_KEYS)
.with(HeaderProperties.CLOSE_ON_CLICK_LISTENER, closeOnClickRunnable)
.with(HeaderProperties.TYPE, HeaderType.VERIFY)
.build()));

addAccounts(mIdpUrl, Arrays.asList(account), /*areAccountsClickable=*/false);
showContent();
}

void hideBottomSheet() {
if (mVisible) hideContent();
}

void showAccounts(GURL rpUrl, GURL idpUrl, List<Account> accounts,
IdentityProviderMetadata idpMetadata, ClientIdMetadata clientMetadata,
boolean isAutoSignIn) {
Expand Down Expand Up @@ -218,6 +239,8 @@ private void showAccountsInternal(GURL rpUrl, GURL idpUrl, List<Account> account
@VisibleForTesting
void showContent() {
if (mBottomSheetController.requestShowContent(mBottomSheetContent, true)) {
if (mVisible) return;

mVisible = true;
mBottomSheetController.addObserver(mBottomSheetObserver);
} else {
Expand Down Expand Up @@ -275,8 +298,9 @@ void onAccountSelected(Account selectedAccount) {
mClientMetadata, /*isAutoSignIn=*/false);
return;
}
hideContent();

mDelegate.onAccountSelected(selectedAccount);
showVerifySheet(selectedAccount);
}

void onDismissed(@StateChangeReason int reason) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ private AccountProperties() {}
* sheet.
*/
static class HeaderProperties {
public enum HeaderType { SINGLE_ACCOUNT, MULTIPLE_ACCOUNT, SIGN_IN }
public enum HeaderType { SINGLE_ACCOUNT, MULTIPLE_ACCOUNT, SIGN_IN, VERIFY }
static final ReadableObjectPropertyKey<Runnable> CLOSE_ON_CLICK_LISTENER =
new ReadableObjectPropertyKey<>("close_on_click_listener");
static final ReadableObjectPropertyKey<String> FORMATTED_IDP_URL =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,9 @@ static void bindHeaderView(PropertyModel model, View view, PropertyKey key) {
case SIGN_IN:
titleStringId = R.string.sign_in_sheet_title;
break;
case VERIFY:
titleStringId = R.string.verify_sheet_title;
break;
}

String title = String.format(view.getContext().getString(titleStringId),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,9 @@ interface Delegate {
void showAccounts(GURL rpUrl, GURL idpUrl, List<Account> accounts,
IdentityProviderMetadata idpMetadata, ClientIdMetadata clientMetadata,
boolean isAutoSignIn);

/**
* Hides the outstanding bottom sheet.
*/
void hideBottomSheet();
}

0 comments on commit 9dd2e3d

Please sign in to comment.