Skip to content

Commit

Permalink
[Android Bulk Tab Restore] Cleanup flags
Browse files Browse the repository at this point in the history
Cleanup flags as this feature is launched.

Delete any unused code or changes.

Bug: 1326950
Change-Id: I7bbd26f319076c007f6e8c3d052ae32ce864ef86
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3913413
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1051379}
  • Loading branch information
ckitagawa-work authored and Chromium LUCI CQ committed Sep 26, 2022
1 parent ee691b9 commit 5f9bac0
Show file tree
Hide file tree
Showing 19 changed files with 38 additions and 246 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.invalidation.SessionsInvalidationManager;
import org.chromium.chrome.browser.ntp.ForeignSessionHelper.ForeignSession;
import org.chromium.chrome.browser.ntp.ForeignSessionHelper.ForeignSessionTab;
Expand Down Expand Up @@ -192,15 +191,8 @@ public void destroy() {
}

private void updateRecentlyClosedEntries() {
if (ChromeFeatureList.isEnabled(ChromeFeatureList.BULK_TAB_RESTORE)) {
mRecentlyClosedEntries = mRecentlyClosedTabManager.getRecentlyClosedEntries(
RECENTLY_CLOSED_MAX_ENTRY_COUNT);
} else {
mRecentlyClosedEntries =
(List<RecentlyClosedEntry>) (List<? extends RecentlyClosedEntry>)
mRecentlyClosedTabManager.getRecentlyClosedTabs(
RECENTLY_CLOSED_MAX_ENTRY_COUNT);
}
mRecentlyClosedEntries =
mRecentlyClosedTabManager.getRecentlyClosedEntries(RECENTLY_CLOSED_MAX_ENTRY_COUNT);
for (RecentlyClosedEntry entry : mRecentlyClosedEntries) {
if (entry instanceof RecentlyClosedTab
&& !mTabSessionIdsRestored.containsKey(entry.getSessionId())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@

import org.chromium.base.metrics.RecordHistogram;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.ntp.ForeignSessionHelper.ForeignSession;
import org.chromium.chrome.browser.ntp.ForeignSessionHelper.ForeignSessionTab;
import org.chromium.chrome.browser.ntp.ForeignSessionHelper.ForeignSessionWindow;
Expand Down Expand Up @@ -552,7 +551,6 @@ public void configureChildView(int childPosition, ViewHolder viewHolder) {
R.dimen.recent_tabs_foreign_session_group_item_height);
RecentlyClosedEntry entry = getChild(childPosition);
if (!(entry instanceof RecentlyClosedTab)) {
assert ChromeFeatureList.isEnabled(ChromeFeatureList.BULK_TAB_RESTORE);
int tabCount = 0;
if (entry instanceof RecentlyClosedGroup) {
RecentlyClosedGroup recentlyClosedGroup = (RecentlyClosedGroup) entry;
Expand Down Expand Up @@ -638,7 +636,6 @@ public boolean onChildClick(int childPosition) {
(RecentlyClosedTab) entry, WindowOpenDisposition.CURRENT_TAB);
return true;
}
assert ChromeFeatureList.isEnabled(ChromeFeatureList.BULK_TAB_RESTORE);
mRecentTabsManager.openRecentlyClosedEntry(entry);
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,6 @@ public class RecentlyClosedBridge implements RecentlyClosedTabManager {
@Nullable
private Runnable mEntriesUpdatedRunnable;

// TODO(crbug/1307345): Remove in favor of generic entries.
@CalledByNative
private static void pushTab(List<RecentlyClosedTab> tabs, int id, long timestamp, String title,
GURL url, String groupId) {
RecentlyClosedTab tab = new RecentlyClosedTab(id, timestamp, title, url, groupId);
tabs.add(tab);
}

private static void addTabs(List<RecentlyClosedTab> tabs, int[] tabIds, long[] tabTimestamps,
String[] tabTitles, GURL[] tabUrls, String[] tabGroupIds) {
assert tabIds.length == tabTimestamps.length;
Expand Down Expand Up @@ -132,14 +124,6 @@ public void setEntriesUpdatedRunnable(@Nullable Runnable runnable) {
mEntriesUpdatedRunnable = runnable;
}

@Override
public List<RecentlyClosedTab> getRecentlyClosedTabs(int maxTabCount) {
List<RecentlyClosedTab> tabs = new ArrayList<RecentlyClosedTab>();
boolean received = RecentlyClosedBridgeJni.get().getRecentlyClosedTabs(
mNativeBridge, tabs, maxTabCount);
return received ? tabs : null;
}

@Override
public List<RecentlyClosedEntry> getRecentlyClosedEntries(int maxEntryCount) {
List<RecentlyClosedEntry> entries = new ArrayList<RecentlyClosedEntry>();
Expand Down Expand Up @@ -187,8 +171,6 @@ private void onUpdated() {
public interface Natives {
long init(RecentlyClosedBridge caller, Profile profile);
void destroy(long nativeRecentlyClosedTabsBridge);
boolean getRecentlyClosedTabs(
long nativeRecentlyClosedTabsBridge, List<RecentlyClosedTab> tabs, int maxTabCount);
boolean getRecentlyClosedEntries(long nativeRecentlyClosedTabsBridge,
List<RecentlyClosedEntry> entries, int maxEntryCount);
boolean openRecentlyClosedTab(long nativeRecentlyClosedTabsBridge, TabModel tabModel,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,6 @@ public interface RecentlyClosedTabManager {
*/
void setEntriesUpdatedRunnable(@Nullable Runnable runnable);

/**
* @param maxTabCount The maximum number of recently closed tabs to return.
* @return A snapshot of the list of recently closed tabs, with up to maxTabCount elements.
*/
List<RecentlyClosedTab> getRecentlyClosedTabs(int maxTabCount);

// TODO(crbug/1307345): Replace calls to getRecentlyClosedTabs() with this method.
/**
* @param maxEntryCount The maximum number of recently closed entries to return.
* @return A snapshot of the list of recently closed entries, with up to maxEntryCount elements.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

package org.chromium.chrome.browser.tab.tab_restore;

import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.state.CriticalPersistedTabData;
import org.chromium.chrome.browser.tabmodel.TabModel;
Expand Down Expand Up @@ -39,11 +38,6 @@ public void destroy() {

@Override
public void onFinishingMultipleTabClosure(List<Tab> tabs) {
if (!ChromeFeatureList.isEnabled(ChromeFeatureList.BULK_TAB_RESTORE)) {
legacyCreateHistoricalTabs(tabs);
return;
}

if (tabs.isEmpty()) return;

if (tabs.size() == 1) {
Expand All @@ -54,16 +48,6 @@ public void onFinishingMultipleTabClosure(List<Tab> tabs) {
buildGroupsAndCreateClosure(tabs);
}

/**
* Creates one historical tab entry per {@link Tab} in {@code tabs}. This is approximately
* identical to what occurred prior to {@link ChromeFeatureList.BULK_TAB_RESTORE}.
*/
private void legacyCreateHistoricalTabs(List<Tab> tabs) {
for (Tab tab : tabs) {
mHistoricalTabSaver.createHistoricalTab(tab);
}
}

private void buildGroupsAndCreateClosure(List<Tab> tabs) {
HashMap<Integer, HistoricalEntry> idToGroup = new HashMap<>();
List<HistoricalEntry> entries = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,6 @@ public void setEntriesUpdatedRunnable(@Nullable Runnable runnable) {
mEntriesUpdatedRunnable = runnable;
}

@Override
public List<RecentlyClosedTab> getRecentlyClosedTabs(int maxTabCount) {
List<RecentlyClosedTab> tabs = new ArrayList<>();
for (int i = 0; i < maxTabCount && i < mTabs.size(); i++) {
tabs.add((RecentlyClosedTab) mTabs.get(i));
}
return tabs;
}

@Override
public List<RecentlyClosedEntry> getRecentlyClosedEntries(int maxEntryCount) {
List<RecentlyClosedEntry> entries = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.chromium.chrome.test.util.ChromeRenderTestRule;
import org.chromium.chrome.test.util.RecentTabsPageTestUtils;
import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
import org.chromium.chrome.test.util.browser.signin.AccountManagerTestRule;
import org.chromium.chrome.test.util.browser.signin.SigninTestRule;
import org.chromium.components.embedder_support.util.UrlConstants;
Expand Down Expand Up @@ -137,7 +136,6 @@ public void testRecentlyClosedTabs() throws ExecutionException {
@Test
@LargeTest
@Feature({"RecentTabsPage", "RenderTest"})
@EnableFeatures({ChromeFeatureList.BULK_TAB_RESTORE})
// Disable sign-in to suppress sync promo, as it's unrelated to this render test.
@Policies.Add(@Policies.Item(key = "BrowserSignin", string = "0"))
public void testRecentlyClosedGroup_WithTitle() throws Exception {
Expand Down Expand Up @@ -173,7 +171,6 @@ public void testRecentlyClosedGroup_WithTitle() throws Exception {
@Test
@LargeTest
@Feature({"RecentTabsPage", "RenderTest"})
@EnableFeatures({ChromeFeatureList.BULK_TAB_RESTORE})
// Disable sign-in to suppress sync promo, as it's unrelated to this render test.
@Policies.Add(@Policies.Item(key = "BrowserSignin", string = "0"))
public void testRecentlyClosedGroup_WithoutTitle() throws Exception {
Expand Down Expand Up @@ -210,7 +207,6 @@ public void testRecentlyClosedGroup_WithoutTitle() throws Exception {
@Test
@LargeTest
@Feature({"RecentTabsPage", "RenderTest"})
@EnableFeatures({ChromeFeatureList.BULK_TAB_RESTORE})
// Disable sign-in to suppress sync promo, as it's unrelated to this render test.
@Policies.Add(@Policies.Item(key = "BrowserSignin", string = "0"))
public void testRecentlyClosedBulkEvent() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public void setUp() {
mActivity.getTabModelSelectorSupplier().get());
mRecentlyClosedBridge.clearRecentlyClosedEntries();
Assert.assertEquals(
0, mRecentlyClosedBridge.getRecentlyClosedTabs(MAX_ENTRY_COUNT).size());
0, mRecentlyClosedBridge.getRecentlyClosedEntries(MAX_ENTRY_COUNT).size());
});
mActivity = sActivityTestRule.getActivity();
mTabModelSelector = mActivity.getTabModelSelectorSupplier().get();
Expand All @@ -107,14 +107,13 @@ public void tearDown() {
TestThreadUtils.runOnUiThreadBlocking(() -> {
mRecentlyClosedBridge.clearRecentlyClosedEntries();
Assert.assertEquals(
0, mRecentlyClosedBridge.getRecentlyClosedTabs(MAX_ENTRY_COUNT).size());
0, mRecentlyClosedBridge.getRecentlyClosedEntries(MAX_ENTRY_COUNT).size());
mRecentlyClosedBridge.destroy();
});
}

/**
* Tests opening the most recently closed tab in the background. This is a legacy test prior to
* {@link ChromeFeatureList.BULK_TAB_RESTORE}.
* Tests opening the most recently closed tab in the background.
*/
@Test
@MediumTest
Expand All @@ -136,7 +135,9 @@ public void testOpenMostRecentlyClosedEntry_Tab_InBackground() {
final int[] tabCount = new int[1];
TestThreadUtils.runOnUiThreadBlocking(() -> {
tabCount[0] = mTabModel.getCount();
recentTabs.addAll(mRecentlyClosedBridge.getRecentlyClosedTabs(MAX_ENTRY_COUNT));
recentTabs.addAll(
(List<RecentlyClosedTab>) (List<? extends RecentlyClosedEntry>)
mRecentlyClosedBridge.getRecentlyClosedEntries(MAX_ENTRY_COUNT));
mRecentlyClosedBridge.openMostRecentlyClosedEntry(mTabModel);
});
// 1. Blank Tab
Expand All @@ -157,9 +158,7 @@ public void testOpenMostRecentlyClosedEntry_Tab_InBackground() {
}

/**
* Tests opening a specific closed {@link Tab} as a new background tab. This is a legacy test
* prior to
* {@link ChromeFeatureList.BULK_TAB_RESTORE}.
* Tests opening a specific closed {@link Tab} as a new background tab.
*/
@Test
@MediumTest
Expand All @@ -175,14 +174,17 @@ public void testOpenRecentlyClosedTab_InCurrentTab() {
mTabModel.setIndex(mTabModel.indexOf(tabC), TabSelectionType.FROM_USER, false);
titles[0] = tabA.getTitle();
titles[1] = tabB.getTitle();
mTabModel.closeMultipleTabs(Arrays.asList(new Tab[] {tabB, tabA}), false);
mTabModel.closeTab(tabB);
mTabModel.closeTab(tabA);
});

final List<RecentlyClosedTab> recentTabs = new ArrayList<>();
final int[] tabCount = new int[1];
TestThreadUtils.runOnUiThreadBlocking(() -> {
tabCount[0] = mTabModel.getCount();
recentTabs.addAll(mRecentlyClosedBridge.getRecentlyClosedTabs(MAX_ENTRY_COUNT));
recentTabs.addAll(
(List<RecentlyClosedTab>) (List<? extends RecentlyClosedEntry>)
mRecentlyClosedBridge.getRecentlyClosedEntries(MAX_ENTRY_COUNT));
mRecentlyClosedBridge.openRecentlyClosedTab(
mTabModel, recentTabs.get(1), WindowOpenDisposition.CURRENT_TAB);
});
Expand Down Expand Up @@ -215,9 +217,7 @@ public void testOpenRecentlyClosedTab_InCurrentTab() {
}

/**
* Tests opening a specific closed {@link Tab} that was frozen as a new background tab. This is
* a legacy test prior to
* {@link ChromeFeatureList.BULK_TAB_RESTORE}.
* Tests opening a specific closed {@link Tab} that was frozen as a new background tab.
*/
@Test
@MediumTest
Expand All @@ -241,7 +241,9 @@ public void testOpenRecentlyClosedTab_Frozen_InBackground() {
final int[] tabCount = new int[1];
TestThreadUtils.runOnUiThreadBlocking(() -> {
tabCount[0] = mTabModel.getCount();
recentTabs.addAll(mRecentlyClosedBridge.getRecentlyClosedTabs(MAX_ENTRY_COUNT));
recentTabs.addAll(
(List<RecentlyClosedTab>) (List<? extends RecentlyClosedEntry>)
mRecentlyClosedBridge.getRecentlyClosedEntries(MAX_ENTRY_COUNT));
mRecentlyClosedBridge.openRecentlyClosedTab(
mTabModel, recentTabs.get(0), WindowOpenDisposition.NEW_BACKGROUND_TAB);
});
Expand All @@ -266,7 +268,6 @@ public void testOpenRecentlyClosedTab_Frozen_InBackground() {
*/
@Test
@MediumTest
@EnableFeatures({ChromeFeatureList.BULK_TAB_RESTORE})
public void testOpenRecentlyClosedTab_FromBulkClosure_InNewTab() {
// Tab order is inverted in RecentlyClosedEntry as most recent comes first so log data in
// reverse.
Expand Down Expand Up @@ -308,7 +309,7 @@ public void testOpenRecentlyClosedTab_FromBulkClosure_InNewTab() {
*/
@Test
@MediumTest
@EnableFeatures({ChromeFeatureList.BULK_TAB_RESTORE, ChromeFeatureList.TAB_GROUPS_ANDROID})
@EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@Restriction({Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
public void testOpenRecentlyClosedTab_FromGroupInBulkClosure_InBackgroundTab() {
if (mTabGroupModelFilter == null) return;
Expand Down Expand Up @@ -363,7 +364,7 @@ public void testOpenRecentlyClosedTab_FromGroupInBulkClosure_InBackgroundTab() {
*/
@Test
@MediumTest
@EnableFeatures({ChromeFeatureList.BULK_TAB_RESTORE, ChromeFeatureList.TAB_GROUPS_ANDROID})
@EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@Restriction({Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
public void testOpenRecentlyClosedTab_FromGroupClosure_InCurrentTab() {
if (mTabGroupModelFilter == null) return;
Expand Down Expand Up @@ -425,7 +426,7 @@ public void testOpenRecentlyClosedTab_FromGroupClosure_InCurrentTab() {
*/
@Test
@MediumTest
@EnableFeatures({ChromeFeatureList.BULK_TAB_RESTORE, ChromeFeatureList.TAB_GROUPS_ANDROID})
@EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@Restriction({Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
public void testOpenRecentlyClosedEntry_Tab_FromMultipleTabs() {
if (mTabGroupModelFilter == null) return;
Expand Down Expand Up @@ -489,7 +490,7 @@ public void testOpenRecentlyClosedEntry_Tab_FromMultipleTabs() {
*/
@Test
@MediumTest
@EnableFeatures({ChromeFeatureList.BULK_TAB_RESTORE, ChromeFeatureList.TAB_GROUPS_ANDROID})
@EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@Restriction({Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
public void testOpenRecentlyClosedEntry_Tab_FromGroupClosure() {
if (mTabGroupModelFilter == null) return;
Expand Down Expand Up @@ -540,7 +541,7 @@ public void testOpenRecentlyClosedEntry_Tab_FromGroupClosure() {
*/
@Test
@MediumTest
@EnableFeatures({ChromeFeatureList.BULK_TAB_RESTORE, ChromeFeatureList.TAB_GROUPS_ANDROID})
@EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@Restriction({Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
public void testOpenRecentlyClosedEntry_Group_FromGroupClosure() {
if (mTabGroupModelFilter == null) return;
Expand Down Expand Up @@ -593,7 +594,7 @@ public void testOpenRecentlyClosedEntry_Group_FromGroupClosure() {
*/
@Test
@LargeTest
@EnableFeatures({ChromeFeatureList.BULK_TAB_RESTORE, ChromeFeatureList.TAB_GROUPS_ANDROID})
@EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@Restriction({Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
public void testOpenRecentlyClosedEntry_Group_FromGroupClosure_WithRestart() {
if (mTabGroupModelFilter == null) return;
Expand Down Expand Up @@ -691,7 +692,7 @@ public void testOpenRecentlyClosedEntry_Group_FromGroupClosure_WithRestart() {
*/
@Test
@MediumTest
@EnableFeatures({ChromeFeatureList.BULK_TAB_RESTORE, ChromeFeatureList.TAB_GROUPS_ANDROID})
@EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@Restriction({Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
public void testOpenRecentlyClosedEntry_Tab_FromBulkClosure() {
if (mTabGroupModelFilter == null) return;
Expand Down Expand Up @@ -741,7 +742,7 @@ public void testOpenRecentlyClosedEntry_Tab_FromBulkClosure() {
*/
@Test
@MediumTest
@EnableFeatures({ChromeFeatureList.BULK_TAB_RESTORE, ChromeFeatureList.TAB_GROUPS_ANDROID})
@EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@Restriction({Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
public void testOpenRecentlyClosedEntry_Bulk_FromBulkClosure() {
if (mTabGroupModelFilter == null) return;
Expand Down Expand Up @@ -801,7 +802,7 @@ public void testOpenRecentlyClosedEntry_Bulk_FromBulkClosure() {
*/
@Test
@MediumTest
@EnableFeatures({ChromeFeatureList.BULK_TAB_RESTORE, ChromeFeatureList.TAB_GROUPS_ANDROID})
@EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@Restriction({Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
public void testOpenMostRecentlyClosedEntry_Group() {
if (mTabGroupModelFilter == null) return;
Expand Down Expand Up @@ -881,7 +882,7 @@ public void testOpenMostRecentlyClosedEntry_Group() {
*/
@Test
@MediumTest
@EnableFeatures({ChromeFeatureList.BULK_TAB_RESTORE, ChromeFeatureList.TAB_GROUPS_ANDROID})
@EnableFeatures({ChromeFeatureList.TAB_GROUPS_ANDROID})
@Restriction({Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE})
public void testOpenMostRecentlyClosedEntry_Bulk() {
if (mTabGroupModelFilter == null) return;
Expand Down
Loading

0 comments on commit 5f9bac0

Please sign in to comment.