Fix crash when starting in incognito mode.

https://chromium-review.googlesource.com/c/chromium/src/+/1749845
inadvertently removed the ability to change the current tab model
before tab models have been initialized. This CL fixes that by
refactoring how initial incognito state is set when the
tabmodelselector is initialized.

Bug: 995680
Change-Id: I1136d1365078db50decd5868f12445bdbfad65a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1761485
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688699}

chrome/android/java/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityTabFactory.java

@@ -58,8 +58,8 @@ public class CustomTabActivityTabFactory {
 
     /** Creates a {@link TabModelSelector} for the custom tab. */
     public TabModelSelectorImpl createTabModelSelector() {
-        mTabModelSelector =
-                new TabModelSelectorImpl(mActivity, mActivity, mPersistencePolicy, false, false);
+        mTabModelSelector = new TabModelSelectorImpl(
+                mActivity, mActivity, mPersistencePolicy, false, false, false);
         return mTabModelSelector;
     }
 

chrome/android/java/src/org/chromium/chrome/browser/tabmodel/SingleTabModelSelector.java

@@ -14,8 +14,8 @@ import org.chromium.chrome.browser.tab.Tab;
 public class SingleTabModelSelector extends TabModelSelectorBase {
     public SingleTabModelSelector(
             Activity activity, TabCreatorManager tabCreatorManager, boolean incognito) {
-        super(tabCreatorManager);
-        initialize(incognito, new SingleTabModel(activity, incognito));
+        super(tabCreatorManager, incognito);
+        initialize(new SingleTabModel(activity, incognito));
 
         TabModelObserver tabModelObserver = new EmptyTabModelObserver() {
             @Override

chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorBase.java

@@ -30,23 +30,24 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
     private TabModelFilterProvider mTabModelFilterProvider = new TabModelFilterProvider();
 
     private int mActiveModelIndex;
-    private final ObserverList<TabModelSelectorObserver> mObservers =
-            new ObserverList<TabModelSelectorObserver>();
+    private final ObserverList<TabModelSelectorObserver> mObservers = new ObserverList<>();
     private boolean mTabStateInitialized;
+    private boolean mStartIncognito;
 
     private final TabCreatorManager mTabCreatorManager;
 
-    protected TabModelSelectorBase(TabCreatorManager tabCreatorManager) {
+    protected TabModelSelectorBase(TabCreatorManager tabCreatorManager, boolean startIncognito) {
         mTabCreatorManager = tabCreatorManager;
+        mStartIncognito = startIncognito;
     }
 
-    protected final void initialize(boolean startIncognito, TabModel... models) {
+    protected final void initialize(TabModel... models) {
         // Only normal and incognito supported for now.
         assert mTabModels.isEmpty();
         assert models.length > 0;
 
         Collections.addAll(mTabModels, models);
-        mActiveModelIndex = getModelIndex(startIncognito);
+        mActiveModelIndex = getModelIndex(mStartIncognito);
         assert mActiveModelIndex != MODEL_NOT_FOUND;
         mTabModelFilterProvider = new TabModelFilterProvider(mTabModels);
 
@@ -84,6 +85,10 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
 
     @Override
     public void selectModel(boolean incognito) {
+        if (mTabModels.size() == 0) {
+            mStartIncognito = incognito;
+            return;
+        }
         int newIndex = getModelIndex(incognito);
         assert newIndex != MODEL_NOT_FOUND;
         if (newIndex == mActiveModelIndex) return;
@@ -150,6 +155,7 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
 
     @Override
     public boolean isIncognitoSelected() {
+        if (mTabModels.size() == 0) return mStartIncognito;
         return getCurrentModel().isIncognito();
     }
 

chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorImpl.java

@@ -57,8 +57,9 @@ public class TabModelSelectorImpl extends TabModelSelectorBase implements TabMod
      * @param supportUndo Whether a tab closure can be undone.
      */
     public TabModelSelectorImpl(Activity activity, TabCreatorManager tabCreatorManager,
-            TabPersistencePolicy persistencePolicy, boolean supportUndo, boolean isTabbedActivity) {
-        super(tabCreatorManager);
+            TabPersistencePolicy persistencePolicy, boolean supportUndo, boolean isTabbedActivity,
+            boolean startIncognito) {
+        super(tabCreatorManager, startIncognito);
         mUma = new TabModelSelectorUma(activity);
         final TabPersistentStoreObserver persistentStoreObserver =
                 new TabPersistentStoreObserver() {
@@ -126,7 +127,7 @@ public class TabModelSelectorImpl extends TabModelSelectorBase implements TabMod
         TabModel incognitoModel = new IncognitoTabModel(new IncognitoTabModelImplCreator(
                 regularTabCreator, incognitoTabCreator, mUma, mOrderController,
                 mTabContentManager, mTabSaver, this));
-        initialize(isIncognitoSelected(), normalModel, incognitoModel);
+        initialize(normalModel, incognitoModel);
         regularTabCreator.setTabModel(normalModel, mOrderController);
         incognitoTabCreator.setTabModel(incognitoModel, mOrderController);
         mTabSaver.setTabContentManager(mTabContentManager);
@@ -208,7 +209,7 @@ public class TabModelSelectorImpl extends TabModelSelectorBase implements TabMod
      */
     @VisibleForTesting
     public void initializeForTesting(TabModel normalModel, TabModel incognitoModel) {
-        initialize(isIncognitoSelected(), normalModel, incognitoModel);
+        initialize(normalModel, incognitoModel);
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabWindowManager.java

@@ -218,7 +218,7 @@ public class TabWindowManager implements ActivityStateListener {
             TabPersistencePolicy persistencePolicy = new TabbedModeTabPersistencePolicy(
                     selectorIndex, mergeTabs);
             return new TabModelSelectorImpl(
-                    activity, tabCreatorManager, persistencePolicy, true, true);
+                    activity, tabCreatorManager, persistencePolicy, true, true, false);
         }
     }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabTabPersistencePolicyTest.java

@@ -463,7 +463,7 @@ public class CustomTabTabPersistencePolicyTest {
         CustomTabActivity activity = new CustomTabActivity();
         ApplicationStatus.onStateChangeForTesting(activity, ActivityState.CREATED);
         TabModelSelectorImpl selector = new TabModelSelectorImpl(
-                activity, activity, buildTestPersistencePolicy(), false, false);
+                activity, activity, buildTestPersistencePolicy(), false, false, false);
         selector.initializeForTesting(normalTabModel, incognitoTabModel);
         ApplicationStatus.onStateChangeForTesting(activity, ActivityState.DESTROYED);
         return selector;

chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/ContextMenuLoadUrlParamsTest.java

@@ -81,7 +81,7 @@ public class ContextMenuLoadUrlParamsTest {
         public RecordingTabModelSelector(
                 Activity activity, TabCreatorManager tabCreatorManager, int selectorIndex) {
             super(activity, tabCreatorManager,
-                    new TabbedModeTabPersistencePolicy(selectorIndex, false), false, false);
+                    new TabbedModeTabPersistencePolicy(selectorIndex, false), false, false, false);
         }
     }
 

chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/IncognitoTabModelTest.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.tabmodel;
 
+import android.support.test.filters.MediumTest;
 import android.support.test.filters.SmallTest;
 
 import org.junit.Assert;
@@ -16,10 +17,14 @@ import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.RetryOnFailure;
 import org.chromium.chrome.browser.ChromeSwitches;
+import org.chromium.chrome.browser.ChromeTabbedActivity;
 import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.chrome.browser.tab.TabState;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
+import org.chromium.chrome.test.util.ApplicationTestUtils;
 import org.chromium.content_public.browser.LoadUrlParams;
+import org.chromium.content_public.browser.test.util.CriteriaHelper;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 
 /**
@@ -70,4 +75,19 @@ public class IncognitoTabModelTest {
         createTabOnUiThread();
         Assert.assertEquals(1, mTabModel.getCount());
     }
+
+    @Test
+    @MediumTest
+    @Feature({"OffTheRecord"})
+    public void testRecreateInIncognito() {
+        createTabOnUiThread();
+        // Need to wait for contentsState to be initialized for the tab to restore correctly.
+        CriteriaHelper.pollUiThread(() -> {
+            return TabState.from(mActivityTestRule.getActivity().getActivityTab()).contentsState
+                    != null;
+        });
+        ChromeTabbedActivity newActivity =
+                ApplicationTestUtils.recreateActivity(mActivityTestRule.getActivity());
+        CriteriaHelper.pollUiThread(() -> newActivity.getTabModelSelector().isIncognitoSelected());
+    }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorObserverTestRule.java

@@ -67,7 +67,7 @@ public class TabModelSelectorObserverTestRule extends ChromeBrowserTestRule {
                                                    .getTargetContext()
                                                    .getApplicationContext());
 
-        mSelector = new TabModelSelectorBase(null) {
+        mSelector = new TabModelSelectorBase(null, false) {
             @Override
             public Tab openNewTab(LoadUrlParams loadUrlParams, @TabLaunchType int type, Tab parent,
                     boolean incognito) {
@@ -127,7 +127,7 @@ public class TabModelSelectorObserverTestRule extends ChromeBrowserTestRule {
         mIncognitoTabModel = new TabModelSelectorTestTabModel(
                 true, orderController, tabContentManager, tabPersistentStore, delegate);
 
-        mSelector.initialize(false, mNormalTabModel, mIncognitoTabModel);
+        mSelector.initialize(mNormalTabModel, mIncognitoTabModel);
     }
 
     /**

chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorTabModelObserverTest.java

@@ -64,7 +64,7 @@ public class TabModelSelectorTabModelObserverTest {
     @UiThreadTest
     @SmallTest
     public void testUninitializedSelector() throws InterruptedException, TimeoutException {
-        mSelector = new TabModelSelectorBase(null) {
+        mSelector = new TabModelSelectorBase(null, false) {
             @Override
             public Tab openNewTab(LoadUrlParams loadUrlParams, @TabLaunchType int type, Tab parent,
                     boolean incognito) {
@@ -79,8 +79,7 @@ public class TabModelSelectorTabModelObserverTest {
                         registrationCompleteCallback.notifyCalled();
                     }
                 };
-        mSelector.initialize(
-                false, mTestRule.getNormalTabModel(), mTestRule.getIncognitoTabModel());
+        mSelector.initialize(mTestRule.getNormalTabModel(), mTestRule.getIncognitoTabModel());
         registrationCompleteCallback.waitForCallback(0);
         assertAllModelsHaveObserver(mSelector, observer);
     }

chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorTabObserverTest.java

@@ -110,7 +110,7 @@ public class TabModelSelectorTabObserverTest {
     @Test
     @SmallTest
     public void testObserverAddedBeforeInitialize() {
-        TabModelSelectorBase selector = new TabModelSelectorBase(null) {
+        TabModelSelectorBase selector = new TabModelSelectorBase(null, false) {
             @Override
             public Tab openNewTab(LoadUrlParams loadUrlParams, @TabLaunchType int type, Tab parent,
                     boolean incognito) {
@@ -118,7 +118,7 @@ public class TabModelSelectorTabObserverTest {
             }
         };
         TestTabModelSelectorTabObserver observer = createTabModelSelectorTabObserver();
-        selector.initialize(false, mTestRule.getNormalTabModel(), mTestRule.getIncognitoTabModel());
+        selector.initialize(mTestRule.getNormalTabModel(), mTestRule.getIncognitoTabModel());
 
         Tab normalTab1 = createTestTab(false);
         addTab(mTestRule.getNormalTabModel(), normalTab1);

chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabPersistentStoreTest.java

@@ -171,7 +171,7 @@ public class TabPersistentStoreTest {
         private final TabModelOrderController mTabModelOrderController;
 
         public TestTabModelSelector() throws Exception {
-            super(new MockTabCreatorManager());
+            super(new MockTabCreatorManager(), false);
             ((MockTabCreatorManager) getTabCreatorManager()).initialize(this);
             mTabPersistentStoreObserver = new MockTabPersistentStoreObserver();
             mTabPersistentStore =
@@ -202,7 +202,7 @@ public class TabPersistentStoreTest {
                     new IncognitoTabModelImplCreator(getTabCreatorManager().getTabCreator(false),
                             getTabCreatorManager().getTabCreator(true), null,
                             mTabModelOrderController, null, mTabPersistentStore, this));
-            initialize(false, regularTabModel, incognitoTabModel);
+            initialize(regularTabModel, incognitoTabModel);
         }
 
         @Override

chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/tabmodel/MockTabModelSelector.java

@@ -23,8 +23,8 @@ public class MockTabModelSelector extends TabModelSelectorBase {
 
     public MockTabModelSelector(
             int tabCount, int incognitoTabCount, MockTabModel.MockTabModelDelegate delegate) {
-        super(null);
-        initialize(false, new MockTabModel(false, delegate), new MockTabModel(true, delegate));
+        super(null, false);
+        initialize(new MockTabModel(false, delegate), new MockTabModel(true, delegate));
         for (int i = 0; i < tabCount; i++) {
             addMockTab();
         }