Dismiss CloseTabSuggestion card immediately after closing tab

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

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

@@ -8,6 +8,7 @@ import static android.support.test.espresso.Espresso.onView;
 import static android.support.test.espresso.action.ViewActions.click;
 import static android.support.test.espresso.assertion.ViewAssertions.doesNotExist;
 import static android.support.test.espresso.assertion.ViewAssertions.matches;
+import static android.support.test.espresso.contrib.RecyclerViewActions.actionOnItemAtPosition;
 import static android.support.test.espresso.matcher.ViewMatchers.Visibility.GONE;
 import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed;
 import static android.support.test.espresso.matcher.ViewMatchers.withContentDescription;
@@ -515,7 +516,7 @@ public class StartSurfaceLayoutTest {
             }
             int count = getCaptureCount();
             onView(withId(R.id.tab_list_view))
-                    .perform(RecyclerViewActions.actionOnItemAtPosition(targetIndex, click()));
+                    .perform(actionOnItemAtPosition(targetIndex, click()));
             CriteriaHelper.pollUiThread(() -> {
                 boolean doneHiding =
                         !mActivityTestRule.getActivity().getLayoutManager().overviewVisible();
@@ -638,8 +639,7 @@ public class StartSurfaceLayoutTest {
         onView(withId(R.id.tab_list_view))
                 .check(TabCountAssertion.havingTabCount(1));
 
-        onView(withId(R.id.tab_list_view))
-                .perform(RecyclerViewActions.actionOnItemAtPosition(0, click()));
+        onView(withId(R.id.tab_list_view)).perform(actionOnItemAtPosition(0, click()));
         CriteriaHelper.pollInstrumentationThread(
                 () -> !mActivityTestRule.getActivity().getLayoutManager().overviewVisible());
 
@@ -859,6 +859,46 @@ public class StartSurfaceLayoutTest {
         verifyOnlyOneTabSuggestionMessageCardIsShowing();
     }
 
+    @Test
+    @MediumTest
+    @Feature("TabSuggestion")
+    // clang-format off
+    @Features.EnableFeatures({ChromeFeatureList.CLOSE_TAB_SUGGESTIONS + "<Study"})
+    @CommandLineFlags.Add({BASE_PARAMS + "/baseline_tab_suggestions/true"})
+    public void testTabSuggestionMessageCardDismissAfterTabClosing() throws InterruptedException {
+        // clang-format on
+        prepareTabs(3, 0, mUrl);
+        CriteriaHelper.pollInstrumentationThread(
+                ()
+                        -> TabSuggestionMessageService.isSuggestionAvailableForTesting()
+                        && mActivityTestRule.getActivity()
+                                        .getTabModelSelector()
+                                        .getCurrentModel()
+                                        .getCount()
+                                == 3);
+
+        enterGTSWithThumbnailChecking();
+        CriteriaHelper.pollInstrumentationThread(
+                TabSwitcherCoordinator::hasAppendedMessagesForTesting);
+        onView(withId(R.id.tab_grid_message_item)).check(matches(isDisplayed()));
+
+        closeFirstTabInTabSwitcher();
+
+        CriteriaHelper.pollInstrumentationThread(
+                ()
+                        -> !TabSuggestionMessageService.isSuggestionAvailableForTesting()
+                        && mActivityTestRule.getActivity()
+                                        .getTabModelSelector()
+                                        .getCurrentModel()
+                                        .getCount()
+                                == 2);
+
+        onView(withId(R.id.tab_list_view))
+                .check(TabUiTestHelper.ChildrenCountAssertion.havingTabSuggestionMessageCardCount(
+                        0));
+        onView(withId(R.id.tab_grid_message_item)).check(doesNotExist());
+    }
+
     @Test
     @MediumTest
     @Feature("NewTabTile")

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

@@ -199,9 +199,11 @@ public class TabContext {
             List<Tab> relatedTabs = tabModelFilter.getRelatedTabList(currentTab.getId());
 
             if (relatedTabs != null && relatedTabs.size() > 1) {
+                List<Tab> nonClosingTabs = getNonClosingTabs(relatedTabs);
                 existingGroups.add(new TabGroupInfo(
-                        ((TabImpl) currentTab).getRootId(), createTabInfoList(relatedTabs)));
+                        ((TabImpl) currentTab).getRootId(), createTabInfoList(nonClosingTabs)));
             } else {
+                if (currentTab.isClosing()) continue;
                 ungroupedTabs.add(TabInfo.createFromTab(currentTab));
             }
         }
@@ -209,6 +211,16 @@ public class TabContext {
         return new TabContext(ungroupedTabs, existingGroups);
     }
 
+    private static List<Tab> getNonClosingTabs(List<Tab> tabs) {
+        List<Tab> nonClosingTabs = new ArrayList<>();
+        for (int i = 0; i < tabs.size(); i++) {
+            Tab tab = tabs.get(i);
+            if (tab.isClosing()) continue;
+            nonClosingTabs.add(tab);
+        }
+        return nonClosingTabs;
+    }
+
     private static List<TabInfo> createTabInfoList(List<Tab> tabs) {
         List<TabInfo> tabInfoList = new ArrayList<>();
         for (Tab tab : tabs) {

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

@@ -43,7 +43,12 @@ public abstract class TabContextObserver {
             }
 
             @Override
-            public void didCloseTab(int tabId, boolean incognito) {
+            public void tabClosureUndone(Tab tab) {
+                onTabContextChanged(TabContextChangeReason.TAB_CLOSURE_UNDONE);
+            }
+
+            @Override
+            public void willCloseTab(Tab tab, boolean animate) {
                 onTabContextChanged(TabContextChangeReason.TAB_CLOSED);
             }
         };
@@ -62,6 +67,7 @@ public abstract class TabContextObserver {
         int TAB_ADDED = 1;
         int TAB_CLOSED = 2;
         int FIRST_VISUALLY_NON_EMPTY_PAINT = 3;
+        int TAB_CLOSURE_UNDONE = 4;
     }
 
     /**

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

@@ -40,6 +40,8 @@ import android.support.test.espresso.contrib.RecyclerViewActions;
 import android.support.v7.widget.RecyclerView;
 import android.view.View;
 
+import androidx.annotation.IntDef;
+
 import org.hamcrest.Description;
 import org.hamcrest.Matcher;
 import org.hamcrest.TypeSafeMatcher;
@@ -61,6 +63,8 @@ import org.chromium.content_public.browser.test.util.CriteriaHelper;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 
 import java.io.File;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -541,23 +545,49 @@ public class TabUiTestHelper {
      * of children.
      */
     public static class ChildrenCountAssertion implements ViewAssertion {
-        private int mExpectedTabCount;
+        @IntDef({ChildrenType.TAB, ChildrenType.TAB_SUGGESTION_MESSAGE})
+        @Retention(RetentionPolicy.SOURCE)
+        public @interface ChildrenType {
+            int TAB = 0;
+            int TAB_SUGGESTION_MESSAGE = 1;
+        }
+
+        private int mExpectedCount;
+        @ChildrenType
+        private int mExpectedChildrenType;
 
         public static ChildrenCountAssertion havingTabCount(int tabCount) {
-            return new ChildrenCountAssertion(tabCount);
+            return new ChildrenCountAssertion(ChildrenType.TAB, tabCount);
         }
 
-        public ChildrenCountAssertion(int expectedTabCount) {
-            mExpectedTabCount = expectedTabCount;
+        public static ChildrenCountAssertion havingTabSuggestionMessageCardCount(int count) {
+            return new ChildrenCountAssertion(ChildrenType.TAB_SUGGESTION_MESSAGE, count);
+        }
+
+        public ChildrenCountAssertion(@ChildrenType int expectedChildrenType, int expectedCount) {
+            mExpectedChildrenType = expectedChildrenType;
+            mExpectedCount = expectedCount;
         }
 
         @Override
         public void check(View view, NoMatchingViewException noMatchException) {
             if (noMatchException != null) throw noMatchException;
 
+            switch (mExpectedChildrenType) {
+                case ChildrenType.TAB:
+                    checkTabCount(view);
+                    break;
+                case ChildrenType.TAB_SUGGESTION_MESSAGE:
+                    checkTabSuggestionMessageCard(view);
+                    break;
+            }
+        }
+
+        private void checkTabCount(View view) {
             RecyclerView recyclerView = ((RecyclerView) view);
             recyclerView.setItemAnimator(null); // Disable animation to reduce flakiness.
             RecyclerView.Adapter adapter = recyclerView.getAdapter();
+
             int itemCount = adapter.getItemCount();
             int nonTabCardCount = 0;
 
@@ -571,7 +601,26 @@ public class TabUiTestHelper {
                     nonTabCardCount += 1;
                 }
             }
-            assertEquals(mExpectedTabCount + nonTabCardCount, itemCount);
+            assertEquals(mExpectedCount + nonTabCardCount, itemCount);
+        }
+
+        private void checkTabSuggestionMessageCard(View view) {
+            RecyclerView recyclerView = ((RecyclerView) view);
+            recyclerView.setItemAnimator(null); // Disable animation to reduce flakiness.
+            RecyclerView.Adapter adapter = recyclerView.getAdapter();
+
+            int itemCount = adapter.getItemCount();
+            int tabSuggestionMessageCount = 0;
+
+            for (int i = 0; i < itemCount; i++) {
+                RecyclerView.ViewHolder viewHolder =
+                        recyclerView.findViewHolderForAdapterPosition(i);
+                if (viewHolder == null) return;
+                if (viewHolder.getItemViewType() == TabProperties.UiType.MESSAGE) {
+                    tabSuggestionMessageCount += 1;
+                }
+            }
+            assertEquals(mExpectedCount, tabSuggestionMessageCount);
         }
     }
 }

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

@@ -94,11 +94,20 @@ public class TabContextObserverTest {
     public void testCloseTab() {
         TabContextObserverTestHelper tabContextObserverTestHelper =
                 new TabContextObserverTestHelper(mTabModelSelector);
-        tabContextObserverTestHelper.mTabModelObserver.didCloseTab(0, false);
+        tabContextObserverTestHelper.mTabModelObserver.willCloseTab(null, false);
         Assert.assertEquals(TabContextObserver.TabContextChangeReason.TAB_CLOSED,
                 tabContextObserverTestHelper.getChangeReason());
     }
 
+    @Test
+    public void testUndoClosedTab() {
+        TabContextObserverTestHelper tabContextObserverTestHelper =
+                new TabContextObserverTestHelper(mTabModelSelector);
+        tabContextObserverTestHelper.mTabModelObserver.tabClosureUndone(null);
+        Assert.assertEquals(TabContextObserver.TabContextChangeReason.TAB_CLOSURE_UNDONE,
+                tabContextObserverTestHelper.getChangeReason());
+    }
+
     @Test
     public void testDidFirstVisuallyNonEmptyPaint() {
         TabContextObserverTestHelper tabContextObserverTestHelper =

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

@@ -42,6 +42,8 @@ public class TabContextTest {
     private static final int TAB_0_ID = 0;
     private static final int RELATED_TAB_0_ID = 1;
     private static final int RELATED_TAB_1_ID = 2;
+    private static final int NEW_TAB_1_ID = 3;
+    private static final int NEW_TAB_2_ID = 4;
     private static final int LAST_COMMITTED_INDEX = 1;
 
     @Rule
@@ -56,12 +58,12 @@ public class TabContextTest {
     @Mock
     private TabModelFilter mTabModelFilter;
 
-    private static Tab sTab0 = mockTab(TAB_0_ID, 6, "mock_title_tab_0", "mock_url_tab_0",
+    private Tab mTab0 = mockTab(TAB_0_ID, 6, "mock_title_tab_0", "mock_url_tab_0",
             "mock_original_url_tab_0", "mock_referrer_url_tab_0", 100);
-    private static Tab sRelatedTab0 =
+    private Tab mRelatedTab0 =
             mockTab(RELATED_TAB_0_ID, 6, "mock_title_related_tab_0", "mock_url_related_tab_0",
                     "mock_original_url_related_tab_0", "mock_referrer_url_related_tab_0", 200);
-    private static Tab sRelatedTab1 =
+    private Tab mRelatedTab1 =
             mockTab(RELATED_TAB_1_ID, 6, "mock_title_related_tab_1", "mock_url_related_tab_1",
                     "mock_original_url_related_tab_1", "mock_referrer_url_related_tab_1", 300);
 
@@ -102,9 +104,9 @@ public class TabContextTest {
      */
     @Test
     public void testRelatedTabsExist() {
-        doReturn(sTab0).when(mTabModelFilter).getTabAt(eq(TAB_0_ID));
+        doReturn(mTab0).when(mTabModelFilter).getTabAt(eq(TAB_0_ID));
         doReturn(1).when(mTabModelFilter).getCount();
-        doReturn(Arrays.asList(sTab0, sRelatedTab0, sRelatedTab1))
+        doReturn(Arrays.asList(mTab0, mRelatedTab0, mRelatedTab1))
                 .when(mTabModelFilter)
                 .getRelatedTabList(eq(TAB_0_ID));
         TabContext tabContext = TabContext.createCurrentContext(mTabModelSelector);
@@ -123,9 +125,9 @@ public class TabContextTest {
      */
     @Test
     public void testFindNoRelatedTabs() {
-        doReturn(sTab0).when(mTabModelFilter).getTabAt(eq(TAB_0_ID));
+        doReturn(mTab0).when(mTabModelFilter).getTabAt(eq(TAB_0_ID));
         doReturn(1).when(mTabModelFilter).getCount();
-        doReturn(Arrays.asList(sTab0)).when(mTabModelFilter).getRelatedTabList(eq(TAB_0_ID));
+        doReturn(Arrays.asList(mTab0)).when(mTabModelFilter).getRelatedTabList(eq(TAB_0_ID));
         TabContext tabContext = TabContext.createCurrentContext(mTabModelSelector);
         Assert.assertEquals(tabContext.getUngroupedTabs().size(), 1);
         List<TabContext.TabGroupInfo> tabGroups = tabContext.getTabGroups();
@@ -134,4 +136,35 @@ public class TabContextTest {
         Assert.assertEquals(1, ungroupedTabs.size());
         Assert.assertEquals(TAB_0_ID, ungroupedTabs.get(0).id);
     }
+
+    @Test
+    public void testExcludeClosingTabs() {
+        Tab newTab1 = mockTab(NEW_TAB_1_ID, NEW_TAB_1_ID, "", "", "", "", 0);
+        Tab newTab2 = mockTab(NEW_TAB_2_ID, NEW_TAB_2_ID, "", "", "", "", 0);
+        doReturn(mTab0).when(mTabModelFilter).getTabAt(eq(TAB_0_ID));
+        doReturn(newTab1).when(mTabModelFilter).getTabAt(eq(TAB_0_ID + 1));
+        doReturn(newTab2).when(mTabModelFilter).getTabAt(eq(TAB_0_ID + 2));
+        doReturn(3).when(mTabModelFilter).getCount();
+        doReturn(Arrays.asList(mTab0, mRelatedTab0, mRelatedTab1))
+                .when(mTabModelFilter)
+                .getRelatedTabList(eq(TAB_0_ID));
+        doReturn(Arrays.asList(newTab1)).when(mTabModelFilter).getRelatedTabList(eq(NEW_TAB_1_ID));
+        doReturn(Arrays.asList(newTab2)).when(mTabModelFilter).getRelatedTabList(eq(NEW_TAB_2_ID));
+
+        TabContext tabContext = TabContext.createCurrentContext(mTabModelSelector);
+        Assert.assertEquals(2, tabContext.getUngroupedTabs().size());
+        Assert.assertEquals(1, tabContext.getTabGroups().size());
+        Assert.assertEquals(3, tabContext.getTabGroups().get(0).tabs.size());
+
+        // close newTab1
+        doReturn(true).when(newTab1).isClosing();
+        tabContext = TabContext.createCurrentContext(mTabModelSelector);
+        Assert.assertEquals(1, tabContext.getUngroupedTabs().size());
+
+        // close mTab0
+        doReturn(true).when(mTab0).isClosing();
+        tabContext = TabContext.createCurrentContext(mTabModelSelector);
+        Assert.assertEquals(1, tabContext.getTabGroups().size());
+        Assert.assertEquals(2, tabContext.getTabGroups().get(0).tabs.size());
+    }
 }

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

@@ -91,6 +91,7 @@ public class TabSuggestionsOrchestratorTest {
                 .when(mTabModelFilterProvider)
                 .addTabModelFilterObserver(any(TabModelObserver.class));
         doReturn(mTabModelFilter).when(mTabModelFilterProvider).getCurrentTabModelFilter();
+        // TODO(1049917): According to TabModelFilter, getRelatedTabList returns non-null.
         doReturn(null).when(mTabModelFilter).getRelatedTabList(anyInt());
         RecordUserAction.setDisabledForTests(true);
         RecordHistogram.setDisabledForTests(true);