[CCTToSFRE] Use SkipTosDialogPolicyListener for ToS Fragment

Introducing SkipTosDialogPolicyListener that loads TosDialogBehavior
policy and determines if we should skip the ToS dialog.

Update the ToS Fragment to make use of SkipTosDialogPolicyListener.

This CL also changes semantics for two metrics we used to record
in the ToS Fragment, so bump up the metrics version.

Bug: 1108549
Change-Id: I4a61c96d815dce619f39a6252ef36b25095a73da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2503312
Commit-Queue: Wenyu Fu <wenyufu@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825644}

chrome/android/chrome_java_sources.gni

@@ -717,6 +717,7 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/firstrun/LightweightFirstRunActivity.java",
   "java/src/org/chromium/chrome/browser/firstrun/PolicyLoadListener.java",
   "java/src/org/chromium/chrome/browser/firstrun/SigninFirstRunFragment.java",
+  "java/src/org/chromium/chrome/browser/firstrun/SkipTosDialogPolicyListener.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/TosAndUmaFirstRunFragmentWithEnterpriseSupport.java",

chrome/android/chrome_junit_test_java_sources.gni

@@ -98,6 +98,7 @@ chrome_junit_test_java_sources = [
   "junit/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencerTest.java",
   "junit/src/org/chromium/chrome/browser/firstrun/FirstRunIntegrationUnitTest.java",
   "junit/src/org/chromium/chrome/browser/firstrun/PolicyLoadListenerUnitTest.java",
+  "junit/src/org/chromium/chrome/browser/firstrun/SkipTosDialogPolicyListenerUnitTest.java",
   "junit/src/org/chromium/chrome/browser/fullscreen/BrowserControlsManagerUnitTest.java",
   "junit/src/org/chromium/chrome/browser/gcore/GoogleApiClientHelperTest.java",
   "junit/src/org/chromium/chrome/browser/gsa/GSAStateUnitTest.java",

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

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.firstrun;
+
+import android.os.SystemClock;
+import android.text.TextUtils;
+
+import androidx.annotation.Nullable;
+import androidx.annotation.VisibleForTesting;
+
+import org.chromium.base.Callback;
+import org.chromium.base.CallbackController;
+import org.chromium.base.metrics.RecordHistogram;
+import org.chromium.base.supplier.OneshotSupplier;
+import org.chromium.base.supplier.OneshotSupplierImpl;
+import org.chromium.chrome.browser.policy.EnterpriseInfo;
+import org.chromium.components.policy.PolicyService;
+
+/**
+ * Class that listens to signals related to ToSDialogBehavior. Supplies whether ToS dialog should be
+ * skipped given policy settings.
+ *
+ * To be more specific:
+ *  - Supplies [True] if the ToS dialog is not enabled by policy while device is fully managed;
+ *  - Supplies [False] otherwise.
+ */
+final class SkipTosDialogPolicyListener implements OneshotSupplier<Boolean> {
+    /**
+     * Interface that provides histogram to be recorded when signals are available in this listener.
+     */
+    interface HistogramNameProvider {
+        /**
+         * Name of time histogram to be recorded when signal "whether device is fully managed" is
+         * available. The duration between creation of {@link SkipTosDialogPolicyListener} and
+         * signal received will be recorded.
+         *
+         * @return Name of histogram to be recorded when signal is available.
+         */
+        String getOnDeviceOwnedDetectedTimeHistogramName();
+
+        /**
+         * Name of time histogram to be recorded when signal "whether the Tos dialog is enabled on
+         * the device" is available. This histogram is not recorded when the value of policy
+         * TosDialogBehavior is not used to determine the output of this listener.
+         *
+         * The duration between creation of {@link SkipTosDialogPolicyListener} and signal received
+         * will be recorded.
+         *
+         * @return Name of histogram to be recorded when signal is available.
+         */
+        String getOnPolicyAvailableTimeHistogramName();
+    }
+
+    private final CallbackController mCallbackController;
+    private final OneshotSupplierImpl<Boolean> mSkipTosDialogPolicySupplier;
+    private final PolicyLoadListener mPolicyLoadListener;
+    private final HistogramNameProvider mHistNameProvider;
+    private final long mObjectCreatedTimeMs;
+
+    /**
+     * 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
+     * is not ready yet.
+     */
+    private @Nullable Boolean mTosDialogEnabled;
+    /**
+     * Whether the current device is organization owned. This will start null before the check
+     * occurs. The FRE can only be skipped if the device is organization owned.
+     */
+    private @Nullable Boolean mIsDeviceOwned;
+
+    /**
+     * @param firstRunAppRestrictionInfo Source that providers app restriction information.
+     * @param policyServiceSupplier Supplier that providers PolicyService when native initialized.
+     * @param enterpriseInfo Source that provides whether device is managed.
+     * @param histogramNameProvider Provider that provides histogram names when signals are
+     *         available.
+     */
+    public SkipTosDialogPolicyListener(FirstRunAppRestrictionInfo firstRunAppRestrictionInfo,
+            OneshotSupplier<PolicyService> policyServiceSupplier, EnterpriseInfo enterpriseInfo,
+            HistogramNameProvider histogramNameProvider) {
+        this(new PolicyLoadListener(firstRunAppRestrictionInfo, policyServiceSupplier),
+                enterpriseInfo, histogramNameProvider);
+    }
+
+    @VisibleForTesting
+    SkipTosDialogPolicyListener(PolicyLoadListener policyLoadListener,
+            EnterpriseInfo enterpriseInfo, HistogramNameProvider histogramNameProvider) {
+        mObjectCreatedTimeMs = SystemClock.elapsedRealtime();
+        mSkipTosDialogPolicySupplier = new OneshotSupplierImpl<>();
+        mCallbackController = new CallbackController();
+        mHistNameProvider = histogramNameProvider;
+        mPolicyLoadListener = policyLoadListener;
+
+        Boolean hasPolicy = mPolicyLoadListener.onAvailable(
+                mCallbackController.makeCancelable(this::onPolicyLoadListenerAvailable));
+        if (hasPolicy != null) {
+            onPolicyLoadListenerAvailable(hasPolicy);
+        }
+
+        // Check EnterpriseInfo if still needed.
+        if (mSkipTosDialogPolicySupplier.get() == null) {
+            enterpriseInfo.getDeviceEnterpriseInfo(
+                    mCallbackController.makeCancelable(this::onIsDeviceOwnedDetected));
+        }
+    }
+
+    /**
+     * Destroy the instance and remove all its dependencies.
+     */
+    public void destroy() {
+        mCallbackController.destroy();
+        mPolicyLoadListener.destroy();
+    }
+
+    @Override
+    public Boolean onAvailable(Callback<Boolean> callback) {
+        return mSkipTosDialogPolicySupplier.onAvailable(callback);
+    }
+
+    /**
+     * @return Whether the ToS dialog should be skipped given settings on device.
+     */
+    @Override
+    public Boolean get() {
+        return mSkipTosDialogPolicySupplier.get();
+    }
+
+    private void onPolicyLoadListenerAvailable(boolean mightHavePolicy) {
+        if (mTosDialogEnabled != null) return;
+
+        if (!mightHavePolicy) {
+            mTosDialogEnabled = true;
+        } else {
+            mTosDialogEnabled = FirstRunUtils.isCctTosDialogEnabled();
+            String histogramOnPolicyLoaded =
+                    mHistNameProvider.getOnPolicyAvailableTimeHistogramName();
+            assert !TextUtils.isEmpty(histogramOnPolicyLoaded);
+            RecordHistogram.recordTimesHistogram(
+                    histogramOnPolicyLoaded, SystemClock.elapsedRealtime() - mObjectCreatedTimeMs);
+        }
+
+        setSupplierIfDecidable();
+    }
+
+    private void onIsDeviceOwnedDetected(EnterpriseInfo.OwnedState ownedState) {
+        if (mIsDeviceOwned != null) return;
+
+        mIsDeviceOwned = ownedState != null && ownedState.mDeviceOwned;
+        String histogramOnEnterpriseInfoLoaded =
+                mHistNameProvider.getOnDeviceOwnedDetectedTimeHistogramName();
+        assert !TextUtils.isEmpty(histogramOnEnterpriseInfoLoaded);
+        RecordHistogram.recordTimesHistogram(histogramOnEnterpriseInfoLoaded,
+                SystemClock.elapsedRealtime() - mObjectCreatedTimeMs);
+
+        setSupplierIfDecidable();
+    }
+
+    private void setSupplierIfDecidable() {
+        if (mSkipTosDialogPolicySupplier.get() != null) return;
+
+        boolean confirmedDeviceNotOwned = mIsDeviceOwned != null && !mIsDeviceOwned;
+        boolean confirmedTosDialogEnabled = mTosDialogEnabled != null && mTosDialogEnabled;
+        boolean hasOutstandingSignal = mIsDeviceOwned == null || mTosDialogEnabled == null;
+
+        if (!hasOutstandingSignal) {
+            mSkipTosDialogPolicySupplier.set(!mTosDialogEnabled && mIsDeviceOwned);
+        } else if (confirmedTosDialogEnabled || confirmedDeviceNotOwned) {
+            mSkipTosDialogPolicySupplier.set(false);
+        }
+    }
+}

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

@@ -236,20 +236,6 @@ public class TosAndUmaFirstRunFragmentWithEnterpriseSupportTest {
                 SpeedComparedToInflation.NOT_RECORDED, SpeedComparedToInflation.SLOWER);
     }
 
