Skip popup Tabstrip update when opening tabs from background

Currently, we use AnchoredPopupWindow#onRectChanged to force update
popup strip layout when new tabs are added. This could lead to a race
condition where RecyclerView tries to recycle a view that has been
temporarily detached due to the re-layouting process. This is only
reproducible when there is recycling happening, and when tab is added
from background.

This CL bypasses this crash by disabling the force update for adding
tabs from background. Note that it is already a known issue that strip
is not properly updated when tab is added from background, so this
CL doesn't introduce any UI difference before/after.

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

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

@@ -110,7 +110,10 @@ public class TabGroupPopupUiMediator {
 
             @Override
             public void didAddTab(Tab tab, int type) {
-                if (isTabStripShowing()) {
+                // TODO(crbug.com/1050846): The popup tab strip doesn't update accordingly when tab
+                // is opened in background. This might require us to specify how the view measures
+                // in this case. Skip it for now to avoid crash crbug.com/1045944.
+                if (isTabStripShowing() && type != TabLaunchType.FROM_LONGPRESS_BACKGROUND) {
                     mUiUpdater.updateTabGroupPopUi();
                     return;
                 }

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

@@ -40,6 +40,7 @@ import org.junit.Test;
 import org.junit.rules.TestRule;
 import org.junit.runner.RunWith;
 
+import org.chromium.base.test.util.CloseableOnMainThread;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Restriction;
 import org.chromium.chrome.browser.ChromeSwitches;
@@ -56,6 +57,7 @@ import org.chromium.chrome.tab_ui.R;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
 import org.chromium.chrome.test.util.browser.Features;
+import org.chromium.chrome.test.util.browser.contextmenu.RevampedContextMenuUtils;
 import org.chromium.content_public.browser.test.util.CriteriaHelper;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import org.chromium.net.test.EmbeddedTestServer;
@@ -211,16 +213,10 @@ public class TabGroupPopupUiTest {
 
         // Click on the current tab in strip to close, and the closure should trigger tab strip
         // update.
-        onView(withId(R.id.tab_list_view))
-                .inRoot(withDecorView(not(cta.getWindow().getDecorView())))
-                .perform(RecyclerViewActions.actionOnItemAtPosition(
-                        getCurrentTabIndexInGroup(cta), click()));
+        clickOnNthItemInStrip(cta, getCurrentTabIndexInGroup(cta));
         verifyShowingTabStrip(cta, 3);
 
-        onView(withId(R.id.tab_list_view))
-                .inRoot(withDecorView(not(cta.getWindow().getDecorView())))
-                .perform(RecyclerViewActions.actionOnItemAtPosition(
-                        getCurrentTabIndexInGroup(cta), click()));
+        clickOnNthItemInStrip(cta, getCurrentTabIndexInGroup(cta));
         verifyShowingTabStrip(cta, 2);
     }
 
@@ -260,6 +256,44 @@ public class TabGroupPopupUiTest {
                 });
     }
 
+    @Test
+    @MediumTest
+    public void testAddTabFromBackground() throws Throwable {
+        launchActivity();
+        final ChromeTabbedActivity cta = mActivityTestRule.getActivity();
+        EmbeddedTestServer embeddedTestServer =
+                EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext());
+        String contextMenuTestUrl = embeddedTestServer.getURL(
+                "/chrome/test/data/android/contextmenu/context_menu_test.html");
+
+        mActivityTestRule.loadUrl(contextMenuTestUrl);
+        try (CloseableOnMainThread ignored = CloseableOnMainThread.StrictMode.allowDiskWrites()) {
+            RevampedContextMenuUtils.selectContextMenuItem(
+                    InstrumentationRegistry.getInstrumentation(), cta,
+                    cta.getTabModelSelector().getCurrentTab(), "testLink",
+                    R.id.contextmenu_open_in_new_tab);
+        }
+        verifyShowingTabStrip(cta, 2);
+
+        // Add enough tabs so that adding new items later requires recyclerView to recycle old
+        // views.
+        for (int i = 0; i < 6; i++) {
+            onView(withId(R.id.toolbar_right_button))
+                    .inRoot(withDecorView(not(cta.getWindow().getDecorView())))
+                    .perform(click());
+        }
+        verifyShowingTabStrip(cta, 8);
+
+        clickOnNthItemInStrip(cta, 0);
+        try (CloseableOnMainThread ignored = CloseableOnMainThread.StrictMode.allowDiskWrites()) {
+            RevampedContextMenuUtils.selectContextMenuItem(
+                    InstrumentationRegistry.getInstrumentation(), cta,
+                    cta.getTabModelSelector().getCurrentTab(), "testLink",
+                    R.id.contextmenu_open_in_new_tab);
+        }
+        verifyShowingTabStrip(cta, 9);
+    }
+
     private void createTabGroupAndEnterTabPage(ChromeTabbedActivity cta, int tabCount, String url) {
         if (url == null) {
             createTabs(cta, false, tabCount);
@@ -351,4 +385,10 @@ public class TabGroupPopupUiTest {
                                             .getTabModelFilterProvider()
                                             .getCurrentTabModelFilter()::isTabModelRestored);
     }
+
+    private void clickOnNthItemInStrip(ChromeTabbedActivity cta, int index) {
+        onView(withId(R.id.tab_list_view))
+                .inRoot(withDecorView(not(cta.getWindow().getDecorView())))
+                .perform(RecyclerViewActions.actionOnItemAtPosition(index, click()));
+    }
 }

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

@@ -307,6 +307,22 @@ public class TabGroupPopupUiMediatorUnitTest {
         verify(mUpdater, never()).updateTabGroupPopUi();
     }
 
+    @Test
+    public void tabAddition_NotUpdate_Background() {
+        // Mock that the strip is showing.
+        mModel.set(TabGroupPopupUiProperties.IS_VISIBLE, true);
+        // Mock that tab1, tab2 and tab3 are in the same group, and tab3 has just been created from
+        // background.
+        List<Tab> tabGroup = new ArrayList<>(Arrays.asList(mTab1, mTab2, mTab3));
+        createTabGroup(tabGroup, TAB1_ID);
+
+        mTabModelObserverCaptor.getValue().didAddTab(
+                mTab3, TabLaunchType.FROM_LONGPRESS_BACKGROUND);
+
+        assertThat(mModel.get(TabGroupPopupUiProperties.IS_VISIBLE), equalTo(true));
+        verify(mUpdater, never()).updateTabGroupPopUi();
+    }
+
     @Test
     public void tabClosureUndone_Show() {
         // Mock that the strip is hiding.