Adds timestamp to session restore for tabs and windows.

BUG=13134

TEST=None

Review URL: http://codereview.chromium.org/155609

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@21095 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 173 insertions, 23 deletions

diff --git a/chrome/browser/sessions/tab_restore_service.cc b/chrome/browser/sessions/tab_restore_service.cc
index fc44204..3eefdf2 100644
--- a/chrome/browser/sessions/tab_restore_service.cc
+++ b/chrome/browser/sessions/tab_restore_service.cc
@@ -62,19 +62,31 @@ namespace {
 
 typedef int32 RestoredEntryPayload;
 
-// Payload used for the start of a window close.
+// Payload used for the start of a window close. This is the old struct that is
+// used for backwards compat when it comes to reading the session files.
 struct WindowPayload {
   SessionID::id_type window_id;
   int32 selected_tab_index;
   int32 num_tabs;
 };
 
-// Payload used for the start of a tab close.
+// Payload used for the start of a tab close. This is the old struct that is
+// used for backwards compat when it comes to reading the session files.
 struct SelectedNavigationInTabPayload {
   SessionID::id_type id;
   int32 index;
 };
 
+// Payload used for the start of a window close.
+struct WindowPayload2 : WindowPayload {
+  int64 timestamp;
+};
+
+// Payload used for the start of a tab close.
+struct SelectedNavigationInTabPayload2 : SelectedNavigationInTabPayload {
+  int64 timestamp;
+};
+
 // Only written if the tab is pinned.
 typedef bool PinnedStatePayload;
 
@@ -97,14 +109,16 @@ void RemoveEntryByID(SessionID::id_type id,
 
 }  // namespace
 
-TabRestoreService::TabRestoreService(Profile* profile)
+TabRestoreService::TabRestoreService(Profile* profile,
+    TabRestoreService::TimeFactory* time_factory)
     : BaseSessionService(BaseSessionService::TAB_RESTORE, profile,
                          FilePath()),
       load_state_(NOT_LOADED),
       restoring_(false),
       reached_max_(false),
       entries_to_write_(0),
-      entries_written_(0) {
+      entries_written_(0),
+      time_factory_(time_factory) {
 }
 
 TabRestoreService::~TabRestoreService() {
@@ -114,6 +128,7 @@ TabRestoreService::~TabRestoreService() {
   FOR_EACH_OBSERVER(Observer, observer_list_, TabRestoreServiceDestroyed(this));
   STLDeleteElements(&entries_);
   STLDeleteElements(&staging_entries_);
+  time_factory_ = NULL;
 }
 
 void TabRestoreService::AddObserver(Observer* observer) {
@@ -149,6 +164,7 @@ void TabRestoreService::BrowserClosing(Browser* browser) {
 
   Window* window = new Window();
   window->selected_tab_index = browser->selected_index();
+  window->timestamp = TimeNow();
   window->tabs.resize(browser->tab_count());
   size_t entry_index = 0;
   for (int tab_index = 0; tab_index < browser->tab_count(); ++tab_index) {
@@ -358,6 +374,7 @@ void TabRestoreService::PopulateTab(Tab* tab,
         controller->pending_entry() : controller->GetEntryAtIndex(i);
     tab->navigations[i].SetFromNavigationEntry(*entry);
   }
+  tab->timestamp = TimeNow();
   tab->current_navigation_index = controller->GetCurrentEntryIndex();
   if (tab->current_navigation_index == -1 && entry_count > 0)
     tab->current_navigation_index = 0;
@@ -424,7 +441,8 @@ void TabRestoreService::ScheduleCommandsForWindow(const Window& window) {
   ScheduleCommand(
       CreateWindowCommand(window.id,
                           std::min(real_selected_tab, valid_tab_count - 1),
-                          valid_tab_count));
+                          valid_tab_count,
+                          window.timestamp));
 
   for (size_t i = 0; i < window.tabs.size(); ++i) {
     int selected_index = GetSelectedNavigationIndexToPersist(window.tabs[i]);
@@ -452,7 +470,8 @@ void TabRestoreService::ScheduleCommandsForTab(const Tab& tab,
   // Write the command that identifies the selected tab.
   ScheduleCommand(
       CreateSelectedNavigationInTabCommand(tab.id,
-                                           valid_count_before_selected));
+                                           valid_count_before_selected,
+                                           tab.timestamp));
 
   if (tab.pinned) {
     PinnedStatePayload payload = true;
@@ -480,11 +499,13 @@ void TabRestoreService::ScheduleCommandsForTab(const Tab& tab,
 
 SessionCommand* TabRestoreService::CreateWindowCommand(SessionID::id_type id,
                                                        int selected_tab_index,
-                                                       int num_tabs) {
-  WindowPayload payload = { 0 };
+                                                       int num_tabs,
+                                                       Time timestamp) {
+  WindowPayload2 payload;
   payload.window_id = id;
   payload.selected_tab_index = selected_tab_index;
   payload.num_tabs = num_tabs;
+  payload.timestamp = timestamp.ToInternalValue();
 
   SessionCommand* command =
       new SessionCommand(kCommandWindow, sizeof(payload));
@@ -494,10 +515,12 @@ SessionCommand* TabRestoreService::CreateWindowCommand(SessionID::id_type id,
 
 SessionCommand* TabRestoreService::CreateSelectedNavigationInTabCommand(
     SessionID::id_type tab_id,
-    int32 index) {
-  SelectedNavigationInTabPayload payload = { 0 };
+    int32 index,
+    Time timestamp) {
+  SelectedNavigationInTabPayload2 payload;
   payload.id = tab_id;
   payload.index = index;
+  payload.timestamp = timestamp.ToInternalValue();
   SessionCommand* command =
       new SessionCommand(kCommandSelectedNavigationInTab, sizeof(payload));
   memcpy(command->contents(), &payload, sizeof(payload));
@@ -588,15 +611,28 @@ void TabRestoreService::CreateEntriesFromCommands(
       }
 
       case kCommandWindow: {
-        WindowPayload payload;
+        WindowPayload2 payload;
         if (pending_window_tabs > 0) {
           // Should never receive a window command while waiting for all the
           // tabs in a window.
           return;
         }
 
-        if (!command.GetPayload(&payload, sizeof(payload)))
-          return;
+        // Try the new payload first
+        if (!command.GetPayload(&payload, sizeof(payload))) {
+          // then the old payload
+          WindowPayload old_payload;
+          if (!command.GetPayload(&old_payload, sizeof(old_payload)))
+            return;
+
+          // Copy the old payload data to the new payload.
+          payload.window_id = old_payload.window_id;
+          payload.selected_tab_index = old_payload.selected_tab_index;
+          payload.num_tabs = old_payload.num_tabs;
+          // Since we don't have a time use time 0 which is used to mark as an
+          // unknown timestamp.
+          payload.timestamp = 0;
+        }
 
         pending_window_tabs = payload.num_tabs;
         if (pending_window_tabs <= 0) {
@@ -608,15 +644,24 @@ void TabRestoreService::CreateEntriesFromCommands(
 
         current_window = new Window();
         current_window->selected_tab_index = payload.selected_tab_index;
+        current_window->timestamp = Time::FromInternalValue(payload.timestamp);
         entries->push_back(current_window);
         id_to_entry[payload.window_id] = current_window;
         break;
       }
 
       case kCommandSelectedNavigationInTab: {
-        SelectedNavigationInTabPayload payload;
-        if (!command.GetPayload(&payload, sizeof(payload)))
-          return;
+        SelectedNavigationInTabPayload2 payload;
+        if (!command.GetPayload(&payload, sizeof(payload))) {
+          SelectedNavigationInTabPayload old_payload;
+          if (!command.GetPayload(&old_payload, sizeof(old_payload)))
+            return;
+          payload.id = old_payload.id;
+          payload.index = old_payload.index;
+          // Since we don't have a time use time 0 which is used to mark as an
+          // unknown timestamp.
+          payload.timestamp = 0;
+        }
 
         if (pending_window_tabs > 0) {
           if (!current_window) {
@@ -632,6 +677,7 @@ void TabRestoreService::CreateEntriesFromCommands(
           RemoveEntryByID(payload.id, &id_to_entry, &(entries.get()));
           current_tab = new Tab();
           id_to_entry[payload.id] = current_tab;
+          current_tab->timestamp = Time::FromInternalValue(payload.timestamp);
           entries->push_back(current_tab);
         }
         current_tab->current_navigation_index = payload.index;
@@ -775,6 +821,7 @@ bool TabRestoreService::ConvertSessionWindowToWindow(
       tab.navigations.swap(session_window->tabs[i]->navigations);
       tab.current_navigation_index =
           session_window->tabs[i]->current_navigation_index;
+      tab.timestamp = Time();
     }
   }
   if (window->tabs.empty())
@@ -783,6 +830,7 @@ bool TabRestoreService::ConvertSessionWindowToWindow(
   window->selected_tab_index =
       std::min(session_window->selected_tab_index,
                static_cast<int>(window->tabs.size() - 1));
+  window->timestamp = Time();
   return true;
 }
 
@@ -829,3 +877,8 @@ void TabRestoreService::LoadStateChanged() {
 
   PruneAndNotify();
 }
+
+Time TabRestoreService::TimeNow() const {
+  return time_factory_ ? time_factory_->TimeNow() : Time::Now();
+}
+
diff --git a/chrome/browser/sessions/tab_restore_service.h b/chrome/browser/sessions/tab_restore_service.h
index 1c28da8..a3e611c 100644
--- a/chrome/browser/sessions/tab_restore_service.h
+++ b/chrome/browser/sessions/tab_restore_service.h
@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/observer_list.h"
+#include "base/time.h"
 #include "chrome/browser/sessions/base_session_service.h"
 #include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/sessions/session_types.h"
@@ -42,6 +43,13 @@ class TabRestoreService : public BaseSessionService {
     virtual void TabRestoreServiceDestroyed(TabRestoreService* service) = 0;
   };
 
+  // Interface used to allow the test to provide a custom time.
+  class TimeFactory {
+   public:
+    virtual ~TimeFactory() {}
+    virtual base::Time TimeNow() = 0;
+  };
+
   // The type of entry.
   enum Type {
     TAB,
@@ -59,6 +67,9 @@ class TabRestoreService : public BaseSessionService {
 
     // The type of the entry.
     Type type;
+
+    // The time when the window or tab was closed.
+    base::Time timestamp;
   };
 
   // Represents a previously open tab.
@@ -102,8 +113,12 @@ class TabRestoreService : public BaseSessionService {
 
   typedef std::list<Entry*> Entries;
 
-  // Creates a new TabRestoreService.
-  explicit TabRestoreService(Profile* profile);
+  // Creates a new TabRestoreService and provides an object that provides the
+  // current time. The TabRestoreService does not take ownership of the
+  // |time_factory_|.
+  explicit TabRestoreService(Profile* profile,
+                             TimeFactory* time_factory_ = NULL);
+
   virtual ~TabRestoreService();
 
   // Adds/removes an observer. TabRestoreService does not take ownership of
@@ -204,12 +219,14 @@ class TabRestoreService : public BaseSessionService {
   // Creates a window close command.
   SessionCommand* CreateWindowCommand(SessionID::id_type id,
                                       int selected_tab_index,
-                                      int num_tabs);
+                                      int num_tabs,
+                                      base::Time timestamp);
 
   // Creates a tab close command.
   SessionCommand* CreateSelectedNavigationInTabCommand(
       SessionID::id_type tab_id,
-      int32 index);
+      int32 index,
+      base::Time timestamp);
 
   // Creates a restore command.
   SessionCommand* CreateRestoredEntryCommand(SessionID::id_type entry_id);
@@ -257,7 +274,8 @@ class TabRestoreService : public BaseSessionService {
       std::vector<SessionWindow*>* windows,
       std::vector<Entry*>* entries);
 
-  // Converts a SessionWindow into a Window, returning true on success.
+  // 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.
   bool ConvertSessionWindowToWindow(
       SessionWindow* session_window,
       Window* window);
@@ -267,6 +285,9 @@ class TabRestoreService : public BaseSessionService {
   // observers are notified.
   void LoadStateChanged();
 
+  // Gets the current time. This uses the time_factory_ if there is one.
+  base::Time TimeNow() const;
+
   // Set of entries.
   Entries entries_;
 
@@ -302,7 +323,10 @@ class TabRestoreService : public BaseSessionService {
   // entries_.
   std::vector<Entry*> staging_entries_;
 
+  TimeFactory* time_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(TabRestoreService);
 };
 
 #endif  // CHROME_BROWSER_SESSIONS_TAB_RESTORE_SERVICE_H_
+
diff --git a/chrome/browser/sessions/tab_restore_service_unittest.cc b/chrome/browser/sessions/tab_restore_service_unittest.cc
index 72ce546..d351e47 100644
--- a/chrome/browser/sessions/tab_restore_service_unittest.cc
+++ b/chrome/browser/sessions/tab_restore_service_unittest.cc
@@ -9,6 +9,24 @@
 #include "chrome/test/testing_profile.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+
+
+// Create subclass that overrides TimeNow so that we can control the time used
+// for closed tabs and windows.
+class TabRestoreTimeFactory : public TabRestoreService::TimeFactory {
+ public:
+  TabRestoreTimeFactory() : time_(base::Time::Now()) {}
+
+  virtual ~TabRestoreTimeFactory() {}
+
+  virtual base::Time TimeNow() {
+    return time_;
+  }
+
+ private:
+  base::Time time_;
+};
+
 class TabRestoreServiceTest : public RenderViewHostTestHarness {
  public:
   TabRestoreServiceTest() {
@@ -24,10 +42,12 @@ class TabRestoreServiceTest : public RenderViewHostTestHarness {
   // testing::Test overrides
   virtual void SetUp() {
     RenderViewHostTestHarness::SetUp();
-    service_ = new TabRestoreService(profile());
+    time_factory_ = new TabRestoreTimeFactory();
+    service_ = new TabRestoreService(profile(), time_factory_);
   }
   virtual void TearDown() {
     service_ = NULL;
+    delete time_factory_;
     RenderViewHostTestHarness::TearDown();
   }
 
@@ -50,7 +70,7 @@ class TabRestoreServiceTest : public RenderViewHostTestHarness {
     // Must set service to null first so that it is destroyed before the new
     // one is created.
     service_ = NULL;
-    service_ = new TabRestoreService(profile());
+    service_ = new TabRestoreService(profile(), time_factory_);
     service_->LoadTabsFromLastSession();
   }
 
@@ -90,6 +110,7 @@ class TabRestoreServiceTest : public RenderViewHostTestHarness {
   GURL url2_;
   GURL url3_;
   scoped_refptr<TabRestoreService> service_;
+  TabRestoreTimeFactory* time_factory_;
 };
 
 TEST_F(TabRestoreServiceTest, Basic) {
@@ -111,6 +132,8 @@ TEST_F(TabRestoreServiceTest, Basic) {
   EXPECT_TRUE(url2_ == tab->navigations[1].url());
   EXPECT_TRUE(url3_ == tab->navigations[2].url());
   EXPECT_EQ(2, tab->current_navigation_index);
+  EXPECT_EQ(time_factory_->TimeNow().ToInternalValue(),
+            tab->timestamp.ToInternalValue());
 
   NavigateToIndex(1);
 
@@ -130,6 +153,8 @@ TEST_F(TabRestoreServiceTest, Basic) {
   EXPECT_TRUE(url2_ == tab->navigations[1].url());
   EXPECT_TRUE(url3_ == tab->navigations[2].url());
   EXPECT_EQ(1, tab->current_navigation_index);
+  EXPECT_EQ(time_factory_->TimeNow().ToInternalValue(),
+            tab->timestamp.ToInternalValue());
 }
 
 // Make sure TabRestoreService doesn't create an entry for a tab with no
@@ -162,6 +187,8 @@ TEST_F(TabRestoreServiceTest, Restore) {
   EXPECT_TRUE(url2_ == tab->navigations[1].url());
   EXPECT_TRUE(url3_ == tab->navigations[2].url());
   EXPECT_EQ(2, tab->current_navigation_index);
+  EXPECT_EQ(time_factory_->TimeNow().ToInternalValue(),
+            tab->timestamp.ToInternalValue());
 }
 
 // Tests restoring a single pinned tab.
@@ -223,6 +250,8 @@ TEST_F(TabRestoreServiceTest, DontPersistPostData) {
       static_cast<const TabRestoreService::Tab*>(restored_entry);
   // There should be 3 navs.
   ASSERT_EQ(3U, restored_tab->navigations.size());
+  EXPECT_EQ(time_factory_->TimeNow().ToInternalValue(),
+            restored_tab->timestamp.ToInternalValue());
 }
 
 // Make sure we don't persist entries to disk that have post data. This
@@ -259,9 +288,11 @@ TEST_F(TabRestoreServiceTest, LoadPreviousSession) {
   TabRestoreService::Window* window =
       static_cast<TabRestoreService::Window*>(entry2);
   ASSERT_EQ(1U, window->tabs.size());
+  EXPECT_EQ(0, window->timestamp.ToInternalValue());
   EXPECT_EQ(0, window->selected_tab_index);
   ASSERT_EQ(1U, window->tabs[0].navigations.size());
   EXPECT_EQ(0, window->tabs[0].current_navigation_index);
+  EXPECT_EQ(0, window->tabs[0].timestamp.ToInternalValue());
   EXPECT_TRUE(url1_ == window->tabs[0].navigations[0].url());
 }
 
@@ -313,8 +344,10 @@ TEST_F(TabRestoreServiceTest, LoadPreviousSessionAndTabs) {
       static_cast<TabRestoreService::Window*>(entry);
   ASSERT_EQ(1U, window->tabs.size());
   EXPECT_EQ(0, window->selected_tab_index);
+  EXPECT_EQ(0, window->timestamp.ToInternalValue());
   ASSERT_EQ(1U, window->tabs[0].navigations.size());
   EXPECT_EQ(0, window->tabs[0].current_navigation_index);
+  EXPECT_EQ(0, window->tabs[0].timestamp.ToInternalValue());
   EXPECT_TRUE(url1_ == window->tabs[0].navigations[0].url());
 
   // Then the closed tab.
@@ -324,6 +357,8 @@ TEST_F(TabRestoreServiceTest, LoadPreviousSessionAndTabs) {
   ASSERT_FALSE(tab->pinned);
   ASSERT_EQ(3U, tab->navigations.size());
   EXPECT_EQ(2, tab->current_navigation_index);
+  EXPECT_EQ(time_factory_->TimeNow().ToInternalValue(),
+            tab->timestamp.ToInternalValue());
   EXPECT_TRUE(url1_ == tab->navigations[0].url());
   EXPECT_TRUE(url2_ == tab->navigations[1].url());
   EXPECT_TRUE(url3_ == tab->navigations[2].url());
@@ -395,7 +430,45 @@ TEST_F(TabRestoreServiceTest, ManyWindowsInSessionService) {
       static_cast<TabRestoreService::Window*>(entry);
   ASSERT_EQ(1U, window->tabs.size());
   EXPECT_EQ(0, window->selected_tab_index);
+  EXPECT_EQ(0, window->timestamp.ToInternalValue());
   ASSERT_EQ(1U, window->tabs[0].navigations.size());
   EXPECT_EQ(0, window->tabs[0].current_navigation_index);
+  EXPECT_EQ(0, window->tabs[0].timestamp.ToInternalValue());
   EXPECT_TRUE(url1_ == window->tabs[0].navigations[0].url());
 }
+
+// Makes sure we restore the time stamp correctly.
+TEST_F(TabRestoreServiceTest, TimestampSurvivesRestore) {
+  base::Time tab_timestamp(base::Time::FromInternalValue(123456789));
+
+  AddThreeNavigations();
+
+  // Have the service record the tab.
+  service_->CreateHistoricalTab(&controller());
+
+  // Make sure an entry was created.
+  ASSERT_EQ(1U, service_->entries().size());
+
+  // Make sure the entry matches.
+  TabRestoreService::Entry* entry = service_->entries().front();
+  ASSERT_EQ(TabRestoreService::TAB, entry->type);
+  TabRestoreService::Tab* tab = static_cast<TabRestoreService::Tab*>(entry);
+  tab->timestamp = tab_timestamp;
+
+  // Set this, otherwise previous session won't be loaded.
+  profile()->set_last_session_exited_cleanly(false);
+
+  RecreateService();
+
+  // One entry should be created.
+  ASSERT_EQ(1U, service_->entries().size());
+
+  // And verify the entry.
+  TabRestoreService::Entry* restored_entry = service_->entries().front();
+  ASSERT_EQ(TabRestoreService::TAB, restored_entry->type);
+  TabRestoreService::Tab* restored_tab =
+      static_cast<TabRestoreService::Tab*>(restored_entry);
+  EXPECT_EQ(tab_timestamp.ToInternalValue(),
+            restored_tab->timestamp.ToInternalValue());
+}
+