Skip to content

Commit

Permalink
[Android Tab Restore] HistoricalTabSaver bulk closure
Browse files Browse the repository at this point in the history
Expand HistoricalTabSaver to support groups and bulk closures. Also
convert HistoricalTabSaver to an interface with the implementation
HistoricalTabSaverImpl - this will make testing easier in a follow up
CL.

Once CL [1] lands we just need to hook this up to a didCloseTabs
observer that knows about groups and saving should be working.

This also implements basic reading of Group/Bulk/Tab entries for
RecentlyClosedBridge which will be required to for the UI, but was
helpful for testing.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/3514563

Bug: 1306278, 1307345
Change-Id: I50e4f98570b65391190f36e226efe1683534fe7a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3550461
Reviewed-by: Fred Mello <fredmello@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/main@{#989013}
  • Loading branch information
ckitagawa-work authored and Chromium LUCI CQ committed Apr 5, 2022
1 parent 0fc0ef7 commit e445dc5
Show file tree
Hide file tree
Showing 29 changed files with 1,595 additions and 285 deletions.
2 changes: 1 addition & 1 deletion chrome/android/BUILD.gn
Expand Up @@ -3985,11 +3985,11 @@ generate_jni("chrome_jni_headers") {
"java/src/org/chromium/chrome/browser/suggestions/mostvisited/MostVisitedSitesBridge.java",
"java/src/org/chromium/chrome/browser/survey/SurveyHttpClientBridge.java",
"java/src/org/chromium/chrome/browser/sync/TrustedVaultClient.java",
"java/src/org/chromium/chrome/browser/tab/HistoricalTabSaver.java",
"java/src/org/chromium/chrome/browser/tab/TabBrowserControlsConstraintsHelper.java",
"java/src/org/chromium/chrome/browser/tab/TabFavicon.java",
"java/src/org/chromium/chrome/browser/tab/TabImpl.java",
"java/src/org/chromium/chrome/browser/tab/TabWebContentsDelegateAndroidImpl.java",
"java/src/org/chromium/chrome/browser/tab/tab_restore/HistoricalTabSaverImpl.java",
"java/src/org/chromium/chrome/browser/tabmodel/TabModelJniBridge.java",
"java/src/org/chromium/chrome/browser/tabmodel/TabModelObserverJniBridge.java",
"java/src/org/chromium/chrome/browser/usage_stats/NotificationSuspender.java",
Expand Down
7 changes: 6 additions & 1 deletion chrome/android/chrome_java_sources.gni
Expand Up @@ -811,6 +811,9 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/ntp/RecentTabsPagePrefs.java",
"java/src/org/chromium/chrome/browser/ntp/RecentTabsRowAdapter.java",
"java/src/org/chromium/chrome/browser/ntp/RecentlyClosedBridge.java",
"java/src/org/chromium/chrome/browser/ntp/RecentlyClosedBulkEvent.java",
"java/src/org/chromium/chrome/browser/ntp/RecentlyClosedEntry.java",
"java/src/org/chromium/chrome/browser/ntp/RecentlyClosedGroup.java",
"java/src/org/chromium/chrome/browser/ntp/RecentlyClosedTab.java",
"java/src/org/chromium/chrome/browser/ntp/RecentlyClosedTabManager.java",
"java/src/org/chromium/chrome/browser/ntp/RevampedIncognitoDescriptionView.java",
Expand Down Expand Up @@ -1094,7 +1097,6 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/tab/AccessibilityVisibilityHandler.java",
"java/src/org/chromium/chrome/browser/tab/AuthenticatorNavigationInterceptorTabHelper.java",
"java/src/org/chromium/chrome/browser/tab/AutofillSessionLifetimeController.java",
"java/src/org/chromium/chrome/browser/tab/HistoricalTabSaver.java",
"java/src/org/chromium/chrome/browser/tab/InterceptNavigationDelegateClientImpl.java",
"java/src/org/chromium/chrome/browser/tab/InterceptNavigationDelegateTabHelper.java",
"java/src/org/chromium/chrome/browser/tab/RedirectHandlerTabHelper.java",
Expand All @@ -1120,6 +1122,9 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/tab/TabViewManagerImpl.java",
"java/src/org/chromium/chrome/browser/tab/TabWebContentsDelegateAndroidImpl.java",
"java/src/org/chromium/chrome/browser/tab/TabWebContentsObserver.java",
"java/src/org/chromium/chrome/browser/tab/tab_restore/HistoricalEntry.java",
"java/src/org/chromium/chrome/browser/tab/tab_restore/HistoricalTabSaver.java",
"java/src/org/chromium/chrome/browser/tab/tab_restore/HistoricalTabSaverImpl.java",
"java/src/org/chromium/chrome/browser/tabbed_mode/TabbedAppMenuPropertiesDelegate.java",
"java/src/org/chromium/chrome/browser/tabbed_mode/TabbedNavigationBarColorController.java",
"java/src/org/chromium/chrome/browser/tabbed_mode/TabbedRootUiCoordinator.java",
Expand Down
1 change: 1 addition & 0 deletions chrome/android/chrome_junit_test_java_sources.gni
Expand Up @@ -215,6 +215,7 @@ chrome_junit_test_java_sources = [
"junit/src/org/chromium/chrome/browser/tab/TabUnitTest.java",
"junit/src/org/chromium/chrome/browser/tab/TabViewAndroidDelegateTest.java",
"junit/src/org/chromium/chrome/browser/tab/TabViewManagerUnitTest.java",
"junit/src/org/chromium/chrome/browser/tab/tab_restore/HistoricalTabSaverImplUnitTest.java",
"junit/src/org/chromium/chrome/browser/tabbed_mode/TabbedAppMenuPropertiesDelegateUnitTest.java",
"junit/src/org/chromium/chrome/browser/tabmodel/PendingTabClosureManagerTest.java",
"junit/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorImplTest.java",
Expand Down
2 changes: 1 addition & 1 deletion chrome/android/chrome_test_java_sources.gni
Expand Up @@ -556,7 +556,6 @@ chrome_test_java_sources = [
"javatests/src/org/chromium/chrome/browser/sync/ui/PassphraseActivityTest.java",
"javatests/src/org/chromium/chrome/browser/sync/ui/PassphraseTypeDialogFragmentTest.java",
"javatests/src/org/chromium/chrome/browser/sync/ui/SyncErrorMessageTest.java",
"javatests/src/org/chromium/chrome/browser/tab/HistoricalTabSaverTest.java",
"javatests/src/org/chromium/chrome/browser/tab/InterceptNavigationDelegateTest.java",
"javatests/src/org/chromium/chrome/browser/tab/RepostFormWarningTest.java",
"javatests/src/org/chromium/chrome/browser/tab/RequestDesktopSiteTest.java",
Expand All @@ -580,6 +579,7 @@ chrome_test_java_sources = [
"javatests/src/org/chromium/chrome/browser/tab/state/ShoppingPersistedTabDataTest.java",
"javatests/src/org/chromium/chrome/browser/tab/state/ShoppingPersistedTabDataTestUtils.java",
"javatests/src/org/chromium/chrome/browser/tab/state/StorePersistedTabDataTest.java",
"javatests/src/org/chromium/chrome/browser/tab/tab_restore/HistoricalTabSaverImplTest.java",
"javatests/src/org/chromium/chrome/browser/tabbed_mode/TabbedNavigationBarColorControllerTest.java",
"javatests/src/org/chromium/chrome/browser/tabmodel/AsyncTabCreationParamsManagerTest.java",
"javatests/src/org/chromium/chrome/browser/tabmodel/ChromeTabCreatorTest.java",
Expand Down
Expand Up @@ -310,9 +310,9 @@ public void deleteForeignSession(ForeignSession session) {
/**
* Clears the list of recently closed tabs.
*/
public void clearRecentlyClosedTabs() {
public void clearRecentlyClosedEntries() {
if (mIsDestroyed) return;
mRecentlyClosedTabManager.clearRecentlyClosedTabs();
mRecentlyClosedTabManager.clearRecentlyClosedEntries();
}

/**
Expand Down
Expand Up @@ -545,15 +545,15 @@ public void configureChildView(int childPosition, ViewHolder viewHolder) {
mActivity.getResources().getDimensionPixelSize(
R.dimen.recent_tabs_foreign_session_group_item_height);
RecentlyClosedTab tab = getChild(childPosition);
String title = TitleUtil.getTitleForDisplay(tab.title, tab.url);
String title = TitleUtil.getTitleForDisplay(tab.getTitle(), tab.getUrl());
viewHolder.textView.setText(title);

String domain = UrlUtilities.getDomainAndRegistry(tab.url.getSpec(), false);
String domain = UrlUtilities.getDomainAndRegistry(tab.getUrl().getSpec(), false);
if (!TextUtils.isEmpty(domain)) {
viewHolder.domainView.setText(domain);
viewHolder.domainView.setVisibility(View.VISIBLE);
}
loadFavicon(viewHolder, tab.url, FaviconLocality.LOCAL);
loadFavicon(viewHolder, tab.getUrl(), FaviconLocality.LOCAL);
}

@Override
Expand Down Expand Up @@ -594,7 +594,7 @@ public void onCreateContextMenuForChild(final int childPosition, ContextMenu men
OnMenuItemClickListener listener = item -> {
switch (item.getItemId()) {
case ID_REMOVE_ALL:
mRecentTabsManager.clearRecentlyClosedTabs();
mRecentTabsManager.clearRecentlyClosedEntries();
break;
case ID_OPEN_IN_NEW_TAB:
mRecentTabsManager.openRecentlyClosedTab(
Expand Down
Expand Up @@ -27,12 +27,60 @@ public class RecentlyClosedBridge implements RecentlyClosedTabManager {
@Nullable
private Runnable mTabsUpdatedRunnable;

// TODO(crbug/1307345): Remove in favor of generic entries.
@CalledByNative
private static void pushTab(List<RecentlyClosedTab> tabs, int id, String title, GURL url) {
RecentlyClosedTab tab = new RecentlyClosedTab(id, title, url);
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;
assert tabIds.length == tabTitles.length;
assert tabIds.length == tabUrls.length;
assert tabIds.length == tabGroupIds.length;
for (int i = 0; i < tabIds.length; i++) {
tabs.add(new RecentlyClosedTab(
tabIds[i], tabTimestamps[i], tabTitles[i], tabUrls[i], tabGroupIds[i]));
}
}

@CalledByNative
private static void addTabToEntries(List<RecentlyClosedEntry> entries, int id, long timestamp,
String title, GURL url, String groupId) {
RecentlyClosedTab tab = new RecentlyClosedTab(id, timestamp, title, url, groupId);
entries.add(tab);
}

@CalledByNative
private static void addGroupToEntries(List<RecentlyClosedEntry> entries, int id,
long groupTimestamp, String groupTitle, int[] tabIds, long[] tabTimestamps,
String[] tabTitles, GURL[] tabUrls, String[] tabGroupIds) {
RecentlyClosedGroup group = new RecentlyClosedGroup(id, groupTimestamp, groupTitle);

addTabs(group.getTabs(), tabIds, tabTimestamps, tabTitles, tabUrls, tabGroupIds);

entries.add(group);
}

@CalledByNative
private static void addBulkEventToEntries(List<RecentlyClosedEntry> entries, int id,
long eventTimestamp, String[] groupIds, String[] groupsTitles, int[] tabIds,
long[] tabTimestamps, String[] tabTitles, GURL[] tabUrls, String[] tabGroupIds) {
RecentlyClosedBulkEvent event = new RecentlyClosedBulkEvent(id, eventTimestamp);

assert groupIds.length == groupsTitles.length;
for (int i = 0; i < groupIds.length; i++) {
event.getGroupIdToTitleMap().put(groupIds[i], groupsTitles[i]);
}

addTabs(event.getTabs(), tabIds, tabTimestamps, tabTitles, tabUrls, tabGroupIds);

entries.add(event);
}

/**
* Initializes this class with the given profile.
* @param profile The Profile whose recently closed tabs will be queried.
Expand Down Expand Up @@ -62,11 +110,19 @@ public List<RecentlyClosedTab> getRecentlyClosedTabs(int maxTabCount) {
return received ? tabs : null;
}

@Override
public List<RecentlyClosedEntry> getRecentlyClosedEntries(int maxEntryCount) {
List<RecentlyClosedEntry> entries = new ArrayList<RecentlyClosedEntry>();
boolean received = RecentlyClosedBridgeJni.get().getRecentlyClosedEntries(
mNativeBridge, entries, maxEntryCount);
return received ? entries : null;
}

@Override
public boolean openRecentlyClosedTab(
TabModel tabModel, RecentlyClosedTab recentTab, int windowOpenDisposition) {
return RecentlyClosedBridgeJni.get().openRecentlyClosedTab(
mNativeBridge, tabModel, recentTab.id, windowOpenDisposition);
mNativeBridge, tabModel, recentTab.getSessionId(), windowOpenDisposition);
}

@Override
Expand All @@ -75,8 +131,8 @@ public void openMostRecentlyClosedTab(TabModel tabModel) {
}

@Override
public void clearRecentlyClosedTabs() {
RecentlyClosedBridgeJni.get().clearRecentlyClosedTabs(mNativeBridge);
public void clearRecentlyClosedEntries() {
RecentlyClosedBridgeJni.get().clearRecentlyClosedEntries(mNativeBridge);
}

/**
Expand All @@ -94,9 +150,11 @@ public interface Natives {
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,
int recentTabId, int windowOpenDisposition);
boolean openMostRecentlyClosedTab(long nativeRecentlyClosedTabsBridge, TabModel tabModel);
void clearRecentlyClosedTabs(long nativeRecentlyClosedTabsBridge);
void clearRecentlyClosedEntries(long nativeRecentlyClosedTabsBridge);
}
}
@@ -0,0 +1,36 @@
// Copyright 2022 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.ntp;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/**
* Represents a recent closure of multiple tabs and groups (AKA Window) from TabRestoreService.
*/
public class RecentlyClosedBulkEvent extends RecentlyClosedEntry {
private final List<RecentlyClosedTab> mTabs = new ArrayList<>();
private final Map<String, String> mGroups = new HashMap<>();

public RecentlyClosedBulkEvent(int sessionId, long timestamp) {
super(sessionId, timestamp);
}

/**
* @return list of {@link RecentlyClosedTab} in this event.
*/
public List<RecentlyClosedTab> getTabs() {
return mTabs;
}

/**
* @return map of {@link RecentlyClosedTab#getGroupId()} to group titles.
*/
public Map<String, String> getGroupIdToTitleMap() {
return mGroups;
}
}
@@ -0,0 +1,38 @@
// Copyright 2022 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.ntp;

import java.util.Date;

/**
* Represents a recently closed entry from TabRestoreService.
*/
public class RecentlyClosedEntry {
private final int mSessionId;
private final Date mDate;

/**
* @param sessionId The Session ID of this entry.
* @param timestamp The milliseconds since the Unix Epoch this entry was created.
*/
protected RecentlyClosedEntry(int sessionId, long timestamp) {
mSessionId = sessionId;
mDate = new Date(timestamp);
}

/**
* @return the Session ID of the entry in TabRestoreService.
*/
public int getSessionId() {
return mSessionId;
}

/**
* @return the {@link Date} when this entry was created.
*/
public Date getDate() {
return mDate;
}
}
@@ -0,0 +1,36 @@
// Copyright 2022 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.ntp;

import java.util.ArrayList;
import java.util.List;

/**
* Represents a recently closed group from TabRestoreService.
*/
public class RecentlyClosedGroup extends RecentlyClosedEntry {
private final String mTitle;
private final List<RecentlyClosedTab> mTabs = new ArrayList<>();

public RecentlyClosedGroup(int sessionId, long timestamp, String title) {
super(sessionId, timestamp);
mTitle = title;
}

/**
* @return title of the group this may be an empty string if the default title was used when
* saving.
*/
public String getTitle() {
return mTitle;
}

/**
* @return the list of tabs for this group.
*/
public List<RecentlyClosedTab> getTabs() {
return mTabs;
}
}
Expand Up @@ -7,16 +7,46 @@
import org.chromium.url.GURL;

/**
* Represents a recently closed tab.
* Represents a recently closed tab from TabRestoreService.
*/
public class RecentlyClosedTab {
public final int id;
public final String title;
public final GURL url;

public RecentlyClosedTab(int id, String title, GURL url) {
this.id = id;
this.title = title;
this.url = url;
public class RecentlyClosedTab extends RecentlyClosedEntry {
private final String mTitle;
private final GURL mUrl;
/**
* Native tab_groups::TabGroupId. This is NOT equal to {@link RecentlyClosedEntry#id} for the
* corresponding {@link RecentlyClosedGroup}.
*/
private final String mGroupId;

public RecentlyClosedTab(
int sessionId, long timestamp, String title, GURL url, String groupId) {
super(sessionId, timestamp);
mTitle = title;
mUrl = url;

// Treat null and empty strings as equivalent.
mGroupId = (groupId == null || groupId.isEmpty()) ? null : groupId;
}

/**
* @return Title of tab.
*/
public String getTitle() {
return mTitle;
}

/**
* @return URL of the tab.
*/
public GURL getUrl() {
return mUrl;
}

/**
* @return Group ID of the tab. Useful when displaying groups from a {@link
* RecentlyClosedBulkEvent}.
*/
public String getGroupId() {
return mGroupId;
}
}

0 comments on commit e445dc5

Please sign in to comment.