Skip to content

Commit

Permalink
[FRE] Add a progress spinner for sign(in/out) in signin FRE
Browse files Browse the repository at this point in the history
This cl adds a progress spinner that is shown when the user presses on
continue or dismiss button on the signin FRE and signin or signout
process is running on the background. While the spinner is shown the
bottom group (selected account row, continue and dismiss button and the
footer string) is hidden.
Signout is invoked when the user signs in through the fre, goes to the
sync consent page and then presses back to return to the FRE.

The transition animation starts after a 300ms delay and takes 300ms to
finish the transition. If the signin/signout process finishes before
then the user is redirected to the next page.

Video links: https://crbug.com/1294994#c8

Bug: 1294994
Change-Id: Ida6a472f8bf83be25d4f65d6bb995b7d5e823471
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3545155
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Commit-Queue: Tanmoy Mollik <triploblastic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#989905}
  • Loading branch information
Tanmoy Mollik authored and Chromium LUCI CQ committed Apr 7, 2022
1 parent e5e8bad commit e6c4b56
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 52 deletions.
Expand Up @@ -85,6 +85,7 @@
import org.chromium.components.policy.PolicyService;
import org.chromium.components.signin.base.CoreAccountInfo;
import org.chromium.components.signin.identitymanager.ConsentLevel;
import org.chromium.components.signin.identitymanager.IdentityManager;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.ui.test.util.ViewUtils;

