Allow fetching variations while FRE is showing on Android first run.

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

Index: chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencer.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencer.java b/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencer.java
index a57bb4d60329c84147f516220cc6a5a48511c4ce..5560dfc482a65bb35678dccbee1d3575dd72fc17 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencer.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencer.java
@@ -49,8 +49,10 @@ public abstract class FirstRunFlowSequencer  {
      * If the properties is null, the First Run experience needs to finish and
      * restart the original intent if necessary.
      * @param freProperties Properties to be used in the First Run activity, or null.
+     * @param setUpPropertiesPostNative Runnable that should be run to finish initialization of
+     *   freProperties after native initialization.

[Ted C, 2016/12/01 21:59:06]

should be aligned with "Runnable that..."

[Alexei Svitkine (slow), 2016/12/02 18:34:54]

Removed param.

      */
-    public abstract void onFlowIsKnown(Bundle freProperties);
+    public abstract void onFlowIsKnown(Bundle freProperties, Runnable setUpPropertiesPostNative);
 
     public FirstRunFlowSequencer(Activity activity, Bundle launcherProvidedProperties) {
         mActivity = activity;
@@ -64,12 +66,7 @@ public abstract class FirstRunFlowSequencer  {
     public void start() {
         if (CommandLine.getInstance().hasSwitch(ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE)
                 || ApiCompatibilityUtils.isDemoUser(mActivity)) {
-            onFlowIsKnown(null);
-            return;
-        }
-
-        if (!mLaunchProperties.getBoolean(FirstRunActivity.EXTRA_USE_FRE_FLOW_SEQUENCER)) {
-            onFlowIsKnown(mLaunchProperties);
+            onFlowIsKnown(null, null);
             return;
         }
 
@@ -135,63 +132,81 @@ public abstract class FirstRunFlowSequencer  {
                 mActivity.getApplicationContext(), true);
     }
 
-    void processFreEnvironment(boolean androidEduDevice, boolean hasChildAccount) {
+    void processFreEnvironment(boolean androidEduDevice, final boolean hasChildAccount) {
         if (isFirstRunFlowComplete()) {
             assert isFirstRunEulaAccepted();
             // We do not need any interactive FRE.
-            onFlowIsKnown(null);
+            onFlowIsKnown(null, null);
             return;
         }
 
-        Bundle freProperties = new Bundle();
-        freProperties.putAll(mLaunchProperties);
-        freProperties.remove(FirstRunActivity.EXTRA_USE_FRE_FLOW_SEQUENCER);
-
-        Account[] googleAccounts = getGoogleAccounts();
-        boolean onlyOneAccount = googleAccounts.length == 1;
+        final Account[] googleAccounts = getGoogleAccounts();
+        final boolean onlyOneAccount = googleAccounts.length == 1;
 
         // EDU devices should always have exactly 1 google account, which will be automatically
         // signed-in. All FRE screens are skipped in this case.
-        boolean forceEduSignIn = androidEduDevice && onlyOneAccount && !isSignedIn();
+        final boolean forceEduSignIn = androidEduDevice && onlyOneAccount && !isSignedIn();
+
+        boolean useFlowSequencer =
+                mLaunchProperties.getBoolean(FirstRunActivity.EXTRA_USE_FRE_FLOW_SEQUENCER);
+        // Note: freProperties is bound to the Runnable below.
+        final Bundle freProperties = useFlowSequencer ? new Bundle() : mLaunchProperties;
+
+        Runnable setUpPropertiesPostNative = new Runnable() {

[Ted C, 2016/12/01 21:59:06]

instead of passing a Runnable, I think this should stuff a new boolean into the
launch properties that is something like
EXTRA_FRE_FLOW_SEQUENCER_NATIVE_INIT_PENDING.  Then on native init, you'd check
that boolean and call into this class again.

I think that is more useful because EXTRA_USE_FRE_FLOW_SEQUENCER no longer
indicates that the sequencer is complete like it did before.  So, I think we
want a new boolean that can be used in combo with the existing one to know if it
really is complete.

Ooooh, you already have POST_NATIVE_SETUP_NEEDED.  I think we should just use
that and not pass the runnable.  Have FirstRunActivity keep a reference to the
sequencer and call onNativeInitialized or something on it to do the rest.

[Alexei Svitkine (slow), 2016/12/02 18:34:54]

Done. Compared to the previous approach, this required adding
mIsAndroidEduDevice and mHasChildAccount members so we can later use them in
onNativeInitialized() as well as some duplication of logic (e.g. onlyOneAccount)
- which previously we could re-use by having them be bound to the Runnable.
However, I agree that the new approach is a bit cleaner.

+            @Override
+            public void run() {
+                // We show the sign-in page if sync is allowed, and not signed in, and this is not

[Ted C, 2016/12/01 21:59:06]

should this remove POST_NATIVE_SETUP_NEEDED?

[Alexei Svitkine (slow), 2016/12/02 18:34:54]

Done.

+                // an EDU device, and
+                // - no "skip the first use hints" is set, or
+                // - "skip the first use hints" is set, but there is at least one account.
+                boolean offerSignInOk = isSyncAllowed() && !isSignedIn() && !forceEduSignIn
+                        && (!shouldSkipFirstUseHints() || googleAccounts.length > 0);
+                freProperties.putBoolean(FirstRunActivity.SHOW_SIGNIN_PAGE, offerSignInOk);
+                if (offerSignInOk || forceEduSignIn) {
+                    // If the user has accepted the ToS in the Setup Wizard and there is exactly
+                    // one account, or if the device has a child account, or if the device is an
+                    // Android EDU device and there is exactly one account, preselect the sign-in
+                    // account and force the selection if necessary.
+                    if ((hasAnyUserSeenToS() && onlyOneAccount) || hasChildAccount
+                            || forceEduSignIn) {
+                        freProperties.putString(AccountFirstRunFragment.FORCE_SIGNIN_ACCOUNT_TO,
+                                googleAccounts[0].name);
+                        freProperties.putBoolean(
+                                AccountFirstRunFragment.PRESELECT_BUT_ALLOW_TO_CHANGE,
+                                !forceEduSignIn && !hasChildAccount);
+                    }
+                }
+
+                freProperties.putBoolean(
+                        FirstRunActivity.SHOW_DATA_REDUCTION_PAGE, shouldShowDataReductionPage());
+            }
+        };
+
+        if (!useFlowSequencer) {
+            // If EXTRA_USE_FRE_FLOW_SEQUENCER is not set, it means we should use the properties as
+            // provided instead of setting them up. However, the properties as provided may not yet
+            // have post-native properties computed, so the Runnable still needs to be passed.
+            onFlowIsKnown(mLaunchProperties, setUpPropertiesPostNative);
+            return;
+        }
+
+        freProperties.putAll(mLaunchProperties);
+        freProperties.remove(FirstRunActivity.EXTRA_USE_FRE_FLOW_SEQUENCER);
 
         // In the full FRE we always show the Welcome page, except on EDU devices.
         boolean showWelcomePage = !forceEduSignIn;
         freProperties.putBoolean(FirstRunActivity.SHOW_WELCOME_PAGE, showWelcomePage);
+        freProperties.putBoolean(AccountFirstRunFragment.IS_CHILD_ACCOUNT, hasChildAccount);
+
+        // Set a boolean to indicate we need to do post native setup via the runnable below.
+        freProperties.putBoolean(FirstRunActivity.POST_NATIVE_SETUP_NEEDED, true);
 
         // Initialize usage and crash reporting according to the default value.
         // The user can explicitly enable or disable the reporting on the Welcome page.
         // This is controlled by the administrator via a policy on EDU devices.
         setDefaultMetricsAndCrashReporting();
 
-        // We show the sign-in page if sync is allowed, and not signed in, and this is not an EDU
-        // device, and
-        // - no "skip the first use hints" is set, or
-        // - "skip the first use hints" is set, but there is at least one account.
-        final boolean offerSignInOk = isSyncAllowed()
-                && !isSignedIn()
-                && !forceEduSignIn
-                && (!shouldSkipFirstUseHints() || googleAccounts.length > 0);
-        freProperties.putBoolean(FirstRunActivity.SHOW_SIGNIN_PAGE, offerSignInOk);
-
-        if (offerSignInOk || forceEduSignIn) {
-            // If the user has accepted the ToS in the Setup Wizard and there is exactly
-            // one account, or if the device has a child account, or if the device is an
-            // Android EDU device and there is exactly one account, preselect the sign-in
-            // account and force the selection if necessary.
-            if ((hasAnyUserSeenToS() && onlyOneAccount) || hasChildAccount || forceEduSignIn) {
-                freProperties.putString(AccountFirstRunFragment.FORCE_SIGNIN_ACCOUNT_TO,
-                        googleAccounts[0].name);
-                freProperties.putBoolean(AccountFirstRunFragment.PRESELECT_BUT_ALLOW_TO_CHANGE,
-                        !forceEduSignIn && !hasChildAccount);
-            }
-        }
-
-        freProperties.putBoolean(AccountFirstRunFragment.IS_CHILD_ACCOUNT, hasChildAccount);
-
-        freProperties.putBoolean(FirstRunActivity.SHOW_DATA_REDUCTION_PAGE,
-                shouldShowDataReductionPage());
-
-        onFlowIsKnown(freProperties);
+        onFlowIsKnown(freProperties, setUpPropertiesPostNative);
         if (hasChildAccount || forceEduSignIn) {
             // Child and Edu forced signins are processed independently.
             setFirstRunFlowSignInComplete();