chromium/src
0
Fork 0
Code Activity

Set the restored most recently closed tab as current tab.

Browse Source 
On other desktop platforms(linux, mac, chromeos,etc), when
user uses keyboard shortcut "ctrl-shift-t" to re-open the
most recently closed tab, after the tab is restored, it is
set as the current tab. On Clank, however, the restored tab
is inactive. This makes the user experience inconsistent
on clank. This cl fixes this issue by always setting the
tab(most recently closed) restored as the current tab.

This cl fixes the case for restoring a tab without snackbar
showing up, i.e. from tab restore service.

Bug: 348478081
Change-Id: I912143efb508a6ab4bec4cfd50e37eb3d6a964c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5669932
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Jenny Zhang <jennyz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1332509}
This commit is contained in:
Jenny Zhang
2024-07-24 19:02:12 +00:00
committed by Chromium LUCI CQ
parent da10677dc7
commit 5d08744a14
32 changed files with 243 additions and 57 deletions

5
components/sessions/core/live_tab_context.h

@ -72,9 +72,12 @@ class SESSIONS_EXPORT LiveTabContext {
// platform-specific data).
// |tab.id| is the tab's unique SessionID. Only present if a historical tab
// has been created by TabRestoreService.
// |original_session_type| indicates the type of session entry the tab
// belongs to.
virtual LiveTab* AddRestoredTab(const tab_restore::Tab& tab,
int tab_index,
bool select) = 0;
bool select,
tab_restore::Type original_session_type) = 0;
// Note: |tab.platform_data| may be null (e.g., if restoring from last session
// as this data is not persisted, or if the platform does not provide

21
components/sessions/core/tab_restore_service_helper.cc

@ -471,7 +471,8 @@ LiveTabContext* TabRestoreServiceHelper::RestoreTabOrGroupFromWindow(
// Restore the tab.
restored_tab_browser_id = tab.browser_id;
LiveTab* restored_tab = nullptr;
context = RestoreTab(tab, context, disposition, &restored_tab);
context = RestoreTab(tab, context, disposition,
sessions::tab_restore::WINDOW, &restored_tab);
live_tabs->push_back(restored_tab);
std::optional<tab_groups::TabGroupId> group_id = tab.group;
@ -521,7 +522,8 @@ LiveTabContext* TabRestoreServiceHelper::RestoreTabOrGroupFromWindow(
// Restore the tab.
LiveTab* restored_tab = nullptr;
LiveTabContext* new_context =
RestoreTab(tab, context, disposition, &restored_tab);
RestoreTab(tab, context, disposition, sessions::tab_restore::WINDOW,
&restored_tab);
if (tab_i != 0) {
// CHECK that the context should be the same except for the first tab.
DCHECK_EQ(new_context, context);
@ -596,7 +598,8 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
}
LiveTab* restored_tab = nullptr;
context = RestoreTab(tab, context, disposition, &restored_tab);
context =
RestoreTab(tab, context, disposition, entry.type, &restored_tab);
live_tabs.push_back(restored_tab);
context->ShowBrowserWindow();
break;
@ -634,7 +637,7 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
for (const auto& tab : window.tabs) {
const bool select_tab = tab->id == selected_tab_id;
LiveTab* restored_tab = context->AddRestoredTab(
*tab.get(), context->GetTabCount(), select_tab);
*tab.get(), context->GetTabCount(), select_tab, entry.type);
if (restored_tab) {
client_->OnTabRestored(
@ -684,7 +687,8 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
if (entry_id_matches_restore_id) {
for (const auto& tab : group.tabs) {
LiveTab* restored_tab = context->AddRestoredTab(
*tab.get(), context->GetTabCount(), group.tabs[0]->id == tab->id);
*tab.get(), context->GetTabCount(), group.tabs[0]->id == tab->id,
entry.type);
live_tabs.push_back(restored_tab);
}
} else {
@ -694,7 +698,8 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
const Tab& tab = *group.tabs[i];
if (tab.id == id) {
LiveTab* restored_tab = nullptr;
context = RestoreTab(tab, context, disposition, &restored_tab);
context = RestoreTab(tab, context, disposition, entry.type,
&restored_tab);
live_tabs.push_back(restored_tab);
CHECK(ValidateGroup(group));
group.tabs.erase(group.tabs.begin() + i);
@ -940,6 +945,7 @@ LiveTabContext* TabRestoreServiceHelper::RestoreTab(
const Tab& tab,
LiveTabContext* context,
WindowOpenDisposition disposition,
sessions::tab_restore::Type session_restore_type,
LiveTab** live_tab) {
LiveTab* restored_tab;
if (disposition == WindowOpenDisposition::CURRENT_TAB && context) {
@ -989,7 +995,8 @@ LiveTabContext* TabRestoreServiceHelper::RestoreTab(
restored_tab = context->AddRestoredTab(
tab, tab_index,
/*select=*/disposition != WindowOpenDisposition::NEW_BACKGROUND_TAB);
/*select=*/disposition != WindowOpenDisposition::NEW_BACKGROUND_TAB,
session_restore_type);
}
client_->OnTabRestored(

3
components/sessions/core/tab_restore_service_helper.h

@ -154,9 +154,12 @@ class SESSIONS_EXPORT TabRestoreServiceHelper
// will be respected instead. If a new LiveTabContext needs to be created for
// this tab, If present, |live_tab| will be populated with the LiveTab of the
// restored tab.
// |original_session_type| indicates the type of session entry the tab
// belongs to.
LiveTabContext* RestoreTab(const Tab& tab,
LiveTabContext* context,
WindowOpenDisposition disposition,
sessions::tab_restore::Type session_restore_type,
LiveTab** live_tab);
// This is a helper function for RestoreEntryById(). Restores a single entry