Avoid sending null TabSuggestion via TabSuggestionFeedback callback

Because TabContextObserver listens to TabModel changing events to
invalidate TabSuggestion, closing the selected tabs before running the
TabSuggestionFeedback callback results in using a null TabSuggestion for
the callback. This CL addresses the null usage by running the callback
before closing the selected tabs.

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

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

@@ -134,12 +134,13 @@ public class TabSuggestionMessageService extends MessageService implements TabSu
                             List<Tab> selectedTabs, TabModelSelector tabModelSelector) {
                         int totalTabCountBeforeProcess =
                                 tabModelSelector.getCurrentModel().getCount();
-                        super.processSelectedTabs(selectedTabs, tabModelSelector);
                         List<Integer> selectedTabIds = new ArrayList<>();
                         for (int i = 0; i < selectedTabs.size(); i++) {
                             selectedTabIds.add(selectedTabs.get(i).getId());
                         }
                         accepted(selectedTabIds, totalTabCountBeforeProcess);
+
+                        super.processSelectedTabs(selectedTabs, tabModelSelector);
                     }
                 };
             default:
@@ -194,12 +195,14 @@ public class TabSuggestionMessageService extends MessageService implements TabSu
     @VisibleForTesting
     public void dismiss() {
         assert mCurrentTabSuggestionFeedback != null;
+        assert mCurrentBestTabSuggestion != null;
         mCurrentTabSuggestionFeedback.onResult(
                 new TabSuggestionFeedback(mCurrentBestTabSuggestion, NOT_CONSIDERED, null, 0));
     }
 
     private void accepted(List<Integer> selectedTabIds, int totalTabCount) {
         assert mCurrentTabSuggestionFeedback != null;
+        assert mCurrentBestTabSuggestion != null;
         mCurrentTabSuggestionFeedback.onResult(new TabSuggestionFeedback(
                 mCurrentBestTabSuggestion, ACCEPTED, selectedTabIds, totalTabCount));
     }
@@ -220,6 +223,7 @@ public class TabSuggestionMessageService extends MessageService implements TabSu
     @Override
     public void onTabSuggestionInvalidated() {
         mCurrentBestTabSuggestion = null;
+        mCurrentTabSuggestionFeedback = null;
         sSuggestionAvailableForTesting = false;
         sendInvalidNotification();
     }

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

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.tasks.tab_management;
+
+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.matches;
+import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed;
+import static android.support.test.espresso.matcher.ViewMatchers.withId;
+import static android.support.test.espresso.matcher.ViewMatchers.withParent;
+
+import static org.hamcrest.core.AllOf.allOf;
+
+import android.support.test.filters.MediumTest;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TestRule;
+import org.junit.runner.RunWith;
+
+import org.chromium.base.test.util.CommandLineFlags;
+import org.chromium.base.test.util.Restriction;
+import org.chromium.chrome.browser.flags.ChromeFeatureList;
+import org.chromium.chrome.browser.flags.ChromeSwitches;
+import org.chromium.chrome.browser.tabmodel.TabModel;
+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.content_public.browser.test.util.Criteria;
+import org.chromium.content_public.browser.test.util.CriteriaHelper;
+import org.chromium.ui.test.util.UiRestriction;
+
+/**
+ * End-to-end tests for TabSuggestion.
+ */
+@RunWith(ChromeJUnit4ClassRunner.class)
+// clang-format off
+@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
+@Features.EnableFeatures({ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID,
+        ChromeFeatureList.CLOSE_TAB_SUGGESTIONS+"<Study"})
+@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE, "force-fieldtrials=Study/Group",
+        "force-fieldtrial-params=Study.Group:baseline_tab_suggestions/true"})
+public class TabSuggestionMessageCardTest {
+    // clang-format on
+    @Rule
+    public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
+
+    @Rule
+    public TestRule mProcessor = new Features.InstrumentationProcessor();
+
+    @Before
+    public void setUp() {
+        mActivityTestRule.startMainActivityOnBlankPage();
+        TabUiTestHelper.prepareTabsWithThumbnail(mActivityTestRule, 3, 0, "about:blank");
+    }
+
+    @Test
+    @MediumTest
+    public void closeTabSuggestionReviewedAndAccepted() {
+        TabModel currentTabModel =
+                mActivityTestRule.getActivity().getTabModelSelector().getCurrentModel();
+        CriteriaHelper.pollUiThread(Criteria.equals(3, currentTabModel::getCount));
+        CriteriaHelper.pollUiThread(TabSuggestionMessageService::isSuggestionAvailableForTesting);
+
+        TabUiTestHelper.enterTabSwitcher(mActivityTestRule.getActivity());
+        CriteriaHelper.pollUiThread(TabSwitcherCoordinator::hasAppendedMessagesForTesting);
+        onView(withId(R.id.tab_grid_message_item)).check(matches(isDisplayed()));
+
+        onView(allOf(withId(R.id.action_button), withParent(withId(R.id.tab_grid_message_item))))
+                .perform(click());
+        TabSelectionEditorTestingRobot tabSelectionEditorTestingRobot =
+                new TabSelectionEditorTestingRobot();
+        tabSelectionEditorTestingRobot.resultRobot.verifyTabSelectionEditorIsVisible();
+
+        tabSelectionEditorTestingRobot.actionRobot.clickToolbarActionButton();
+        tabSelectionEditorTestingRobot.resultRobot.verifyTabSelectionEditorIsHidden();
+        CriteriaHelper.pollUiThread(Criteria.equals(0, currentTabModel::getCount));
+    }
+
+    @Test
+    @MediumTest
+    public void closeTabSuggestionReviewedAndDismissed() {
+        TabModel currentTabModel =
+                mActivityTestRule.getActivity().getTabModelSelector().getCurrentModel();
+        CriteriaHelper.pollUiThread(Criteria.equals(3, currentTabModel::getCount));
+        CriteriaHelper.pollUiThread(TabSuggestionMessageService::isSuggestionAvailableForTesting);
+
+        TabUiTestHelper.enterTabSwitcher(mActivityTestRule.getActivity());
+        CriteriaHelper.pollUiThread(TabSwitcherCoordinator::hasAppendedMessagesForTesting);
+        onView(withId(R.id.tab_grid_message_item)).check(matches(isDisplayed()));
+
+        onView(allOf(withId(R.id.action_button), withParent(withId(R.id.tab_grid_message_item))))
+                .perform(click());
+        TabSelectionEditorTestingRobot tabSelectionEditorTestingRobot =
+                new TabSelectionEditorTestingRobot();
+        tabSelectionEditorTestingRobot.resultRobot.verifyTabSelectionEditorIsVisible();
+
+        tabSelectionEditorTestingRobot.actionRobot.clickToolbarNavigationButton();
+        tabSelectionEditorTestingRobot.resultRobot.verifyTabSelectionEditorIsHidden();
+        CriteriaHelper.pollUiThread(Criteria.equals(3, currentTabModel::getCount));
+    }
+}

chrome/android/features/tab_ui/tab_management_java_sources.gni

@@ -47,6 +47,7 @@ tab_management_test_java_sources = [
   "//chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorLayoutBinderTest.java",
   "//chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorTest.java",
   "//chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabSelectionEditorTestingRobot.java",
+  "//chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabSuggestionMessageCardTest.java",
   "//chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabSwitcherMultiWindowTest.java",
   "//chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabUiTestHelper.java",
   "//chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TestRecyclerViewSimpleViewBinder.java",