diff options
author | mad@chromium.org <mad@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-10-10 15:20:38 +0000 |
---|---|---|
committer | mad@chromium.org <mad@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-10-10 15:20:38 +0000 |
commit | bf219e9eb433014439ef0f40a722f021e18bc1ca (patch) | |
tree | c0c2a44120f228e732ff82159f0295be395c60d1 /chrome/browser | |
parent | 5dca49ebea4644ca5ca961996af2450876ceb30e (diff) | |
download | chromium_src-bf219e9eb433014439ef0f40a722f021e18bc1ca.zip chromium_src-bf219e9eb433014439ef0f40a722f021e18bc1ca.tar.gz chromium_src-bf219e9eb433014439ef0f40a722f021e18bc1ca.tar.bz2 |
Activate appropriate window from a session restore.
BUG=151979
Review URL: https://chromiumcodereview.appspot.com/10982098
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@161116 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
-rw-r--r-- | chrome/browser/sessions/session_restore.cc | 45 | ||||
-rw-r--r-- | chrome/browser/sessions/session_service.cc | 112 | ||||
-rw-r--r-- | chrome/browser/sessions/session_service.h | 51 | ||||
-rw-r--r-- | chrome/browser/sessions/session_service_test_helper.cc | 12 | ||||
-rw-r--r-- | chrome/browser/sessions/session_service_test_helper.h | 8 | ||||
-rw-r--r-- | chrome/browser/sessions/session_service_unittest.cc | 216 | ||||
-rw-r--r-- | chrome/browser/sessions/tab_restore_service.cc | 3 | ||||
-rw-r--r-- | chrome/browser/sessions/tab_restore_service.h | 6 |
8 files changed, 338 insertions, 115 deletions
diff --git a/chrome/browser/sessions/session_restore.cc b/chrome/browser/sessions/session_restore.cc index 3ccac2d..0dee8cf 100644 --- a/chrome/browser/sessions/session_restore.cc +++ b/chrome/browser/sessions/session_restore.cc @@ -490,6 +490,7 @@ class SessionRestoreImpl : public content::NotificationObserver { clobber_existing_tab_(clobber_existing_tab), always_create_tabbed_browser_(always_create_tabbed_browser), urls_to_open_(urls_to_open), + active_window_id_(0), restore_started_(base::TimeTicks::Now()), browser_shown_(false) { @@ -527,7 +528,7 @@ class SessionRestoreImpl : public content::NotificationObserver { MessageLoop::ScopedNestableTaskAllower allow(MessageLoop::current()); MessageLoop::current()->Run(); } - Browser* browser = ProcessSessionWindows(&windows_); + Browser* browser = ProcessSessionWindows(&windows_, active_window_id_); delete this; return browser; } @@ -708,7 +709,8 @@ class SessionRestoreImpl : public content::NotificationObserver { } void OnGotSession(SessionService::Handle handle, - std::vector<SessionWindow*>* windows) { + std::vector<SessionWindow*>* windows, + SessionID::id_type active_window_id) { base::TimeDelta time_to_got_sessions = base::TimeTicks::Now() - restore_started_; UMA_HISTOGRAM_CUSTOM_TIMES( @@ -724,14 +726,16 @@ class SessionRestoreImpl : public content::NotificationObserver { if (synchronous_) { // See comment above windows_ as to why we don't process immediately. windows_.swap(*windows); + active_window_id_ = active_window_id; MessageLoop::current()->QuitNow(); return; } - ProcessSessionWindows(windows); + ProcessSessionWindows(windows, active_window_id); } - Browser* ProcessSessionWindows(std::vector<SessionWindow*>* windows) { + Browser* ProcessSessionWindows(std::vector<SessionWindow*>* windows, + SessionID::id_type active_window_id) { VLOG(1) << "ProcessSessionWindows " << windows->size(); base::TimeDelta time_to_process_sessions = base::TimeTicks::Now() - restore_started_; @@ -761,6 +765,11 @@ class SessionRestoreImpl : public content::NotificationObserver { Browser* last_browser = NULL; bool has_tabbed_browser = false; + // After the for loop, this contains the browser to activate, if one of the + // windows has the same id as specified in active_window_id. + Browser* browser_to_activate = NULL; + int selected_tab_to_activate = -1; + // Determine if there is a visible window. bool has_visible_browser = false; for (std::vector<SessionWindow*>::iterator i = windows->begin(); @@ -833,6 +842,10 @@ class SessionRestoreImpl : public content::NotificationObserver { selected_tab_index = RestoreTabsToBrowser(*(*i), browser, selected_tab_index); ShowBrowser(browser, selected_tab_index); + if ((*i)->window_id.id() == active_window_id) { + browser_to_activate = browser; + selected_tab_to_activate = selected_tab_index; + } if (clobber_existing_tab_ && i == windows->begin() && (*i)->type == Browser::TYPE_TABBED && active_tab && browser == browser_ && browser->tab_count() > initial_tab_count) { @@ -844,12 +857,22 @@ class SessionRestoreImpl : public content::NotificationObserver { NotifySessionServiceOfRestoredTabs(browser, initial_tab_count); } + if (browser_to_activate && browser_to_activate->is_type_tabbed()) + last_browser = browser_to_activate; + if (last_browser && !urls_to_open_.empty()) AppendURLsToBrowser(last_browser, urls_to_open_); #if defined(OS_CHROMEOS) chromeos::BootTimesLoader::Get()->AddLoginTimeMarker( "SessionRestore-CreatingTabs-End", false); #endif + if (browser_to_activate) + browser_to_activate->window()->Activate(); + +#if defined(OS_WIN) + if (base::win::IsMetroProcess() && browser_to_activate) + ShowBrowser(browser_to_activate, selected_tab_to_activate); +#endif // If last_browser is NULL and urls_to_open_ is non-empty, // FinishedTabCreation will create a new TabbedBrowser and add the urls to // it. @@ -889,14 +912,19 @@ class SessionRestoreImpl : public content::NotificationObserver { VLOG(1) << "RestoreTabsToBrowser " << window.tabs.size(); DCHECK(!window.tabs.empty()); WebContents* selected_web_contents = NULL; + // If browser already has tabs, we want to restore the new ones after the + // existing ones. E.g., this happens in Win8 Metro where we merge windows. + int tab_index_offset = browser->tab_count(); for (int i = 0; i < static_cast<int>(window.tabs.size()); ++i) { const SessionTab& tab = *(window.tabs[i]); // Don't schedule a load for the selected tab, as ShowBrowser() will do // that. - if (i == selected_tab_index) - selected_web_contents = RestoreTab(tab, i, browser, false); - else - RestoreTab(tab, i, browser, true); + if (i == selected_tab_index) { + selected_web_contents = RestoreTab( + tab, tab_index_offset + i, browser, false); + } else { + RestoreTab(tab, tab_index_offset + i, browser, true); + } } return selected_web_contents ? chrome::GetIndexOfTab(browser, selected_web_contents) : 0; @@ -1065,6 +1093,7 @@ class SessionRestoreImpl : public content::NotificationObserver { // loop take a while) we cache the SessionWindows here and create the actual // windows when the nested message loop exits. std::vector<SessionWindow*> windows_; + SessionID::id_type active_window_id_; content::NotificationRegistrar registrar_; diff --git a/chrome/browser/sessions/session_service.cc b/chrome/browser/sessions/session_service.cc index a2f37dc..3a68eb5 100644 --- a/chrome/browser/sessions/session_service.cc +++ b/chrome/browser/sessions/session_service.cc @@ -76,6 +76,7 @@ static const SessionCommand::id_type kCommandTabClosed = 16; static const SessionCommand::id_type kCommandWindowClosed = 17; static const SessionCommand::id_type kCommandSetTabUserAgentOverride = 18; static const SessionCommand::id_type kCommandSessionStorageAssociated = 19; +static const SessionCommand::id_type kCommandSetActiveWindow = 20; // Every kWritesPerReset commands triggers recreating the file. static const int kWritesPerReset = 250; @@ -129,6 +130,8 @@ struct WindowBoundsPayload3 { int32 show_state; }; +typedef SessionID::id_type ActiveWindowPayload; + struct IDAndIndexPayload { SessionID::id_type id; int32 index; @@ -220,6 +223,9 @@ SessionService::SessionService(const FilePath& save_path) } SessionService::~SessionService() { + // The BrowserList should outlive the SessionService since it's static and + // the SessionService is a ProfileKeyedService. + BrowserList::RemoveObserver(this); Save(); } @@ -533,6 +539,8 @@ void SessionService::Init() { registrar_.Add( this, chrome::NOTIFICATION_TAB_CONTENTS_APPLICATION_EXTENSION_CHANGED, content::NotificationService::AllSources()); + + BrowserList::AddObserver(this); } bool SessionService::ShouldNewWindowStartSession() { @@ -587,11 +595,10 @@ void SessionService::Observe(int type, switch (type) { case chrome::NOTIFICATION_BROWSER_WINDOW_READY: { Browser* browser = content::Source<Browser>(source).ptr(); - AppType app_type = browser->is_app() ? TYPE_APP : TYPE_NORMAL; - if (browser->profile() != profile() || - !should_track_changes_for_browser_type(browser->type(), app_type)) + if (!ShouldTrackBrowser(browser)) return; + AppType app_type = browser->is_app() ? TYPE_APP : TYPE_NORMAL; RestoreIfNecessary(std::vector<GURL>(), browser); SetWindowType(browser->session_id(), browser->type(), app_type); SetWindowAppName(browser->session_id(), browser->app_name()); @@ -696,6 +703,11 @@ void SessionService::Observe(int type, } } +void SessionService::OnBrowserSetLastActive(Browser* browser) { + if (ShouldTrackBrowser(browser)) + ScheduleCommand(CreateSetActiveWindowCommand(browser->session_id())); +} + void SessionService::SetTabExtensionAppID( const SessionID& window_id, const SessionID& tab_id, @@ -831,6 +843,16 @@ SessionCommand* SessionService::CreateSessionStorageAssociatedCommand( return new SessionCommand(kCommandSessionStorageAssociated, pickle); } +SessionCommand* SessionService::CreateSetActiveWindowCommand( + const SessionID& window_id) { + ActiveWindowPayload payload = 0; + payload = window_id.id(); + SessionCommand* command = + new SessionCommand(kCommandSetActiveWindow, sizeof(payload)); + memcpy(command->contents(), &payload, sizeof(payload)); + return command; +} + void SessionService::OnGotSessionCommands( Handle handle, scoped_refptr<InternalGetCommandsRequest> request) { @@ -838,20 +860,22 @@ void SessionService::OnGotSessionCommands( return; ScopedVector<SessionWindow> valid_windows; + SessionID::id_type active_window_id = 0; RestoreSessionFromCommands( - request->commands, &(valid_windows.get())); - static_cast<InternalSessionRequest*>(request.get())-> - real_callback.Run(request->handle(), &(valid_windows.get())); + request->commands, &(valid_windows.get()), &active_window_id); + static_cast<InternalSessionRequest*>(request.get())->real_callback.Run( + request->handle(), &(valid_windows.get()), active_window_id); } void SessionService::RestoreSessionFromCommands( const std::vector<SessionCommand*>& commands, - std::vector<SessionWindow*>* valid_windows) { + std::vector<SessionWindow*>* valid_windows, + SessionID::id_type* active_window_id) { std::map<int, SessionTab*> tabs; std::map<int, SessionWindow*> windows; VLOG(1) << "RestoreSessionFromCommands " << commands.size(); - if (CreateTabsAndWindows(commands, &tabs, &windows)) { + if (CreateTabsAndWindows(commands, &tabs, &windows, active_window_id)) { AddTabsToWindows(&tabs, &windows); SortTabsBasedOnVisualOrderAndPrune(&windows, valid_windows); UpdateSelectedTabIndex(valid_windows); @@ -950,7 +974,7 @@ void SessionService::SortTabsBasedOnVisualOrderAndPrune( // Valid window; sort the tabs and add it to the list of valid windows. std::sort(window->tabs.begin(), window->tabs.end(), &TabVisualIndexSortFunction); - // Add the window such that older windows appear first. + // Otherwise, add the window such that older windows appear first. if (valid_windows->empty()) { valid_windows->push_back(window); } else { @@ -998,7 +1022,8 @@ void SessionService::AddTabsToWindows(std::map<int, SessionTab*>* tabs, bool SessionService::CreateTabsAndWindows( const std::vector<SessionCommand*>& data, std::map<int, SessionTab*>* tabs, - std::map<int, SessionWindow*>* windows) { + std::map<int, SessionWindow*>* windows, + SessionID::id_type* active_window_id) { // If the file is corrupt (command with wrong size, or unknown command), we // still return true and attempt to restore what we we can. VLOG(1) << "CreateTabsAndWindows"; @@ -1241,6 +1266,16 @@ bool SessionService::CreateTabsAndWindows( break; } + case kCommandSetActiveWindow: { + ActiveWindowPayload payload; + if (!command->GetPayload(&payload, sizeof(payload))) { + VLOG(1) << "Failed reading command " << command->id(); + return true; + } + *active_window_id = payload; + break; + } + default: VLOG(1) << "Failed reading an unknown command " << command->id(); return true; @@ -1387,9 +1422,7 @@ void SessionService::BuildCommandsFromBrowsers( // for us to get a handle to a browser that is about to be removed. If // the tab count is 0 or the window is NULL, the browser is about to be // deleted, so we ignore it. - AppType app_type = browser->is_app() ? TYPE_APP : TYPE_NORMAL; - if (should_track_changes_for_browser_type(browser->type(), app_type) && - browser->tab_count() && + if (ShouldTrackBrowser(browser) && browser->tab_count() && browser->window()) { BuildCommandsForBrowser(browser, commands, tab_to_available_range, windows_to_track); @@ -1414,23 +1447,25 @@ void SessionService::ScheduleReset() { } bool SessionService::ReplacePendingCommand(SessionCommand* command) { - // We only optimize page navigations, which can happen quite frequently and - // are expensive. If necessary, other commands could be searched for as - // well. - if (command->id() != kCommandUpdateTabNavigation) - return false; - scoped_ptr<Pickle> command_pickle(command->PayloadAsPickle()); - PickleIterator iterator(*command_pickle); - SessionID::id_type command_tab_id; - int command_nav_index; - if (!command_pickle->ReadInt(&iterator, &command_tab_id) || - !command_pickle->ReadInt(&iterator, &command_nav_index)) { + // We optimize page navigations, which can happen quite frequently and + // are expensive. And activation is like Highlander, there can only be one! + if (command->id() != kCommandUpdateTabNavigation && + command->id() != kCommandSetActiveWindow) { return false; } for (std::vector<SessionCommand*>::reverse_iterator i = pending_commands().rbegin(); i != pending_commands().rend(); ++i) { SessionCommand* existing_command = *i; - if (existing_command->id() == kCommandUpdateTabNavigation) { + if (command->id() == kCommandUpdateTabNavigation && + existing_command->id() == kCommandUpdateTabNavigation) { + scoped_ptr<Pickle> command_pickle(command->PayloadAsPickle()); + PickleIterator iterator(*command_pickle); + SessionID::id_type command_tab_id; + int command_nav_index; + if (!command_pickle->ReadInt(&iterator, &command_tab_id) || + !command_pickle->ReadInt(&iterator, &command_nav_index)) { + return false; + } SessionID::id_type existing_tab_id; int existing_nav_index; { @@ -1456,6 +1491,12 @@ bool SessionService::ReplacePendingCommand(SessionCommand* command) { } return false; } + if (command->id() == kCommandSetActiveWindow && + existing_command->id() == kCommandSetActiveWindow) { + *i = command; + delete existing_command; + return true; + } } return false; } @@ -1489,7 +1530,7 @@ void SessionService::CommitPendingCloses() { pending_window_close_ids_.clear(); } -bool SessionService::IsOnlyOneTabLeft() { +bool SessionService::IsOnlyOneTabLeft() const { if (!profile()) { // We're testing, always return false. return false; @@ -1500,9 +1541,7 @@ bool SessionService::IsOnlyOneTabLeft() { i != BrowserList::end(); ++i) { Browser* browser = *i; const SessionID::id_type window_id = browser->session_id().id(); - AppType app_type = browser->is_app() ? TYPE_APP : TYPE_NORMAL; - if (should_track_changes_for_browser_type(browser->type(), app_type) && - browser->profile() == profile() && + if (ShouldTrackBrowser(browser) && window_closing_ids_.find(window_id) == window_closing_ids_.end()) { if (++window_count > 1) return false; @@ -1515,7 +1554,8 @@ bool SessionService::IsOnlyOneTabLeft() { return true; } -bool SessionService::HasOpenTrackableBrowsers(const SessionID& window_id) { +bool SessionService::HasOpenTrackableBrowsers( + const SessionID& window_id) const { if (!profile()) { // We're testing, always return false. return true; @@ -1525,21 +1565,25 @@ bool SessionService::HasOpenTrackableBrowsers(const SessionID& window_id) { i != BrowserList::end(); ++i) { Browser* browser = *i; const SessionID::id_type browser_id = browser->session_id().id(); - AppType app_type = browser->is_app() ? TYPE_APP : TYPE_NORMAL; if (browser_id != window_id.id() && window_closing_ids_.find(browser_id) == window_closing_ids_.end() && - should_track_changes_for_browser_type(browser->type(), app_type) && - browser->profile() == profile()) { + ShouldTrackBrowser(browser)) { return true; } } return false; } -bool SessionService::ShouldTrackChangesToWindow(const SessionID& window_id) { +bool SessionService::ShouldTrackChangesToWindow( + const SessionID& window_id) const { return windows_tracking_.find(window_id.id()) != windows_tracking_.end(); } +bool SessionService::ShouldTrackBrowser(Browser* browser) const { + AppType app_type = browser->is_app() ? TYPE_APP : TYPE_NORMAL; + return browser->profile() == profile() && + should_track_changes_for_browser_type(browser->type(), app_type); +} bool SessionService::should_track_changes_for_browser_type(Browser::Type type, AppType app_type) { diff --git a/chrome/browser/sessions/session_service.h b/chrome/browser/sessions/session_service.h index 67fd97d..b9d36ec 100644 --- a/chrome/browser/sessions/session_service.h +++ b/chrome/browser/sessions/session_service.h @@ -16,6 +16,7 @@ #include "chrome/browser/sessions/session_id.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_list.h" +#include "chrome/browser/ui/browser_list_observer.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" #include "ui/base/ui_base_types.h" @@ -52,7 +53,8 @@ class WebContents; // SessionService rebuilds the contents of the file from the open state // of the browser. class SessionService : public BaseSessionService, - public content::NotificationObserver { + public content::NotificationObserver, + public chrome::BrowserListObserver { friend class SessionServiceTestHelper; public: // Used to distinguish an application window from a normal one. @@ -181,9 +183,10 @@ class SessionService : public BaseSessionService, // The contents of the supplied vector are deleted after the callback is // notified. To take ownership of the vector clear it before returning. // - // The time gives the time the session was closed. - typedef base::Callback<void(Handle, std::vector<SessionWindow*>*)> - SessionCallback; + // The session ID is the id of the window that was last active. + typedef base::Callback<void(Handle, + std::vector<SessionWindow*>*, + SessionID::id_type)> SessionCallback; // Fetches the contents of the last session, notifying the callback when // done. If the callback is supplied an empty vector of SessionWindows @@ -200,6 +203,10 @@ class SessionService : public BaseSessionService, virtual void Save() OVERRIDE; private: + // Allow tests to access our innards for testing purposes. + FRIEND_TEST_ALL_PREFIXES(SessionServiceTest, RestoreActivation1); + FRIEND_TEST_ALL_PREFIXES(SessionServiceTest, RestoreActivation2); + typedef std::map<SessionID::id_type, std::pair<int, int> > IdToRange; typedef std::map<SessionID::id_type, SessionTab*> IdToSessionTab; typedef std::map<SessionID::id_type, SessionWindow*> IdToSessionWindow; @@ -225,6 +232,11 @@ class SessionService : public BaseSessionService, const content::NotificationSource& source, const content::NotificationDetails& details) OVERRIDE; + // chrome::BrowserListObserver + virtual void OnBrowserAdded(Browser* browser) OVERRIDE {} + virtual void OnBrowserRemoved(Browser* browser) OVERRIDE {} + virtual void OnBrowserSetLastActive(Browser* browser) OVERRIDE; + // Sets the application extension id of the specified tab. void SetTabExtensionAppID(const SessionID& window_id, const SessionID& tab_id, @@ -263,6 +275,8 @@ class SessionService : public BaseSessionService, const SessionID& tab_id, const std::string& session_storage_persistent_id); + SessionCommand* CreateSetActiveWindowCommand(const SessionID& window_id); + // Callback from the backend for getting the commands from the save file. // Converts the commands in SessionWindows and notifies the real callback. void OnGotSessionCommands( @@ -271,12 +285,12 @@ class SessionService : public BaseSessionService, // Converts the commands into SessionWindows. On return any valid // windows are added to valid_windows. It is up to the caller to delete - // the windows added to valid_windows. - // - // If ignore_recent_closes is true, any window/tab closes within in a certain - // time frame are ignored. + // the windows added to valid_windows. |active_window_id| will be set with the + // id of the last active window, but it's only valid when this id corresponds + // to the id of one of the windows in valid_windows. void RestoreSessionFromCommands(const std::vector<SessionCommand*>& commands, - std::vector<SessionWindow*>* valid_windows); + std::vector<SessionWindow*>* valid_windows, + SessionID::id_type* active_window_id); // Iterates through the vector updating the selected_tab_index of each // SessionWindow based on the actual tabs that were restored. @@ -317,15 +331,17 @@ class SessionService : public BaseSessionService, void AddTabsToWindows(std::map<int, SessionTab*>* tabs, std::map<int, SessionWindow*>* windows); - // Creates tabs and windows from the specified commands. The created tabs - // and windows are added to |tabs| and |windows| respectively. It is up to - // the caller to delete the tabs and windows added to |tabs| and |windows|. + // Creates tabs and windows from the commands specified in |data|. The created + // tabs and windows are added to |tabs| and |windows| respectively, with the + // id of the active window set in |active_window_id|. It is up to the caller + // to delete the tabs and windows added to |tabs| and |windows|. // // This does NOT add any created SessionTabs to SessionWindow.tabs, that is // done by AddTabsToWindows. bool CreateTabsAndWindows(const std::vector<SessionCommand*>& data, std::map<int, SessionTab*>* tabs, - std::map<int, SessionWindow*>* windows); + std::map<int, SessionWindow*>* windows, + SessionID::id_type* active_window_id); // Adds commands to commands that will recreate the state of the specified // tab. This adds at most kMaxNavigationCountToPersist navigations (in each @@ -376,16 +392,19 @@ class SessionService : public BaseSessionService, // Returns true if there is only one window open with a single tab that shares // our profile. - bool IsOnlyOneTabLeft(); + bool IsOnlyOneTabLeft() const; // Returns true if there are open trackable browser windows whose ids do // match |window_id| with our profile. A trackable window is a window from // which |should_track_changes_for_browser_type| returns true. See // |should_track_changes_for_browser_type| for details. - bool HasOpenTrackableBrowsers(const SessionID& window_id); + bool HasOpenTrackableBrowsers(const SessionID& window_id) const; // Returns true if changes to tabs in the specified window should be tracked. - bool ShouldTrackChangesToWindow(const SessionID& window_id); + bool ShouldTrackChangesToWindow(const SessionID& window_id) const; + + // Returns true if we track changes to the specified browser. + bool ShouldTrackBrowser(Browser* browser) const; // Returns true if we track changes to the specified browser type. static bool should_track_changes_for_browser_type( diff --git a/chrome/browser/sessions/session_service_test_helper.cc b/chrome/browser/sessions/session_service_test_helper.cc index 62aac7c..fc2108e7 100644 --- a/chrome/browser/sessions/session_service_test_helper.cc +++ b/chrome/browser/sessions/session_service_test_helper.cc @@ -21,12 +21,6 @@ SessionServiceTestHelper::SessionServiceTestHelper(SessionService* service) SessionServiceTestHelper::~SessionServiceTestHelper() {} -void SessionServiceTestHelper::RestoreSessionFromCommands( - const std::vector<SessionCommand*>& commands, - std::vector<SessionWindow*>* valid_windows) { - service()->RestoreSessionFromCommands(commands, valid_windows); -} - void SessionServiceTestHelper::PrepareTabInWindow(const SessionID& window_id, const SessionID& tab_id, int visual_index, @@ -59,11 +53,13 @@ void SessionServiceTestHelper::SetForceBrowserNotAliveWithNoWindows( // Be sure and null out service to force closing the file. void SessionServiceTestHelper::ReadWindows( - std::vector<SessionWindow*>* windows) { + std::vector<SessionWindow*>* windows, + SessionID::id_type* active_window_id) { Time last_time; ScopedVector<SessionCommand> read_commands; backend()->ReadLastSessionCommandsImpl(&(read_commands.get())); - RestoreSessionFromCommands(read_commands.get(), windows); + service()->RestoreSessionFromCommands( + read_commands.get(), windows, active_window_id); } void SessionServiceTestHelper::AssertTabEquals(SessionID& window_id, diff --git a/chrome/browser/sessions/session_service_test_helper.h b/chrome/browser/sessions/session_service_test_helper.h index cccb1d3..dd481c3 100644 --- a/chrome/browser/sessions/session_service_test_helper.h +++ b/chrome/browser/sessions/session_service_test_helper.h @@ -10,10 +10,10 @@ #include "base/basictypes.h" #include "base/memory/scoped_ptr.h" +#include "chrome/browser/sessions/session_id.h" class SessionBackend; class SessionCommand; -class SessionID; class SessionService; struct SessionTab; struct SessionWindow; @@ -27,9 +27,6 @@ class SessionServiceTestHelper { explicit SessionServiceTestHelper(SessionService* service); ~SessionServiceTestHelper(); - void RestoreSessionFromCommands(const std::vector<SessionCommand*>& commands, - std::vector<SessionWindow*>* valid_windows); - void PrepareTabInWindow(const SessionID& window_id, const SessionID& tab_id, int visual_index, @@ -47,7 +44,8 @@ class SessionServiceTestHelper { bool force_browser_not_alive_with_no_windows); // Reads the contents of the last session. - void ReadWindows(std::vector<SessionWindow*>* windows); + void ReadWindows(std::vector<SessionWindow*>* windows, + SessionID::id_type* active_window_id); void AssertTabEquals(SessionID& window_id, SessionID& tab_id, diff --git a/chrome/browser/sessions/session_service_unittest.cc b/chrome/browser/sessions/session_service_unittest.cc index 3152b3b..dd33e12 100644 --- a/chrome/browser/sessions/session_service_unittest.cc +++ b/chrome/browser/sessions/session_service_unittest.cc @@ -84,13 +84,19 @@ class SessionServiceTest : public BrowserWithTestWindowTest, } } - void ReadWindows(std::vector<SessionWindow*>* windows) { + void ReadWindows(std::vector<SessionWindow*>* windows, + SessionID::id_type* active_window_id) { // Forces closing the file. helper_.set_service(NULL); SessionService* session_service = new SessionService(path_); helper_.set_service(session_service); - helper_.ReadWindows(windows); + + SessionID::id_type* non_null_active_window_id = active_window_id; + SessionID::id_type dummy_active_window_id = 0; + if (!non_null_active_window_id) + non_null_active_window_id = &dummy_active_window_id; + helper_.ReadWindows(windows, non_null_active_window_id); } // Configures the session service with one window with one tab and a single @@ -109,7 +115,7 @@ class SessionServiceTest : public BrowserWithTestWindowTest, helper_.service()->SetPinnedState(window_id, tab_id, pinned_state); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); EXPECT_EQ(1U, windows.size()); if (HasFatalFailure()) @@ -124,6 +130,31 @@ class SessionServiceTest : public BrowserWithTestWindowTest, return tab->pinned; } + void CreateAndWriteSessionWithTwoWindows( + const SessionID& window2_id, + const SessionID& tab1_id, + const SessionID& tab2_id, + TabNavigation* nav1, + TabNavigation* nav2) { + *nav1 = + SessionTypesTestHelper::CreateNavigation("http://google.com", "abc"); + *nav2 = + SessionTypesTestHelper::CreateNavigation("http://google2.com", "abcd"); + + helper_.PrepareTabInWindow(window_id, tab1_id, 0, true); + UpdateNavigation(window_id, tab1_id, *nav1, true); + + const gfx::Rect window2_bounds(3, 4, 5, 6); + service()->SetWindowType( + window2_id, Browser::TYPE_TABBED, SessionService::TYPE_NORMAL); + service()->SetWindowBounds(window2_id, + window2_bounds, + ui::SHOW_STATE_MAXIMIZED); + helper_.PrepareTabInWindow(window2_id, tab2_id, 0, true); + UpdateNavigation(window2_id, tab2_id, *nav2, true); + + } + SessionService* service() { return helper_.service(); } SessionBackend* backend() { return helper_.backend(); } @@ -154,7 +185,7 @@ TEST_F(SessionServiceTest, Basic) { UpdateNavigation(window_id, tab_id, nav1, true); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); ASSERT_EQ(1U, windows.size()); ASSERT_TRUE(window_bounds == windows[0]->bounds); @@ -182,7 +213,7 @@ TEST_F(SessionServiceTest, PersistPostData) { UpdateNavigation(window_id, tab_id, nav1, true); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); helper_.AssertSingleWindowWithSingleTab(windows.get(), 1); } @@ -205,7 +236,7 @@ TEST_F(SessionServiceTest, ClosingTabStaysClosed) { service()->TabClosed(window_id, tab2_id, false); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); ASSERT_EQ(1U, windows.size()); ASSERT_EQ(0, windows[0]->selected_tab_index); @@ -235,7 +266,7 @@ TEST_F(SessionServiceTest, Pruning) { service()->TabNavigationPathPrunedFromBack(window_id, tab_id, 3); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); ASSERT_EQ(1U, windows.size()); ASSERT_EQ(0, windows[0]->selected_tab_index); @@ -256,26 +287,14 @@ TEST_F(SessionServiceTest, TwoWindows) { SessionID window2_id; SessionID tab1_id; SessionID tab2_id; + TabNavigation nav1; + TabNavigation nav2; - TabNavigation nav1 = - SessionTypesTestHelper::CreateNavigation("http://google.com", "abc"); - TabNavigation nav2 = - SessionTypesTestHelper::CreateNavigation("http://google2.com", "abcd"); - - helper_.PrepareTabInWindow(window_id, tab1_id, 0, true); - UpdateNavigation(window_id, tab1_id, nav1, true); - - const gfx::Rect window2_bounds(3, 4, 5, 6); - service()->SetWindowType( - window2_id, Browser::TYPE_TABBED, SessionService::TYPE_NORMAL); - service()->SetWindowBounds(window2_id, - window2_bounds, - ui::SHOW_STATE_MAXIMIZED); - helper_.PrepareTabInWindow(window2_id, tab2_id, 0, true); - UpdateNavigation(window2_id, tab2_id, nav2, true); + CreateAndWriteSessionWithTwoWindows( + window2_id, tab1_id, tab2_id, &nav1, &nav2); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); ASSERT_EQ(2U, windows.size()); ASSERT_EQ(0, windows[0]->selected_tab_index); @@ -328,7 +347,7 @@ TEST_F(SessionServiceTest, WindowWithNoTabsGetsPruned) { helper_.PrepareTabInWindow(window2_id, tab2_id, 0, true); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); ASSERT_EQ(1U, windows.size()); ASSERT_EQ(0, windows[0]->selected_tab_index); @@ -359,7 +378,7 @@ TEST_F(SessionServiceTest, ClosingWindowDoesntCloseTabs) { service()->WindowClosing(window_id); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); ASSERT_EQ(1U, windows.size()); ASSERT_EQ(0, windows[0]->selected_tab_index); @@ -403,7 +422,7 @@ TEST_F(SessionServiceTest, WindowCloseCommittedAfterNavigate) { service()->WindowClosed(window2_id); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); ASSERT_EQ(1U, windows.size()); ASSERT_EQ(0, windows[0]->selected_tab_index); @@ -443,7 +462,7 @@ TEST_F(SessionServiceTest, IgnorePopups) { UpdateNavigation(window2_id, tab2_id, nav2, true); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); ASSERT_EQ(1U, windows.size()); ASSERT_EQ(0, windows[0]->selected_tab_index); @@ -483,7 +502,7 @@ TEST_F(SessionServiceTest, RestorePopup) { UpdateNavigation(window2_id, tab2_id, nav2, true); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); ASSERT_EQ(2U, windows.size()); int tabbed_index = windows[0]->type == Browser::TYPE_TABBED ? @@ -533,7 +552,7 @@ TEST_F(SessionServiceTest, RestoreApp) { UpdateNavigation(window2_id, tab2_id, nav2, true); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); ASSERT_EQ(2U, windows.size()); int tabbed_index = windows[0]->type == Browser::TYPE_TABBED ? @@ -557,7 +576,7 @@ TEST_F(SessionServiceTest, RestoreApp) { helper_.AssertTabEquals(window2_id, tab2_id, 0, 0, 1, *tab); helper_.AssertNavigationEquals(nav2, tab->navigations[0]); } -#endif +#endif // defined (USE_AURA) // Tests pruning from the front. TEST_F(SessionServiceTest, PruneFromFront) { @@ -580,7 +599,7 @@ TEST_F(SessionServiceTest, PruneFromFront) { // Read back in. ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); ASSERT_EQ(1U, windows.size()); ASSERT_EQ(0, windows[0]->selected_tab_index); @@ -623,7 +642,7 @@ TEST_F(SessionServiceTest, PruneToEmpty) { // Read back in. ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); ASSERT_EQ(0U, windows.size()); } @@ -657,7 +676,7 @@ TEST_F(SessionServiceTest, PersistApplicationExtensionID) { helper_.SetTabExtensionAppID(window_id, tab_id, app_id); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); helper_.AssertSingleWindowWithSingleTab(windows.get(), 1); EXPECT_TRUE(app_id == windows[0]->tabs[0]->extension_app_id); @@ -680,7 +699,7 @@ TEST_F(SessionServiceTest, PersistUserAgentOverrides) { helper_.SetTabUserAgentOverride(window_id, tab_id, user_agent_override); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); helper_.AssertSingleWindowWithSingleTab(windows.get(), 1); SessionTab* tab = windows[0]->tabs[0]; @@ -711,7 +730,7 @@ TEST_F(SessionServiceTest, CloseTabUserGesture) { service()->TabClosed(window_id, tab_id, true); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); ASSERT_TRUE(windows.empty()); } @@ -729,7 +748,7 @@ TEST_F(SessionServiceTest, DontPersistDefault) { ui::SHOW_STATE_DEFAULT); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); ASSERT_EQ(1U, windows.size()); EXPECT_EQ(ui::SHOW_STATE_NORMAL, windows[0]->show_state); } @@ -768,7 +787,7 @@ TEST_F(SessionServiceTest, KeepPostDataWithoutPasswords) { UpdateNavigation(window_id, tab_id, nav2, true); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); helper_.AssertSingleWindowWithSingleTab(windows.get(), 2); @@ -803,7 +822,7 @@ TEST_F(SessionServiceTest, RemovePostDataWithPasswords) { UpdateNavigation(window_id, tab_id, nav1, true); ScopedVector<SessionWindow> windows; - ReadWindows(&(windows.get())); + ReadWindows(&(windows.get()), NULL); helper_.AssertSingleWindowWithSingleTab(windows.get(), 1); @@ -834,8 +853,123 @@ TEST_F(SessionServiceTest, CanOpenV1TabClosed) { SessionService* session_service = new SessionService(path_); helper_.set_service(session_service); ScopedVector<SessionWindow> windows; - helper_.ReadWindows(&(windows.get())); + SessionID::id_type active_window_id = 0; + helper_.ReadWindows(&(windows.get()), &active_window_id); ASSERT_EQ(1u, windows.size()); EXPECT_EQ(1u, windows[0]->tabs.size()); } -#endif +#endif // defined(OS_CHROMEOS) + +TEST_F(SessionServiceTest, ReplacePendingNavigation) { + const std::string base_url("http://google.com/"); + SessionID tab_id; + + helper_.PrepareTabInWindow(window_id, tab_id, 0, true); + + // Add 5 navigations, some with the same index + for (int i = 0; i < 5; ++i) { + TabNavigation nav = + SessionTypesTestHelper::CreateNavigation( + base_url + base::IntToString(i), "a"); + nav.set_index(i / 2); + UpdateNavigation(window_id, tab_id, nav, true); + } + + // Read back in. + ScopedVector<SessionWindow> windows; + ReadWindows(&(windows.get()), NULL); + + // The ones with index 0, and 2 should have been replaced by 1 and 3. + ASSERT_EQ(1U, windows.size()); + ASSERT_EQ(1U, windows[0]->tabs.size()); + EXPECT_EQ(3U, windows[0]->tabs[0]->navigations.size()); + EXPECT_EQ(GURL(base_url + base::IntToString(1)), + windows[0]->tabs[0]->navigations[0].virtual_url()); + EXPECT_EQ(GURL(base_url + base::IntToString(3)), + windows[0]->tabs[0]->navigations[1].virtual_url()); + EXPECT_EQ(GURL(base_url + base::IntToString(4)), + windows[0]->tabs[0]->navigations[2].virtual_url()); +} + +TEST_F(SessionServiceTest, ReplacePendingNavigationAndPrune) { + const std::string base_url("http://google.com/"); + SessionID tab_id; + + helper_.PrepareTabInWindow(window_id, tab_id, 0, true); + + for (int i = 0; i < 5; ++i) { + TabNavigation nav = + SessionTypesTestHelper::CreateNavigation( + base_url + base::IntToString(i), "a"); + nav.set_index(i); + UpdateNavigation(window_id, tab_id, nav, true); + } + + // Prune all those navigations. + helper_.service()->TabNavigationPathPrunedFromFront(window_id, tab_id, 5); + + // Add another navigation to replace the last one. + TabNavigation nav = + SessionTypesTestHelper::CreateNavigation( + base_url + base::IntToString(5), "a"); + nav.set_index(4); + UpdateNavigation(window_id, tab_id, nav, true); + + // Read back in. + ScopedVector<SessionWindow> windows; + ReadWindows(&(windows.get()), NULL); + + // We should still have that last navigation at the end, + // even though it replaced one that was set before the prune. + ASSERT_EQ(1U, windows.size()); + ASSERT_EQ(1U, windows[0]->tabs.size()); + ASSERT_EQ(1U, windows[0]->tabs[0]->navigations.size()); + EXPECT_EQ(GURL(base_url + base::IntToString(5)), + windows[0]->tabs[0]->navigations[0].virtual_url()); +} + +TEST_F(SessionServiceTest, RestoreActivation1) { + SessionID window2_id; + SessionID tab1_id; + SessionID tab2_id; + TabNavigation nav1; + TabNavigation nav2; + + CreateAndWriteSessionWithTwoWindows( + window2_id, tab1_id, tab2_id, &nav1, &nav2); + + service()->ScheduleCommand( + service()->CreateSetActiveWindowCommand(window2_id)); + service()->ScheduleCommand( + service()->CreateSetActiveWindowCommand(window_id)); + + ScopedVector<SessionWindow> windows; + SessionID::id_type active_window_id = 0; + ReadWindows(&(windows.get()), &active_window_id); + EXPECT_EQ(window_id.id(), active_window_id); +} + +// It's easier to have two separate tests with setup/teardown than to manualy +// reset the state for the different flavors of the test. +TEST_F(SessionServiceTest, RestoreActivation2) { + SessionID window2_id; + SessionID tab1_id; + SessionID tab2_id; + TabNavigation nav1; + TabNavigation nav2; + + CreateAndWriteSessionWithTwoWindows( + window2_id, tab1_id, tab2_id, &nav1, &nav2); + + service()->ScheduleCommand( + service()->CreateSetActiveWindowCommand(window2_id)); + service()->ScheduleCommand( + service()->CreateSetActiveWindowCommand(window_id)); + service()->ScheduleCommand( + service()->CreateSetActiveWindowCommand(window2_id)); + + ScopedVector<SessionWindow> windows; + SessionID::id_type active_window_id = 0; + ReadWindows(&(windows.get()), &active_window_id); + EXPECT_EQ(window2_id.id(), active_window_id); +} diff --git a/chrome/browser/sessions/tab_restore_service.cc b/chrome/browser/sessions/tab_restore_service.cc index a83f9d1..4003edb 100644 --- a/chrome/browser/sessions/tab_restore_service.cc +++ b/chrome/browser/sessions/tab_restore_service.cc @@ -1162,7 +1162,8 @@ void TabRestoreService::UpdateTabBrowserIDs(SessionID::id_type old_id, void TabRestoreService::OnGotPreviousSession( Handle handle, - std::vector<SessionWindow*>* windows) { + std::vector<SessionWindow*>* windows, + SessionID::id_type ignored_active_window) { std::vector<Entry*> entries; CreateEntriesFromWindows(windows, &entries); // Previous session tabs go first. diff --git a/chrome/browser/sessions/tab_restore_service.h b/chrome/browser/sessions/tab_restore_service.h index 3fea7fb..13d202f 100644 --- a/chrome/browser/sessions/tab_restore_service.h +++ b/chrome/browser/sessions/tab_restore_service.h @@ -327,9 +327,11 @@ class TabRestoreService : public BaseSessionService { // Callback from SessionService when we've received the windows from the // previous session. This creates and add entries to |staging_entries_| - // and invokes LoadStateChanged. + // and invokes LoadStateChanged. |ignored_active_window| is ignored because + // we don't need to restore activation. void OnGotPreviousSession(Handle handle, - std::vector<SessionWindow*>* windows); + std::vector<SessionWindow*>* windows, + SessionID::id_type ignored_active_window); // Converts a SessionWindow into a Window, returning true on success. We use 0 // as the timestamp here since we do not know when the window/tab was closed. |