Simplify SingleTabModelSelector and delete TabModelSelector#getModelAt

No functionality changes intended, this is simply cleanup.

getModelAt was only exposed for testing, and the API contract was
confusing (SingleTabModelSelector always just returned the index 0
model).

Change-Id: I7f18ad407e14540de180c7f5b8d624e3148266af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1749845Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688206}

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

@@ -9,65 +9,17 @@ import android.app.Activity;
 import org.chromium.chrome.browser.tab.Tab;
 
 /**
- * Simple TabModelSelector that assumes that only the regular TabModel type exists.
+ * Simple TabModelSelector that assumes that only a single TabModel exists at a time.
  */
 public class SingleTabModelSelector extends TabModelSelectorBase {
-    private final SingleTabModel mTabModel;
-
     public SingleTabModelSelector(Activity activity, TabCreatorManager tabCreatorManager,
             boolean incognito, boolean blockNewWindows) {
         super(tabCreatorManager);
-        mTabModel = new SingleTabModel(activity, incognito, blockNewWindows);
-        initialize(false, mTabModel);
+        initialize(incognito, new SingleTabModel(activity, incognito, blockNewWindows));
     }
 
     public void setTab(Tab tab) {
-        mTabModel.setTab(tab);
+        ((SingleTabModel) getCurrentModel()).setTab(tab);
         markTabStateInitialized();
     }
-
-    @Override
-    public void selectModel(boolean incognito) {
-        assert incognito == mTabModel.isIncognito();
-    }
-
-    @Override
-    public TabModel getModel(boolean incognito) {
-        return super.getModel(incognito);
-    }
-
-    @Override
-    public TabModel getCurrentModel() {
-        assert super.getCurrentModel() == mTabModel;
-        return mTabModel;
-    }
-
-    @Override
-    public boolean isIncognitoSelected() {
-        return mTabModel.isIncognito();
-    }
-
-    @Override
-    public void closeAllTabs() {
-        mTabModel.closeAllTabs();
-    }
-
-    @Override
-    public int getTotalTabCount() {
-        assert mTabModel != null;
-        return mTabModel.getCount();
-    }
-
-    @Override
-    public Tab getTabById(int id) {
-        Tab currentTab = getCurrentTab();
-        if (currentTab != null && currentTab.getId() == id) return currentTab;
-        return null;
-    }
-
-    @Override
-    public TabModel getModelAt(int index) {
-        assert index == INCOGNITO_TAB_MODEL_INDEX || index == NORMAL_TAB_MODEL_INDEX;
-        return mTabModel;
-    }
 }

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

@@ -6,7 +6,6 @@ package org.chromium.chrome.browser.tabmodel;
 
 import android.support.annotation.Nullable;
 
