Avoid discarding freshly created background tabs

The Chrome OS out-of-memory tab discarder uses the "last selected" time as a signal for which tab to discard. This starts at 0 for freshly created background tabs, like those created with a control-click. The user probably doesn't want to discard those tabs, since they just opened them.

Since the only other user of WebContents::GetLastSelectedTime() is devtools, which uses it for a similar kind of sorting based on activity, rename the method to GetLastActiveTime() and start the value with the tab creation time.

BUG=338735
TEST=content_unittests WebContentsImpl.GetLastActiveTime
TBR=torne@chromium.org for rename touching android_webview

Review URL: https://codereview.chromium.org/138913021

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@251154 0039d316-1c4b-4281-b951-d872f2087c98
parent 0b64c917
...@@ -72,7 +72,7 @@ Target::Target(WebContents* web_contents) { ...@@ -72,7 +72,7 @@ Target::Target(WebContents* web_contents) {
description_ = GetViewDescription(web_contents); description_ = GetViewDescription(web_contents);
title_ = base::UTF16ToUTF8(web_contents->GetTitle()); title_ = base::UTF16ToUTF8(web_contents->GetTitle());
url_ = web_contents->GetURL(); url_ = web_contents->GetURL();
last_activity_time_ = web_contents->GetLastSelectedTime(); last_activity_time_ = web_contents->GetLastActiveTime();
} }
// Delegate implementation for the devtools http handler for WebView. A new // Delegate implementation for the devtools http handler for WebView. A new
......
...@@ -86,7 +86,7 @@ class TargetBase : public content::DevToolsTarget { ...@@ -86,7 +86,7 @@ class TargetBase : public content::DevToolsTarget {
: title_(base::UTF16ToUTF8(web_contents->GetTitle())), : title_(base::UTF16ToUTF8(web_contents->GetTitle())),
url_(web_contents->GetURL()), url_(web_contents->GetURL()),
favicon_url_(GetFaviconURL(web_contents)), favicon_url_(GetFaviconURL(web_contents)),
last_activity_time_(web_contents->GetLastSelectedTime()) { last_activity_time_(web_contents->GetLastActiveTime()) {
} }
TargetBase(const base::string16& title, const GURL& url) TargetBase(const base::string16& title, const GURL& url)
......
...@@ -449,8 +449,8 @@ bool OomPriorityManager::CompareTabStats(TabStats first, ...@@ -449,8 +449,8 @@ bool OomPriorityManager::CompareTabStats(TabStats first,
// for beforeUnload handlers, which are likely to present a dialog asking // for beforeUnload handlers, which are likely to present a dialog asking
// if the user wants to discard state. crbug.com/123049 // if the user wants to discard state. crbug.com/123049
// Being more recently selected is more important. // Being more recently active is more important.
return first.last_selected > second.last_selected; return first.last_active > second.last_active;
} }
void OomPriorityManager::AdjustFocusedTabScoreOnFileThread() { void OomPriorityManager::AdjustFocusedTabScoreOnFileThread() {
...@@ -578,7 +578,7 @@ OomPriorityManager::TabStatsList OomPriorityManager::GetTabStatsOnUIThread() { ...@@ -578,7 +578,7 @@ OomPriorityManager::TabStatsList OomPriorityManager::GetTabStatsOnUIThread() {
stats.is_pinned = model->IsTabPinned(i); stats.is_pinned = model->IsTabPinned(i);
stats.is_selected = browser_active && model->IsTabSelected(i); stats.is_selected = browser_active && model->IsTabSelected(i);
stats.is_discarded = model->IsTabDiscarded(i); stats.is_discarded = model->IsTabDiscarded(i);
stats.last_selected = contents->GetLastSelectedTime(); stats.last_active = contents->GetLastActiveTime();
stats.renderer_handle = contents->GetRenderProcessHost()->GetHandle(); stats.renderer_handle = contents->GetRenderProcessHost()->GetHandle();
stats.title = contents->GetTitle(); stats.title = contents->GetTitle();
stats.tab_contents_id = IdFromWebContents(contents); stats.tab_contents_id = IdFromWebContents(contents);
......
...@@ -78,7 +78,7 @@ class OomPriorityManager : public content::NotificationObserver, ...@@ -78,7 +78,7 @@ class OomPriorityManager : public content::NotificationObserver,
bool is_pinned; bool is_pinned;
bool is_selected; // selected in the currently active browser window bool is_selected; // selected in the currently active browser window
bool is_discarded; bool is_discarded;
base::TimeTicks last_selected; base::TimeTicks last_active;
base::ProcessHandle renderer_handle; base::ProcessHandle renderer_handle;
base::string16 title; base::string16 title;
int64 tab_contents_id; // unique ID per WebContents int64 tab_contents_id; // unique ID per WebContents
......
...@@ -62,21 +62,21 @@ TEST_F(OomPriorityManagerTest, Comparator) { ...@@ -62,21 +62,21 @@ TEST_F(OomPriorityManagerTest, Comparator) {
{ {
OomPriorityManager::TabStats stats; OomPriorityManager::TabStats stats;
stats.last_selected = now - base::TimeDelta::FromSeconds(10); stats.last_active = now - base::TimeDelta::FromSeconds(10);
stats.renderer_handle = kRecent; stats.renderer_handle = kRecent;
test_list.push_back(stats); test_list.push_back(stats);
} }
{ {
OomPriorityManager::TabStats stats; OomPriorityManager::TabStats stats;
stats.last_selected = now - base::TimeDelta::FromMinutes(15); stats.last_active = now - base::TimeDelta::FromMinutes(15);
stats.renderer_handle = kOld; stats.renderer_handle = kOld;
test_list.push_back(stats); test_list.push_back(stats);
} }
{ {
OomPriorityManager::TabStats stats; OomPriorityManager::TabStats stats;
stats.last_selected = now - base::TimeDelta::FromDays(365); stats.last_active = now - base::TimeDelta::FromDays(365);
stats.renderer_handle = kReallyOld; stats.renderer_handle = kReallyOld;
test_list.push_back(stats); test_list.push_back(stats);
} }
...@@ -84,7 +84,7 @@ TEST_F(OomPriorityManagerTest, Comparator) { ...@@ -84,7 +84,7 @@ TEST_F(OomPriorityManagerTest, Comparator) {
{ {
OomPriorityManager::TabStats stats; OomPriorityManager::TabStats stats;
stats.is_pinned = true; stats.is_pinned = true;
stats.last_selected = now - base::TimeDelta::FromDays(365); stats.last_active = now - base::TimeDelta::FromDays(365);
stats.renderer_handle = kOldButPinned; stats.renderer_handle = kOldButPinned;
test_list.push_back(stats); test_list.push_back(stats);
} }
......
...@@ -70,7 +70,7 @@ RenderViewHostTarget::RenderViewHostTarget(RenderViewHost* rvh, bool is_tab) { ...@@ -70,7 +70,7 @@ RenderViewHostTarget::RenderViewHostTarget(RenderViewHost* rvh, bool is_tab) {
content::NavigationEntry* entry = controller.GetActiveEntry(); content::NavigationEntry* entry = controller.GetActiveEntry();
if (entry != NULL && entry->GetURL().is_valid()) if (entry != NULL && entry->GetURL().is_valid())
favicon_url_ = entry->GetFavicon().url; favicon_url_ = entry->GetFavicon().url;
last_activity_time_ = web_contents->GetLastSelectedTime(); last_activity_time_ = web_contents->GetLastActiveTime();
if (is_tab) { if (is_tab) {
type_ = kTargetTypePage; type_ = kTargetTypePage;
......
...@@ -308,6 +308,7 @@ WebContentsImpl::WebContentsImpl( ...@@ -308,6 +308,7 @@ WebContentsImpl::WebContentsImpl(
notify_disconnection_(false), notify_disconnection_(false),
dialog_manager_(NULL), dialog_manager_(NULL),
is_showing_before_unload_dialog_(false), is_showing_before_unload_dialog_(false),
last_active_time_(base::TimeTicks::Now()),
closed_by_user_gesture_(false), closed_by_user_gesture_(false),
minimum_zoom_percent_(static_cast<int>(kMinimumZoomFactor * 100)), minimum_zoom_percent_(static_cast<int>(kMinimumZoomFactor * 100)),
maximum_zoom_percent_(static_cast<int>(kMaximumZoomFactor * 100)), maximum_zoom_percent_(static_cast<int>(kMaximumZoomFactor * 100)),
...@@ -901,8 +902,8 @@ void WebContentsImpl::NotifyNavigationStateChanged(unsigned changed_flags) { ...@@ -901,8 +902,8 @@ void WebContentsImpl::NotifyNavigationStateChanged(unsigned changed_flags) {
delegate_->NavigationStateChanged(this, changed_flags); delegate_->NavigationStateChanged(this, changed_flags);
} }
base::TimeTicks WebContentsImpl::GetLastSelectedTime() const { base::TimeTicks WebContentsImpl::GetLastActiveTime() const {
return last_selected_time_; return last_active_time_;
} }
void WebContentsImpl::WasShown() { void WebContentsImpl::WasShown() {
...@@ -916,7 +917,7 @@ void WebContentsImpl::WasShown() { ...@@ -916,7 +917,7 @@ void WebContentsImpl::WasShown() {
#endif #endif
} }
last_selected_time_ = base::TimeTicks::Now(); last_active_time_ = base::TimeTicks::Now();
// The resize rect might have changed while this was inactive -- send the new // The resize rect might have changed while this was inactive -- send the new
// one to make sure it's up to date. // one to make sure it's up to date.
......
...@@ -233,7 +233,7 @@ class CONTENT_EXPORT WebContentsImpl ...@@ -233,7 +233,7 @@ class CONTENT_EXPORT WebContentsImpl
virtual base::TerminationStatus GetCrashedStatus() const OVERRIDE; virtual base::TerminationStatus GetCrashedStatus() const OVERRIDE;
virtual bool IsBeingDestroyed() const OVERRIDE; virtual bool IsBeingDestroyed() const OVERRIDE;
virtual void NotifyNavigationStateChanged(unsigned changed_flags) OVERRIDE; virtual void NotifyNavigationStateChanged(unsigned changed_flags) OVERRIDE;
virtual base::TimeTicks GetLastSelectedTime() const OVERRIDE; virtual base::TimeTicks GetLastActiveTime() const OVERRIDE;
virtual void WasShown() OVERRIDE; virtual void WasShown() OVERRIDE;
virtual void WasHidden() OVERRIDE; virtual void WasHidden() OVERRIDE;
virtual bool NeedToFireBeforeUnload() OVERRIDE; virtual bool NeedToFireBeforeUnload() OVERRIDE;
...@@ -626,6 +626,7 @@ class CONTENT_EXPORT WebContentsImpl ...@@ -626,6 +626,7 @@ class CONTENT_EXPORT WebContentsImpl
CrossSiteCantPreemptAfterUnload); CrossSiteCantPreemptAfterUnload);
FRIEND_TEST_ALL_PREFIXES(WebContentsImplTest, PendingContents); FRIEND_TEST_ALL_PREFIXES(WebContentsImplTest, PendingContents);
FRIEND_TEST_ALL_PREFIXES(WebContentsImplTest, FrameTreeShape); FRIEND_TEST_ALL_PREFIXES(WebContentsImplTest, FrameTreeShape);
FRIEND_TEST_ALL_PREFIXES(WebContentsImplTest, GetLastActiveTime);
FRIEND_TEST_ALL_PREFIXES(FormStructureBrowserTest, HTMLFiles); FRIEND_TEST_ALL_PREFIXES(FormStructureBrowserTest, HTMLFiles);
FRIEND_TEST_ALL_PREFIXES(NavigationControllerTest, HistoryNavigate); FRIEND_TEST_ALL_PREFIXES(NavigationControllerTest, HistoryNavigate);
FRIEND_TEST_ALL_PREFIXES(RenderFrameHostManagerTest, PageDoesBackAndReload); FRIEND_TEST_ALL_PREFIXES(RenderFrameHostManagerTest, PageDoesBackAndReload);
...@@ -988,8 +989,9 @@ class CONTENT_EXPORT WebContentsImpl ...@@ -988,8 +989,9 @@ class CONTENT_EXPORT WebContentsImpl
// Settings that get passed to the renderer process. // Settings that get passed to the renderer process.
RendererPreferences renderer_preferences_; RendererPreferences renderer_preferences_;
// The time that this tab was last selected. // The time that this WebContents was last made active. The initial value is
base::TimeTicks last_selected_time_; // the WebContents creation time.
base::TimeTicks last_active_time_;
// See description above setter. // See description above setter.
bool closed_by_user_gesture_; bool closed_by_user_gesture_;
......
...@@ -2216,4 +2216,19 @@ TEST_F(WebContentsImplTest, CapturerOverridesPreferredSize) { ...@@ -2216,4 +2216,19 @@ TEST_F(WebContentsImplTest, CapturerOverridesPreferredSize) {
EXPECT_EQ(original_preferred_size, contents()->GetPreferredSize()); EXPECT_EQ(original_preferred_size, contents()->GetPreferredSize());
} }
// Tests that GetLastActiveTime starts with a real, non-zero time and updates
// on activity.
TEST_F(WebContentsImplTest, GetLastActiveTime) {
// The WebContents starts with a valid creation time.
EXPECT_FALSE(contents()->GetLastActiveTime().is_null());
// Reset the last active time to a known-bad value.
contents()->last_active_time_ = base::TimeTicks();
ASSERT_TRUE(contents()->GetLastActiveTime().is_null());
// Simulate activating the WebContents. The active time should update.
contents()->WasShown();
EXPECT_FALSE(contents()->GetLastActiveTime().is_null());
}
} // namespace content } // namespace content
...@@ -314,8 +314,9 @@ class WebContents : public PageNavigator, ...@@ -314,8 +314,9 @@ class WebContents : public PageNavigator,
// change. See InvalidateType enum. // change. See InvalidateType enum.
virtual void NotifyNavigationStateChanged(unsigned changed_flags) = 0; virtual void NotifyNavigationStateChanged(unsigned changed_flags) = 0;
// Get the last time that the WebContents was made visible with WasShown() // Get the last time that the WebContents was made active (either when it was
virtual base::TimeTicks GetLastSelectedTime() const = 0; // created or shown with WasShown()).
virtual base::TimeTicks GetLastActiveTime() const = 0;
// Invoked when the WebContents becomes shown/hidden. // Invoked when the WebContents becomes shown/hidden.
virtual void WasShown() = 0; virtual void WasShown() = 0;
......
...@@ -113,7 +113,7 @@ Target::Target(WebContents* web_contents) { ...@@ -113,7 +113,7 @@ Target::Target(WebContents* web_contents) {
content::NavigationEntry* entry = controller.GetActiveEntry(); content::NavigationEntry* entry = controller.GetActiveEntry();
if (entry != NULL && entry->GetURL().is_valid()) if (entry != NULL && entry->GetURL().is_valid())
favicon_url_ = entry->GetFavicon().url; favicon_url_ = entry->GetFavicon().url;
last_activity_time_ = web_contents->GetLastSelectedTime(); last_activity_time_ = web_contents->GetLastActiveTime();
} }
bool Target::Activate() const { bool Target::Activate() const {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment