Skip to content

Commit

Permalink
Fix infinite spinner on welcome page
Browse files Browse the repository at this point in the history
When the spinner becomes visible when welcome page is pending on proceed
to the next page, we never switch it back to show ToS. As a result, when
user navigate to the previous page by pack pressing the small spinner
will stay there forever.

This fix introduce a "reset" method to all FRE fragment. When pages are
revisited through backpress, we'll reset the fragment UI to how it
supposed to look like. For the case of welcome screen, we'll show the
ToS and UMA checkbox if applicable.

Change-Id: I1acf9f530efacf4650720d6c3a1f1eb83cac1823
Bug: 1192854
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2787669
Commit-Queue: Wenyu Fu <wenyufu@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#869254}
  • Loading branch information
fwy423 authored and Chromium LUCI CQ committed Apr 5, 2021
1 parent 24d8114 commit 831a331
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 3 deletions.
Expand Up @@ -562,13 +562,18 @@ private boolean setCurrentItemForPager(int position) {
return false;
}

int oldPosition = mPager.getCurrentItem();
mPager.setCurrentItem(position, false);

// Set A11y focus if possible. See https://crbug.com/1094064 for more context.
// The screen reader can lose focus when switching between pages with ViewPager2.
FirstRunFragment currentFragment = mPagerAdapter.getFirstRunFragment(position);
if (currentFragment != null) {
currentFragment.setInitialA11yFocus();
if (oldPosition > position) {
// If the fragment is revisited through back press, reset its state.
currentFragment.reset();
}
}
return true;
}
Expand Down
Expand Up @@ -38,4 +38,9 @@ default void onNativeInitialized() {}
default FirstRunPageDelegate getPageDelegate() {
return (FirstRunPageDelegate) getActivity();
}

/**
* Reset the fragment state. This can be used when the fragment is revisited with back button.
*/
default void reset() {}
}
Expand Up @@ -175,6 +175,15 @@ public void onNativeInitialized() {
tryMarkTermsAccepted(false);
}

@Override
public void reset() {
// We cannot pass the welcome page when native or policy is not initialized. When this page
// is revisited, this means this page is persist and we should re-show the ToS And UMA.
assert !isWaitingForNativeAndPolicyInit();

setSpinnerVisible(false);
}

private void onTosButtonClicked() {
mTosButtonClicked = true;
mTosAcceptedTime = SystemClock.elapsedRealtime();
Expand Down
Expand Up @@ -4,6 +4,7 @@

package org.chromium.chrome.browser.firstrun;

import static org.hamcrest.Matchers.is;
import static org.mockito.ArgumentMatchers.any;

import android.app.Activity;
Expand All @@ -14,6 +15,7 @@
import android.net.Uri;
import android.os.Bundle;
import android.support.test.InstrumentationRegistry;
import android.view.View;
import android.widget.Button;

import androidx.test.filters.MediumTest;
Expand Down Expand Up @@ -47,6 +49,8 @@
import org.chromium.chrome.browser.locale.LocaleManager;
import org.chromium.chrome.browser.locale.LocaleManager.SearchEnginePromoType;
import org.chromium.chrome.browser.policy.EnterpriseInfo;
import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import org.chromium.chrome.browser.privacy.settings.PrivacyPreferencesManagerImpl;
import org.chromium.chrome.browser.search_engines.TemplateUrlServiceFactory;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
Expand All @@ -55,6 +59,7 @@
import org.chromium.components.policy.AbstractAppRestrictionsProvider;
import org.chromium.components.search_engines.TemplateUrl;
import org.chromium.content_public.browser.UiThreadTaskTraits;
import org.chromium.content_public.browser.test.util.TestThreadUtils;

import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -142,10 +147,10 @@ private <T extends Activity> T waitForActivity(Class<T> activityClass) {
return (T) mLastActivity;
}

private void setHasAppRestrictionForMock() {
private void setHasAppRestrictionForMock(boolean hasAppRestriction) {
Mockito.doAnswer(invocation -> {
Callback<Boolean> callback = invocation.getArgument(0);
callback.onResult(true);
callback.onResult(hasAppRestriction);
return null;
})
.when(mMockAppRestrictionInfo)
Expand All @@ -165,7 +170,7 @@ private void setDeviceOwnedForMock() {
}

private void skipTosDialogViaPolicy() {
setHasAppRestrictionForMock();
setHasAppRestrictionForMock(true);
Bundle restrictions = new Bundle();
restrictions.putInt("TosDialogBehavior", TosDialogBehavior.SKIP);
AbstractAppRestrictionsProvider.setTestRestrictions(restrictions);
Expand Down Expand Up @@ -397,6 +402,38 @@ public void testFastDestroy() {
mContext.startActivity(intent);
}

@Test
@MediumTest
public void testResetOnBackPress() throws Exception {
// Inspired by crbug.com/1192854.
// When the policy initialization is finishing after ToS accepted, the small loading circle
// will be shown on the screen. If user decide to go back with backpress, the UI should be
// reset with ToS UI visible.
FirstRunStatus.setSkipWelcomePage(true);
SharedPreferencesManager.getInstance().writeBoolean(
ChromePreferenceKeys.FIRST_RUN_CACHED_TOS_ACCEPTED, true);
setHasAppRestrictionForMock(false);
FirstRunActivity freActivity = launchFirstRunActivity();

// ToS page should be skipped and jumped to the next page, since ToS is marked accepted in
// shared preference.
mTestObserver.flowIsKnownCallback.waitForCallback("Failed to finalize the flow.", 0);
CriteriaHelper.pollUiThread(
() -> Criteria.checkThat(freActivity.isNativeSideIsInitializedForTest(), is(true)));
mTestObserver.jumpToPageCallback.waitForCallback(
"ToS should be skipped after native initialized.", 0);

// Press back button.
TestThreadUtils.runOnUiThreadBlocking(() -> mLastActivity.onBackPressed());

View tosAndPrivacy = mLastActivity.findViewById(R.id.tos_and_privacy);
View umaCheckbox = mLastActivity.findViewById(R.id.send_report_checkbox);
Assert.assertNotNull(tosAndPrivacy);
Assert.assertNotNull(umaCheckbox);
Assert.assertEquals(View.VISIBLE, tosAndPrivacy.getVisibility());
Assert.assertEquals(View.VISIBLE, umaCheckbox.getVisibility());
}

private void clickButton(final Activity activity, final int id, final String message) {
CriteriaHelper.pollUiThread(
() -> Criteria.checkThat(activity.findViewById(id), Matchers.notNullValue()));
Expand Down

0 comments on commit 831a331

Please sign in to comment.