[Android FRE] Simplify FirstRunActivity state restoring

This CL limits the state that FirstRunActivity restores to the state sent via
FirstRunFlowSequencer#createGenericFirstRunIntent(). This enables
deleting the FirstRunActivity#EXTRA_USE_FRE_FLOW_SEQUENCER and
FirstRunActivity#POST_NATIVE_SETUP_NEEDED extras

Bug=755646

Change-Id: I1a8f4dd7c13ab5649ac2656e5a0b20c34b875e90
Reviewed-on: https://chromium-review.googlesource.com/670359Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506442}

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

@@ -61,7 +61,6 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
 
     // Incoming parameters:
     public static final String EXTRA_COMING_FROM_CHROME_ICON = "Extra.ComingFromChromeIcon";
-    public static final String EXTRA_USE_FRE_FLOW_SEQUENCER = "Extra.UseFreFlowSequencer";
     public static final String EXTRA_CHROME_LAUNCH_INTENT = "Extra.FreChromeLaunchIntent";
 
     static final String SHOW_WELCOME_PAGE = "ShowWelcome";
@@ -69,8 +68,6 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
     static final String SHOW_SEARCH_ENGINE_PAGE = "ShowSearchEnginePage";
     static final String SHOW_SIGNIN_PAGE = "ShowSignIn";
 
-    static final String POST_NATIVE_SETUP_NEEDED = "PostNativeSetupNeeded";
-
     // Outgoing results:
     public static final String EXTRA_FIRST_RUN_ACTIVITY_RESULT = "Extra.FreActivityResult";
     public static final String EXTRA_FIRST_RUN_COMPLETE = "Extra.FreComplete";
@@ -118,7 +115,13 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
 
     private FirstRunFlowSequencer mFirstRunFlowSequencer;
 
-    protected Bundle mFreProperties;
+    private Bundle mFreProperties;
+
+    /**
+     * Whether the first run activity was launched as a result of the user launching Chrome from the
+     * Android app list.
+     */
+    private boolean mLaunchedFromChromeIcon;
 
     private List<Callable<FirstRunPage>> mPages;
 
