diff options
| -rw-r--r-- | chrome/browser/sessions/session_restore.cc | 36 | ||||
| -rw-r--r-- | chrome/browser/sessions/session_restore.h | 4 | ||||
| -rw-r--r-- | chrome/browser/sessions/session_restore_browsertest.cc | 122 | ||||
| -rw-r--r-- | chrome/browser/sessions/session_restore_delegate.cc | 51 | ||||
| -rw-r--r-- | chrome/browser/sessions/session_restore_delegate.h | 24 | ||||
| -rw-r--r-- | chrome/browser/sessions/session_restore_stats_collector.cc | 6 | ||||
| -rw-r--r-- | chrome/browser/sessions/tab_loader.cc | 8 |
7 files changed, 227 insertions, 24 deletions
diff --git a/chrome/browser/sessions/session_restore.cc b/chrome/browser/sessions/session_restore.cc index 8402192..afafc0a 100644 --- a/chrome/browser/sessions/session_restore.cc +++ b/chrome/browser/sessions/session_restore.cc @@ -179,7 +179,7 @@ class SessionRestoreImpl : public content::NotificationObserver { } // Always create in a new window. - FinishedTabCreation(true, true, created_contents); + FinishedTabCreation(true, true, &created_contents); on_session_restored_callbacks_->Notify( static_cast<int>(created_contents.size())); @@ -280,10 +280,9 @@ class SessionRestoreImpl : public content::NotificationObserver { // have been loaded. // // Returns the Browser that was created, if any. - Browser* FinishedTabCreation( - bool succeeded, - bool created_tabbed_browser, - const std::vector<RestoredTab>& contents_created) { + Browser* FinishedTabCreation(bool succeeded, + bool created_tabbed_browser, + std::vector<RestoredTab>* contents_created) { Browser* browser = nullptr; if (!created_tabbed_browser && always_create_tabbed_browser_) { browser = @@ -299,7 +298,9 @@ class SessionRestoreImpl : public content::NotificationObserver { if (succeeded) { // Start Loading tabs. - SessionRestoreDelegate::RestoreTabs(contents_created, restore_started_); + if (SessionRestore::SmartLoadingEnabled()) + std::stable_sort(contents_created->begin(), contents_created->end()); + SessionRestoreDelegate::RestoreTabs(*contents_created, restore_started_); } if (!synchronous_) { @@ -362,7 +363,7 @@ class SessionRestoreImpl : public content::NotificationObserver { content::BrowserContext::GetDefaultStoragePartition(profile_) ->GetDOMStorageContext() ->StartScavengingUnusedSessionStorage(); - return FinishedTabCreation(false, false, *created_contents); + return FinishedTabCreation(false, false, created_contents); } #if defined(OS_CHROMEOS) @@ -462,7 +463,7 @@ class SessionRestoreImpl : public content::NotificationObserver { // FinishedTabCreation will create a new TabbedBrowser and add the urls to // it. Browser* finished_browser = - FinishedTabCreation(true, has_tabbed_browser, *created_contents); + FinishedTabCreation(true, has_tabbed_browser, created_contents); if (finished_browser) last_browser = finished_browser; @@ -518,9 +519,8 @@ class SessionRestoreImpl : public content::NotificationObserver { if (!contents) continue; - RestoredTab restored_tab; - restored_tab.contents = contents; - restored_tab.is_active = is_selected_tab; + RestoredTab restored_tab(contents, is_selected_tab, + tab.extension_app_id.empty(), tab.pinned); created_contents->push_back(restored_tab); // If this isn't the selected tab, there's nothing else to do. @@ -543,9 +543,8 @@ class SessionRestoreImpl : public content::NotificationObserver { WebContents* contents = RestoreTab(tab, tab_index_offset + i, browser, false); if (contents) { - RestoredTab restored_tab; - restored_tab.contents = contents; - restored_tab.is_active = false; + RestoredTab restored_tab(contents, false, + tab.extension_app_id.empty(), tab.pinned); created_contents->push_back(restored_tab); } } @@ -819,5 +818,14 @@ SessionRestore::CallbackSubscription } // static +bool SessionRestore::SmartLoadingEnabled() { + base::FieldTrial* trial = + base::FieldTrialList::Find("SessionRestoreBackgroundLoading"); + if (!trial || trial->group_name() != "Smart") + return false; + return true; +} + +// static base::CallbackList<void(int)>* SessionRestore::on_session_restored_callbacks_ = nullptr; diff --git a/chrome/browser/sessions/session_restore.h b/chrome/browser/sessions/session_restore.h index e6cb781..9117abe 100644 --- a/chrome/browser/sessions/session_restore.h +++ b/chrome/browser/sessions/session_restore.h @@ -98,6 +98,10 @@ class SessionRestore { static CallbackSubscription RegisterOnSessionRestoredCallback( const base::Callback<void(int)>& callback); + // Returns true if smart session restore is enabled (ie. background tabs are + // sorted before being loaded). + static bool SmartLoadingEnabled(); + private: SessionRestore(); diff --git a/chrome/browser/sessions/session_restore_browsertest.cc b/chrome/browser/sessions/session_restore_browsertest.cc index 5d39701..cea36b2 100644 --- a/chrome/browser/sessions/session_restore_browsertest.cc +++ b/chrome/browser/sessions/session_restore_browsertest.cc @@ -212,6 +212,55 @@ class SessionRestoreTest : public InProcessBrowserTest { const BrowserList* active_browser_list_; }; +// Activates the smart restore behaviour and can track the loading of tabs. +class SmartSessionRestoreTest : public SessionRestoreTest, + public content::NotificationObserver { + public: + SmartSessionRestoreTest() {} + void StartObserving(int num_tabs) { + num_tabs_ = num_tabs; + registrar_.Add(this, content::NOTIFICATION_LOAD_START, + content::NotificationService::AllSources()); + } + void Observe(int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) override { + switch (type) { + case content::NOTIFICATION_LOAD_START: { + content::NavigationController* controller = + content::Source<content::NavigationController>(source).ptr(); + web_contents_.push_back(controller->GetWebContents()); + if (web_contents_.size() == static_cast<size_t>(num_tabs_)) + message_loop_runner_->Quit(); + break; + } + } + } + const std::vector<content::WebContents*>& web_contents() const { + return web_contents_; + } + + void WaitForAllTabsToStartLoading() { + message_loop_runner_ = new content::MessageLoopRunner; + message_loop_runner_->Run(); + } + + protected: + void SetUpCommandLine(base::CommandLine* command_line) override { + base::CommandLine::ForCurrentProcess()->AppendSwitchASCII( + switches::kForceFieldTrials, "SessionRestoreBackgroundLoading/Smart/"); + } + + private: + content::NotificationRegistrar registrar_; + // Ordered by load start order. + std::vector<content::WebContents*> web_contents_; + scoped_refptr<content::MessageLoopRunner> message_loop_runner_; + int num_tabs_; + + DISALLOW_COPY_AND_ASSIGN(SmartSessionRestoreTest); +}; + // Verifies that restored tabs have a root window. This is important // otherwise the wrong information is communicated to the renderer. // (http://crbug.com/342672). @@ -1280,3 +1329,76 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, SessionStorageAfterTabReplace) { ASSERT_EQ(1u, active_browser_list_->size()); EXPECT_EQ(1, new_browser->tab_strip_model()->count()); } + +IN_PROC_BROWSER_TEST_F(SmartSessionRestoreTest, CorrectLoadingOrder) { + ASSERT_TRUE(SessionRestore::SmartLoadingEnabled()); + + const int NumTabs = 6; + + // Start observing the loading of tabs, to make sure the order is correct. + StartObserving(NumTabs); + + struct TabInfo { + GURL url; + bool pinned; + int expected_load_order; + }; + + TabInfo tab_info[NumTabs] = { + // This will be the foreground tab and will always load first. + {GURL("http://google.com/1"), false, 1}, + {GURL("http://google.com/2"), false, 3}, + // Internal page, should load last. + {GURL(chrome::kChromeUINewTabURL), false, 6}, + {GURL("http://google.com/4"), false, 4}, + {GURL("http://google.com/5"), true, 2}, // Pinned, should load second. + {GURL("http://google.com/6"), false, 5}, + }; + + // Set up the restore data. + std::vector<const sessions::SessionWindow*> session; + sessions::SessionWindow window; + sessions::SessionTab tab[NumTabs]; + + for (int i = 0; i < NumTabs; i++) { + SerializedNavigationEntry nav = + SerializedNavigationEntryTestHelper::CreateNavigation( + tab_info[i].url.spec(), tab_info[i].url.spec().c_str()); + sync_pb::SessionTab sync_data; + sync_data.set_tab_visual_index(0); + sync_data.set_current_navigation_index(0); + sync_data.add_navigation()->CopyFrom(nav.ToSyncData()); + sync_data.set_pinned(tab_info[i].pinned); + tab[i].SetFromSyncData(sync_data, base::Time::Now()); + window.tabs.push_back(tab + i); + } + + session.push_back(&window); + Profile* profile = browser()->profile(); + ui_test_utils::BrowserAddedObserver window_observer; + std::vector<Browser*> browsers = SessionRestore::RestoreForeignSessionWindows( + profile, browser()->host_desktop_type(), session.begin(), session.end()); + + ASSERT_EQ(1u, browsers.size()); + ASSERT_TRUE(browsers[0]); + ASSERT_EQ(NumTabs, browsers[0]->tab_strip_model()->count()); + + WaitForAllTabsToStartLoading(); + + ASSERT_EQ(static_cast<size_t>(NumTabs), web_contents().size()); + + // Make sure that contents are loaded in the correct order, ie. each tab rank + // is higher that its preceding one. + std::map<GURL, int> ranks; + for (auto t : tab_info) + ranks[t.url] = t.expected_load_order; + for (size_t i = 1; i < web_contents().size(); i++) { + int current_rank = ranks[web_contents()[i]->GetLastCommittedURL()]; + int previous_rank = ranks[web_contents()[i - 1]->GetLastCommittedURL()]; + ASSERT_LT(previous_rank, current_rank); + } + + // The SessionWindow destructor deletes the tabs, so we have to clear them + // here to avoid a crash. + window.tabs.clear(); +} diff --git a/chrome/browser/sessions/session_restore_delegate.cc b/chrome/browser/sessions/session_restore_delegate.cc index 2c458ec..5f016e0 100644 --- a/chrome/browser/sessions/session_restore_delegate.cc +++ b/chrome/browser/sessions/session_restore_delegate.cc @@ -7,8 +7,59 @@ #include "base/metrics/field_trial.h" #include "chrome/browser/sessions/session_restore_stats_collector.h" #include "chrome/browser/sessions/tab_loader.h" +#include "chrome/common/url_constants.h" #include "components/favicon/content/content_favicon_driver.h" +namespace { + +bool IsInternalPage(const GURL& url) { + // There are many chrome:// UI URLs, but only look for the ones that users + // are likely to have open. Most of the benefit is from the NTP URL. + const char* const kReloadableUrlPrefixes[] = { + chrome::kChromeUIDownloadsURL, + chrome::kChromeUIHistoryURL, + chrome::kChromeUINewTabURL, + chrome::kChromeUISettingsURL, + }; + // Prefix-match against the table above. Use strncmp to avoid allocating + // memory to convert the URL prefix constants into std::strings. + for (size_t i = 0; i < arraysize(kReloadableUrlPrefixes); ++i) { + if (!strncmp(url.spec().c_str(), kReloadableUrlPrefixes[i], + strlen(kReloadableUrlPrefixes[i]))) + return true; + } + return false; +} + +} // namespace + +SessionRestoreDelegate::RestoredTab::RestoredTab(content::WebContents* contents, + bool is_active, + bool is_app, + bool is_pinned) + : contents_(contents), + is_active_(is_active), + is_app_(is_app), + is_internal_page_(IsInternalPage(contents->GetLastCommittedURL())), + is_pinned_(is_pinned) { +} + +bool SessionRestoreDelegate::RestoredTab::operator<( + const RestoredTab& right) const { + // Tab with internal web UI like NTP or Settings are good choices to + // defer loading. + if (is_internal_page_ != right.is_internal_page_) + return !is_internal_page_; + // Pinned tabs should be loaded first. + if (is_pinned_ != right.is_pinned_) + return is_pinned_; + // Apps should be loaded before normal tabs. + if (is_app_ != right.is_app_) + return is_app_; + // TODO(georgesak): Add criterion based on recency. + return false; +} + // static void SessionRestoreDelegate::RestoreTabs( const std::vector<RestoredTab>& tabs, diff --git a/chrome/browser/sessions/session_restore_delegate.h b/chrome/browser/sessions/session_restore_delegate.h index 019ddac..63e40ee 100644 --- a/chrome/browser/sessions/session_restore_delegate.h +++ b/chrome/browser/sessions/session_restore_delegate.h @@ -17,9 +17,27 @@ class WebContents; // stats collector. class SessionRestoreDelegate { public: - struct RestoredTab { - content::WebContents* contents; - bool is_active; + class RestoredTab { + public: + RestoredTab(content::WebContents* contents, + bool is_active, + bool is_app, + bool is_pinned); + + bool operator<(const RestoredTab& right) const; + + content::WebContents* contents() const { return contents_; } + bool is_active() const { return is_active_; } + bool is_app() const { return is_app_; } + bool is_internal_page() const { return is_internal_page_; } + bool is_pinned() const { return is_pinned_; } + + private: + content::WebContents* contents_; + bool is_active_; + bool is_app_; // Browser window is an app. + bool is_internal_page_; // Internal web UI page, like NTP or Settings. + bool is_pinned_; }; static void RestoreTabs(const std::vector<RestoredTab>& tabs, diff --git a/chrome/browser/sessions/session_restore_stats_collector.cc b/chrome/browser/sessions/session_restore_stats_collector.cc index 30159f9..f593b6e 100644 --- a/chrome/browser/sessions/session_restore_stats_collector.cc +++ b/chrome/browser/sessions/session_restore_stats_collector.cc @@ -165,10 +165,10 @@ void SessionRestoreStatsCollector::AddTabs( const std::vector<SessionRestoreDelegate::RestoredTab>& tabs) { tab_count_ += tabs.size(); for (auto& tab : tabs) { - RegisterForNotifications(&tab.contents->GetController()); - if (tab.is_active) { + RegisterForNotifications(&tab.contents()->GetController()); + if (tab.is_active()) { RenderWidgetHost* render_widget_host = - GetRenderWidgetHost(&tab.contents->GetController()); + GetRenderWidgetHost(&tab.contents()->GetController()); render_widget_hosts_loading_.insert(render_widget_host); } } diff --git a/chrome/browser/sessions/tab_loader.cc b/chrome/browser/sessions/tab_loader.cc index df77a8c..28b80ef 100644 --- a/chrome/browser/sessions/tab_loader.cc +++ b/chrome/browser/sessions/tab_loader.cc @@ -85,11 +85,11 @@ void TabLoader::StartLoading(const std::vector<RestoredTab>& tabs) { // Add the tabs to the list of tabs loading/to load and register them for // notifications. for (auto& restored_tab : tabs) { - if (!restored_tab.is_active) - tabs_to_load_.push_back(&restored_tab.contents->GetController()); + if (!restored_tab.is_active()) + tabs_to_load_.push_back(&restored_tab.contents()->GetController()); else - tabs_loading_.insert(&restored_tab.contents->GetController()); - RegisterForNotifications(&restored_tab.contents->GetController()); + tabs_loading_.insert(&restored_tab.contents()->GetController()); + RegisterForNotifications(&restored_tab.contents()->GetController()); } // When multiple profiles are using the same TabLoader, another profile might // already have started loading. In that case, the tabs scheduled for loading |