[Instant Start] Fix observer setup in TabListMediator

Since with instant start TabListMediator could be created before
native is ready, we can no longer depend on looking at the type of
current TabModelFilter in TabListMediator constructor for observer
setup. Therefore, this CL moves the observer setup to initWithNative()
where we are confident about using the type of TabModelFilter to decide
whether we should set up observers for tab group.

Bug: 1125451, 1125471
Change-Id: If156674137d64b829728402555e2cf4e06916519
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2429467Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: Mei Liang <meiliang@chromium.org>
Commit-Queue: Yue Zhang <yuezhanggg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810703}

chrome/android/features/start_surface/internal/javatests/src/org/chromium/chrome/features/start_surface/StartSurfaceLayoutTest.java

@@ -38,6 +38,7 @@ import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.e
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.finishActivity;
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.getSwipeToDismissAction;
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.leaveTabSwitcher;
+import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.mergeAllNormalTabsToAGroup;
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.rotateDeviceToOrientation;
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.switchTabModel;
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.verifyTabModelTabCount;
@@ -2080,6 +2081,14 @@ public class StartSurfaceLayoutTest {
         // closure.
         closeFirstTabInTabSwitcher();
         verifyTabSwitcherCardCount(cta, 0);
+
+        // Verify TabGroupModelFilter is correctly setup by checking if tab switcher changes with
+        // tab grouping.
+        createTabs(cta, false, 3);
+        enterTabSwitcher(cta);
+        verifyTabSwitcherCardCount(cta, 2);
+        mergeAllNormalTabsToAGroup(cta);
+        verifyTabSwitcherCardCount(cta, 1);
     }
 
     private void enterTabGroupManualSelection(ChromeTabbedActivity cta) {

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

@@ -568,6 +568,67 @@ class TabListMediator {
             }
         };
 
