Revert of Change WebContents::last_active_time_ to Time instead of Ti…

…meticks. (patchset #1 id:140001 of https://codereview.chromium.org/1140083004/) Reason for revert: Reverting for georgesak@ (his committer access hasn't propagated yet it looks like). After discussion with sky@, decided to keep TimeTicks. Original issue's description: > Change WebContents::last_active_time_ to Time instead of Timeticks. > > For context, last_active_time_ is going to be used by session restore to order the loading of background tabs using MRU. In order for this to be robust, last_active_time_ must be saved and restore between sessions. Timeticks cannot be reliably restored as it's dependent on the current OS session. MRU code for session restore is being implemented in https://codereview.chromium.org/1131373003 > > Notes: > - In dev tools, replaced "activity" with "active" for consistency > - In OomPriorityManagerTest.Comparator, initialized last_active_time for all tabs, as this is necessary with Time (a default TimeTicks can go back in time, not the case with Time). > > BUG=472772 > > Committed: https://crrev.com/041ef9c96d9a5b6f206a24385b0e6e4b3dbf9f20 > Cr-Commit-Position: refs/heads/master@{#330029} [email protected],[email protected],[email protected] NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=472772 Review URL: https://codereview.chromium.org/1145233002 Cr-Commit-Position: refs/heads/master@{#330778}
huningxin · May 20, 2015 · 0dccfef · 0dccfef
1 parent a06788a
commit 0dccfef
Show file tree

Hide file tree

Showing 11 changed files with 23 additions and 25 deletions.
diff --git a/chrome/browser/android/dev_tools_discovery_provider_android.cc b/chrome/browser/android/dev_tools_discovery_provider_android.cc
@@ -65,7 +65,9 @@ class TabDescriptor : public devtools_discovery::DevToolsTargetDescriptor {
     return favicon_url_;
   }
 
-  base::Time GetLastActiveTime() const override { return last_active_time_; }
+  base::TimeTicks GetLastActivityTime() const override {
+    return last_activity_time_;
+  }
 
   std::string GetId() const override {
     return base::IntToString(tab_id_);
@@ -132,7 +134,8 @@ class TabDescriptor : public devtools_discovery::DevToolsTargetDescriptor {
         title_(base::UTF16ToUTF8(web_contents->GetTitle())),
         url_(web_contents->GetURL()),
         favicon_url_(CalculateFaviconURL()),
-        last_active_time_(web_contents->GetLastActiveTime()) {}
+        last_activity_time_(web_contents->GetLastActiveTime()) {
+  }
 
   TabDescriptor(int tab_id, const base::string16& title, const GURL& url)
       : tab_id_(tab_id),
@@ -188,7 +191,7 @@ class TabDescriptor : public devtools_discovery::DevToolsTargetDescriptor {
   const std::string title_;
   const GURL url_;
   const GURL favicon_url_;
-  const base::Time last_active_time_;
+  const base::TimeTicks last_activity_time_;
 
   DISALLOW_COPY_AND_ASSIGN(TabDescriptor);
 };

diff --git a/chrome/browser/chromeos/memory/oom_priority_manager.h b/chrome/browser/chromeos/memory/oom_priority_manager.h
@@ -79,7 +79,7 @@ class OomPriorityManager : public content::NotificationObserver {
     bool is_pinned;
     bool is_selected;  // selected in the currently active browser window
     bool is_discarded;
-    base::Time last_active;
+    base::TimeTicks last_active;
     base::ProcessHandle renderer_handle;
     int child_process_host_id;
     base::string16 title;

diff --git a/chrome/browser/chromeos/memory/oom_priority_manager_unittest.cc b/chrome/browser/chromeos/memory/oom_priority_manager_unittest.cc
@@ -35,13 +35,12 @@ enum TestIndicies {
 // desired order.
 TEST_F(OomPriorityManagerTest, Comparator) {
   chromeos::OomPriorityManager::TabStatsList test_list;
-  const base::Time now = base::Time::Now();
+  const base::TimeTicks now = base::TimeTicks::Now();
 
   // Add kSelected last to verify we are sorting the array.
 
   {
     OomPriorityManager::TabStats stats;
-    stats.last_active = now;
     stats.is_pinned = true;
     stats.renderer_handle = kPinned;
     stats.child_process_host_id = kPinned;
@@ -50,7 +49,6 @@ TEST_F(OomPriorityManagerTest, Comparator) {
 
   {
     OomPriorityManager::TabStats stats;
-    stats.last_active = now;
     stats.is_app = true;
     stats.renderer_handle = kApp;
     stats.child_process_host_id = kApp;
@@ -59,7 +57,6 @@ TEST_F(OomPriorityManagerTest, Comparator) {
 
   {
     OomPriorityManager::TabStats stats;
-    stats.last_active = now;
     stats.is_playing_audio = true;
     stats.renderer_handle = kPlayingAudio;
     stats.child_process_host_id = kPlayingAudio;
@@ -101,7 +98,6 @@ TEST_F(OomPriorityManagerTest, Comparator) {
 
   {
     OomPriorityManager::TabStats stats;
-    stats.last_active = now;
     stats.is_reloadable_ui = true;
     stats.renderer_handle = kReloadableUI;
     stats.child_process_host_id = kReloadableUI;
@@ -112,7 +108,6 @@ TEST_F(OomPriorityManagerTest, Comparator) {
   // we are actually sorting the array.
   {
     OomPriorityManager::TabStats stats;
-    stats.last_active = now;
     stats.is_selected = true;
     stats.renderer_handle = kSelected;
     stats.child_process_host_id = kSelected;

diff --git a/components/devtools_discovery/basic_target_descriptor.cc b/components/devtools_discovery/basic_target_descriptor.cc
@@ -47,7 +47,7 @@ BasicTargetDescriptor::BasicTargetDescriptor(
     content::NavigationEntry* entry = controller.GetActiveEntry();
     if (entry != NULL && entry->GetURL().is_valid())
       favicon_url_ = entry->GetFavicon().url;
-    last_active_time_ = web_contents->GetLastActiveTime();
+    last_activity_time_ = web_contents->GetLastActiveTime();
   }
 }
 
@@ -82,8 +82,8 @@ GURL BasicTargetDescriptor::GetFaviconURL() const {
   return favicon_url_;
 }
 
-base::Time BasicTargetDescriptor::GetLastActiveTime() const {
-  return last_active_time_;
+base::TimeTicks BasicTargetDescriptor::GetLastActivityTime() const {
+  return last_activity_time_;
 }
 
 bool BasicTargetDescriptor::IsAttached() const {

diff --git a/components/devtools_discovery/basic_target_descriptor.h b/components/devtools_discovery/basic_target_descriptor.h
@@ -28,7 +28,7 @@ class BasicTargetDescriptor : public DevToolsTargetDescriptor {
   std::string GetDescription() const override;
   GURL GetURL() const override;
   GURL GetFaviconURL() const override;
-  base::Time GetLastActiveTime() const override;
+  base::TimeTicks GetLastActivityTime() const override;
   bool IsAttached() const override;
   scoped_refptr<content::DevToolsAgentHost> GetAgentHost() const override;
   bool Activate() const override;
@@ -50,7 +50,7 @@ class BasicTargetDescriptor : public DevToolsTargetDescriptor {
   std::string description_;
   GURL url_;
   GURL favicon_url_;
-  base::Time last_active_time_;
+  base::TimeTicks last_activity_time_;
 };
 
 }  // namespace devtools_discovery

diff --git a/components/devtools_discovery/devtools_target_descriptor.h b/components/devtools_discovery/devtools_target_descriptor.h
@@ -50,7 +50,7 @@ class DevToolsTargetDescriptor {
   virtual GURL GetFaviconURL() const = 0;
 
   // Returns the time when the target was last active.
-  virtual base::Time GetLastActiveTime() const = 0;
+  virtual base::TimeTicks GetLastActivityTime() const = 0;
 
   // Returns true if the debugger is attached to the target.
   virtual bool IsAttached() const = 0;

diff --git a/components/devtools_http_handler/devtools_http_handler.cc b/components/devtools_http_handler/devtools_http_handler.cc
@@ -325,7 +325,7 @@ class DevToolsAgentHostClientImpl : public DevToolsAgentHostClient {
 
 static bool TimeComparator(const DevToolsTargetDescriptor* desc1,
                            const DevToolsTargetDescriptor* desc2) {
-  return desc1->GetLastActiveTime() > desc2->GetLastActiveTime();
+  return desc1->GetLastActivityTime() > desc2->GetLastActivityTime();
 }
 
 // DevToolsHttpHandler::ServerSocketFactory ----------------------------------

diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
@@ -321,7 +321,7 @@ WebContentsImpl::WebContentsImpl(BrowserContext* browser_context,
       notify_disconnection_(false),
       dialog_manager_(NULL),
       is_showing_before_unload_dialog_(false),
-      last_active_time_(base::Time::Now()),
+      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)),
@@ -1048,7 +1048,7 @@ void WebContentsImpl::NotifyNavigationStateChanged(
     delegate_->NavigationStateChanged(this, changed_flags);
 }
 
-base::Time WebContentsImpl::GetLastActiveTime() const {
+base::TimeTicks WebContentsImpl::GetLastActiveTime() const {
   return last_active_time_;
 }
 
@@ -1064,7 +1064,7 @@ void WebContentsImpl::WasShown() {
     }
   }
 
-  last_active_time_ = base::Time::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
@@ -271,7 +271,7 @@ class CONTENT_EXPORT WebContentsImpl
   base::TerminationStatus GetCrashedStatus() const override;
   bool IsBeingDestroyed() const override;
   void NotifyNavigationStateChanged(InvalidateTypes changed_flags) override;
-  base::Time GetLastActiveTime() const override;
+  base::TimeTicks GetLastActiveTime() const override;
   void WasShown() override;
   void WasHidden() override;
   bool NeedToFireBeforeUnload() override;
@@ -1136,7 +1136,7 @@ class CONTENT_EXPORT WebContentsImpl
 
   // The time that this WebContents was last made active. The initial value is
   // the WebContents creation time.
-  base::Time last_active_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
@@ -2570,7 +2570,7 @@ TEST_F(WebContentsImplTest, GetLastActiveTime) {
   EXPECT_FALSE(contents()->GetLastActiveTime().is_null());
 
   // Reset the last active time to a known-bad value.
-  contents()->last_active_time_ = base::Time();
+  contents()->last_active_time_ = base::TimeTicks();
   ASSERT_TRUE(contents()->GetLastActiveTime().is_null());
 
   // Simulate activating the WebContents. The active time should update.

diff --git a/content/public/browser/web_contents.h b/content/public/browser/web_contents.h
@@ -32,7 +32,7 @@
 
 namespace base {
 class DictionaryValue;
-class Time;
+class TimeTicks;
 }
 
 namespace blink {
@@ -353,7 +353,7 @@ class WebContents : public PageNavigator,
 
   // Get the last time that the WebContents was made active (either when it was
   // created or shown with WasShown()).
-  virtual base::Time GetLastActiveTime() const = 0;
+  virtual base::TimeTicks GetLastActiveTime() const = 0;
 
   // Invoked when the WebContents becomes shown/hidden.
   virtual void WasShown() = 0;