-import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.content_public.browser.LoadUrlParams;
@@ -64,12 +63,6 @@ public interface TabModelSelector {
      */
     List<TabModel> getModels();
 
-    /**
-     * @return the model at {@code index} or null if no model exist for that index.
-     */
-    @VisibleForTesting
-    TabModel getModelAt(int index);
-
     /**
      * Get the current tab model.
      * @return Never returns null.  Returns a stub when real model is uninitialized.

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

@@ -17,12 +17,11 @@ import java.util.List;
  * Implement methods shared across the different model implementations.
  */
 public abstract class TabModelSelectorBase implements TabModelSelector {
-    public static final int NORMAL_TAB_MODEL_INDEX = 0;
-    public static final int INCOGNITO_TAB_MODEL_INDEX = 1;
+    private static final int MODEL_NOT_FOUND = -1;
 
     private static TabModelSelectorObserver sObserver;
 
-    private List<TabModel> mTabModels = Collections.emptyList();
+    private List<TabModel> mTabModels = new ArrayList<>();
 
     /**
      * This is a dummy implementation intended to stub out TabModelFilterProvider before native is
@@ -30,7 +29,7 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
      */
     private TabModelFilterProvider mTabModelFilterProvider = new TabModelFilterProvider();
 
-    private int mActiveModelIndex = NORMAL_TAB_MODEL_INDEX;
+    private int mActiveModelIndex;
     private final ObserverList<TabModelSelectorObserver> mObservers =
             new ObserverList<TabModelSelectorObserver>();
     private boolean mTabStateInitialized;
@@ -45,16 +44,10 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
         // Only normal and incognito supported for now.
         assert mTabModels.isEmpty();
         assert models.length > 0;
-        if (startIncognito) {
-            assert models.length > INCOGNITO_TAB_MODEL_INDEX;
-        }
 
-        List<TabModel> tabModels = new ArrayList<TabModel>();
-        for (int i = 0; i < models.length; i++) {
-            tabModels.add(models[i]);
-        }
-        mActiveModelIndex = startIncognito ? INCOGNITO_TAB_MODEL_INDEX : NORMAL_TAB_MODEL_INDEX;
-        mTabModels = Collections.unmodifiableList(tabModels);
+        Collections.addAll(mTabModels, models);
+        mActiveModelIndex = getModelIndex(startIncognito);
+        assert mActiveModelIndex != MODEL_NOT_FOUND;
         mTabModelFilterProvider = new TabModelFilterProvider(mTabModels);
 
         TabModelObserver tabModelObserver = new EmptyTabModelObserver() {
@@ -91,27 +84,21 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
 
     @Override
     public void selectModel(boolean incognito) {
-        TabModel previousModel = getCurrentModel();
-        mActiveModelIndex = incognito ? INCOGNITO_TAB_MODEL_INDEX : NORMAL_TAB_MODEL_INDEX;
-        TabModel newModel = getCurrentModel();
+        int newIndex = getModelIndex(incognito);
+        assert newIndex != MODEL_NOT_FOUND;
+        if (newIndex == mActiveModelIndex) return;
 
-        if (previousModel != newModel) {
-            for (TabModelSelectorObserver listener : mObservers) {
-                listener.onTabModelSelected(newModel, previousModel);
-            }
+        TabModel newModel = mTabModels.get(newIndex);
+        TabModel previousModel = mTabModels.get(mActiveModelIndex);
+        mActiveModelIndex = newIndex;
+        for (TabModelSelectorObserver listener : mObservers) {
+            listener.onTabModelSelected(newModel, previousModel);
         }
     }
 
-    @Override
-    public TabModel getModelAt(int index) {
-        assert (index < mTabModels.size() && index >= 0) :
-            "requested index " + index + " size " + mTabModels.size();
-        return mTabModels.get(index);
-    }
-
     @Override
     public Tab getCurrentTab() {
-        return getCurrentModel() == null ? null : TabModelUtils.getCurrentTab(getCurrentModel());
+        return TabModelUtils.getCurrentTab(getCurrentModel());
     }
 
     @Override
@@ -133,7 +120,8 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
 
     @Override
     public TabModel getCurrentModel() {
-        return getModelAt(mActiveModelIndex);
+        if (mTabModels.size() == 0) return EmptyTabModel.getInstance();
+        return mTabModels.get(mActiveModelIndex);
     }
 
     @Override
@@ -143,8 +131,16 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
 
     @Override
     public TabModel getModel(boolean incognito) {
-        int index = incognito ? INCOGNITO_TAB_MODEL_INDEX : NORMAL_TAB_MODEL_INDEX;
-        return getModelAt(index);
+        int index = getModelIndex(incognito);
+        if (index == MODEL_NOT_FOUND) return EmptyTabModel.getInstance();
+        return mTabModels.get(index);
+    }
+
+    private int getModelIndex(boolean incognito) {
+        for (int i = 0; i < mTabModels.size(); i++) {
+            if (incognito == mTabModels.get(i).isIncognito()) return i;
+        }
+        return MODEL_NOT_FOUND;
     }
 
     @Override
@@ -154,7 +150,7 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
 
     @Override
     public boolean isIncognitoSelected() {
-        return mActiveModelIndex == INCOGNITO_TAB_MODEL_INDEX;
+        return getCurrentModel().isIncognito();
     }
 
     @Override
@@ -172,7 +168,7 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
     @Override
     public boolean closeTab(Tab tab) {
         for (int i = 0; i < getModels().size(); i++) {
-            TabModel model = getModelAt(i);
+            TabModel model = mTabModels.get(i);
             if (model.indexOf(tab) >= 0) {
                 return model.closeTab(tab);
             }
@@ -191,7 +187,7 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
     @Override
     public Tab getTabById(int id) {
         for (int i = 0; i < getModels().size(); i++) {
-            Tab tab = TabModelUtils.getTabById(getModelAt(i), id);
+            Tab tab = TabModelUtils.getTabById(mTabModels.get(i), id);
             if (tab != null) return tab;
         }
         return null;
@@ -205,7 +201,7 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
     @Override
     public void closeAllTabs(boolean uponExit) {
         for (int i = 0; i < getModels().size(); i++) {
-            getModelAt(i).closeAllTabs(!uponExit, uponExit);
+            mTabModels.get(i).closeAllTabs(!uponExit, uponExit);
         }
     }
 
@@ -213,7 +209,7 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
     public int getTotalTabCount() {
         int count = 0;
         for (int i = 0; i < getModels().size(); i++)  {
-            count += getModelAt(i).getCount();
+            count += mTabModels.get(i).getCount();
         }
         return count;
     }
@@ -235,6 +231,7 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
      * Marks the task state being initialized and notifies observers.
      */
     public void markTabStateInitialized() {
+        if (mTabStateInitialized) return;
         mTabStateInitialized = true;
         for (TabModelSelectorObserver listener : mObservers) listener.onTabStateInitialized();
     }
@@ -253,7 +250,8 @@ public abstract class TabModelSelectorBase implements TabModelSelector {
     @Override
     public void destroy() {
         mTabModelFilterProvider.destroy();
-        for (int i = 0; i < getModels().size(); i++) getModelAt(i).destroy();
+        for (int i = 0; i < getModels().size(); i++) mTabModels.get(i).destroy();
+        mTabModels.clear();
     }
 
     /**

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

@@ -30,10 +30,6 @@ public class TabModelSelectorImpl extends TabModelSelectorBase implements TabMod
             new AtomicBoolean(true);
     private final TabPersistentStore mTabSaver;
 
-    // This flag signifies the object has gotten an onNativeReady callback and
-    // has not been destroyed.
-    private boolean mActiveState;
-
     private boolean mIsUndoSupported;
 
     // Whether the Activity that owns that TabModelSelector is tabbed or not.
@@ -117,7 +113,7 @@ public class TabModelSelectorImpl extends TabModelSelectorBase implements TabMod
      * @param tabContentProvider                      A {@link TabContentManager} instance.
      */
     public void onNativeLibraryReady(TabContentManager tabContentProvider) {
-        assert !mActiveState : "onNativeLibraryReady called twice!";
+        assert mTabContentManager == null : "onNativeLibraryReady called twice!";
         mTabContentManager = tabContentProvider;
 
         ChromeTabCreator regularTabCreator =
@@ -147,8 +143,6 @@ public class TabModelSelectorImpl extends TabModelSelectorBase implements TabMod
             }
         });
 
-        mActiveState = true;
-
         new TabModelSelectorTabObserver(this) {
             @Override
             public void onUrlUpdated(Tab tab) {
@@ -215,7 +209,6 @@ public class TabModelSelectorImpl extends TabModelSelectorBase implements TabMod
     @VisibleForTesting
     public void initializeForTesting(TabModel normalModel, TabModel incognitoModel) {
         initialize(isIncognitoSelected(), normalModel, incognitoModel);
-        mActiveState = true;
     }
 
     @Override
@@ -223,11 +216,6 @@ public class TabModelSelectorImpl extends TabModelSelectorBase implements TabMod
         mCloseAllTabsDelegate = delegate;
     }
 
-    @Override
-    public TabModel getModelAt(int index) {
-        return mActiveState ? super.getModelAt(index) : EmptyTabModel.getInstance();
-    }
-
     @Override
     public void selectModel(boolean incognito) {
         TabModel oldModel = getCurrentModel();
@@ -254,7 +242,7 @@ public class TabModelSelectorImpl extends TabModelSelectorBase implements TabMod
     @Override
     public void commitAllTabClosures() {
         for (int i = 0; i < getModels().size(); i++) {
-            getModelAt(i).commitAllTabClosures();
+            getModels().get(i).commitAllTabClosures();
         }
     }
 
@@ -324,7 +312,6 @@ public class TabModelSelectorImpl extends TabModelSelectorBase implements TabMod
         mTabSaver.destroy();
         mUma.destroy();
         super.destroy();
-        mActiveState = false;
     }
 
     /**

chrome/android/javatests/src/org/chromium/chrome/browser/TabsOpenedFromExternalAppTest.java

@@ -479,10 +479,9 @@ public class TabsOpenedFromExternalAppTest {
 
         Assert.assertEquals("Selected tab is not on the right URL.", url1,
                 mActivityTestRule.getActivity().getActivityTab().getUrl());
-        int originalTabCount = ChromeTabUtils.getNumOpenTabs(mActivityTestRule.getActivity());
 
         // Launch the same URL without app ID. It should open a new tab.
-        originalTabCount = ChromeTabUtils.getNumOpenTabs(mActivityTestRule.getActivity());
+        int originalTabCount = ChromeTabUtils.getNumOpenTabs(mActivityTestRule.getActivity());
         launchUrlFromExternalApp(url1, null, false);
         int newTabCount = ChromeTabUtils.getNumOpenTabs(mActivityTestRule.getActivity());
         Assert.assertEquals("Incorrect number of tabs open", originalTabCount + 1, newTabCount);

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

@@ -283,7 +283,7 @@ public class UndoTabModelTest {
                 () -> { ((TabModelSelectorImpl) selector).saveState(); });
 
         for (int i = 0; i < selector.getModels().size(); i++) {
-            TabList tabs = selector.getModelAt(i).getComprehensiveModel();
+            TabList tabs = selector.getModels().get(i).getComprehensiveModel();
             for (int j = 0; j < tabs.getCount(); j++) {
                 Assert.assertFalse(tabs.isClosurePending(tabs.getTabAt(j).getId()));
             }

chrome/android/touchless/java/src/org/chromium/chrome/browser/touchless/NoTouchActivity.java

@@ -147,8 +147,6 @@ public class NoTouchActivity extends SingleTabActivity {
 
         ((TouchlessTabCreator) getTabCreator(false))
                 .setTabModel(getTabModelSelector().getModel(false));
-        ((TouchlessTabCreator) getTabCreator(true))
-                .setTabModel(getTabModelSelector().getModel(true));
 
         super.initializeState();
 

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

@@ -28,12 +28,12 @@ public class MockTabModelSelector extends TabModelSelectorBase {
         for (int i = 0; i < tabCount; i++) {
             addMockTab();
         }
-        if (tabCount > 0) TabModelUtils.setIndex(getModelAt(0), 0);
+        if (tabCount > 0) TabModelUtils.setIndex(getModel(false), 0);
 
         for (int i = 0; i < incognitoTabCount; i++) {
             addMockIncognitoTab();
         }
-        if (incognitoTabCount > 0) TabModelUtils.setIndex(getModelAt(1), 0);
+        if (incognitoTabCount > 0) TabModelUtils.setIndex(getModel(true), 0);
     }
 
     private static int nextIdOffset() {
@@ -41,11 +41,11 @@ public class MockTabModelSelector extends TabModelSelectorBase {
     }
 
     public Tab addMockTab() {
-        return ((MockTabModel) getModelAt(0)).addTab(ID_OFFSET + nextIdOffset());
+        return ((MockTabModel) getModel(false)).addTab(ID_OFFSET + nextIdOffset());
     }
 
     public Tab addMockIncognitoTab() {
-        return ((MockTabModel) getModelAt(1)).addTab(INCOGNITO_ID_OFFSET + nextIdOffset());
+        return ((MockTabModel) getModel(true)).addTab(INCOGNITO_ID_OFFSET + nextIdOffset());
     }
 
     @Override