+        // TODO(meiliang): follow up with unit tests to test the close signal is sent correctly with
+        // the recommendedNextTab.
+        mTabClosedListener = new TabActionListener() {
+            @Override
+            public void run(int tabId) {
+                // TODO(crbug.com/990698): Consider disabling all touch events during animation.
+                if (mModel.indexFromId(tabId) == TabModel.INVALID_TAB_INDEX) return;
+                Tab closingTab =
+                        TabModelUtils.getTabById(mTabModelSelector.getCurrentModel(), tabId);
+                if (closingTab == null) return;
+
+                RecordUserAction.record("MobileTabClosed." + mComponentName);
+
+                if (mActionsOnAllRelatedTabs) {
+                    List<Tab> related = getRelatedTabsForId(tabId);
+                    if (related.size() > 1) {
+                        onGroupClosedFrom(tabId);
+                        mTabModelSelector.getCurrentModel().closeMultipleTabs(related, true);
+                        return;
+                    }
+                }
+                onTabClosedFrom(tabId, mComponentName);
+
+                Tab currentTab = mTabModelSelector.getCurrentTab();
+                Tab nextTab = currentTab == closingTab ? getNextTab(tabId) : null;
+
+                mTabModelSelector.getCurrentModel().closeTab(
+                        closingTab, nextTab, false, false, true);
+            }
+
+            private Tab getNextTab(int closingTabId) {
+                int closingTabIndex = mModel.indexFromId(closingTabId);
+
+                if (closingTabIndex == TabModel.INVALID_TAB_INDEX) {
+                    assert false;
+                    return null;
+                }
+
+                int nextTabId = Tab.INVALID_TAB_ID;
+                if (mModel.size() > 1) {
+                    PropertyModel nextCardModel = closingTabIndex == 0
+                            ? mModel.get(closingTabIndex + 1).model
+                            : mModel.get(closingTabIndex - 1).model;
+                    nextTabId = nextCardModel.get(CARD_TYPE) == TAB
+                            ? nextCardModel.get(TabProperties.TAB_ID)
+                            : Tab.INVALID_TAB_ID;
+                }
+
+                return TabModelUtils.getTabById(mTabModelSelector.getCurrentModel(), nextTabId);
+            }
+        };
+
+        mTabGridItemTouchHelperCallback =
+                new TabGridItemTouchHelperCallback(mModel, mTabModelSelector, mTabClosedListener,
+                        mTabGridDialogHandler, mComponentName, mActionsOnAllRelatedTabs);
+    }
+
+    public void initWithNative(Profile profile) {
+        mTabListFaviconProvider.initWithNative(profile);
+        mTabModelSelector.getTabModelFilterProvider().addTabModelFilterObserver(mTabModelObserver);
+
         if (mTabModelSelector.getTabModelFilterProvider().getCurrentTabModelFilter()
                         instanceof TabGroupModelFilter) {
             mTabGroupObserver = new EmptyTabGroupModelFilterObserver() {
@@ -706,67 +767,6 @@ class TabListMediator {
                     .addTabGroupObserver(mTabGroupObserver);
         }
 
-        // TODO(meiliang): follow up with unit tests to test the close signal is sent correctly with
-        // the recommendedNextTab.
-        mTabClosedListener = new TabActionListener() {
-            @Override
-            public void run(int tabId) {
-                // TODO(crbug.com/990698): Consider disabling all touch events during animation.
-                if (mModel.indexFromId(tabId) == TabModel.INVALID_TAB_INDEX) return;
-                Tab closingTab =
-                        TabModelUtils.getTabById(mTabModelSelector.getCurrentModel(), tabId);
-                if (closingTab == null) return;
-
-                RecordUserAction.record("MobileTabClosed." + mComponentName);
-
-                if (mActionsOnAllRelatedTabs) {
-                    List<Tab> related = getRelatedTabsForId(tabId);
-                    if (related.size() > 1) {
-                        onGroupClosedFrom(tabId);
-                        mTabModelSelector.getCurrentModel().closeMultipleTabs(related, true);
-                        return;
-                    }
-                }
-                onTabClosedFrom(tabId, mComponentName);
-
-                Tab currentTab = mTabModelSelector.getCurrentTab();
-                Tab nextTab = currentTab == closingTab ? getNextTab(tabId) : null;
-
-                mTabModelSelector.getCurrentModel().closeTab(
-                        closingTab, nextTab, false, false, true);
-            }
-
-            private Tab getNextTab(int closingTabId) {
-                int closingTabIndex = mModel.indexFromId(closingTabId);
-
-                if (closingTabIndex == TabModel.INVALID_TAB_INDEX) {
-                    assert false;
-                    return null;
-                }
-
-                int nextTabId = Tab.INVALID_TAB_ID;
-                if (mModel.size() > 1) {
-                    PropertyModel nextCardModel = closingTabIndex == 0
-                            ? mModel.get(closingTabIndex + 1).model
-                            : mModel.get(closingTabIndex - 1).model;
-                    nextTabId = nextCardModel.get(CARD_TYPE) == TAB
-                            ? nextCardModel.get(TabProperties.TAB_ID)
-                            : Tab.INVALID_TAB_ID;
-                }
-
-                return TabModelUtils.getTabById(mTabModelSelector.getCurrentModel(), nextTabId);
-            }
-        };
-
-        mTabGridItemTouchHelperCallback =
-                new TabGridItemTouchHelperCallback(mModel, mTabModelSelector, mTabClosedListener,
-                        mTabGridDialogHandler, mComponentName, mActionsOnAllRelatedTabs);
-    }
-
-    public void initWithNative(Profile profile) {
-        mTabListFaviconProvider.initWithNative(profile);
-        mTabModelSelector.getTabModelFilterProvider().addTabModelFilterObserver(mTabModelObserver);
-
         if (TabUiFeatureUtilities.isTabGroupsAndroidContinuationEnabled()) {
             mTabGroupTitleEditor = new TabGroupTitleEditor(mTabModelSelector) {
                 @Override

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

@@ -748,7 +748,6 @@ public class TabGridDialogTest {
 
     @Test
     @MediumTest
-    @DisabledTest(message = "Re-enable this when crbug.com/1116985 is resolved.")
     @Features.EnableFeatures({ChromeFeatureList.INSTANT_START})
     public void testSetup_WithInstantStart() {
         final ChromeTabbedActivity cta = mActivityTestRule.getActivity();

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

@@ -2420,7 +2420,11 @@ public class TabListMediatorUnitTest {
                 mTabContentManager::getTabThumbnailWithCallback, mTitleProvider,
                 mTabListFaviconProvider, actionOnRelatedTabs, null, null, handler,
                 getClass().getSimpleName(), uiType);
+
+        // TabGroupModelFilterObserver is registered when native is ready.
+        assertThat(mTabGroupModelFilterObserverCaptor.getAllValues().isEmpty(), equalTo(true));
         mMediator.initWithNative(mProfile);
+        assertThat(mTabGroupModelFilterObserverCaptor.getAllValues().isEmpty(), equalTo(false));
 
         // There are two TabModelObserver and two TabGroupModelFilter.Observer added when
         // initializing TabListMediator, one set from TabListMediator and the other from