Commit f169f08f authored by Mei Liang's avatar Mei Liang Committed by Commit Bot

Avoid showing duplicate closing suggestion cards

This CL cleans all existing message cards if GTS can be shown quickly,
then re-appends them to the end of GTS. These steps are required, since
the internal model resets only when the GTS can't be shown quickly.

Change-Id: I30fce1a2eda27e38a42bf05fd6090be769e40427
Bug: 1043202
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2007344
Commit-Queue: Mei Liang <meiliang@chromium.org>
Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738434}
parent 17c9e0dc
......@@ -12,6 +12,7 @@ import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed;
import static android.support.test.espresso.matcher.ViewMatchers.withContentDescription;
import static android.support.test.espresso.matcher.ViewMatchers.withId;
import static android.support.test.espresso.matcher.ViewMatchers.withParent;
import static android.support.test.espresso.matcher.ViewMatchers.withText;
import static org.hamcrest.core.AllOf.allOf;
import static org.junit.Assert.assertEquals;
......@@ -100,6 +101,7 @@ import java.io.IOException;
import java.lang.ref.WeakReference;
import java.util.LinkedList;
import java.util.List;
import java.util.Locale;
// clang-format off
/** Tests for the {@link StartSurfaceLayout} */
......@@ -820,6 +822,31 @@ public class StartSurfaceLayoutTest {
tabSelectionEditorTestingRobot.resultRobot.verifyTabSelectionEditorIsVisible();
}
@Test
@MediumTest
@Feature("TabSuggestion")
// clang-format off
@Features.EnableFeatures({ChromeFeatureList.CLOSE_TAB_SUGGESTIONS + "<Study"})
@CommandLineFlags.Add({BASE_PARAMS +
"/cleanup-delay/10000/close_tab_suggestions_stale_time_ms/0"})
public void testShowOnlyOneTabSuggestionMessageCard_withSoftCleanup()
throws InterruptedException {
// clang-format on
verifyOnlyOneTabSuggestionMessageCardIsShowing();
}
@Test
@MediumTest
@Feature("TabSuggestion")
// clang-format off
@Features.EnableFeatures({ChromeFeatureList.CLOSE_TAB_SUGGESTIONS + "<Study"})
@CommandLineFlags.Add({BASE_PARAMS + "/close_tab_suggestions_stale_time_ms/0"})
public void testShowOnlyOneTabSuggestionMessageCard_withHardCleanup()
throws InterruptedException {
// clang-format on
verifyOnlyOneTabSuggestionMessageCardIsShowing();
}
@Test
@MediumTest
@Feature("NewTabTile")
......@@ -1447,4 +1474,36 @@ public class StartSurfaceLayoutTest {
Math.abs(bitmapRatio - ratio) <= TabContentManager.ASPECT_RATIO_PRECISION);
}
}
private void verifyOnlyOneTabSuggestionMessageCardIsShowing() throws InterruptedException {
String suggestionMessageTemplate = mActivityTestRule.getActivity().getString(
org.chromium.chrome.tab_ui.R.string.tab_suggestion_close_stale_message);
String suggestionMessage =
String.format(Locale.getDefault(), suggestionMessageTemplate, "3");
prepareTabs(3, 0, mUrl);
CriteriaHelper.pollInstrumentationThread(
()
-> TabSuggestionMessageService.isSuggestionAvailableForTesting()
&& mActivityTestRule.getActivity()
.getTabModelSelector()
.getCurrentModel()
.getCount()
== 3);
enterGTSWithThumbnailChecking();
CriteriaHelper.pollInstrumentationThread(
TabSwitcherCoordinator::hasAppendedMessagesForTesting);
onView(allOf(withText(suggestionMessage), withParent(withId(R.id.tab_grid_message_item))))
.check(matches(isDisplayed()));
leaveGTSAndVerifyThumbnailsAreReleased();
// With soft or hard clean up depends on the soft-cleanup-delay and cleanup-delay params.
enterGTSWithThumbnailChecking();
CriteriaHelper.pollInstrumentationThread(
TabSwitcherCoordinator::hasAppendedMessagesForTesting);
// This will fail with error "matched multiple views" when there is more than one suggestion
// message card.
onView(allOf(withText(suggestionMessage), withParent(withId(R.id.tab_grid_message_item))))
.check(matches(isDisplayed()));
}
}
......@@ -18,11 +18,12 @@ import java.lang.annotation.RetentionPolicy;
* understands.
*/
public class MessageService {
@IntDef({MessageType.TAB_SUGGESTION})
@IntDef({MessageType.TAB_SUGGESTION, MessageType.ALL})
@Retention(RetentionPolicy.SOURCE)
public @interface MessageType {
int FOR_TESTING = 0;
int TAB_SUGGESTION = 1;
int ALL = 2;
}
/**
......
......@@ -1358,6 +1358,13 @@ class TabListMediator {
void removeSpecialItemFromModel(@UiType int uiType, int itemIdentifier) {
int index = TabModel.INVALID_TAB_INDEX;
if (uiType == UiType.MESSAGE) {
if (itemIdentifier == MessageService.MessageType.ALL) {
while (mModel.lastIndexForMessageItem() != TabModel.INVALID_TAB_INDEX) {
index = mModel.lastIndexForMessageItem();
mModel.removeAt(index);
}
return;
}
index = mModel.lastIndexForMessageItemFromType(itemIdentifier);
} else if (uiType == UiType.NEW_TAB_TILE) {
index = mModel.getIndexForNewTabTile();
......
......@@ -84,6 +84,19 @@ class TabListModel extends ModelList {
return TabModel.INVALID_TAB_INDEX;
}
/**
* Get the last index of a message item.
*/
public int lastIndexForMessageItem() {
for (int i = size() - 1; i >= 0; i--) {
PropertyModel model = get(i).model;
if (model.get(CARD_TYPE) == MESSAGE) {
return i;
}
}
return TabModel.INVALID_TAB_INDEX;
}
/**
* Get the index that matches the new tab tile in TabListModel.
* @return The index within the model.
......
......@@ -304,7 +304,6 @@ public class TabSwitcherCoordinator
// ResetHandler implementation.
@Override
public boolean resetWithTabList(@Nullable TabList tabList, boolean quickMode, boolean mruMode) {
sAppendedMessagesForTesting = false;
List<Tab> tabs = null;
if (tabList != null) {
tabs = new ArrayList<>();
......@@ -317,6 +316,7 @@ public class TabSwitcherCoordinator
boolean showQuickly = mTabListCoordinator.resetWithListOfTabs(tabs, quickMode, mruMode);
if (showQuickly) {
mTabListCoordinator.removeSpecialListItem(TabProperties.UiType.NEW_TAB_TILE, 0);
removeAllAppendedMessage();
}
int cardsCount = tabs == null ? 0 : tabs.size();
......@@ -330,7 +330,13 @@ public class TabSwitcherCoordinator
return showQuickly;
}
private void removeAllAppendedMessage() {
mTabListCoordinator.removeSpecialListItem(
TabProperties.UiType.MESSAGE, MessageService.MessageType.ALL);
}
private void appendMessagesTo(int index) {
sAppendedMessagesForTesting = false;
List<MessageCardProviderMediator.Message> messages =
mMessageCardProviderCoordinator.getMessageItems();
for (int i = 0; i < messages.size(); i++) {
......
......@@ -498,6 +498,8 @@ const FeatureEntry::FeatureVariation kForceDarkVariations[] = {
#endif // !OS_CHROMEOS
#if defined(OS_ANDROID)
const FeatureEntry::FeatureParam kCloseTabSuggestionsStale_Immediate[] = {
{"close_tab_suggestions_stale_time_ms", "0"}};
const FeatureEntry::FeatureParam kCloseTabSuggestionsStale_4Hours[] = {
{"close_tab_suggestions_stale_time_ms", "14400000"}};
const FeatureEntry::FeatureParam kCloseTabSuggestionsStale_8Hours[] = {
......@@ -505,6 +507,8 @@ const FeatureEntry::FeatureParam kCloseTabSuggestionsStale_8Hours[] = {
const FeatureEntry::FeatureParam kCloseTabSuggestionsStale_7Days[] = {
{"close_tab_suggestions_stale_time_ms", "604800000"}};
const FeatureEntry::FeatureVariation kCloseTabSuggestionsStaleVariations[] = {
{"Immediate", kCloseTabSuggestionsStale_Immediate,
base::size(kCloseTabSuggestionsStale_Immediate), nullptr},
{"4 hours", kCloseTabSuggestionsStale_4Hours,
base::size(kCloseTabSuggestionsStale_4Hours), nullptr},
{"8 hours", kCloseTabSuggestionsStale_8Hours,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment