[FirstRun] Waiting until policy loaded before leaving welcome page

Wait on the welcome page if policy exists on device. This applies to all
the FRE scenarios, and paves the road for the following pages to load
enterprise policies without additional logics to wait on loading.

This change also makes |BrowserSignin| policy works as expected. First
run fragments will be added to pager after policy services is
initialized,  so SignInManager#isSyncAllowed will be populated with
value that matches the policy setting. If the |BrowserSignin| is set to
disabled, the sign in fragment will not be added into FRE.

Change-Id: I88942f50093bc77432981ebc78042d777bb0bcaf
Bug: 1155715
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2601329
Commit-Queue: Wenyu Fu <wenyufu@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843060}

chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java

@@ -91,7 +91,7 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
     private boolean mResultShowSignInSettings;
 
     private boolean mFlowIsKnown;
-    private boolean mPostNativePageSequenceCreated;
+    private boolean mPostNativeAndPolicyPagesCreated;
     private boolean mNativeSideIsInitialized;
     private Set<FirstRunFragment> mPagesToNotifyOfNativeInit;
     private boolean mDeferredCompleteFRE;
@@ -115,8 +115,6 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
      */
     private long mIntentCreationElapsedRealtimeMs;
 
-    private final FirstRunAppRestrictionInfo mFirstRunAppRestrictionInfo;
-
     private final List<FirstRunPage> mPages = new ArrayList<>();
     private final List<Integer> mFreProgressStates = new ArrayList<>();
 
@@ -125,10 +123,6 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
      */
     private FirstRunPagerAdapter mPagerAdapter;
 
-    public FirstRunActivity() {
-        mFirstRunAppRestrictionInfo = FirstRunAppRestrictionInfo.takeMaybeInitialized();
-    }
-
     /**
      * Defines a sequence of pages to be shown (depending on parameters etc).
      */
@@ -137,8 +131,8 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
                         ? new TosAndUmaFirstRunFragmentWithEnterpriseSupport.Page()
                         : new ToSAndUMAFirstRunFragment.Page());
         mFreProgressStates.add(FRE_PROGRESS_WELCOME_SHOWN);
-        // Other pages will be created by createPostNativePageSequence() after
-        // native has been initialized.
+        // Other pages will be created by createPostNativeAndPoliciesPageSequence() after
+        // native and policy service have been initialized.
     }
 
     private boolean shouldCreateEnterpriseCctTosPage() {
@@ -150,12 +144,20 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
         return mLaunchedFromCCT && !FirstRunStatus.shouldSkipWelcomePage();
     }
 
-    private void createPostNativePageSequence() {
+    /**
+     * Create the page sequence which requires native initialized, and policies loaded if any
+     * on-device policies may exists.
+     *
+     * @see #areNativeAndPoliciesInitialized()
+     */
+    private void createPostNativeAndPoliciesPageSequence() {
         // 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
-        // performed in a previous session.
-        if (mPostNativePageSequenceCreated) return;
-        mFirstRunFlowSequencer.onNativeInitialized(mFreProperties);
+        // populates |mPages| which needs to be done even if onNativeDependenciesFullyInitialized()
+        // was performed in a previous session.
+        if (mPostNativeAndPolicyPagesCreated) return;
+
+        assert areNativeAndPoliciesInitialized();
+        mFirstRunFlowSequencer.onNativeAndPoliciesInitialized(mFreProperties);
 
         boolean notifyAdapter = false;
         // An optional Data Saver page.
@@ -182,7 +184,7 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
         if (notifyAdapter && mPagerAdapter != null) {
             mPagerAdapter.notifyDataSetChanged();
         }
-        mPostNativePageSequenceCreated = true;
+        mPostNativeAndPolicyPagesCreated = true;
     }
 
     @Override
@@ -234,8 +236,8 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
                 }
 
                 createPageSequence();