@@ -194,14 +197,7 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
 
     @Override
     public void setContentView() {
-        Bundle savedInstanceState = getSavedInstanceState();
-        if (savedInstanceState != null) {
-            mFreProperties = savedInstanceState;
-        } else if (getIntent() != null) {
-            mFreProperties = getIntent().getExtras();
-        } else {
-            mFreProperties = new Bundle();
-        }
+        initializeStateFromLaunchData();
 
         setFinishOnTouchOutside(true);
 
@@ -210,7 +206,7 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
         mPager.setOffscreenPageLimit(3);
         setContentView(mPager);
 
-        mFirstRunFlowSequencer = new FirstRunFlowSequencer(this, mFreProperties) {
+        mFirstRunFlowSequencer = new FirstRunFlowSequencer(this) {
             @Override
             public void onFlowIsKnown(Bundle freProperties) {
                 mFlowIsKnown = true;
@@ -309,7 +305,8 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
     @Override
     protected void onSaveInstanceState(Bundle outState) {
         super.onSaveInstanceState(outState);
-        outState.putAll(mFreProperties);
+        outState.putBoolean(
+                FirstRunActivity.EXTRA_COMING_FROM_CHROME_ICON, mLaunchedFromChromeIcon);
     }
 
     @Override
@@ -460,6 +457,18 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
         jumpToPage(mPager.getCurrentItem() + 1);
     }
 
+    /** Initialize local state from launch intent and from saved instance state. */
+    private void initializeStateFromLaunchData() {
+        Bundle readFrom = new Bundle();
+        if (getSavedInstanceState() != null) {
+            readFrom = getSavedInstanceState();
+        } else if (getIntent() != null) {
+            readFrom = getIntent().getExtras();
+        }
+        mLaunchedFromChromeIcon =
+                readFrom.getBoolean(FirstRunActivity.EXTRA_COMING_FROM_CHROME_ICON);
+    }
+
     /**
      * Transitions to a given page.
      * @return Whether the transition to a given page was allowed.
@@ -498,7 +507,7 @@ public class FirstRunActivity extends FirstRunActivityBase implements FirstRunPa
     }
 
     private void recordFreProgressHistogram(int state) {
-        if (mFreProperties.getBoolean(FirstRunActivity.EXTRA_COMING_FROM_CHROME_ICON)) {
+        if (mLaunchedFromChromeIcon) {
             sMobileFreProgressMainIntentHistogram.record(state);
         } else {
             sMobileFreProgressViewIntentHistogram.record(state);

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

@@ -48,7 +48,6 @@ public abstract class FirstRunFlowSequencer  {
     private static final String TAG = "firstrun";
 
     private final Activity mActivity;
-    private final Bundle mLaunchProperties;
 
     // The following are initialized via initializeSharedState().
     private boolean mIsAndroidEduDevice;
@@ -65,9 +64,8 @@ public abstract class FirstRunFlowSequencer  {
      */
     public abstract void onFlowIsKnown(Bundle freProperties);
 
-    public FirstRunFlowSequencer(Activity activity, Bundle launcherProvidedProperties) {
+    public FirstRunFlowSequencer(Activity activity) {
         mActivity = activity;
-        mLaunchProperties = launcherProvidedProperties;
     }
 
     /**
@@ -168,26 +166,13 @@ public abstract class FirstRunFlowSequencer  {
             return;
         }
 
-        if (!mLaunchProperties.getBoolean(FirstRunActivity.EXTRA_USE_FRE_FLOW_SEQUENCER)) {
-            // 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);
-            return;
-        }
-
         Bundle freProperties = new Bundle();
-        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 = !mForceEduSignIn;
         freProperties.putBoolean(FirstRunActivity.SHOW_WELCOME_PAGE, showWelcomePage);
         freProperties.putBoolean(AccountFirstRunFragment.IS_CHILD_ACCOUNT, mHasChildAccount);
 
-        // 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.
@@ -202,12 +187,9 @@ public abstract class FirstRunFlowSequencer  {
 
     /**
      * Called onNativeInitialized() a given flow as completed.
-     * @param activity An activity.
      * @param data Resulting FRE properties bundle.
      */
     public void onNativeInitialized(Bundle freProperties) {
-        if (!freProperties.getBoolean(FirstRunActivity.POST_NATIVE_SETUP_NEEDED)) return;
-
         // 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
@@ -232,7 +214,6 @@ public abstract class FirstRunFlowSequencer  {
                 FirstRunActivity.SHOW_DATA_REDUCTION_PAGE, shouldShowDataReductionPage());
         freProperties.putBoolean(
                 FirstRunActivity.SHOW_SEARCH_ENGINE_PAGE, shouldShowSearchEnginePage());
-        freProperties.remove(FirstRunActivity.POST_NATIVE_SETUP_NEEDED);
     }
 
     /**
@@ -281,8 +262,7 @@ public abstract class FirstRunFlowSequencer  {
             if (preferLightweightFre) {
                 if (!FirstRunStatus.shouldSkipWelcomePage()
                         && !FirstRunStatus.getLightweightFirstRunFlowComplete()) {
-                    return createFirstRunIntent(
-                            context, LightweightFirstRunActivity.class.getName(), fromChromeIcon);
+                    return createLightweightFirstRunIntent(context);
                 }
             } else {
                 return createGenericFirstRunIntent(context, fromChromeIcon);
@@ -299,21 +279,16 @@ public abstract class FirstRunFlowSequencer  {
      * @param fromChromeIcon Whether Chrome is opened via the Chrome icon.
      */
     public static Intent createGenericFirstRunIntent(Context context, boolean fromChromeIcon) {
-        return createFirstRunIntent(context, FirstRunActivity.class.getName(), fromChromeIcon);
+        Intent intent = new Intent();
+        intent.setClassName(context, FirstRunActivity.class.getName());
+        intent.putExtra(FirstRunActivity.EXTRA_COMING_FROM_CHROME_ICON, fromChromeIcon);
+        return intent;
     }
 
-    /**
-     * @return An intent to show the First Run Activity.
-     * @param context                   The context.
-     * @param firstRunActivityClassName The class name of the first run activity to start.
-     * @param fromChromeIcon            Whether Chrome is opened via the Chrome icon.
-     */
-    public static Intent createFirstRunIntent(
-            Context context, String firstRunActivityClassName, boolean fromChromeIcon) {
+    /** Returns an intent to show lightweight first run activity. */
+    public static Intent createLightweightFirstRunIntent(Context context) {
         Intent intent = new Intent();
-        intent.setClassName(context, firstRunActivityClassName);
-        intent.putExtra(FirstRunActivity.EXTRA_COMING_FROM_CHROME_ICON, fromChromeIcon);
-        intent.putExtra(FirstRunActivity.EXTRA_USE_FRE_FLOW_SEQUENCER, true);
+        intent.setClassName(context, LightweightFirstRunActivity.class.getName());
         return intent;
     }
 

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

@@ -25,25 +25,15 @@ import org.chromium.ui.text.SpanApplier.SpanInfo;
 */
 public class LightweightFirstRunActivity extends FirstRunActivityBase {
     private FirstRunFlowSequencer mFirstRunFlowSequencer;
-    private Bundle mFreProperties;
     private Button mOkButton;
     private boolean mNativeInitialized;
     private boolean mTriggerAcceptAfterNativeInit;
 
     @Override
     public void setContentView() {
-        Bundle savedInstanceState = getSavedInstanceState();
-        if (savedInstanceState != null) {
-            mFreProperties = savedInstanceState;
-        } else if (getIntent() != null) {
-            mFreProperties = getIntent().getExtras();
-        } else {
-            mFreProperties = new Bundle();
-        }
-
         setFinishOnTouchOutside(true);
 
-        mFirstRunFlowSequencer = new FirstRunFlowSequencer(this, mFreProperties) {
+        mFirstRunFlowSequencer = new FirstRunFlowSequencer(this) {
             @Override
             public void onFlowIsKnown(Bundle freProperties) {
                 if (freProperties == null) {
@@ -51,9 +41,8 @@ public class LightweightFirstRunActivity extends FirstRunActivityBase {
                     return;
                 }
 
-                mFreProperties = freProperties;
                 onChildAccountKnown(
-                        mFreProperties.getBoolean(AccountFirstRunFragment.IS_CHILD_ACCOUNT));
+                        freProperties.getBoolean(AccountFirstRunFragment.IS_CHILD_ACCOUNT));
             }
         };
         mFirstRunFlowSequencer.start();
@@ -126,12 +115,6 @@ public class LightweightFirstRunActivity extends FirstRunActivityBase {
         if (mTriggerAcceptAfterNativeInit) acceptTermsOfService();
     }
 
-    @Override
-    protected void onSaveInstanceState(Bundle outState) {
-        super.onSaveInstanceState(outState);
-        outState.putAll(mFreProperties);
-    }
-
     @Override
     public void onBackPressed() {
         abortFirstRunExperience();

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

@@ -57,8 +57,8 @@ public class FirstRunFlowSequencerTest {
         public boolean shouldShowDataReductionPage;
         public boolean shouldShowSearchEnginePage;
 
-        public TestFirstRunFlowSequencer(Activity activity, Bundle launcherProvidedProperties) {
-            super(activity, launcherProvidedProperties);
+        public TestFirstRunFlowSequencer(Activity activity) {
+            super(activity);
         }
 
         @Override
@@ -130,10 +130,7 @@ public class FirstRunFlowSequencerTest {
     @Before
     public void setUp() throws Exception {
         mActivityController = Robolectric.buildActivity(Activity.class);
-        Bundle launchProperties = new Bundle();
-        launchProperties.putBoolean(FirstRunActivity.EXTRA_USE_FRE_FLOW_SEQUENCER, true);
-        mSequencer =
-                new TestFirstRunFlowSequencer(mActivityController.setup().get(), launchProperties);
+        mSequencer = new TestFirstRunFlowSequencer(mActivityController.setup().get());
     }
 
     @After