Add a range check when restoring tabs in TabListMediator

When user switches between light and dark mode, incognito tabs are
dumped and then restored. This exposes a missed range check in
TabListMediator. When restoring an incognito tab group, the tab is
added to TabModel but corresponding PropertyModel is not yet added to
TabListModel, which leads to a temporary async. This async is also
exposed in other cases where we try to sync TabListModel with TabModel
through restoring, e.g. exiting multi-window mode. This CL fixes this
issue.

Bug: 1034385, 1034921
Change-Id: I3e712070cb3455a38d43cb7dae74adc68ebf65e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1970735
Commit-Queue: Yue Zhang <yuezhanggg@chromium.org>
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#731567}

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediator.java

@@ -510,16 +510,20 @@ class TabListMediator {
                 if (!isTabModelRestoreCompleted) return;
                 onTabAdded(tab, !mActionsOnAllRelatedTabs);
                 if (type == TabLaunchType.FROM_RESTORE && mActionsOnAllRelatedTabs) {
-                    // When tab is restored after restoring stage (e.g. exiting multi-window mode),
-                    // we need to update related property models.
+                    // When tab is restored after restoring stage (e.g. exiting multi-window mode,
+                    // switching between dark/light mode in incognito), we need to update related
+                    // property models.
                     TabModelFilter filter = mTabModelSelector.getTabModelFilterProvider()
                                                     .getCurrentTabModelFilter();
                     int index = filter.indexOf(tab);
                     if (index == TabList.INVALID_TAB_INDEX) return;
                     Tab currentGroupSelectedTab = filter.getTabAt(index);
-
-                    assert mModel.indexFromId(currentGroupSelectedTab.getId()) == index;
-
+                    // TabModel and TabListModel may be in the process of syncing up through
+                    // restoring. Examples of this situation are switching between light/dark mode
+                    // in incognito, exiting multi-window mode, etc.
+                    if (mModel.indexFromId(currentGroupSelectedTab.getId()) != index) {
+                        return;
+                    }
                     updateTab(index, currentGroupSelectedTab,
                             mModel.get(index).model.get(TabProperties.IS_SELECTED), false, false);
                 }

chrome/android/features/tab_ui/junit/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediatorUnitTest.java

@@ -230,6 +230,7 @@ public class TabListMediatorUnitTest {
     private RecyclerView.ViewHolder mDummyViewHolder2;
     private View mItemView1 = mock(View.class);
     private View mItemView2 = mock(View.class);
+    private TabModelObserver mMediatorTabModelObserver;
     private TabGroupModelFilter.Observer mMediatorTabGroupModelFilterObserver;
 
     @Before
@@ -650,6 +651,33 @@ public class TabListMediatorUnitTest {
         assertThat(mModel.size(), equalTo(2));
     }
 
+    @Test
+    @Features.EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID,
+            ChromeFeatureList.TAB_GROUPS_UI_IMPROVEMENTS_ANDROID})
+    // clang-format off
+    public void tabAddition_Restore_SyncingTabListModelWithTabModel() {
+        // clang-format on
+        setUpForTabGroupOperation(TabListMediatorType.TAB_SWITCHER);
+        // Mock that tab restoring stage is over.
+        doReturn(true).when(mTabGroupModelFilter).isTabModelRestored();
+
+        // Mock that tab1 and tab2 are in the same group, and they are being restored. The
+        // TabListModel has been cleaned out before the restoring happens. This case could happen
+        // within a incognito tab group when user switches between light/dark mode.
+        createTabGroup(new ArrayList<>(Arrays.asList(mTab1, mTab2)), TAB1_ID);
+        doReturn(POSITION1).when(mTabGroupModelFilter).indexOf(mTab1);
+        doReturn(POSITION1).when(mTabGroupModelFilter).indexOf(mTab2);
+        doReturn(mTab1).when(mTabGroupModelFilter).getTabAt(POSITION1);
+        doReturn(1).when(mTabGroupModelFilter).getCount();
+        mModel.clear();
+
+        mMediatorTabModelObserver.didAddTab(mTab2, TabLaunchType.FROM_RESTORE);
+        assertThat(mModel.size(), equalTo(0));
+
+        mMediatorTabModelObserver.didAddTab(mTab1, TabLaunchType.FROM_RESTORE);
+        assertThat(mModel.size(), equalTo(1));
+    }
+
     @Test
     public void tabAddition_GTS() {
         initAndAssertAllProperties();
@@ -1814,9 +1842,10 @@ public class TabListMediatorUnitTest {
                 mTabListFaviconProvider, actionOnRelatedTabs, null, null, null, handler,
                 getClass().getSimpleName(), 0);
 
-        // There are two TabGroupModelFilter.Observer added when initializing TabListMediator, one
-        // from TabListMediator and the other from TabGroupTitleEditor. Here we only test the one
-        // from TabListMediator.
+        // There are two TabModelObserver and two TabGroupModelFilter.Observer added when
+        // initializing TabListMediator, one set from TabListMediator and the other from
+        // TabGroupTitleEditor. Here we only test the ones from TabListMediator.
+        mMediatorTabModelObserver = mTabModelObserverCaptor.getAllValues().get(1);
         mMediatorTabGroupModelFilterObserver =
                 mTabGroupModelFilterObserverCaptor.getAllValues().get(0);