-                if (mNativeSideIsInitialized) {
-                    createPostNativePageSequence();
+                if (areNativeAndPoliciesInitialized()) {
+                    createPostNativeAndPoliciesPageSequence();
                 }
 
                 if (mPages.size() == 0) {
@@ -247,7 +249,7 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
                 stopProgressionIfNotAcceptedTermsOfService();
                 mPager.setAdapter(mPagerAdapter);
 
-                if (mNativeSideIsInitialized) {
+                if (areNativeAndPoliciesInitialized()) {
                     skipPagesIfNecessary();
                 }
 
@@ -256,7 +258,7 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
                 long inflationCompletion = SystemClock.elapsedRealtime();
                 RecordHistogram.recordTimesHistogram("MobileFre.FromLaunch.FirstFragmentInflatedV2",
                         inflationCompletion - mIntentCreationElapsedRealtimeMs);
-                mFirstRunAppRestrictionInfo.getCompletionElapsedRealtimeMs(
+                getFirstRunAppRestrictionInfo().getCompletionElapsedRealtimeMs(
                         restrictionsCompletion -> {
                             if (restrictionsCompletion > inflationCompletion) {
                                 RecordHistogram.recordTimesHistogram(
@@ -294,29 +296,47 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
         TemplateUrlServiceFactory.get().runWhenLoaded(onNativeFinished);
     }
 
-    public boolean isNativeSideIsInitializedForTest() {
-        return mNativeSideIsInitialized;
-    }
-
     private void onNativeDependenciesFullyInitialized() {
         mNativeSideIsInitialized = true;
         if (mDeferredCompleteFRE) {
             completeFirstRunExperience();
             mDeferredCompleteFRE = false;
         } else if (mFlowIsKnown) {
+            boolean doCreatePostPolicyPageSequence = areNativeAndPoliciesInitialized();
             // Note: If mFlowIsKnown is false, then we're not ready to create the post native page
             // sequence - in that case this will be done when onFlowIsKnown() gets called.
-            createPostNativePageSequence();
+            if (doCreatePostPolicyPageSequence) {
+                createPostNativeAndPoliciesPageSequence();
+            }
+
             if (mPagesToNotifyOfNativeInit != null) {
                 for (FirstRunFragment page : mPagesToNotifyOfNativeInit) {
                     page.onNativeInitialized();
                 }
+                mPagesToNotifyOfNativeInit = null;
             }
-            mPagesToNotifyOfNativeInit = null;
+
+            if (doCreatePostPolicyPageSequence) {
+                skipPagesIfNecessary();
+            }
+        }
+    }
+
+    @Override
+    protected void onPolicyLoadListenerAvailable(boolean onDevicePolicyFound) {
+        super.onPolicyLoadListenerAvailable(onDevicePolicyFound);
+
+        if (areNativeAndPoliciesInitialized()) {
+            createPostNativeAndPoliciesPageSequence();
             skipPagesIfNecessary();
         }
     }
 
+    private boolean areNativeAndPoliciesInitialized() {
+        return mNativeSideIsInitialized && mFlowIsKnown
+                && this.getPolicyLoadListener().get() != null;
+    }
+
     // Activity:
 
     @Override
@@ -352,14 +372,6 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
         stopProgressionIfNotAcceptedTermsOfService();
     }
 
-    @Override
-    public void onDestroy() {
-        super.onDestroy();
-
-        // As first run is complete, we no longer need FirstRunAppRestrictionInfo.
-        mFirstRunAppRestrictionInfo.destroy();
-    }
-
     @Override
     public void onBackPressed() {
         // Terminate if we are still waiting for the native or for Android EDU / GAIA Child checks.
@@ -604,9 +616,9 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
                 this, LocalizationUtils.substituteLocalePlaceholder(getString(url)));
     }
 
-    @Override
-    public FirstRunAppRestrictionInfo getFirstRunAppRestrictionInfo() {
-        return mFirstRunAppRestrictionInfo;
+    @VisibleForTesting
+    public boolean isNativeSideIsInitializedForTest() {
+        return mNativeSideIsInitialized;
     }
 
     @VisibleForTesting

chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivityBase.java

@@ -9,13 +9,19 @@ import android.app.PendingIntent;
 import android.app.PendingIntent.CanceledException;
 import android.content.Intent;
 import android.os.Bundle;
+import android.os.SystemClock;
 
 import org.chromium.base.IntentUtils;
 import org.chromium.base.Log;
+import org.chromium.base.metrics.RecordHistogram;
+import org.chromium.base.supplier.OneshotSupplier;
+import org.chromium.base.supplier.OneshotSupplierImpl;
 import org.chromium.chrome.browser.customtabs.CustomTabsConnection;
 import org.chromium.chrome.browser.init.AsyncInitializationActivity;
 import org.chromium.chrome.browser.metrics.UmaUtils;
+import org.chromium.chrome.browser.policy.PolicyServiceFactory;
 import org.chromium.chrome.browser.profiles.ProfileManagerUtils;
+import org.chromium.components.policy.PolicyService;
 
 /** Base class for First Run Experience. */
 public abstract class FirstRunActivityBase extends AsyncInitializationActivity {
@@ -42,6 +48,22 @@ public abstract class FirstRunActivityBase extends AsyncInitializationActivity {
 
     private boolean mNativeInitialized;
 
+    private final FirstRunAppRestrictionInfo mFirstRunAppRestrictionInfo;
+    private final OneshotSupplierImpl<PolicyService> mPolicyServiceSupplier;
+    private final PolicyLoadListener mPolicyLoadListener;
+
+    private final long mStartTime;
+    private long mNativeInitializedTime;
+
+    public FirstRunActivityBase() {
+        mFirstRunAppRestrictionInfo = FirstRunAppRestrictionInfo.takeMaybeInitialized();
+        mPolicyServiceSupplier = new OneshotSupplierImpl<>();
+        mPolicyLoadListener =
+                new PolicyLoadListener(mFirstRunAppRestrictionInfo, mPolicyServiceSupplier);
+        mStartTime = SystemClock.elapsedRealtime();
+        mPolicyLoadListener.onAvailable(this::onPolicyLoadListenerAvailable);
+    }
+
     @Override
     protected boolean requiresFirstRunToBeCompleted(Intent intent) {
         // The user is already in First Run.
@@ -77,6 +99,18 @@ public abstract class FirstRunActivityBase extends AsyncInitializationActivity {
     public void finishNativeInitialization() {
         super.finishNativeInitialization();
         mNativeInitialized = true;
+        mNativeInitializedTime = SystemClock.elapsedRealtime();
+        RecordHistogram.recordTimesHistogram(
+                "MobileFre.NativeInitialized", mNativeInitializedTime - mStartTime);
+        mPolicyServiceSupplier.set(PolicyServiceFactory.getGlobalPolicyService());
+    }
+
+    @Override
+    protected void onDestroy() {
+        super.onDestroy();
+
+        mPolicyLoadListener.destroy();
+        mFirstRunAppRestrictionInfo.destroy();
     }
 
     protected void flushPersistentData() {
@@ -125,6 +159,26 @@ public abstract class FirstRunActivityBase extends AsyncInitializationActivity {
         return false;
     }
 
+    protected FirstRunAppRestrictionInfo getFirstRunAppRestrictionInfo() {
+        return mFirstRunAppRestrictionInfo;
+    }
+
+    protected void onPolicyLoadListenerAvailable(boolean onDevicePolicyFound) {
+        long delayAfterNative = Math.max(0, SystemClock.elapsedRealtime() - mNativeInitializedTime);
+        String histogramName = onDevicePolicyFound
+                ? "MobileFre.PolicyServiceInitDelayAfterNative.WithPolicy"
+                : "MobileFre.PolicyServiceInitDelayAfterNative.WithoutPolicy";
+        RecordHistogram.recordTimesHistogram(histogramName, delayAfterNative);
+    }
+
+    /**
+     * @return PolicyLoadListener used to indicate if policy initialization is complete.
+     * @see PolicyLoadListener for return value expectation.
+     */
+    public OneshotSupplier<Boolean> getPolicyLoadListener() {
+        return mPolicyLoadListener;
+    }
+
     /**
      * If the first run activity was triggered by a custom tab, notify app associated with
      * custom tab whether first run was completed.

chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencer.java

@@ -166,10 +166,11 @@ public abstract class FirstRunFlowSequencer  {
     }
 
     /**
-     * Called onNativeInitialized() a given flow as completed.
+     * Will be called either when policies are initialized, or when native is initialized if we have
+     * no on-device policies.
      * @param freProperties Resulting FRE properties bundle.
      */
-    public void onNativeInitialized(Bundle freProperties) {
+    public void onNativeAndPoliciesInitialized(Bundle freProperties) {
         // We show the sign-in page if sync is allowed, and not signed in, and
         // - no "skip the first use hints" is set, or
         // - "skip the first use hints" is set, but there is at least one account.

chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunPageDelegate.java

@@ -6,6 +6,8 @@ package org.chromium.chrome.browser.firstrun;
 
 import android.os.Bundle;
 
+import org.chromium.base.supplier.OneshotSupplier;
+
 /**
  * Defines the host interface for First Run Experience pages.
  */
@@ -74,6 +76,9 @@ public interface FirstRunPageDelegate {
      */
     void showInfoPage(int url);
 
-    /** Returns the provider of whether the device has app restrictions. */
-    FirstRunAppRestrictionInfo getFirstRunAppRestrictionInfo();
+    /**
+     * The supplier that supplies whether reading policy value is necessary.
+     * See {@link PolicyLoadListener} for details.
+     */
+    OneshotSupplier<Boolean> getPolicyLoadListener();
 }

chrome/android/java/src/org/chromium/chrome/browser/firstrun/LightweightFirstRunActivity.java

@@ -22,13 +22,10 @@ import androidx.annotation.VisibleForTesting;
 import org.chromium.base.IntentUtils;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.metrics.RecordHistogram;
-import org.chromium.base.supplier.OneshotSupplierImpl;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.customtabs.CustomTabActivity;
 import org.chromium.chrome.browser.policy.EnterpriseInfo;
-import org.chromium.chrome.browser.policy.PolicyServiceFactory;
 import org.chromium.components.browser_ui.widget.LoadingView;
-import org.chromium.components.policy.PolicyService;
 import org.chromium.components.signin.ChildAccountStatus;
 import org.chromium.ui.base.LocalizationUtils;
 import org.chromium.ui.text.NoUnderlineClickableSpan;
@@ -44,9 +41,7 @@ public class LightweightFirstRunActivity
     // @Nullable from members below.
     private static boolean sSupportSkippingTos = true;
 
-    private @Nullable FirstRunAppRestrictionInfo mFirstRunAppRestrictionInfo;
     private @Nullable SkipTosDialogPolicyListener mSkipTosDialogPolicyListener;
-    private @Nullable OneshotSupplierImpl<PolicyService> mPolicyServiceSupplier;
 
     private FirstRunFlowSequencer mFirstRunFlowSequencer;
     private TextView mTosAndPrivacyTextView;
@@ -68,16 +63,14 @@ public class LightweightFirstRunActivity
             "org.chromium.chrome.browser.firstrun.AssociatedAppName";
 
     public LightweightFirstRunActivity() {
-        if (sSupportSkippingTos) {
-            mFirstRunAppRestrictionInfo = FirstRunAppRestrictionInfo.takeMaybeInitialized();
-            mPolicyServiceSupplier = new OneshotSupplierImpl<>();
+        super();
 
-            mSkipTosDialogPolicyListener = new SkipTosDialogPolicyListener(
-                    mFirstRunAppRestrictionInfo, mPolicyServiceSupplier,
+        if (sSupportSkippingTos) {
+            mSkipTosDialogPolicyListener = new SkipTosDialogPolicyListener(getPolicyLoadListener(),
                     EnterpriseInfo.getInstance(), new LightWeightTosDialogMetricsNameProvider());
             // We can ignore the result from #onAvailable here, as views are not created at this
             // point.
-            mSkipTosDialogPolicyListener.onAvailable((b) -> onPolicyLoadListenerAvailable());
+            mSkipTosDialogPolicyListener.onAvailable((ignored) -> onPolicyLoadListenerAvailable());
         }
     }
 
@@ -211,13 +204,6 @@ public class LightweightFirstRunActivity
         assert !mNativeInitialized;
 
         mNativeInitialized = true;
-
-        boolean isPolicyServiceNeeded =
-                mSkipTosDialogPolicyListener != null && mSkipTosDialogPolicyListener.get() == null;
-
-        if (mPolicyServiceSupplier != null && isPolicyServiceNeeded) {
-            mPolicyServiceSupplier.set(PolicyServiceFactory.getGlobalPolicyService());
-        }
         if (mTriggerAcceptAfterNativeInit) acceptTermsOfService();
     }
 
@@ -232,8 +218,6 @@ public class LightweightFirstRunActivity
 
         mLoadingView.destroy();
 
-        // As first run is complete, we no longer need FirstRunAppRestrictionInfo.
-        if (mFirstRunAppRestrictionInfo != null) mFirstRunAppRestrictionInfo.destroy();
         if (mSkipTosDialogPolicyListener != null) mSkipTosDialogPolicyListener.destroy();
 
         if (mHandler != null && mExitFreRunnable != null) {

chrome/android/java/src/org/chromium/chrome/browser/firstrun/SkipTosDialogPolicyListener.java

@@ -53,13 +53,18 @@ class SkipTosDialogPolicyListener implements OneshotSupplier<Boolean> {
         String getOnPolicyAvailableTimeHistogramName();
     }
 
-    private final CallbackController mCallbackController;
-    private final OneshotSupplierImpl<Boolean> mSkipTosDialogPolicySupplier;
-    private final PolicyLoadListener mPolicyLoadListener;
+    private final CallbackController mCallbackController = new CallbackController();
+    private final OneshotSupplierImpl<Boolean> mSkipTosDialogPolicySupplier =
+            new OneshotSupplierImpl<>();
     private final long mObjectCreatedTimeMs;
 
     private final @Nullable HistogramNameProvider mHistNameProvider;
 
+    /**
+     * This could be null when the policy load listener is provided and owned by other components.
+     */
+    private @Nullable PolicyLoadListener mPolicyLoadListener;
+
     /**
      * The value of whether the ToS dialog is enabled on the device. If the value is false, it means
      * TosDialogBehavior policy is found and set to SKIP. This can be null when this information
@@ -82,20 +87,32 @@ class SkipTosDialogPolicyListener implements OneshotSupplier<Boolean> {
     public SkipTosDialogPolicyListener(FirstRunAppRestrictionInfo firstRunAppRestrictionInfo,
             OneshotSupplier<PolicyService> policyServiceSupplier, EnterpriseInfo enterpriseInfo,
             @Nullable HistogramNameProvider histogramNameProvider) {
-        this(new PolicyLoadListener(firstRunAppRestrictionInfo, policyServiceSupplier),
-                enterpriseInfo, histogramNameProvider);
+        mObjectCreatedTimeMs = SystemClock.elapsedRealtime();
+        mHistNameProvider = histogramNameProvider;
+        mPolicyLoadListener =
+                new PolicyLoadListener(firstRunAppRestrictionInfo, policyServiceSupplier);
+
+        initInternally(enterpriseInfo, mPolicyLoadListener);
     }
 
-    @VisibleForTesting
-    SkipTosDialogPolicyListener(PolicyLoadListener policyLoadListener,
+    /**
+     * @param policyLoadListener Supplier that provides a boolean value *whether reading policy from
+     *         policy service is necessary*. See {@link PolicyLoadListener} for more information.
+     * @param enterpriseInfo Source that provides whether device is managed.
+     * @param histogramNameProvider Provider that provides histogram names when signals are
+     *         available.
+     */
+    public SkipTosDialogPolicyListener(OneshotSupplier<Boolean> policyLoadListener,
             EnterpriseInfo enterpriseInfo, @Nullable HistogramNameProvider histogramNameProvider) {
         mObjectCreatedTimeMs = SystemClock.elapsedRealtime();
-        mSkipTosDialogPolicySupplier = new OneshotSupplierImpl<>();
-        mCallbackController = new CallbackController();
         mHistNameProvider = histogramNameProvider;
-        mPolicyLoadListener = policyLoadListener;
 
-        Boolean hasPolicy = mPolicyLoadListener.onAvailable(
+        initInternally(enterpriseInfo, policyLoadListener);
+    }
+
+    private void initInternally(
+            EnterpriseInfo enterpriseInfo, OneshotSupplier<Boolean> policyLoadListener) {
+        Boolean hasPolicy = policyLoadListener.onAvailable(
                 mCallbackController.makeCancelable(this::onPolicyLoadListenerAvailable));
         if (hasPolicy != null) {
             onPolicyLoadListenerAvailable(hasPolicy);
@@ -113,7 +130,10 @@ class SkipTosDialogPolicyListener implements OneshotSupplier<Boolean> {
      */
     public void destroy() {
         mCallbackController.destroy();
-        mPolicyLoadListener.destroy();
+        if (mPolicyLoadListener != null) {
+            mPolicyLoadListener.destroy();
+            mPolicyLoadListener = null;
+        }
     }
 
     @Override
@@ -176,4 +196,9 @@ class SkipTosDialogPolicyListener implements OneshotSupplier<Boolean> {
             mSkipTosDialogPolicySupplier.set(false);
         }
     }
+
+    @VisibleForTesting
+    public PolicyLoadListener getPolicyLoadListenerForTesting() {
+        return mPolicyLoadListener;
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/firstrun/ToSAndUMAFirstRunFragment.java

@@ -4,21 +4,24 @@
 
 package org.chromium.chrome.browser.firstrun;
 
+import android.content.Context;
 import android.content.res.Resources;
 import android.os.Bundle;
+import android.os.SystemClock;
 import android.text.method.LinkMovementMethod;
 import android.view.LayoutInflater;
 import android.view.View;
-import android.view.View.OnClickListener;
 import android.view.ViewGroup;
 import android.view.accessibility.AccessibilityEvent;
 import android.widget.Button;
 import android.widget.CheckBox;
 import android.widget.TextView;
 
+import androidx.annotation.NonNull;
 import androidx.annotation.VisibleForTesting;
 import androidx.fragment.app.Fragment;
 
+import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.version.ChromeVersionInfo;
 import org.chromium.components.signin.ChildAccountStatus;
@@ -47,14 +50,16 @@ public class ToSAndUMAFirstRunFragment extends Fragment implements FirstRunFragm
 
     private static boolean sShowUmaCheckBoxForTesting;
 
-    protected boolean mNativeInitialized;
+    private boolean mNativeInitialized;
+    private boolean mTosButtonClicked;
 
     private Button mAcceptButton;
     private CheckBox mSendReportCheckBox;
     private TextView mTosAndPrivacy;
     private View mTitle;
     private View mProgressSpinner;
-    private boolean mTriggerAcceptAfterNativeInit;
+
+    private long mTosAcceptedTime;
 
     @Override
     public View onCreateView(
@@ -62,6 +67,13 @@ public class ToSAndUMAFirstRunFragment extends Fragment implements FirstRunFragm
         return inflater.inflate(R.layout.fre_tosanduma, container, false);
     }
 
+    @Override
+    public void onAttach(@NonNull Context context) {
+        super.onAttach(context);
+        getPageDelegate().getPolicyLoadListener().onAvailable(
+                (ignored) -> tryMarkTermsAccepted(false));
+    }
+
     @Override
     public void onViewCreated(View view, Bundle savedInstanceState) {
         super.onViewCreated(view, savedInstanceState);
@@ -73,12 +85,7 @@ public class ToSAndUMAFirstRunFragment extends Fragment implements FirstRunFragm
         mSendReportCheckBox = (CheckBox) view.findViewById(R.id.send_report_checkbox);
         mTosAndPrivacy = (TextView) view.findViewById(R.id.tos_and_privacy);
 
-        mAcceptButton.setOnClickListener(new OnClickListener() {
-            @Override
-            public void onClick(View v) {
-                acceptTermsOfService();
-            }
-        });
+        mAcceptButton.setOnClickListener((v) -> onTosButtonClicked());
 
         mSendReportCheckBox.setChecked(FirstRunActivity.DEFAULT_METRICS_AND_CRASH_REPORTING);
         if (!canShowUmaCheckBox()) {
@@ -130,7 +137,7 @@ public class ToSAndUMAFirstRunFragment extends Fragment implements FirstRunFragm
         // initialized at which point the activity will skip the page.
         // We distinguish case 1 from case 2 by the value of |mNativeInitialized|, as that is set
         // via onAttachFragment() from FirstRunActivity - which is before this onViewCreated().
-        if (!mNativeInitialized && FirstRunStatus.shouldSkipWelcomePage()) {
+        if (isWaitingForNativeAndPolicyInit() && FirstRunStatus.shouldSkipWelcomePage()) {
             setSpinnerVisible(true);
         }
     }
@@ -165,17 +172,33 @@ public class ToSAndUMAFirstRunFragment extends Fragment implements FirstRunFragm
         assert !mNativeInitialized;
 
         mNativeInitialized = true;
-        if (mTriggerAcceptAfterNativeInit) acceptTermsOfService();
+        tryMarkTermsAccepted(false);
     }
 
-    private void acceptTermsOfService() {
-        if (!mNativeInitialized) {
-            mTriggerAcceptAfterNativeInit = true;
-            setSpinnerVisible(true);
+    private void onTosButtonClicked() {
+        mTosButtonClicked = true;
+        mTosAcceptedTime = SystemClock.elapsedRealtime();
+        tryMarkTermsAccepted(true);
+    }
+
+    /**
+     * This should be called Tos button is clicked for a fresh new FRE, or when native and policies
+     * are initialized if Tos has ever been accepted.
+     *
+     * @param fromButtonClicked Whether called from {@link #onTosButtonClicked()}.
+     */
+    private void tryMarkTermsAccepted(boolean fromButtonClicked) {
+        if (!mTosButtonClicked || isWaitingForNativeAndPolicyInit()) {
+            if (fromButtonClicked) setSpinnerVisible(true);
             return;
         }
 
-        mTriggerAcceptAfterNativeInit = false;
+        // In cases where the attempt is triggered other than button click, the ToS should have been
+        // accepted by the user already.
+        if (!fromButtonClicked) {
+            RecordHistogram.recordTimesHistogram("MobileFre.TosFragment.SpinnerVisibleDuration",
+                    SystemClock.elapsedRealtime() - mTosAcceptedTime);
+        }
         boolean allowCrashUpload = canShowUmaCheckBox() && mSendReportCheckBox.isChecked();
         getPageDelegate().acceptTermsOfService(allowCrashUpload);
     }
@@ -190,6 +213,10 @@ public class ToSAndUMAFirstRunFragment extends Fragment implements FirstRunFragm
         mProgressSpinner.setVisibility(spinnerVisible ? View.VISIBLE : View.GONE);
     }
 
+    private boolean isWaitingForNativeAndPolicyInit() {
+        return !mNativeInitialized || getPageDelegate().getPolicyLoadListener().get() == null;
+    }
+
     // Exposed methods for ToSAndUMACCTFirstRunFragment
 
     protected void setTosAndUmaVisible(boolean isVisible) {

chrome/android/java/src/org/chromium/chrome/browser/firstrun/TosAndUmaFirstRunFragmentWithEnterpriseSupport.java

@@ -104,10 +104,10 @@ public class TosAndUmaFirstRunFragmentWithEnterpriseSupport
         super.onAttach(context);
 
         // TODO(https://crbug.com/1143593): Replace FirstRunAppRestrictionInfo with a supplier.
-        mSkipTosDialogPolicyListener = new SkipTosDialogPolicyListener(
-                getPageDelegate().getFirstRunAppRestrictionInfo(), mPolicyServiceProvider,
-                EnterpriseInfo.getInstance(), new CctTosFragmentMetricsNameProvider());
-        mSkipTosDialogPolicyListener.onAvailable((b) -> onPolicyLoadListenerAvailable());
+        mSkipTosDialogPolicyListener =
+                new SkipTosDialogPolicyListener(getPageDelegate().getPolicyLoadListener(),
+                        EnterpriseInfo.getInstance(), new CctTosFragmentMetricsNameProvider());
+        mSkipTosDialogPolicyListener.onAvailable((ignored) -> onPolicyLoadListenerAvailable());
     }
 
     @Override

chrome/android/javatests/src/org/chromium/chrome/browser/firstrun/FirstRunIntegrationTest.java

@@ -61,6 +61,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.TimeoutException;
 
 /**
  * Integration test suite for the first run experience.
@@ -122,6 +123,18 @@ public class FirstRunIntegrationTest {
         return mMonitorMap.get(activityClass);
     }
 
+    private FirstRunActivity launchFirstRunActivity() {
+        Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse("http://test.com"));
+        intent.setPackage(mContext.getPackageName());
+        intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
+        mContext.startActivity(intent);
+
+        // Because the AsyncInitializationActivity notices that the FRE hasn't been run yet, it
+        // redirects to it.  Once the user closes the FRE, the user should be kicked back into the
+        // startup flow where they were interrupted.
+        return waitForActivity(FirstRunActivity.class);
+    }
+
     private <T extends Activity> T waitForActivity(Class<T> activityClass) {
         Assert.assertTrue(mSupportedActivities.contains(activityClass));
         ActivityMonitor monitor = getMonitor(activityClass);
@@ -238,15 +251,7 @@ public class FirstRunIntegrationTest {
         };
         LocaleManager.setInstanceForTest(mockManager);
 
-        Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse("http://test.com"));
-        intent.setPackage(mContext.getPackageName());
-        intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
-        mContext.startActivity(intent);
-
-        // Because the AsyncInitializationActivity notices that the FRE hasn't been run yet, it
-        // redirects to it.  Once the user closes the FRE, the user should be kicked back into the
-        // startup flow where they were interrupted.
-        waitForActivity(FirstRunActivity.class);
+        launchFirstRunActivity();
 
         mTestObserver.flowIsKnownCallback.waitForCallback("Failed to finalize the flow", 0);
         Bundle freProperties = mTestObserver.freProperties;
@@ -346,6 +351,20 @@ public class FirstRunIntegrationTest {
         CriteriaHelper.pollUiThread(() -> !FirstRunStatus.isFirstRunSkippedByPolicy());
     }
 
+    @Test
+    @MediumTest
+    public void testSkipTosPage() throws TimeoutException {
+        // Test case that verifies when the ToS Page is accepted before thus skipped, and FRE should
+        // transitioning to the next page.
+        FirstRunStatus.setSkipWelcomePage(true);
+
+        FirstRunActivity freActivity = launchFirstRunActivity();
+        CriteriaHelper.pollUiThread(
+                () -> freActivity.getSupportFragmentManager().getFragments().size() > 0);
+
+        mTestObserver.jumpToPageCallback.waitForCallback("Welcome page should be skipped.", 0);
+    }
+
     @Test
     @MediumTest
     // TODO(https://crbug.com/1111490): Change this test case when policy can handle cases when ToS

chrome/android/javatests/src/org/chromium/chrome/browser/firstrun/TosAndUmaFirstRunFragmentWithEnterpriseSupportTest.java

@@ -208,13 +208,9 @@ public class TosAndUmaFirstRunFragmentWithEnterpriseSupportTest {
         assertHistograms(true, SpeedComparedToInflation.SLOWER,
                 SpeedComparedToInflation.NOT_RECORDED, SpeedComparedToInflation.NOT_RECORDED);
 
-        SharedPreferencesManager.getInstance().writeBoolean(
-                ChromePreferenceKeys.PRIVACY_METRICS_REPORTING, false);
-        Assert.assertFalse("Crash report should be disabled by shared preference.",
-                PrivacyPreferencesManagerImpl.getInstance()
-                        .isUsageAndCrashReportingPermittedByUser());
 
         // Try to accept ToS.
+        setMetricsReportDisabled();
         TestThreadUtils.runOnUiThreadBlocking((Runnable) mAcceptButton::performClick);
         Assert.assertTrue("Crash report should be enabled.",
                 PrivacyPreferencesManagerImpl.getInstance()
@@ -233,13 +229,8 @@ public class TosAndUmaFirstRunFragmentWithEnterpriseSupportTest {
         assertHistograms(true, SpeedComparedToInflation.SLOWER,
                 SpeedComparedToInflation.NOT_RECORDED, SpeedComparedToInflation.NOT_RECORDED);
 
-        SharedPreferencesManager.getInstance().writeBoolean(
-                ChromePreferenceKeys.PRIVACY_METRICS_REPORTING, false);
-        Assert.assertFalse("Crash report should be disabled by shared preference.",
-                PrivacyPreferencesManagerImpl.getInstance()
-                        .isUsageAndCrashReportingPermittedByUser());
-
         // Try to accept ToS.
+        setMetricsReportDisabled();
         TestThreadUtils.runOnUiThreadBlocking((Runnable) mAcceptButton::performClick);
         assertUIState(FragmentState.WAITING_UNTIL_NEXT_PAGE);
         Assert.assertFalse("Crash report should not be enabled before native initialized.",
@@ -248,6 +239,9 @@ public class TosAndUmaFirstRunFragmentWithEnterpriseSupportTest {
 
         // ToS should be accepted when native is initialized.
         startNativeInitializationAndWait();
+        String histogram = "MobileFre.TosFragment.SpinnerVisibleDuration";
+        Assert.assertEquals(String.format("Histogram <%s> should be recorded.", histogram), 1,
+                RecordHistogram.getHistogramTotalCountForTesting(histogram));
         Assert.assertTrue("Crash report should be enabled.",
                 PrivacyPreferencesManagerImpl.getInstance()
                         .isUsageAndCrashReportingPermittedByUser());
@@ -279,7 +273,7 @@ public class TosAndUmaFirstRunFragmentWithEnterpriseSupportTest {
         assertHistograms(true, SpeedComparedToInflation.FASTER,
                 SpeedComparedToInflation.NOT_RECORDED, SpeedComparedToInflation.SLOWER);
 
-        // Try to accept Tos.
+        // Try to accept ToS.
         TestThreadUtils.runOnUiThreadBlocking((Runnable) mAcceptButton::performClick);
         Assert.assertTrue("Crash report should be enabled.",
                 PrivacyPreferencesManagerImpl.getInstance()
@@ -312,17 +306,38 @@ public class TosAndUmaFirstRunFragmentWithEnterpriseSupportTest {
 
         assertHistograms(true, SpeedComparedToInflation.FASTER, SpeedComparedToInflation.SLOWER,
                 SpeedComparedToInflation.NOT_RECORDED);
+    }
+
+    @Test
+    @SmallTest
+    public void testNotOwnedDevice_AcceptBeforePolicy() {
+        setAppRestrictionsMockInitialized(true);
+        launchFirstRunThroughCustomTab();
+        assertUIState(FragmentState.LOADING);
+
+        setEnterpriseInfoInitializedWithDeviceOwner(false);
+        assertUIState(FragmentState.NO_POLICY);
 
         // Try to accept Tos.
+        setMetricsReportDisabled();
         TestThreadUtils.runOnUiThreadBlocking((Runnable) mAcceptButton::performClick);
-        Assert.assertTrue("Crash report should be enabled.",
-                PrivacyPreferencesManagerImpl.getInstance()
-                        .isUsageAndCrashReportingPermittedByUser());
+        assertUIState(FragmentState.WAITING_UNTIL_NEXT_PAGE);
+
+        setPolicyServiceMockInitializedWithDialogEnabled(false);
+        CriteriaHelper.pollUiThread(()
+                                            -> PrivacyPreferencesManagerImpl.getInstance()
+                                                       .isUsageAndCrashReportingPermittedByUser());
+        String histogram = "MobileFre.TosFragment.SpinnerVisibleDuration";
+        Assert.assertEquals(String.format("Histogram <%s> should be recorded.", histogram), 1,
+                RecordHistogram.getHistogramTotalCountForTesting(histogram));
+
+        assertHistograms(true, SpeedComparedToInflation.FASTER, SpeedComparedToInflation.SLOWER,
+                SpeedComparedToInflation.SLOWER);
     }
 
     @Test
     @SmallTest
-    public void testNotOwnedDevice_beforeInflation() {
+    public void testNotOwnedDevice_BeforeInflation() {
         setAppRestrictionsMockInitialized(true);
         setEnterpriseInfoInitializedWithDeviceOwner(false);
 
@@ -752,6 +767,14 @@ public class TosAndUmaFirstRunFragmentWithEnterpriseSupportTest {
         });
     }
 
+    private void setMetricsReportDisabled() {
+        SharedPreferencesManager.getInstance().writeBoolean(
+                ChromePreferenceKeys.PRIVACY_METRICS_REPORTING, false);
+        Assert.assertFalse("Crash report should be disabled by shared preference.",
+                PrivacyPreferencesManagerImpl.getInstance()
+                        .isUsageAndCrashReportingPermittedByUser());
+    }
+
     private void renderWithPortraitAndLandscape(View tosAndUmaFragmentView, String testPrefix)
             throws Exception {
         mRenderTestRule.render(tosAndUmaFragmentView, testPrefix + "_portrait");

chrome/android/junit/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencerTest.java

@@ -70,7 +70,7 @@ public class FirstRunFlowSequencerTest {
         @Override
         public void onFlowIsKnown(Bundle freProperties) {
             calledOnFlowIsKnown = true;
-            if (freProperties != null) onNativeInitialized(freProperties);
+            if (freProperties != null) onNativeAndPoliciesInitialized(freProperties);
             returnedBundle = freProperties;
         }
 

chrome/android/junit/src/org/chromium/chrome/browser/firstrun/SkipTosDialogPolicyListenerUnitTest.java

@@ -18,6 +18,7 @@ import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.Spy;
+import org.mockito.internal.util.reflection.FieldSetter;
 import org.mockito.junit.MockitoJUnit;
 import org.mockito.junit.MockitoRule;
 import org.robolectric.annotation.Config;
@@ -27,9 +28,11 @@ import org.robolectric.annotation.Implements;
 import org.chromium.base.Callback;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.metrics.test.ShadowRecordHistogram;
+import org.chromium.base.supplier.OneshotSupplier;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.chrome.browser.policy.EnterpriseInfo;
 import org.chromium.chrome.browser.policy.EnterpriseInfo.OwnedState;
+import org.chromium.components.policy.PolicyService;
 
 /**
  * Unit tests for {@link SkipTosDialogPolicyListener}.
@@ -85,7 +88,7 @@ public class SkipTosDialogPolicyListenerUnitTest {
     @Spy
     public TestHistNameProvider mHistogramNameProvider;
     @Mock
-    public PolicyLoadListener mMockPolicyLoadListener;
+    public OneshotSupplier<Boolean> mMockPolicyLoadListener;
     @Mock
     public EnterpriseInfo mMockEnterpriseInfo;
 
@@ -208,7 +211,6 @@ public class SkipTosDialogPolicyListenerUnitTest {
     @Test
     public void testDestroy_PolicyNotFound() {
         mSkipTosDialogPolicyListener.destroy();
-        Mockito.verify(mMockPolicyLoadListener).destroy();
 
         // Policy signal should be ignored after destroy.
         mPolicyLoadListenerCallback.onResult(false);
@@ -219,7 +221,6 @@ public class SkipTosDialogPolicyListenerUnitTest {
     @Test
     public void testDestroy_DeviceNotOwned() {
         mSkipTosDialogPolicyListener.destroy();
-        Mockito.verify(mMockPolicyLoadListener).destroy();
 
         // Device owned signal should be ignored after destroy.
         setDeviceFullyManaged(false);
@@ -230,7 +231,6 @@ public class SkipTosDialogPolicyListenerUnitTest {
     @Test
     public void testDestroy_SkipTosDialog() {
         mSkipTosDialogPolicyListener.destroy();
-        Mockito.verify(mMockPolicyLoadListener).destroy();
 
         setDeviceFullyManaged(true);
         assertPolicyCheckNotComplete();
@@ -337,6 +337,30 @@ public class SkipTosDialogPolicyListenerUnitTest {
                         HIST_POLICY_LOAD_LISTENER_AVAILABLE));
     }
 
+    @Test
+    public void testCreateAndOwnPolicyLoadListener() throws NoSuchFieldException {
+        FirstRunAppRestrictionInfo mockAppRestrictionInfo =
+                Mockito.mock(FirstRunAppRestrictionInfo.class);
+        OneshotSupplier<PolicyService> mockSupplier =
+                (OneshotSupplier<PolicyService>) Mockito.mock(OneshotSupplier.class);
+
+        SkipTosDialogPolicyListener targetListener = new SkipTosDialogPolicyListener(
+                mockAppRestrictionInfo, mockSupplier, mMockEnterpriseInfo, null);
+
+        Assert.assertNotNull(
+                "SkipTosDialogPolicyListener should create and own a PolicyLoadListener.",
+                targetListener.getPolicyLoadListenerForTesting());
+
+        PolicyLoadListener spyListener =
+                Mockito.spy(targetListener.getPolicyLoadListenerForTesting());
+        FieldSetter.setField(targetListener,
+                SkipTosDialogPolicyListener.class.getDeclaredField("mPolicyLoadListener"),
+                spyListener);
+
+        targetListener.destroy();
+        Mockito.verify(spyListener).destroy();
+    }
+
     private void assertTosDialogEnabled() {
         Assert.assertFalse("ToS dialog should be enabled.", mSkipTosDialogPolicyListener.get());
         Mockito.verify(mTosDialogCallback).onResult(false);

tools/metrics/histograms/histograms_xml/mobile/histograms.xml

@@ -822,6 +822,34 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="MobileFre.NativeInitialized" units="ms"
+    expires_after="2021-07-31">
+  <owner>skym@chromium.org</owner>
+  <owner>wenyufu@chromium.org</owner>
+  <summary>
+    Android: The amount of time spent between FRE started and native
+    initialization. Recorded when native is fully initialized.
+  </summary>
+</histogram>
+
+<histogram name="MobileFre.PolicyServiceInitDelayAfterNative{PolicyPresence}"
+    units="ms" expires_after="2021-07-31">
+  <owner>skym@chromium.org</owner>
+  <owner>wenyufu@chromium.org</owner>
+  <summary>
+    Android: The amount of time spent between native initialized and the
+    decision &quot;whether we need to wait to load policy service from
+    native&quot; has been made. The decision is made when no on-device policy is
+    found, or policy service is fully initialized. If the decision has been made
+    before native initialization, 0 will be recorded. Recorded during FRE only,
+    when on-device policy is {PolicyPresence}
+  </summary>
+  <token key="PolicyPresence">
+    <variant name=".WithoutPolicy" summary="not presented."/>
+    <variant name=".WithPolicy" summary="presented."/>
+  </token>
+</histogram>
+
 <histogram name="MobileFre.Progress" enum="MobileFreProgress"
     expires_after="2021-06-20">
   <owner>bsazonov@chromium.org</owner>
@@ -846,6 +874,16 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="MobileFre.TosFragment.SpinnerVisibleDuration" units="ms"
+    expires_after="2021-07-31">
+  <owner>skym@chromium.org</owner>
+  <owner>wenyufu@chromium.org</owner>
+  <summary>
+    Android: The amount of time that the small spinner, which become visible
+    after the ToS is accepted, is showing on screen.
+  </summary>
+</histogram>
+
 <histogram name="MobileFullscreenVideo.DurationAfterPotraitRotation" units="ms"
     expires_after="M85">
   <owner>qinmin@chromium.org</owner>