Commit 9bedb05b authored by Wenyu Fu's avatar Wenyu Fu Committed by Commit Bot

[CCTToSFRE] Post loading spinner to show/hide

Change the loading spinner to show when waiting time is longer than
500ms, and ensure the loading spinner is displayed for at least 500ms.

For simplicity, we also change the name of CCT fragment and use original
flow when the welcome page is going to be skipped.

Bug: 1108558
Change-Id: If22d74f1afe5a88b1637d4db29be137173c58f6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2338932
Commit-Queue: Wenyu Fu <wenyufu@chromium.org>
Reviewed-by: default avatarSky Malice <skym@chromium.org>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797782}
parent a9db061a
......@@ -745,7 +745,7 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/firstrun/SigninFirstRunFragment.java",
"java/src/org/chromium/chrome/browser/firstrun/TabbedModeFirstRunActivity.java",
"java/src/org/chromium/chrome/browser/firstrun/ToSAndUMAFirstRunFragment.java",
"java/src/org/chromium/chrome/browser/firstrun/TosAndUmaCctFirstRunFragment.java",
"java/src/org/chromium/chrome/browser/firstrun/TosAndUmaFirstRunFragmentWithEnterpriseSupport.java",
"java/src/org/chromium/chrome/browser/flags/ChromeSessionState.java",
"java/src/org/chromium/chrome/browser/fullscreen/BrowserControlsManager.java",
"java/src/org/chromium/chrome/browser/fullscreen/FullscreenHtmlApiHandler.java",
......
......@@ -199,7 +199,7 @@ chrome_test_java_sources = [
"javatests/src/org/chromium/chrome/browser/firstrun/FirstRunActivityTestObserver.java",
"javatests/src/org/chromium/chrome/browser/firstrun/FirstRunIntegrationTest.java",
"javatests/src/org/chromium/chrome/browser/firstrun/FirstRunUtilsTest.java",
"javatests/src/org/chromium/chrome/browser/firstrun/TosAndUmaCctFirstRunFragmentTest.java",
"javatests/src/org/chromium/chrome/browser/firstrun/TosAndUmaFirstRunFragmentWithEnterpriseSupportTest.java",
"javatests/src/org/chromium/chrome/browser/fullscreen/FullscreenManagerTest.java",
"javatests/src/org/chromium/chrome/browser/fullscreen/FullscreenManagerTestUtils.java",
"javatests/src/org/chromium/chrome/browser/gcore/MockConnectedTask.java",
......
......@@ -45,7 +45,7 @@
tools:ignore="ContentDescription"
android:src="@drawable/fre_product_logo" />
<ProgressBar
<org.chromium.components.browser_ui.widget.LoadingView
android:id="@+id/progress_spinner_large"
style="@style/Widget.AppCompat.ProgressBar"
android:layout_marginTop="@dimen/fre_vertical_spacing"
......
......@@ -42,7 +42,10 @@ import java.util.Set;
* The activity might be run more than once, e.g. 1) for ToS and sign-in, and 2) for intro.
*/
public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPageDelegate {
/** Alerted about various events when FirstRunActivity performs them. */
/**
* Alerted about various events when FirstRunActivity performs them.
* TODO(crbug.com/1114319): Rework and use a better testing setup.
* */
public interface FirstRunActivityObserver {
/** See {@link #onFlowIsKnown}. */
void onFlowIsKnown(Bundle freProperties);
......@@ -58,6 +61,9 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
/** See {@link #abortFirstRunExperience}. */
void onAbortFirstRunExperience();
/** See {@link #exitFirstRun()}. */
void onExitFirstRun();
}
// UMA constants.
......@@ -116,14 +122,23 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
* Defines a sequence of pages to be shown (depending on parameters etc).
*/
private void createPageSequence() {
mPages.add(sEnableEnterpriseCCT && mLaunchedFromCCT
? new TosAndUmaCctFirstRunFragment.Page()
mPages.add(shouldCreateEnterpriseCctTosPage()
? new TosAndUmaFirstRunFragmentWithEnterpriseSupport.Page()
: new ToSAndUMAFirstRunFragment.Page());
mFreProgressStates.add(FRE_PROGRESS_WELCOME_SHOWN);
// Other pages will be created by createPostNativePageSequence() after
// native has been initialized.
}
private boolean shouldCreateEnterpriseCctTosPage() {
// TODO(crbug.com/1111490): Revisit case when #shouldSkipWelcomePage = true.
// If the client has already accepted ToS (FirstRunStatus#shouldSkipWelcomePage), do not
// use the subclass ToSAndUmaCCTFirstRunFragment. Instead, use the base class
// (ToSAndUMAFirstRunFragment) which simply shows a loading spinner while waiting for
// native to be loaded.
return sEnableEnterpriseCCT && mLaunchedFromCCT && !FirstRunStatus.shouldSkipWelcomePage();
}
private void createPostNativePageSequence() {
// Note: Can't just use POST_NATIVE_SETUP_NEEDED for the early return, because this
// populates |mPages| which needs to be done even even if onNativeInitialized() was
......@@ -411,6 +426,8 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
}
});
}
if (sObserver != null) sObserver.onExitFirstRun();
}
@Override
......
......@@ -13,15 +13,18 @@ import androidx.annotation.VisibleForTesting;
import org.chromium.base.CallbackController;
import org.chromium.base.Log;
import org.chromium.chrome.R;
import org.chromium.components.browser_ui.widget.LoadingView;
/**
* Another FirstRunFragment that is only used when running with CCT.
*/
public class TosAndUmaCctFirstRunFragment extends ToSAndUMAFirstRunFragment {
private static final String TAG = "TosAndUmaCctFre";
public class TosAndUmaFirstRunFragmentWithEnterpriseSupport
extends ToSAndUMAFirstRunFragment implements LoadingView.Observer {
private static final String TAG = "TosAndUmaFragment";
/** FRE page that instantiates this fragment. */
public static class Page implements FirstRunPage<TosAndUmaCctFirstRunFragment> {
public static class Page
implements FirstRunPage<TosAndUmaFirstRunFragmentWithEnterpriseSupport> {
@Override
public boolean shouldSkipPageOnCreate() {
// TODO(crbug.com/1111490): Revisit during post-MVP.
......@@ -31,13 +34,13 @@ public class TosAndUmaCctFirstRunFragment extends ToSAndUMAFirstRunFragment {
}
@Override
public TosAndUmaCctFirstRunFragment instantiateFragment() {
return new TosAndUmaCctFirstRunFragment();
public TosAndUmaFirstRunFragmentWithEnterpriseSupport instantiateFragment() {
return new TosAndUmaFirstRunFragmentWithEnterpriseSupport();
}
}
private boolean mViewCreated;
private View mLargeSpinner;
private LoadingView mLoadingSpinner;
private CallbackController mCallbackController;
/**
......@@ -53,7 +56,7 @@ public class TosAndUmaCctFirstRunFragment extends ToSAndUMAFirstRunFragment {
private static boolean sBlockPolicyLoadingForTest;
private TosAndUmaCctFirstRunFragment() {
private TosAndUmaFirstRunFragmentWithEnterpriseSupport() {
mCallbackController = new CallbackController();
checkAppRestriction();
}
......@@ -64,6 +67,10 @@ public class TosAndUmaCctFirstRunFragment extends ToSAndUMAFirstRunFragment {
mCallbackController.destroy();
mCallbackController = null;
}
if (mLoadingSpinner != null) {
mLoadingSpinner.destroy();
mLoadingSpinner = null;
}
super.onDestroy();
}
......@@ -71,16 +78,16 @@ public class TosAndUmaCctFirstRunFragment extends ToSAndUMAFirstRunFragment {
public void onViewCreated(View view, Bundle savedInstanceState) {
super.onViewCreated(view, savedInstanceState);
mLargeSpinner = view.findViewById(R.id.progress_spinner_large);
mLoadingSpinner = view.findViewById(R.id.progress_spinner_large);
mViewCreated = true;
if (shouldWaitForPolicyLoading()) {
// TODO(crbug.com/1106987): Post a task to show the LargeSpinner after 500ms and make
// sure it appears 500 ms at least.
mLargeSpinner.setVisibility(View.VISIBLE);
mLoadingSpinner.addObserver(this);
mLoadingSpinner.showLoadingUI();
setTosAndUmaVisible(false);
} else {
updateViewOnEnterpriseChecksComplete();
} else if (confirmedCctTosDialogDisabled()) {
// Skip the FRE if we know dialog is disabled by policy.
exitCctFirstRun();
}
}
......@@ -95,7 +102,19 @@ public class TosAndUmaCctFirstRunFragment extends ToSAndUMAFirstRunFragment {
@Override
protected boolean canShowUmaCheckBox() {
return super.canShowUmaCheckBox() && shouldShowUmaAndTos();
return super.canShowUmaCheckBox() && confirmedToShowUmaAndTos();
}
@Override
public void onHideLoadingUIComplete() {
if (confirmedCctTosDialogDisabled()) {
// TODO(crbug.com/1108564): Show the different UI that has the enterprise disclosure.
exitCctFirstRun();
} else {
// Else, show the UMA as the loading spinner is GONE.
assert confirmedToShowUmaAndTos();
setTosAndUmaVisible(true);
}
}
/**
......@@ -104,12 +123,29 @@ public class TosAndUmaCctFirstRunFragment extends ToSAndUMAFirstRunFragment {
* and can update the UI immediately.
*/
private boolean shouldWaitForPolicyLoading() {
return (mHasRestriction == null || mHasRestriction) && mPolicyCctTosDialogEnabled == null;
return !confirmedNoAppRestriction() && mPolicyCctTosDialogEnabled == null;
}
/**
* This methods will return true only when we know either 1) there's no on-device app
* restrictions or 2) policies has been loaded and first run has not been disabled via policy.
*
* @return Whether we should show TosAndUma components on the UI.
*/
private boolean confirmedToShowUmaAndTos() {
return confirmedNoAppRestriction() || confirmedCctTosDialogEnabled();
}
private boolean shouldShowUmaAndTos() {
return (mHasRestriction != null && !mHasRestriction)
|| (mPolicyCctTosDialogEnabled != null && mPolicyCctTosDialogEnabled);
private boolean confirmedNoAppRestriction() {
return mHasRestriction != null && !mHasRestriction;
}
private boolean confirmedCctTosDialogEnabled() {
return mPolicyCctTosDialogEnabled != null && mPolicyCctTosDialogEnabled;
}
private boolean confirmedCctTosDialogDisabled() {
return mPolicyCctTosDialogEnabled != null && !mPolicyCctTosDialogEnabled;
}
private void checkAppRestriction() {
......@@ -123,7 +159,7 @@ public class TosAndUmaCctFirstRunFragment extends ToSAndUMAFirstRunFragment {
if (!shouldWaitForPolicyLoading() && mViewCreated) {
// TODO(crbug.com/1106812): Unregister policy listener.
updateViewOnEnterpriseChecksComplete();
mLoadingSpinner.hideLoadingUI();
}
}
......@@ -142,34 +178,18 @@ public class TosAndUmaCctFirstRunFragment extends ToSAndUMAFirstRunFragment {
@VisibleForTesting
void onCctTosPolicyDetected(boolean cctTosDialogEnabled) {
mPolicyCctTosDialogEnabled = cctTosDialogEnabled;
updateViewOnEnterpriseChecksComplete();
}
/**
* Update the UI based on aggregated signal whether ToS / UMA should be shown.
*/
private void updateViewOnEnterpriseChecksComplete() {
if (shouldShowUmaAndTos()) {
mLargeSpinner.setVisibility(View.GONE);
setTosAndUmaVisible(true);
return;
if (mViewCreated) {
mLoadingSpinner.hideLoadingUI();
}
assert mPolicyCctTosDialogEnabled != null && !mPolicyCctTosDialogEnabled;
// TODO(crbug.com/1108564): Show the different UI that has the enterprise disclosure.
// TODO(crbug.com/1106987): Post a task to show the LargeSpinner after 500ms and make sure
// it appears 500 ms at least.
mLargeSpinner.setVisibility(View.GONE);
exitCctFirstRun();
}
@VisibleForTesting
void exitCctFirstRun() {
assert confirmedCctTosDialogDisabled();
// TODO(crbug.com/1108564): Fire a signal to end this fragment when disclaimer is ready.
// TODO(crbug.com/1108582): Save a shared pref indicating Enterprise CCT FRE is complete,
// and skip waiting for future cold starts.
Log.d(TAG, "ToSAndUMACCTFirstRunFragment finished.");
Log.d(TAG, "TosAndUmaFirstRunFragmentWithEnterpriseSupport finished.");
getPageDelegate().exitFirstRun();
}
......
......@@ -15,6 +15,7 @@ public class FirstRunActivityTestObserver implements FirstRunActivity.FirstRunAc
public final CallbackHelper jumpToPageCallback = new CallbackHelper();
public final CallbackHelper updateCachedEngineCallback = new CallbackHelper();
public final CallbackHelper abortFirstRunExperienceCallback = new CallbackHelper();
public final CallbackHelper exitFirstRunCallback = new CallbackHelper();
public Bundle freProperties;
......@@ -43,4 +44,9 @@ public class FirstRunActivityTestObserver implements FirstRunActivity.FirstRunAc
public void onAbortFirstRunExperience() {
abortFirstRunExperienceCallback.notifyCalled();
}
@Override
public void onExitFirstRun() {
exitFirstRunCallback.notifyCalled();
}
}
\ No newline at end of file
......@@ -17,6 +17,7 @@ import android.view.View;
import androidx.annotation.IntDef;
import androidx.test.filters.SmallTest;
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
......@@ -33,6 +34,7 @@ import org.chromium.chrome.R;
import org.chromium.chrome.browser.customtabs.CustomTabsTestUtils;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.content_public.browser.test.util.Criteria;
import org.chromium.content_public.browser.test.util.CriteriaHelper;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.ui.test.util.DisableAnimationsTestRule;
......@@ -41,12 +43,12 @@ import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
/**
* Test for first run activity and {@link TosAndUmaCctFirstRunFragment}.
* Test for first run activity and {@link TosAndUmaFirstRunFragmentWithEnterpriseSupport}.
* For the outside signals that used in this test so that the verification is focusing on the
* workflow and UI transition.
*/
@RunWith(ChromeJUnit4ClassRunner.class)
public class TosAndUmaCctFirstRunFragmentTest {
public class TosAndUmaFirstRunFragmentWithEnterpriseSupportTest {
@IntDef({FragmentState.LOADING, FragmentState.NO_POLICY, FragmentState.HAS_POLICY})
@Retention(RetentionPolicy.SOURCE)
@interface FragmentState {
......@@ -63,7 +65,7 @@ public class TosAndUmaCctFirstRunFragmentTest {
private FirstRunActivityTestObserver mTestObserver = new FirstRunActivityTestObserver();
private FirstRunActivity mActivity;
private TosAndUmaCctFirstRunFragment mFragment;
private TosAndUmaFirstRunFragmentWithEnterpriseSupport mFragment;
private View mTosText;
private View mAcceptButton;
......@@ -79,14 +81,14 @@ public class TosAndUmaCctFirstRunFragmentTest {
// Static switches.
FirstRunActivity.setEnableEnterpriseCCTForTest(true);
FirstRunAppRestrictionInfo.setInstanceForTest(mMockAppRestrictionInfo);
TosAndUmaCctFirstRunFragment.setBlockPolicyLoadingForTest(true);
TosAndUmaFirstRunFragmentWithEnterpriseSupport.setBlockPolicyLoadingForTest(true);
}
@After
public void tearDown() {
FirstRunActivity.setEnableEnterpriseCCTForTest(false);
FirstRunAppRestrictionInfo.setInstanceForTest(null);
TosAndUmaCctFirstRunFragment.setBlockPolicyLoadingForTest(false);
TosAndUmaFirstRunFragmentWithEnterpriseSupport.setBlockPolicyLoadingForTest(false);
if (mActivity != null) mActivity.finish();
}
......@@ -109,9 +111,6 @@ public class TosAndUmaCctFirstRunFragmentTest {
waitUntilNativeLoaded();
// TODO(crbug.com/1108118): In our current test setup, #onCctPolicyDetected will actually
// called inside #onNativeInitialized. When we decouple these two signals, we should update
// the test accordingly.
TestThreadUtils.runOnUiThreadBlocking(() -> mFragment.onCctTosPolicyDetected(true));
assertUIState(FragmentState.NO_POLICY);
}
......@@ -128,7 +127,7 @@ public class TosAndUmaCctFirstRunFragmentTest {
TestThreadUtils.runOnUiThreadBlocking(() -> mFragment.onCctTosPolicyDetected(false));
assertUIState(FragmentState.HAS_POLICY);
Mockito.verify(mFragment).exitCctFirstRun();
CriteriaHelper.pollUiThread(() -> mTestObserver.exitFirstRunCallback.getCallCount() > 0);
}
/**
......@@ -162,32 +161,30 @@ public class TosAndUmaCctFirstRunFragmentTest {
CriteriaHelper.pollUiThread(
() -> mActivity.getSupportFragmentManager().getFragments().size() > 0);
mFragment = Mockito.spy((TosAndUmaCctFirstRunFragment) mActivity.getSupportFragmentManager()
.getFragments()
.get(0));
mFragment = (TosAndUmaFirstRunFragmentWithEnterpriseSupport) mActivity
.getSupportFragmentManager()
.getFragments()
.get(0);
mTosText = mActivity.findViewById(R.id.tos_and_privacy);
mAcceptButton = mActivity.findViewById(R.id.tos_and_privacy);
mLargeSpinner = mActivity.findViewById(R.id.progress_spinner_large);
}
private void assertUIState(@FragmentState int fragmentState) {
int tosVisibility = View.INVISIBLE;
int spinnerVisibility = View.GONE;
int tosVisibility =
(fragmentState == FragmentState.NO_POLICY) ? View.VISIBLE : View.INVISIBLE;
int spinnerVisibility = (fragmentState == FragmentState.LOADING) ? View.VISIBLE : View.GONE;
if (fragmentState == FragmentState.NO_POLICY) {
tosVisibility = View.VISIBLE;
}
if (fragmentState == FragmentState.LOADING) {
spinnerVisibility = View.VISIBLE;
}
CriteriaHelper.pollUiThread(
()
-> Criteria.checkThat(
"Visibility of Loading spinner never reached test setting.",
mLargeSpinner.getVisibility(), Matchers.is(spinnerVisibility)));
Assert.assertEquals("Visibility of ToS text is different than the test setting.",
tosVisibility, mTosText.getVisibility());
Assert.assertEquals("Visibility of accept button is different than the test setting.",
tosVisibility, mAcceptButton.getVisibility());
Assert.assertEquals("Visibility of Loading spinner is different than the test setting.",
spinnerVisibility, mLargeSpinner.getVisibility());
}
/** Set up mock FirstRunAppRestrictionInfo that there is app restriction on the device */
......
......@@ -113,6 +113,9 @@ public class FirstRunTest {
@Override
public void onAbortFirstRunExperience() {}
@Override
public void onExitFirstRun() {}
}
private final TestObserver mTestObserver = new TestObserver();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment