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

11 files changed, 39 insertions, 20 deletions

diff --git a/android_webview/native/aw_dev_tools_server.cc b/android_webview/native/aw_dev_tools_server.cc
index 0bedb7a..e1cdc72 100644
--- a/android_webview/native/aw_dev_tools_server.cc
+++ b/android_webview/native/aw_dev_tools_server.cc
@@ -72,7 +72,7 @@ Target::Target(WebContents* web_contents) {
   description_ = GetViewDescription(web_contents);
   title_ = base::UTF16ToUTF8(web_contents->GetTitle());
   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
diff --git a/chrome/browser/android/dev_tools_server.cc b/chrome/browser/android/dev_tools_server.cc
index 038d822..0667d5a 100644
--- a/chrome/browser/android/dev_tools_server.cc
+++ b/chrome/browser/android/dev_tools_server.cc
@@ -86,7 +86,7 @@ class TargetBase : public content::DevToolsTarget {
       : title_(base::UTF16ToUTF8(web_contents->GetTitle())),
         url_(web_contents->GetURL()),
         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)
diff --git a/chrome/browser/chromeos/memory/oom_priority_manager.cc b/chrome/browser/chromeos/memory/oom_priority_manager.cc
index bb9a61e..c5018c2 100644
--- a/chrome/browser/chromeos/memory/oom_priority_manager.cc
+++ b/chrome/browser/chromeos/memory/oom_priority_manager.cc
@@ -449,8 +449,8 @@ bool OomPriorityManager::CompareTabStats(TabStats first,
   // for beforeUnload handlers, which are likely to present a dialog asking
   // if the user wants to discard state.  crbug.com/123049
 
-  // Being more recently selected is more important.
-  return first.last_selected > second.last_selected;
+  // Being more recently active is more important.
+  return first.last_active > second.last_active;
 }
 
 void OomPriorityManager::AdjustFocusedTabScoreOnFileThread() {
@@ -578,7 +578,7 @@ OomPriorityManager::TabStatsList OomPriorityManager::GetTabStatsOnUIThread() {
         stats.is_pinned = model->IsTabPinned(i);
         stats.is_selected = browser_active && model->IsTabSelected(i);
         stats.is_discarded = model->IsTabDiscarded(i);
-        stats.last_selected = contents->GetLastSelectedTime();
+        stats.last_active = contents->GetLastActiveTime();
         stats.renderer_handle = contents->GetRenderProcessHost()->GetHandle();
         stats.title = contents->GetTitle();
         stats.tab_contents_id = IdFromWebContents(contents);
diff --git a/chrome/browser/chromeos/memory/oom_priority_manager.h b/chrome/browser/chromeos/memory/oom_priority_manager.h
index 3df538e..6fe6e04 100644
--- a/chrome/browser/chromeos/memory/oom_priority_manager.h
+++ b/chrome/browser/chromeos/memory/oom_priority_manager.h
@@ -78,7 +78,7 @@ class OomPriorityManager : public content::NotificationObserver,
     bool is_pinned;
     bool is_selected;  // selected in the currently active browser window
     bool is_discarded;
-    base::TimeTicks last_selected;
+    base::TimeTicks last_active;
     base::ProcessHandle renderer_handle;
     base::string16 title;
     int64 tab_contents_id;  // unique ID per WebContents
diff --git a/chrome/browser/chromeos/memory/oom_priority_manager_unittest.cc b/chrome/browser/chromeos/memory/oom_priority_manager_unittest.cc
index c227ac2..3865f70 100644
--- a/chrome/browser/chromeos/memory/oom_priority_manager_unittest.cc
+++ b/chrome/browser/chromeos/memory/oom_priority_manager_unittest.cc
@@ -62,21 +62,21 @@ TEST_F(OomPriorityManagerTest, Comparator) {
 
   {
     OomPriorityManager::TabStats stats;
-    stats.last_selected = now - base::TimeDelta::FromSeconds(10);
+    stats.last_active = now - base::TimeDelta::FromSeconds(10);
     stats.renderer_handle = kRecent;
     test_list.push_back(stats);
   }
 
   {
     OomPriorityManager::TabStats stats;
-    stats.last_selected = now - base::TimeDelta::FromMinutes(15);
+    stats.last_active = now - base::TimeDelta::FromMinutes(15);
     stats.renderer_handle = kOld;
     test_list.push_back(stats);
   }
 
   {
     OomPriorityManager::TabStats stats;
-    stats.last_selected = now - base::TimeDelta::FromDays(365);
+    stats.last_active = now - base::TimeDelta::FromDays(365);
     stats.renderer_handle = kReallyOld;
     test_list.push_back(stats);
   }
@@ -84,7 +84,7 @@ TEST_F(OomPriorityManagerTest, Comparator) {
   {
     OomPriorityManager::TabStats stats;
     stats.is_pinned = true;
-    stats.last_selected = now - base::TimeDelta::FromDays(365);
+    stats.last_active = now - base::TimeDelta::FromDays(365);
     stats.renderer_handle = kOldButPinned;
     test_list.push_back(stats);
   }
diff --git a/chrome/browser/devtools/devtools_target_impl.cc b/chrome/browser/devtools/devtools_target_impl.cc
index 767cad0..14d28b0 100644
--- a/chrome/browser/devtools/devtools_target_impl.cc
+++ b/chrome/browser/devtools/devtools_target_impl.cc
@@ -70,7 +70,7 @@ RenderViewHostTarget::RenderViewHostTarget(RenderViewHost* rvh, bool is_tab) {
   content::NavigationEntry* entry = controller.GetActiveEntry();
   if (entry != NULL && entry->GetURL().is_valid())
     favicon_url_ = entry->GetFavicon().url;
-  last_activity_time_ = web_contents->GetLastSelectedTime();
+  last_activity_time_ = web_contents->GetLastActiveTime();
 
   if (is_tab) {
     type_ = kTargetTypePage;
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 02838bc..63aebf9 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -308,6 +308,7 @@ WebContentsImpl::WebContentsImpl(
       notify_disconnection_(false),
       dialog_manager_(NULL),
       is_showing_before_unload_dialog_(false),
+      last_active_time_(base::TimeTicks::Now()),
       closed_by_user_gesture_(false),
       minimum_zoom_percent_(static_cast<int>(kMinimumZoomFactor * 100)),
       maximum_zoom_percent_(static_cast<int>(kMaximumZoomFactor * 100)),
@@ -901,8 +902,8 @@ void WebContentsImpl::NotifyNavigationStateChanged(unsigned changed_flags) {
     delegate_->NavigationStateChanged(this, changed_flags);
 }
 
-base::TimeTicks WebContentsImpl::GetLastSelectedTime() const {
-  return last_selected_time_;
+base::TimeTicks WebContentsImpl::GetLastActiveTime() const {
+  return last_active_time_;
 }
 
 void WebContentsImpl::WasShown() {
@@ -916,7 +917,7 @@ void WebContentsImpl::WasShown() {
 #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
   // one to make sure it's up to date.
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index 37679f1..c950266 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -233,7 +233,7 @@ class CONTENT_EXPORT WebContentsImpl
   virtual base::TerminationStatus GetCrashedStatus() const OVERRIDE;
   virtual bool IsBeingDestroyed() const 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 WasHidden() OVERRIDE;
   virtual bool NeedToFireBeforeUnload() OVERRIDE;
@@ -626,6 +626,7 @@ class CONTENT_EXPORT WebContentsImpl
                            CrossSiteCantPreemptAfterUnload);
   FRIEND_TEST_ALL_PREFIXES(WebContentsImplTest, PendingContents);
   FRIEND_TEST_ALL_PREFIXES(WebContentsImplTest, FrameTreeShape);
+  FRIEND_TEST_ALL_PREFIXES(WebContentsImplTest, GetLastActiveTime);
   FRIEND_TEST_ALL_PREFIXES(FormStructureBrowserTest, HTMLFiles);
   FRIEND_TEST_ALL_PREFIXES(NavigationControllerTest, HistoryNavigate);
   FRIEND_TEST_ALL_PREFIXES(RenderFrameHostManagerTest, PageDoesBackAndReload);
@@ -988,8 +989,9 @@ class CONTENT_EXPORT WebContentsImpl
   // Settings that get passed to the renderer process.
   RendererPreferences renderer_preferences_;
 
-  // The time that this tab was last selected.
-  base::TimeTicks last_selected_time_;
+  // The time that this WebContents was last made active. The initial value is
+  // the WebContents creation time.
+  base::TimeTicks last_active_time_;
 
   // See description above setter.
   bool closed_by_user_gesture_;
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index 846a861..d7834b0 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -2216,4 +2216,19 @@ TEST_F(WebContentsImplTest, CapturerOverridesPreferredSize) {
   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
diff --git a/content/public/browser/web_contents.h b/content/public/browser/web_contents.h
index db3fe35..ef69439 100644
--- a/content/public/browser/web_contents.h
+++ b/content/public/browser/web_contents.h
@@ -314,8 +314,9 @@ class WebContents : public PageNavigator,
   // change. See InvalidateType enum.
   virtual void NotifyNavigationStateChanged(unsigned changed_flags) = 0;
 
-  // Get the last time that the WebContents was made visible with WasShown()
-  virtual base::TimeTicks GetLastSelectedTime() const = 0;
+  // Get the last time that the WebContents was made active (either when it was
+  // created or shown with WasShown()).
+  virtual base::TimeTicks GetLastActiveTime() const = 0;
 
   // Invoked when the WebContents becomes shown/hidden.
   virtual void WasShown() = 0;
diff --git a/content/shell/browser/shell_devtools_delegate.cc b/content/shell/browser/shell_devtools_delegate.cc
index b583097..fe7f9a9 100644
--- a/content/shell/browser/shell_devtools_delegate.cc
+++ b/content/shell/browser/shell_devtools_delegate.cc
@@ -113,7 +113,7 @@ Target::Target(WebContents* web_contents) {
   content::NavigationEntry* entry = controller.GetActiveEntry();
   if (entry != NULL && entry->GetURL().is_valid())
     favicon_url_ = entry->GetFavicon().url;
-  last_activity_time_ = web_contents->GetLastSelectedTime();
+  last_activity_time_ = web_contents->GetLastActiveTime();
 }
 
 bool Target::Activate() const {