Expand Down Expand Up @@ -141,6 +142,8 @@ void setPageDelegate(FirstRunPageDelegate delegate) {
@Mock
private SigninManager mSigninManagerMock;
@Mock
private IdentityManager mIdentityManagerMock;
@Mock
private SigninChecker mSigninCheckerMock;
@Mock
private IdentityServicesProvider mIdentityServicesProviderMock;
Expand Down Expand Up @@ -528,6 +531,43 @@ public void testContinueButtonWithChildAccount() {
verify(mSigninManagerMock, never()).signinAndEnableSync(anyInt(), any(), any());
}

@Test
@MediumTest
public void testProgressSpinnerOnContinueButtonPress() {
mAccountManagerTestRule.addAccount(TEST_EMAIL1, FULL_NAME1, GIVEN_NAME1, null);
IdentityServicesProvider.setInstanceForTests(mIdentityServicesProviderMock);
TestThreadUtils.runOnUiThreadBlocking(() -> {
when(IdentityServicesProvider.get().getSigninManager(
Profile.getLastUsedRegularProfile()))
.thenReturn(mSigninManagerMock);
// IdentityManager#getPrimaryAccountInfo() is called during this test flow by
// SigninFirstRunMediator.
when(IdentityServicesProvider.get().getIdentityManager(
Profile.getLastUsedRegularProfile()))
.thenReturn(mIdentityManagerMock);
});
TestThreadUtils.runOnUiThreadBlocking(() -> { mFragment.onNativeInitialized(); });
launchActivityWithFragment();

final String continueAsText = mChromeActivityTestRule.getActivity().getString(
R.string.signin_promo_continue_as, GIVEN_NAME1);
onView(withText(continueAsText)).perform(click());

verify(mFirstRunPageDelegateMock).acceptTermsOfService(true);
onView(withId(R.id.fre_signin_progress_spinner)).check(matches(isDisplayed()));
onView(withText(R.string.fre_welcome)).check(matches(isDisplayed()));
onView(withId(R.id.subtitle)).check(matches(not(isDisplayed())));
onView(withText(TEST_EMAIL1)).check(matches(not(isDisplayed())));
onView(withText(FULL_NAME1)).check(matches(not(isDisplayed())));
onView(withId(R.id.signin_fre_selected_account_expand_icon))
.check(matches(not(isDisplayed())));
onView(withText(continueAsText)).check(matches(not(isDisplayed())));
onView(withText(R.string.signin_fre_dismiss_button)).check(matches(not(isDisplayed())));
onView(withId(R.id.signin_fre_footer)).check(matches(not(isDisplayed())));

IdentityServicesProvider.setInstanceForTests(null);
}

@Test
@MediumTest
public void testFragmentWhenClickingOnFooter() {
Expand Down Expand Up @@ -664,7 +704,7 @@ public void testFragmentWhenPolicyIsLoadedAfterNative() {
mAccountManagerTestRule.addAccount(TEST_EMAIL1, FULL_NAME1, GIVEN_NAME1, null);
when(mPolicyLoadListenerMock.get()).thenReturn(null);
launchActivityWithFragment();
checkFragmentWhenLoadingNativeAndPolicyAndHideTheSpinner();
checkFragmentWhenLoadingNativeAndPolicy();

when(mPolicyLoadListenerMock.get()).thenReturn(false);
verify(mPolicyLoadListenerMock, atLeastOnce()).onAvailable(mCallbackCaptor.capture());
Expand All @@ -682,7 +722,7 @@ public void testFragmentWhenPolicyIsLoadedAfterNative() {
public void testFragmentWhenNativeIsLoadedAfterPolicy() {
mAccountManagerTestRule.addAccount(TEST_EMAIL1, FULL_NAME1, GIVEN_NAME1, null);
launchActivityWithFragment();
checkFragmentWhenLoadingNativeAndPolicyAndHideTheSpinner();
checkFragmentWhenLoadingNativeAndPolicy();

TestThreadUtils.runOnUiThreadBlocking(() -> { mFragment.onNativeInitialized(); });

Expand Down Expand Up @@ -726,7 +766,8 @@ public void testFragment_WelcomeToChrome() {
FREMobileIdentityConsistencyFieldTrial.setFirstRunVariationsTrialGroupForTesting(
VariationsGroup.WELCOME_TO_CHROME);
launchActivityWithFragment();
checkAndHideTheSpinner();
onView(withId(R.id.fre_native_and_policy_load_progress_spinner))
.check(matches(isDisplayed()));
onView(withText(R.string.fre_welcome)).check(matches(isDisplayed()));
onView(withId(R.id.subtitle)).check(matches(not(isDisplayed())));

Expand All @@ -742,7 +783,8 @@ public void testFragment_WelcomeToChrome_MostOutOfChrome() {
FREMobileIdentityConsistencyFieldTrial.setFirstRunVariationsTrialGroupForTesting(
VariationsGroup.WELCOME_TO_CHROME_MOST_OUT_OF_CHROME);
launchActivityWithFragment();
checkAndHideTheSpinner();
onView(withId(R.id.fre_native_and_policy_load_progress_spinner))
.check(matches(isDisplayed()));
onView(withId(R.id.title)).check(matches(not(isDisplayed())));
onView(withId(R.id.subtitle)).check(matches(not(isDisplayed())));

Expand All @@ -758,7 +800,6 @@ public void testFragment_WelcomeToChrome_StrongestSecurity() {
FREMobileIdentityConsistencyFieldTrial.setFirstRunVariationsTrialGroupForTesting(
VariationsGroup.WELCOME_TO_CHROME_STRONGEST_SECURITY);
launchActivityWithFragment();
checkAndHideTheSpinner();
onView(withId(R.id.title)).check(matches(not(isDisplayed())));
onView(withId(R.id.subtitle)).check(matches(not(isDisplayed())));

Expand All @@ -774,7 +815,8 @@ public void testFragment_WelcomeToChrome_EasierAcrossDevices() {
FREMobileIdentityConsistencyFieldTrial.setFirstRunVariationsTrialGroupForTesting(
VariationsGroup.WELCOME_TO_CHROME_EASIER_ACROSS_DEVICES);
launchActivityWithFragment();
checkAndHideTheSpinner();
onView(withId(R.id.fre_native_and_policy_load_progress_spinner))
.check(matches(isDisplayed()));
onView(withId(R.id.title)).check(matches(not(isDisplayed())));
onView(withId(R.id.subtitle)).check(matches(not(isDisplayed())));

Expand All @@ -790,7 +832,8 @@ public void testFragment_MostOutOfChrome() {
FREMobileIdentityConsistencyFieldTrial.setFirstRunVariationsTrialGroupForTesting(
VariationsGroup.MOST_OUT_OF_CHROME);
launchActivityWithFragment();
checkAndHideTheSpinner();
onView(withId(R.id.fre_native_and_policy_load_progress_spinner))
.check(matches(isDisplayed()));
onView(withId(R.id.title)).check(matches(not(isDisplayed())));
onView(withId(R.id.subtitle)).check(matches(not(isDisplayed())));

Expand All @@ -806,7 +849,8 @@ public void testFragment_MakeChromeYourOwn() {
FREMobileIdentityConsistencyFieldTrial.setFirstRunVariationsTrialGroupForTesting(
VariationsGroup.MAKE_CHROME_YOUR_OWN);
launchActivityWithFragment();
checkAndHideTheSpinner();
onView(withId(R.id.fre_native_and_policy_load_progress_spinner))
.check(matches(isDisplayed()));
onView(withId(R.id.title)).check(matches(not(isDisplayed())));
onView(withId(R.id.subtitle)).check(matches(not(isDisplayed())));

Expand Down Expand Up @@ -836,22 +880,9 @@ private void checkFragmentWithSelectedAccount(String email, String fullName, Str
onView(withId(R.id.signin_fre_footer)).check(matches(isDisplayed()));
}

private void checkAndHideTheSpinner() {
CriteriaHelper.pollUiThread(() -> {
return mFragment.getView().findViewById(R.id.signin_fre_progress_spinner).isShown();
});
TestThreadUtils.runOnUiThreadBlocking(() -> {
ProgressBar progressBar =
mFragment.getView().findViewById(R.id.signin_fre_progress_spinner);
// Replace the progress bar with a dummy to allow other checks. Currently the
// progress bar cannot be stopped otherwise due to some espresso issues (crbug/1115067).
progressBar.setIndeterminateDrawable(new ColorDrawable(
SemanticColorUtils.getDefaultBgColor(mFragment.getContext())));
});
}

private void checkFragmentWhenLoadingNativeAndPolicyAndHideTheSpinner() {
checkAndHideTheSpinner();
private void checkFragmentWhenLoadingNativeAndPolicy() {
onView(withId(R.id.fre_native_and_policy_load_progress_spinner))
.check(matches(isDisplayed()));
onView(withText(R.string.fre_welcome)).check(matches(isDisplayed()));
onView(withId(R.id.subtitle)).check(matches(not(isDisplayed())));
onView(withText(TEST_EMAIL1)).check(matches(not(isDisplayed())));
Expand Down Expand Up @@ -907,6 +938,18 @@ private void launchActivityWithFragment() {
});
ApplicationTestUtils.waitForActivityState(
mChromeActivityTestRule.getActivity(), Stage.RESUMED);
TestThreadUtils.runOnUiThreadBlocking(() -> {
// Replace all the progress bars with dummies. Currently the progress bar cannot be
// stopped otherwise due to some espresso issues (crbug/1115067).
ProgressBar nativeAndPolicyProgressBar = mFragment.getView().findViewById(
R.id.fre_native_and_policy_load_progress_spinner);
nativeAndPolicyProgressBar.setIndeterminateDrawable(new ColorDrawable(
SemanticColorUtils.getDefaultBgColor(mFragment.getContext())));
ProgressBar signinProgressSpinner =
mFragment.getView().findViewById(R.id.fre_signin_progress_spinner);
signinProgressSpinner.setIndeterminateDrawable(new ColorDrawable(
SemanticColorUtils.getDefaultBgColor(mFragment.getContext())));
});
}

private ViewAction clickOnUMADialogSpan() {
Expand Down
Expand Up @@ -94,7 +94,7 @@ private void navigateThroughFRE() {
IUi2Locator signinContinueButton =
Ui2Locators.withAnyResEntry(R.id.signin_fre_continue_button);
IUi2Locator signinProgressSpinner =
Ui2Locators.withAnyResEntry(R.id.signin_fre_progress_spinner);
Ui2Locators.withAnyResEntry(R.id.fre_native_and_policy_load_progress_spinner);

// Used in DefaultSearchEngineFirstRunFragment FRE page.
IUi2Locator defaultSearchEngineNextButton =
Expand Down
Expand Up @@ -21,7 +21,6 @@
android:src="@drawable/fre_product_logo" />

<LinearLayout
android:id="@+id/signin_fre_content"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginEnd="60dp"
Expand Down Expand Up @@ -69,7 +68,7 @@
</FrameLayout>

<ProgressBar
android:id="@+id/signin_fre_progress_spinner"
android:id="@+id/fre_native_and_policy_load_progress_spinner"
style="@style/Widget.AppCompat.ProgressBar"
android:layout_width="32dp"
android:layout_height="32dp"
Expand All @@ -92,4 +91,15 @@
android:text="@string/signin_fre_footer"
app:leading="@dimen/text_size_small_leading" />
</LinearLayout>

<ProgressBar
android:id="@+id/fre_signin_progress_spinner"
style="@style/Widget.AppCompat.ProgressBar"
android:layout_width="32dp"
android:layout_height="32dp"
android:layout_marginBottom="170dp"
android:layout_marginStart="200dp"
android:layout_alignParentBottom="true"
android:layout_toEndOf="@id/fre_logo"
android:visibility="gone" />
</org.chromium.chrome.browser.ui.signin.fre.SigninFirstRunView>
Expand Up @@ -6,7 +6,6 @@
<org.chromium.chrome.browser.ui.signin.fre.SigninFirstRunView
xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
android:id="@+id/signin_fre_content"
android:layout_width="match_parent"
android:layout_height="match_parent">

Expand Down Expand Up @@ -88,7 +87,7 @@
android:visibility="invisible" />

<ProgressBar
android:id="@+id/signin_fre_progress_spinner"
android:id="@+id/fre_native_and_policy_load_progress_spinner"
style="@style/Widget.AppCompat.ProgressBar"
android:layout_width="32dp"
android:layout_height="32dp"
Expand Down Expand Up @@ -120,4 +119,14 @@
android:text="@string/signin_fre_footer"
app:leading="@dimen/text_size_small_leading" />
</FrameLayout>

<ProgressBar
android:id="@+id/fre_signin_progress_spinner"
style="@style/Widget.AppCompat.ProgressBar"
android:layout_width="32dp"
android:layout_height="32dp"
android:layout_marginBottom="128dp"
android:layout_alignParentBottom="true"
android:layout_centerHorizontal="true"
android:visibility="gone" />
</org.chromium.chrome.browser.ui.signin.fre.SigninFirstRunView>
Expand Up @@ -82,7 +82,7 @@ void destroy() {
}

void reset() {
mModel.set(SigninFirstRunProperties.IS_CONTINUE_OR_DISMISS_CLICKED, false);
mModel.set(SigninFirstRunProperties.SHOW_SIGNIN_PROGRESS_SPINNER, false);
}

void onNativeAndPolicyLoaded(boolean hasPolicies) {
Expand Down Expand Up @@ -186,7 +186,7 @@ private void onContinueAsClicked() {
mDelegate.advanceToNextPage();
return;
}
mModel.set(SigninFirstRunProperties.IS_CONTINUE_OR_DISMISS_CLICKED, true);
mModel.set(SigninFirstRunProperties.SHOW_SIGNIN_PROGRESS_SPINNER, true);
final SigninManager signinManager = IdentityServicesProvider.get().getSigninManager(
Profile.getLastUsedRegularProfile());
signinManager.onFirstRunCheckDone();
Expand Down Expand Up @@ -217,7 +217,7 @@ private void onDismissClicked() {
if (IdentityServicesProvider.get()
.getIdentityManager(Profile.getLastUsedRegularProfile())
.hasPrimaryAccount(ConsentLevel.SIGNIN)) {
mModel.set(SigninFirstRunProperties.IS_CONTINUE_OR_DISMISS_CLICKED, true);
mModel.set(SigninFirstRunProperties.SHOW_SIGNIN_PROGRESS_SPINNER, true);
IdentityServicesProvider.get()
.getSigninManager(Profile.getLastUsedRegularProfile())
.signOut(SignoutReason.ABORT_SIGNIN,
Expand All @@ -235,7 +235,8 @@ private void onDismissClicked() {
* See crbug.com/1294994 for details.
*/
private boolean isContinueOrDismissClicked() {
return mModel.get(SigninFirstRunProperties.IS_CONTINUE_OR_DISMISS_CLICKED);
// This property key is set when continue or dismiss button is clicked.
return mModel.get(SigninFirstRunProperties.SHOW_SIGNIN_PROGRESS_SPINNER);
}

private void setSelectedAccountName(String accountName) {
Expand Down
Expand Up @@ -37,8 +37,11 @@ static class FrePolicy { public boolean metricsReportingDisabledByPolicy; }
static final ReadableObjectPropertyKey<OnClickListener> ON_DISMISS_CLICKED =
new ReadableObjectPropertyKey<>("on_dismiss_clicked");

static final WritableBooleanPropertyKey IS_CONTINUE_OR_DISMISS_CLICKED =
new WritableBooleanPropertyKey("is_continue_or_dismiss_clicked");
// Is not initialized in #createModel(...) to avoid conflicting view changes with
// ARE_NATIVE_AND_POLICY_LOADED. Will be set when |Continue as ...| or dismiss button is
// pressed.
static final WritableBooleanPropertyKey SHOW_SIGNIN_PROGRESS_SPINNER =
new WritableBooleanPropertyKey("show_signin_progress_spinner");

static final WritableBooleanPropertyKey ARE_NATIVE_AND_POLICY_LOADED =
new WritableBooleanPropertyKey("are_native_and_policy_loaded");
Expand All @@ -58,7 +61,7 @@ static class FrePolicy { public boolean metricsReportingDisabledByPolicy; }
IS_SELECTED_ACCOUNT_SUPERVISED,
ON_CONTINUE_AS_CLICKED,
ON_DISMISS_CLICKED,
IS_CONTINUE_OR_DISMISS_CLICKED,
SHOW_SIGNIN_PROGRESS_SPINNER,
ARE_NATIVE_AND_POLICY_LOADED,
FRE_POLICY,
IS_SIGNIN_SUPPORTED,
Expand All @@ -77,7 +80,6 @@ static PropertyModel createModel(Runnable onSelectedAccountClicked,
.with(IS_SELECTED_ACCOUNT_SUPERVISED, false)
.with(ON_CONTINUE_AS_CLICKED, v -> onContinueAsClicked.run())
.with(ON_DISMISS_CLICKED, v -> onDismissClicked.run())
.with(IS_CONTINUE_OR_DISMISS_CLICKED, false)
.with(ARE_NATIVE_AND_POLICY_LOADED, false)
.with(FRE_POLICY, null)
.with(IS_SIGNIN_SUPPORTED, isSigninSupported)
Expand Down
Expand Up @@ -23,16 +23,16 @@

/** View that wraps signin first run welcome screen and caches references to UI elements. **/
public class SigninFirstRunView extends RelativeLayout {
private ViewGroup mContent;
private TextView mTitle;
private TextView mSubtitle;
private View mBrowserManagedHeader;
private ProgressBar mProgressSpinner;
private ProgressBar mInitialLoadProgressSpinner;
private ViewGroup mSelectedAccount;
private ImageView mExpandIcon;
private ButtonCompat mContinueButton;
private ButtonCompat mDismissButton;
private TextViewWithClickableSpans mFooter;
private ProgressBar mSigninProgressSpinner;

public SigninFirstRunView(Context context, @Nullable AttributeSet attrs) {
super(context, attrs);
Expand All @@ -42,32 +42,29 @@ public SigninFirstRunView(Context context, @Nullable AttributeSet attrs) {
protected void onFinishInflate() {
super.onFinishInflate();

mContent = findViewById(R.id.signin_fre_content);
mTitle = findViewById(R.id.title);
mSubtitle = findViewById(R.id.subtitle);
mBrowserManagedHeader = findViewById(R.id.fre_browser_managed_by_organization);
mProgressSpinner = findViewById(R.id.signin_fre_progress_spinner);
mInitialLoadProgressSpinner =
findViewById(R.id.fre_native_and_policy_load_progress_spinner);
mSelectedAccount = findViewById(R.id.signin_fre_selected_account);
mExpandIcon = findViewById(R.id.signin_fre_selected_account_expand_icon);
mContinueButton = findViewById(R.id.signin_fre_continue_button);
mDismissButton = findViewById(R.id.signin_fre_dismiss_button);
mFooter = findViewById(R.id.signin_fre_footer);
mSigninProgressSpinner = findViewById(R.id.fre_signin_progress_spinner);

if (FREMobileIdentityConsistencyFieldTrial.shouldHideTitleUntilPoliciesAreLoaded()) {
mTitle.setVisibility(INVISIBLE);
}
}

ViewGroup getContentView() {
return mContent;
}

View getBrowserManagedHeaderView() {
return mBrowserManagedHeader;
}

ProgressBar getProgressSpinnerView() {
return mProgressSpinner;
ProgressBar getInitialLoadProgressSpinnerView() {
return mInitialLoadProgressSpinner;
}

ViewGroup getSelectedAccountView() {
Expand All @@ -90,6 +87,10 @@ TextViewWithClickableSpans getFooterView() {
return mFooter;
}

ProgressBar getSigninProgressSpinner() {
return mSigninProgressSpinner;
}

/** Updates the title and the subtitle for UI variations on native and policy load. **/
void onNativeAndPoliciesLoaded() {
Pair<Integer, Integer> titleAndSubtitleId =
Expand Down

0 comments on commit e6c4b56

Please sign in to comment.