-    @Test
-    @SmallTest
-    public void testDialogDisabled_NoRestriction() {
-        setPolicyServiceMockInitializedWithDialogEnabled(false);
-        launchFirstRunThroughCustomTab();
-        assertUIState(FragmentState.LOADING);
-
-        setAppRestrictionsMockInitialized(false);
-        assertUIState(FragmentState.NO_POLICY);
-
-        assertHistograms(true, SpeedComparedToInflation.SLOWER,
-                SpeedComparedToInflation.NOT_RECORDED, SpeedComparedToInflation.SLOWER);
-    }
-
     @Test
     @SmallTest
     public void testNotOwnedDevice() {
@@ -522,14 +508,14 @@ public class TosAndUmaFirstRunFragmentWithEnterpriseSupportTest {
         assertSingleHistogram("MobileFre.FragmentInflationSpeed.SlowerThanAppRestriction",
                 appRestrictionMetricsState == SpeedComparedToInflation.FASTER);
 
-        assertSingleHistogram("MobileFre.CctTos.IsDeviceOwnedCheckSpeed.FasterThanInflation",
+        assertSingleHistogram("MobileFre.CctTos.IsDeviceOwnedCheckSpeed2.FasterThanInflation",
                 deviceOwnershipMetricsState == SpeedComparedToInflation.FASTER);
-        assertSingleHistogram("MobileFre.CctTos.IsDeviceOwnedCheckSpeed.SlowerThanInflation",
+        assertSingleHistogram("MobileFre.CctTos.IsDeviceOwnedCheckSpeed2.SlowerThanInflation",
                 deviceOwnershipMetricsState == SpeedComparedToInflation.SLOWER);
 
-        assertSingleHistogram("MobileFre.CctTos.EnterprisePolicyCheckSpeed.FasterThanInflation",
+        assertSingleHistogram("MobileFre.CctTos.EnterprisePolicyCheckSpeed2.FasterThanInflation",
                 policyCheckMetricsState == SpeedComparedToInflation.FASTER);
-        assertSingleHistogram("MobileFre.CctTos.EnterprisePolicyCheckSpeed.SlowerThanInflation",
+        assertSingleHistogram("MobileFre.CctTos.EnterprisePolicyCheckSpeed2.SlowerThanInflation",
                 policyCheckMetricsState == SpeedComparedToInflation.SLOWER);
     }
 

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

@@ -112,6 +112,11 @@ public class PolicyLoadListenerUnitTest {
         setPolicyServiceInitialized();
         Assert.assertTrue(LOADED_POLICY_READY, mPolicyLoadListener.get());
         Mockito.verify(mListener).onResult(true);
+
+        mAppRestrictionsCallback.onResult(true);
+        Assert.assertTrue("App restriction arrives after policy initialized should be ignored.",
+                mPolicyLoadListener.get());
+        Mockito.verify(mListener, never()).onResult(false);
     }
 
     @Test

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

@@ -22,8 +22,12 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
 <histograms>
 
 <variants name="MobileFreInflationSpeedComparison">
-  <variant name=".FasterThanInflation"/>
-  <variant name=".SlowerThanInflation"/>
+  <variant name=".FasterThanInflation"
+      summary="This histogram reports just those timings that were faster
+               than inflation."/>
+  <variant name=".SlowerThanInflation"
+      summary="This histogram reports just those timings that were slower
+               than inflation."/>
 </variants>
 
 <histogram name="Mobile.AppMenu.SimilarSelection"
@@ -666,9 +670,29 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram
+    name="MobileFre.CctTos.EnterprisePolicyCheckSpeed2{MobileFreInflationSpeedComparison}"
+    units="ms" expires_after="2021-07-31">
+  <owner>skym@chromium.org</owner>
+  <owner>wenyufu@chromium.org</owner>
+  <summary>
+    Android: Records the time it takes from ToS fragment attached to the
+    activity to the enterprise policy check completing. This check is often
+    skipped when its result becomes irrelevant.
+    {MobileFreInflationSpeedComparison}
+  </summary>
+  <token key="MobileFreInflationSpeedComparison"
+      variants="MobileFreInflationSpeedComparison"/>
+</histogram>
+
 <histogram
     name="MobileFre.CctTos.EnterprisePolicyCheckSpeed{MobileFreInflationSpeedComparison}"
     units="ms" expires_after="2021-07-31">
+  <obsolete>
+    Deprecated and replaced with MobileFre.CctTos.EnterprisePolicyCheckSpeed2.*.
+    as of 11/2020. The starting time of this metrics is changed to when the
+    fragment attached to the FirstRunActivity.
+  </obsolete>
   <owner>skym@chromium.org</owner>
   <owner>wenyufu@chromium.org</owner>
   <summary>
@@ -681,9 +705,28 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
       variants="MobileFreInflationSpeedComparison"/>
 </histogram>
 
+<histogram
+    name="MobileFre.CctTos.IsDeviceOwnedCheckSpeed2{MobileFreInflationSpeedComparison}"
+    units="ms" expires_after="2021-07-31">
+  <owner>skym@chromium.org</owner>
+  <owner>wenyufu@chromium.org</owner>
+  <summary>
+    Android: Records the time it takes from ToS fragment attached to the
+    activity to the device ownership check completing.
+    {MobileFreInflationSpeedComparison}
+  </summary>
+  <token key="MobileFreInflationSpeedComparison"
+      variants="MobileFreInflationSpeedComparison"/>
+</histogram>
+
 <histogram
     name="MobileFre.CctTos.IsDeviceOwnedCheckSpeed{MobileFreInflationSpeedComparison}"
     units="ms" expires_after="2021-07-31">
+  <obsolete>
+    Deprecated and replaced with MobileFre.CctTos.IsDeviceOwnedCheckSpeed2.*. as
+    of 11/2020. The starting time of this metrics is changed to when the
+    fragment attached to the FirstRunActivity.
+  </obsolete>
   <owner>skym@chromium.org</owner>
   <owner>wenyufu@chromium.org</owner>
   <summary>