Avoid updating URL when Tab does not exist

The crash happens when a tab updates after model switched. The updated
tab does not exist in the current TabModel, therefore a null is returned
when we try to locate the selected tab. The simple fix is to null check
before we access the tab.

This CL also hides the onUrlUpdated logic behind the
TabGroupsContinuation flag.

Change-Id: I81090666328158f8a5a5e37459c54e48d0ef6325
Bug: 1035301
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1996130
Commit-Queue: Mei Liang <meiliang@chromium.org>
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732493}

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

@@ -692,7 +692,7 @@ public class StartSurfaceLayoutTest {
     @CommandLineFlags.Add({BASE_PARAMS})
     @Features.EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID,
                     ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID})
-    public void testUrlUpdatedForUndoableClosedTabNotCrashing() throws Exception {
+    public void testUrlUpdatedNotCrashing_ForUndoableClosedTab() throws Exception {
         // clang-format on
         FeatureUtilities.setTabGroupsAndroidEnabledForTesting(true);
 
@@ -712,6 +712,29 @@ public class StartSurfaceLayoutTest {
                 mUrl, PageTransition.TYPED | PageTransition.FROM_ADDRESS_BAR, tab);
     }
 
+    @Test
+    @MediumTest
+    // clang-format off
+    @CommandLineFlags.Add({BASE_PARAMS})
+    @Features.EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID,
+            ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID})
+    public void testUrlUpdatedNotCrashing_ForTabNotInCurrentModel() throws Exception {
+        // clang-format on
+        FeatureUtilities.setTabGroupsAndroidEnabledForTesting(true);
+
+        // Restart Chrome to have Group.
+        ApplicationTestUtils.finishActivity(mActivityTestRule.getActivity());
+        mActivityTestRule.startMainActivityFromLauncher();
+        prepareTabs(1, 1, null);
+        enterGTSWithThumbnailChecking();
+
+        Tab tab = mActivityTestRule.getActivity().getTabModelSelector().getCurrentTab();
+        switchTabModel(false);
+
+        mActivityTestRule.loadUrlInTab(
+                mUrl, PageTransition.TYPED | PageTransition.FROM_ADDRESS_BAR, tab);
+    }
+
     @Test
     @MediumTest
     @Feature("TabSuggestion")

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

@@ -374,12 +374,13 @@ class TabListMediator {
 
         @Override
         public void onUrlUpdated(Tab tab) {
+            if (!FeatureUtilities.isTabGroupsAndroidContinuationEnabled()) return;
             int index = mModel.indexFromId(tab.getId());
 
-            if (index == TabModel.INVALID_TAB_INDEX && mActionsOnAllRelatedTabs
-                    && FeatureUtilities.isTabGroupsAndroidContinuationEnabled()) {
+            if (index == TabModel.INVALID_TAB_INDEX && mActionsOnAllRelatedTabs) {
                 Tab currentGroupSelectedTab =
                         TabGroupUtils.getSelectedTabInGroupForTab(mTabModelSelector, tab);
+                if (currentGroupSelectedTab == null) return;
                 index = mModel.indexFromId(currentGroupSelectedTab.getId());
             }
 
@@ -1203,6 +1204,7 @@ class TabListMediator {
     }
 
     private String getUrlForTab(Tab tab) {
+        if (!FeatureUtilities.isTabGroupsAndroidContinuationEnabled()) return "";
         if (!mActionsOnAllRelatedTabs) return tab.getUrl();
 
         List<Tab> relatedTabs = getRelatedTabsForId(tab.getId());

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

@@ -115,8 +115,11 @@ import java.util.List;
  */
 @RunWith(LocalRobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
+// clang-format off
 @Features.EnableFeatures(ChromeFeatureList.TAB_TO_GTS_ANIMATION)
+@Features.DisableFeatures(ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID)
 public class TabListMediatorUnitTest {
+    // clang-format on
     @Rule
     public TestRule mProcessor = new Features.JUnitProcessor();
 
@@ -1491,8 +1494,9 @@ public class TabListMediatorUnitTest {
         doReturn(NEW_URL).when(mTab1).getUrl();
         mTabObserverCaptor.getValue().onUrlUpdated(mTab1);
 
-        assertEquals(NEW_URL, mModel.get(POSITION1).model.get(TabProperties.URL));
-        assertEquals(TAB2_URL, mModel.get(POSITION2).model.get(TabProperties.URL));
+        // TabProperties.URL is empty string if TabGroupsAndroidContinuationEnabled is false.
+        assertEquals("", mModel.get(POSITION1).model.get(TabProperties.URL));
+        assertEquals("", mModel.get(POSITION2).model.get(TabProperties.URL));
     }
 
     @Test
@@ -1788,9 +1792,6 @@ public class TabListMediatorUnitTest {
         assertThat(mModel.get(0).model.get(TabProperties.FAVICON), instanceOf(Drawable.class));
         assertThat(mModel.get(1).model.get(TabProperties.FAVICON), instanceOf(Drawable.class));
 
-        assertThat(mModel.get(0).model.get(TabProperties.URL), equalTo(TAB1_URL));
-        assertThat(mModel.get(1).model.get(TabProperties.URL), equalTo(TAB2_URL));
-
         assertThat(mModel.get(0).model.get(TabProperties.IS_SELECTED), equalTo(true));
         assertThat(mModel.get(1).model.get(TabProperties.IS_SELECTED), equalTo(false));