[TabRestore] Represent single tab windows as window entries in the UI
Windows that are closed with a single tab are now represented as window entries with a single tab in the recent tabs submenu. This CL removes the logic from the service meaning other platforms using it the service are not subject to desktop specific logic. Removes SessionRestoreTest.WindowWithOneTab since it is no longer applicable. Updates remaining tests to account for this change. Change-Id: I0168dbadfecd4e9b611b91989180e3900d40627b Bug: 41227458, 40438075, 40846749 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6387072 Reviewed-by: David Pennington <dpenning@chromium.org> Commit-Queue: Darryl James <dljames@chromium.org> Cr-Commit-Position: refs/heads/main@{#1437715}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
cba371a958
commit
9e5585fdb2
chrome/browser
components/sessions/core
@ -703,48 +703,6 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest,
|
||||
EXPECT_EQ(http_status_code, entry->GetHttpStatusCode());
|
||||
}
|
||||
|
||||
// Flaky on Linux. https://crbug.com/537592.
|
||||
// Flaky on Mac. https://crbug.com/1334914.
|
||||
#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_MAC)
|
||||
#define MAYBE_WindowWithOneTab DISABLED_WindowWithOneTab
|
||||
#else
|
||||
#define MAYBE_WindowWithOneTab WindowWithOneTab
|
||||
#endif
|
||||
IN_PROC_BROWSER_TEST_F(SessionRestoreTest, MAYBE_WindowWithOneTab) {
|
||||
GURL url(ui_test_utils::GetTestUrl(
|
||||
base::FilePath(base::FilePath::kCurrentDirectory),
|
||||
base::FilePath(FILE_PATH_LITERAL("title1.html"))));
|
||||
|
||||
// Add a single tab.
|
||||
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
|
||||
|
||||
sessions::TabRestoreService* service =
|
||||
TabRestoreServiceFactory::GetForProfile(browser()->profile());
|
||||
service->ClearEntries();
|
||||
EXPECT_EQ(0U, service->entries().size());
|
||||
|
||||
// Close the window.
|
||||
browser()->window()->Close();
|
||||
|
||||
// Expect the window to be converted to a tab by the TRS.
|
||||
EXPECT_EQ(1U, service->entries().size());
|
||||
ASSERT_EQ(sessions::tab_restore::Type::TAB, service->entries().front()->type);
|
||||
auto* tab = static_cast<const sessions::tab_restore::Tab*>(
|
||||
service->entries().front().get());
|
||||
|
||||
// Restore the tab.
|
||||
std::vector<sessions::LiveTab*> content = service->RestoreEntryById(
|
||||
nullptr, tab->id, WindowOpenDisposition::UNKNOWN);
|
||||
ASSERT_EQ(1U, content.size());
|
||||
ASSERT_TRUE(content[0]);
|
||||
EXPECT_EQ(url, static_cast<sessions::ContentLiveTab*>(content[0])
|
||||
->web_contents()
|
||||
->GetURL());
|
||||
|
||||
// Make sure the restore was successful.
|
||||
EXPECT_EQ(0U, service->entries().size());
|
||||
}
|
||||
|
||||
#if !BUILDFLAG(IS_CHROMEOS)
|
||||
// This test does not apply to ChromeOS as ChromeOS does not do session
|
||||
// restore when a new window is open.
|
||||
|
@ -1515,7 +1515,7 @@ IN_PROC_BROWSER_TEST_F(TabRestoreTest,
|
||||
}
|
||||
#endif // BUILDFLAG(ENABLE_SESSION_SERVICE)
|
||||
|
||||
IN_PROC_BROWSER_TEST_F(TabRestoreTest, PRE_GetRestoreTabType) {
|
||||
IN_PROC_BROWSER_TEST_F(TabRestoreTest, PRE_GetRestoreWindowType) {
|
||||
// The TabRestoreService should get initialized (Loaded)
|
||||
// automatically upon launch.
|
||||
// Wait for robustness because InProcessBrowserTest::PreRunTestOnMainThread
|
||||
@ -1539,12 +1539,12 @@ IN_PROC_BROWSER_TEST_F(TabRestoreTest, PRE_GetRestoreTabType) {
|
||||
browser()->tab_strip_model()->CloseSelectedTabs();
|
||||
destroyed_watcher.Wait();
|
||||
|
||||
// We now should see a Tab as the restore type.
|
||||
// We now should see a Tab as the restore type because we manually closed one.
|
||||
ASSERT_EQ(1u, service->entries().size());
|
||||
EXPECT_EQ(sessions::tab_restore::Type::TAB, service->entries().front()->type);
|
||||
}
|
||||
|
||||
IN_PROC_BROWSER_TEST_F(TabRestoreTest, GetRestoreTabType) {
|
||||
IN_PROC_BROWSER_TEST_F(TabRestoreTest, GetRestoreWindowType) {
|
||||
// The TabRestoreService should get initialized (Loaded)
|
||||
// automatically upon launch.
|
||||
// Wait for robustness because InProcessBrowserTest::PreRunTestOnMainThread
|
||||
@ -1555,9 +1555,10 @@ IN_PROC_BROWSER_TEST_F(TabRestoreTest, GetRestoreTabType) {
|
||||
TabRestoreServiceLoadWaiter waiter(service);
|
||||
waiter.Wait();
|
||||
|
||||
// When we start this time we should get a Tab.
|
||||
// After a restart, we should see a Window since the browser was closed.
|
||||
ASSERT_GE(service->entries().size(), 1u);
|
||||
EXPECT_EQ(sessions::tab_restore::Type::TAB, service->entries().front()->type);
|
||||
EXPECT_EQ(sessions::tab_restore::Type::WINDOW,
|
||||
service->entries().front()->type);
|
||||
}
|
||||
|
||||
IN_PROC_BROWSER_TEST_F(TabRestoreTest, RestoreWindowWithName) {
|
||||
@ -2075,12 +2076,14 @@ IN_PROC_BROWSER_TEST_F(TabRestoreTest, RestoreAfterMultipleRestarts) {
|
||||
EnableSessionService();
|
||||
|
||||
// Restore url2 from one session ago.
|
||||
ASSERT_NO_FATAL_FAILURE(RestoreTab(0, 1));
|
||||
EXPECT_EQ(url2_, browser()->tab_strip_model()->GetWebContentsAt(1)->GetURL());
|
||||
ASSERT_NO_FATAL_FAILURE(RestoreTab(1, 0));
|
||||
Browser* browser_2 = GetBrowser(1);
|
||||
EXPECT_EQ(url2_, browser_2->tab_strip_model()->GetWebContentsAt(0)->GetURL());
|
||||
|
||||
// Restore url1 from two sessions ago.
|
||||
ASSERT_NO_FATAL_FAILURE(RestoreTab(0, 2));
|
||||
EXPECT_EQ(url1_, browser()->tab_strip_model()->GetWebContentsAt(2)->GetURL());
|
||||
ASSERT_NO_FATAL_FAILURE(RestoreTab(2, 0));
|
||||
Browser* browser_3 = GetBrowser(2);
|
||||
EXPECT_EQ(url1_, browser_3->tab_strip_model()->GetWebContentsAt(0)->GetURL());
|
||||
}
|
||||
|
||||
// Test that it is possible to navigate back to a restored about:blank history
|
||||
@ -3175,8 +3178,10 @@ IN_PROC_BROWSER_TEST_P(TabRestoreSavedGroupsTest,
|
||||
// previous group id defined in the PRE step to this test.
|
||||
chrome::RestoreTab(browser());
|
||||
|
||||
Browser* restored_browser = GetBrowser(1);
|
||||
// Verify the browser has a single tab group.
|
||||
TabGroupModel* group_model = browser()->tab_strip_model()->group_model();
|
||||
TabGroupModel* group_model =
|
||||
restored_browser->tab_strip_model()->group_model();
|
||||
EXPECT_EQ(1u, group_model->ListTabGroups().size());
|
||||
|
||||
// Verify there is still only 1 saved group and that it is open now.
|
||||
|
@ -404,6 +404,8 @@ void RecentTabsSubMenuModel::BuildLocalEntries() {
|
||||
}
|
||||
case sessions::tab_restore::Type::WINDOW: {
|
||||
auto& window = static_cast<sessions::tab_restore::Window&>(*entry);
|
||||
// TODO(https://crbug.com/41227458): Consider if we should re-add the
|
||||
// ability for single tab windows to be represented as single tabs.
|
||||
BuildLocalWindowItem(window, ++last_local_model_index_);
|
||||
break;
|
||||
}
|
||||
|
@ -234,16 +234,14 @@ void TabRestoreServiceHelper::BrowserClosing(LiveTabContext* context) {
|
||||
window->tabs.push_back(std::move(tab));
|
||||
}
|
||||
|
||||
if (window->tabs.size() == 1 && window->app_name.empty() &&
|
||||
window->user_title.empty()) {
|
||||
// Short-circuit creating a Window if only 1 tab was present. This fixes
|
||||
// http://crbug.com/56744.
|
||||
AddEntry(std::move(window->tabs[0]), true, true);
|
||||
} else if (!window->tabs.empty()) {
|
||||
window->selected_tab_index = std::min(
|
||||
static_cast<int>(window->tabs.size() - 1), window->selected_tab_index);
|
||||
AddEntry(std::move(window), true, true);
|
||||
if (window->tabs.empty()) {
|
||||
// This can happen in tests.
|
||||
return;
|
||||
}
|
||||
|
||||
window->selected_tab_index = std::min(
|
||||
static_cast<int>(window->tabs.size() - 1), window->selected_tab_index);
|
||||
AddEntry(std::move(window), true, true);
|
||||
}
|
||||
|
||||
void TabRestoreServiceHelper::BrowserClosed(LiveTabContext* context) {
|
||||
|
Reference in New Issue
Block a user