Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(155)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/tab_restore_uitest.cc

Issue 92001: Restore closed tabs into new windows when necessary, and track the windows th... (Closed) Base URL: svn://chrome-svn/chrome/trunk/src/
Patch Set: '' Created 11 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/sessions/tab_restore_service.cc ('k') | chrome/test/automation/browser_proxy.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/tab_restore_uitest.cc
===================================================================
--- chrome/browser/tab_restore_uitest.cc (revision 14184)
+++ chrome/browser/tab_restore_uitest.cc (working copy)
@@ -28,28 +28,78 @@
}
protected:
- void RestoreTab() {
- int tab_count;
+ // Uses the undo-close-tab accelerator to undo a close-tab or close-window
+ // operation. The newly restored tab is expected to appear in the
+ // window at index |expected_window_index|, at the |expected_tabstrip_index|,
+ // and to be active. If |expected_window_index| is equal to the number of
+ // current windows, the restored tab is expected to be created in a new
+ // window (since the index is 0-based).
+ void RestoreTab(int expected_window_index,
+ int expected_tabstrip_index) {
+ int tab_count = 0;
+ int window_count = 0;
- // Reset browser_proxy to new window.
- scoped_ptr<BrowserProxy> browser_proxy(automation()->GetBrowserWindow(0));
+ ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
+ ASSERT_GT(window_count, 0);
+
+ bool expect_new_window = (expected_window_index == window_count);
+ scoped_ptr<BrowserProxy> browser_proxy;
+ if (expect_new_window) {
+ browser_proxy.reset(automation()->GetBrowserWindow(0));
+ } else {
+ ASSERT_GT(window_count, expected_window_index);
+ browser_proxy.reset(
+ automation()->GetBrowserWindow(expected_window_index));
+ }
ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
ASSERT_GT(tab_count, 0);
// Restore the tab.
ASSERT_TRUE(browser_proxy->ApplyAccelerator(IDC_RESTORE_TAB));
- ASSERT_TRUE(browser_proxy->WaitForTabCountToBecome(
- tab_count + 1, action_max_timeout_ms()));
+ if (expect_new_window) {
+ ASSERT_TRUE(automation()->WaitForWindowCountToBecome(
+ ++window_count, action_max_timeout_ms()));
+ browser_proxy.reset(automation()->
+ GetBrowserWindow(expected_window_index));
+ } else {
+ ASSERT_TRUE(browser_proxy->WaitForTabCountToBecome(
+ ++tab_count, action_max_timeout_ms()));
+ }
+
// Get a handle to the restored tab.
ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
+ ASSERT_GT(tab_count, expected_tabstrip_index);
scoped_ptr<TabProxy> restored_tab_proxy(
- browser_proxy->GetTab(tab_count - 1));
+ browser_proxy->GetTab(expected_tabstrip_index));
// Wait for the restored tab to finish loading.
ASSERT_TRUE(restored_tab_proxy->WaitForTabToBeRestored(
action_max_timeout_ms()));
+
+ // Ensure that the tab and window are active.
+ CheckActiveWindow(browser_proxy.get());
+ EXPECT_EQ(expected_tabstrip_index,
+ GetActiveTabIndex(expected_window_index));
}
+ // Adds tabs to the given browser, all navigated to url1_. Returns
+ // the final number of tabs.
+ int AddSomeTabs(BrowserProxy* browser, int how_many) {
+ int starting_tab_count = -1;
+ // Use EXPECT instead of ASSERT throughout to avoid trying to return void.
+ EXPECT_TRUE(browser->GetTabCount(&starting_tab_count));
+
+ for (int i = 0; i < how_many; ++i) {
+ browser->AppendTab(url1_);
+ EXPECT_TRUE(browser->WaitForTabCountToBecome(starting_tab_count + i + 1,
+ action_max_timeout_ms()));
+ }
+ int tab_count;
+ EXPECT_TRUE(browser->GetTabCount(&tab_count));
+ EXPECT_EQ(starting_tab_count + how_many, tab_count);
+ return tab_count;
+ }
+
// Ensure that the given browser occupies the currently active window.
void CheckActiveWindow(const BrowserProxy* browser) {
bool is_active = false;
@@ -87,17 +137,10 @@
TEST_F(TabRestoreUITest, Basic) {
scoped_ptr<BrowserProxy> browser_proxy(automation()->GetBrowserWindow(0));
- int tab_count;
- ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
- int starting_tab_count = tab_count;
+ int starting_tab_count;
+ ASSERT_TRUE(browser_proxy->GetTabCount(&starting_tab_count));
+ int tab_count = AddSomeTabs(browser_proxy.get(), 1);
- // Add a tab
- browser_proxy->AppendTab(url1_);
- ASSERT_TRUE(browser_proxy->WaitForTabCountToBecome(tab_count + 1,
- action_max_timeout_ms()));
- ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
- ASSERT_EQ(starting_tab_count + 1, tab_count);
-
int closed_tab_index = tab_count - 1;
scoped_ptr<TabProxy> new_tab(browser_proxy->GetTab(closed_tab_index));
// Make sure we're at url.
@@ -106,15 +149,15 @@
new_tab->Close(true);
new_tab.reset();
ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
- ASSERT_EQ(starting_tab_count, tab_count);
+ EXPECT_EQ(starting_tab_count, tab_count);
- RestoreTab();
+ RestoreTab(0, closed_tab_index);
// And make sure everything looks right.
ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
- ASSERT_EQ(starting_tab_count + 1, tab_count);
- ASSERT_EQ(closed_tab_index, GetActiveTabIndex());
- ASSERT_EQ(url1_, GetActiveTabURL());
+ EXPECT_EQ(starting_tab_count + 1, tab_count);
+ EXPECT_EQ(closed_tab_index, GetActiveTabIndex());
+ EXPECT_EQ(url1_, GetActiveTabURL());
}
// Close a tab not at the end of the current window, then restore it. The tab
@@ -122,23 +165,10 @@
TEST_F(TabRestoreUITest, MiddleTab) {
scoped_ptr<BrowserProxy> browser_proxy(automation()->GetBrowserWindow(0));
- int tab_count;
- ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
- int starting_tab_count = tab_count;
+ int starting_tab_count;
+ ASSERT_TRUE(browser_proxy->GetTabCount(&starting_tab_count));
+ int tab_count = AddSomeTabs(browser_proxy.get(), 3);
- // Add a couple of tabs
- browser_proxy->AppendTab(url1_);
- ASSERT_TRUE(browser_proxy->WaitForTabCountToBecome(tab_count + 1,
- action_max_timeout_ms()));
- browser_proxy->AppendTab(url1_);
- ASSERT_TRUE(browser_proxy->WaitForTabCountToBecome(tab_count + 2,
- action_max_timeout_ms()));
- browser_proxy->AppendTab(url1_);
- ASSERT_TRUE(browser_proxy->WaitForTabCountToBecome(tab_count + 3,
- action_max_timeout_ms()));
- ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
- ASSERT_EQ(starting_tab_count + 3, tab_count);
-
// Close one in the middle
int closed_tab_index = starting_tab_count + 1;
scoped_ptr<TabProxy> new_tab(browser_proxy->GetTab(closed_tab_index));
@@ -148,15 +178,15 @@
new_tab->Close(true);
new_tab.reset();
ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
- ASSERT_EQ(starting_tab_count + 2, tab_count);
+ EXPECT_EQ(starting_tab_count + 2, tab_count);
- RestoreTab();
+ RestoreTab(0, closed_tab_index);
// And make sure everything looks right.
ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
- ASSERT_EQ(starting_tab_count + 3, tab_count);
- ASSERT_EQ(closed_tab_index, GetActiveTabIndex());
- ASSERT_EQ(url1_, GetActiveTabURL());
+ EXPECT_EQ(starting_tab_count + 3, tab_count);
+ EXPECT_EQ(closed_tab_index, GetActiveTabIndex());
+ EXPECT_EQ(url1_, GetActiveTabURL());
}
// Close a tab, switch windows, then restore the tab. The tab should be in its
@@ -169,23 +199,10 @@
// CheckActiveWindow(). See comments in that function.
CheckActiveWindow(browser_proxy.get());
- int tab_count;
- ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
- int starting_tab_count = tab_count;
+ int starting_tab_count;
+ ASSERT_TRUE(browser_proxy->GetTabCount(&starting_tab_count));
+ int tab_count = AddSomeTabs(browser_proxy.get(), 3);
- // Add a couple of tabs
- browser_proxy->AppendTab(url1_);
- ASSERT_TRUE(browser_proxy->WaitForTabCountToBecome(tab_count + 1,
- action_max_timeout_ms()));
- browser_proxy->AppendTab(url1_);
- ASSERT_TRUE(browser_proxy->WaitForTabCountToBecome(tab_count + 2,
- action_max_timeout_ms()));
- browser_proxy->AppendTab(url1_);
- ASSERT_TRUE(browser_proxy->WaitForTabCountToBecome(tab_count + 3,
- action_max_timeout_ms()));
- ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
- ASSERT_EQ(starting_tab_count + 3, tab_count);
-
// Close one in the middle
int closed_tab_index = starting_tab_count + 1;
scoped_ptr<TabProxy> new_tab(browser_proxy->GetTab(closed_tab_index));
@@ -195,7 +212,7 @@
new_tab->Close(true);
new_tab.reset();
ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
- ASSERT_EQ(starting_tab_count + 2, tab_count);
+ EXPECT_EQ(starting_tab_count + 2, tab_count);
// Create a new browser.
ASSERT_TRUE(automation()->OpenNewBrowserWindow(false));
@@ -204,19 +221,20 @@
CheckActiveWindow(automation()->GetBrowserWindow(1));
- RestoreTab();
+ // Restore tab into original browser.
+ RestoreTab(0, closed_tab_index);
// And make sure everything looks right.
CheckActiveWindow(browser_proxy.get());
ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
EXPECT_EQ(starting_tab_count + 3, tab_count);
- ASSERT_EQ(closed_tab_index, GetActiveTabIndex());
- ASSERT_EQ(url1_, GetActiveTabURL());
+ EXPECT_EQ(closed_tab_index, GetActiveTabIndex(0));
+ EXPECT_EQ(url1_, GetActiveTabURL(0));
}
// Close a tab, open a new window, close the first window, then restore the
-// tab. It should be at the end of the current (new) window's tabstrip.
-TEST_F(TabRestoreUITest, RestoreFromClosedWindow) {
+// tab. It should be in a new window.
+TEST_F(TabRestoreUITest, BasicRestoreFromClosedWindow) {
// This test is disabled on win2k. See bug 1215881.
if (win_util::GetWinVersion() == win_util::WINVERSION_2000)
return;
@@ -245,7 +263,7 @@
2, action_max_timeout_ms()));
CheckActiveWindow(automation()->GetBrowserWindow(1));
- // Close the first browser.
+ // Close the final tab in the first browser.
EXPECT_TRUE(tab_proxy->Close(true));
ASSERT_TRUE(automation()->WaitForWindowCountToBecome(
1, action_max_timeout_ms()));
@@ -254,18 +272,122 @@
tab_proxy.reset();
browser_proxy.reset();
- RestoreTab();
+ RestoreTab(1, 0);
- // Tab should be at the end of the current (only) window.
- browser_proxy.reset(automation()->GetBrowserWindow(0));
+ // Tab should be in a new window.
+ browser_proxy.reset(automation()->GetBrowserWindow(1));
CheckActiveWindow(browser_proxy.get());
tab_proxy.reset(browser_proxy->GetActiveTab());
// And make sure the URLs matches.
- ASSERT_EQ(url2_, GetActiveTabURL());
- ASSERT_TRUE(tab_proxy->GoBack());
- ASSERT_EQ(url1_, GetActiveTabURL());
+ EXPECT_EQ(url2_, GetActiveTabURL(1));
+ EXPECT_TRUE(tab_proxy->GoBack());
+ EXPECT_EQ(url1_, GetActiveTabURL(1));
}
+// Open a window with multiple tabs, close a tab, then close the window.
+// Restore both and make sure the tab goes back into the window.
+TEST_F(TabRestoreUITest, RestoreWindowAndTab) {
+ scoped_ptr<BrowserProxy> browser_proxy(automation()->GetBrowserWindow(0));
+ CheckActiveWindow(browser_proxy.get());
+
+ int starting_tab_count;
+ ASSERT_TRUE(browser_proxy->GetTabCount(&starting_tab_count));
+ int tab_count = AddSomeTabs(browser_proxy.get(), 3);
+
+ // Close one in the middle
+ int closed_tab_index = starting_tab_count + 1;
+ scoped_ptr<TabProxy> new_tab(browser_proxy->GetTab(closed_tab_index));
+ // Make sure we're at url.
+ new_tab->NavigateToURL(url1_);
+ // Close the tab.
+ new_tab->Close(true);
+ new_tab.reset();
+ ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
+ EXPECT_EQ(starting_tab_count + 2, tab_count);
+
+ // Create a new browser.
+ ASSERT_TRUE(automation()->OpenNewBrowserWindow(false));
+ ASSERT_TRUE(automation()->WaitForWindowCountToBecome(
+ 2, action_max_timeout_ms()));
+ CheckActiveWindow(automation()->GetBrowserWindow(1));
+
+ // Close the first browser.
+ bool application_closing;
+ EXPECT_TRUE(CloseBrowser(browser_proxy.get(), &application_closing));
+ EXPECT_FALSE(application_closing);
+ ASSERT_TRUE(automation()->WaitForWindowCountToBecome(
+ 1, action_max_timeout_ms()));
+
+ // Browser is no longer valid.
+ browser_proxy.reset();
+
+ // Restore the first window. The expected_tabstrip_index (second argument)
+ // indicates the expected active tab.
+ RestoreTab(1, starting_tab_count + 1);
+ browser_proxy.reset(automation()->GetBrowserWindow(1));
+ CheckActiveWindow(browser_proxy.get());
+ ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
+ EXPECT_EQ(starting_tab_count + 2, tab_count);
+
+ // Restore the closed tab.
+ RestoreTab(1, closed_tab_index);
+ CheckActiveWindow(browser_proxy.get());
+ ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
+ EXPECT_EQ(starting_tab_count + 3, tab_count);
+ EXPECT_EQ(url1_, GetActiveTabURL(1));
+}
+
+// Open a window with two tabs, close both (closing the window), then restore
+// both. Make sure both restored tabs are in the same window.
+TEST_F(TabRestoreUITest, RestoreIntoSameWindow) {
+ scoped_ptr<BrowserProxy> browser_proxy(automation()->GetBrowserWindow(0));
+ CheckActiveWindow(browser_proxy.get());
+
+ int starting_tab_count;
+ ASSERT_TRUE(browser_proxy->GetTabCount(&starting_tab_count));
+ int tab_count = AddSomeTabs(browser_proxy.get(), 2);
+
+ // Navigate the rightmost one to url2_ for easier identification.
+ scoped_ptr<TabProxy> tab_proxy(browser_proxy->GetTab(tab_count - 1));
+ tab_proxy->NavigateToURL(url2_);
+
+ // Create a new browser.
+ ASSERT_TRUE(automation()->OpenNewBrowserWindow(false));
+ ASSERT_TRUE(automation()->WaitForWindowCountToBecome(
+ 2, action_max_timeout_ms()));
+ CheckActiveWindow(automation()->GetBrowserWindow(1));
+
+ // Close all but one tab in the first browser, left to right.
+ while (tab_count > 1) {
+ scoped_ptr<TabProxy> tab_to_close(browser_proxy->GetTab(0));
+ tab_to_close->Close(true);
+ ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
+ }
+
+ // Close the last tab, closing the browser.
+ tab_proxy.reset(browser_proxy->GetTab(0));
+ EXPECT_TRUE(tab_proxy->Close(true));
+ ASSERT_TRUE(automation()->WaitForWindowCountToBecome(
+ 1, action_max_timeout_ms()));
+ browser_proxy.reset();
+ tab_proxy.reset();
+
+ // Restore the last-closed tab into a new window.
+ RestoreTab(1, 0);
+ browser_proxy.reset(automation()->GetBrowserWindow(1));
+ CheckActiveWindow(browser_proxy.get());
+ ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
+ EXPECT_EQ(1, tab_count);
+ EXPECT_EQ(url2_, GetActiveTabURL(1));
+
+ // Restore the next-to-last-closed tab into the same window.
+ RestoreTab(1, 0);
+ CheckActiveWindow(browser_proxy.get());
+ ASSERT_TRUE(browser_proxy->GetTabCount(&tab_count));
+ EXPECT_EQ(2, tab_count);
+ EXPECT_EQ(url1_, GetActiveTabURL(1));
+}
+
// Tests that a duplicate history entry is not created when we restore a page
// to an existing SiteInstance. (Bug 1230446)
TEST_F(TabRestoreUITest, RestoreWithExistingSiteInstance) {
@@ -300,13 +422,13 @@
browser_proxy->AppendTab(http_url2);
// Restore the closed tab.
- RestoreTab();
+ RestoreTab(0, tab_count - 1);
tab.reset(browser_proxy->GetActiveTab());
// And make sure the URLs match.
- ASSERT_EQ(http_url2, GetActiveTabURL());
- ASSERT_TRUE(tab->GoBack());
- ASSERT_EQ(http_url1, GetActiveTabURL());
+ EXPECT_EQ(http_url2, GetActiveTabURL());
+ EXPECT_TRUE(tab->GoBack());
+ EXPECT_EQ(http_url1, GetActiveTabURL());
}
// Tests that the SiteInstances used for entries in a restored tab's history
@@ -346,19 +468,19 @@
browser_proxy->AppendTab(http_url2);
// Restore the closed tab.
- RestoreTab();
+ RestoreTab(0, tab_count - 1);
tab.reset(browser_proxy->GetActiveTab());
// And make sure the URLs match.
- ASSERT_EQ(url1_, GetActiveTabURL());
- ASSERT_TRUE(tab->GoBack());
- ASSERT_EQ(http_url1, GetActiveTabURL());
+ EXPECT_EQ(url1_, GetActiveTabURL());
+ EXPECT_TRUE(tab->GoBack());
+ EXPECT_EQ(http_url1, GetActiveTabURL());
// Navigating to a new URL should clear the forward list, because the max
// page ID of the renderer should have been updated when we restored the tab.
tab->NavigateToURL(http_url2);
- ASSERT_FALSE(tab->GoForward());
- ASSERT_EQ(http_url2, GetActiveTabURL());
+ EXPECT_FALSE(tab->GoForward());
+ EXPECT_EQ(http_url2, GetActiveTabURL());
}
TEST_F(TabRestoreUITest, RestoreWindow) {
@@ -406,11 +528,11 @@
ASSERT_TRUE(restored_tab_proxy->WaitForTabToBeRestored(action_timeout_ms()));
GURL url;
ASSERT_TRUE(restored_tab_proxy->GetCurrentURL(&url));
- ASSERT_TRUE(url == url1_);
+ EXPECT_TRUE(url == url1_);
restored_tab_proxy.reset(
browser_proxy->GetTab(initial_tab_count + 1));
ASSERT_TRUE(restored_tab_proxy->WaitForTabToBeRestored(action_timeout_ms()));
ASSERT_TRUE(restored_tab_proxy->GetCurrentURL(&url));
- ASSERT_TRUE(url == url2_);
+ EXPECT_TRUE(url == url2_);
}
« no previous file with comments | « chrome/browser/sessions/tab_restore_service.cc ('k') | chrome/test/automation/browser_proxy.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698