[Sync] Add per-navigation timestamps/unique ids to tab sync.

A navigation's timestamps are updated only if the navigation is new
or if the user recently moved back/foward to the navigation entry.
Otherwise, we preserve old timestamps.

To do this, we switch to using SyncedTabNavigations, which also contains
the unique id of the navigation (pulled from NavigationEntry) and the
timestamp of the navgation, and SyncedSessionTab (which we have to
use in order to support SyncedTabNavigations).

Lastly, the protos have been updated to include unique_id/timestamp per
navigation, remove some obsolete fields that were unimportant, and add
comments about what transitions mean and when they're set.

BUG=98892
TEST=unit_tests, manually inspecting timestamps in sync node browser.


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

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

chrome/browser/sessions/session_types.h

@@ -45,7 +45,7 @@ class TabNavigation {
                 const std::string& state,
                 content::PageTransition transition);
   TabNavigation(const TabNavigation& tab);
-  ~TabNavigation();
+  virtual ~TabNavigation();
   TabNavigation& operator=(const TabNavigation& tab);
 
   // Converts this TabNavigation into a NavigationEntry with a page id of
@@ -122,7 +122,7 @@ class TabNavigation {
 // SessionTab corresponds to a NavigationController.
 struct SessionTab {
   SessionTab();
-  ~SessionTab();
+  virtual ~SessionTab();
 
   // Unique id of the window.
   SessionID window_id;

chrome/browser/sync/glue/session_model_associator.h

@@ -36,6 +36,10 @@ class Prefservice;
 class Profile;
 class ProfileSyncService;
 
+namespace content {
+class NavigationEntry;
+}  // namespace content
+
 namespace sync_api {
 class BaseTransaction;
 class ReadNode;
@@ -292,6 +296,7 @@ class SessionModelAssociator
 
   // A pool for managing free/used tab sync nodes. Performs lazy creation
   // of sync nodes when necessary.
+  // TODO(zea): pull this into its own file.
   class TabNodePool {
    public:
     explicit TabNodePool(ProfileSyncService* sync_service);
@@ -411,6 +416,17 @@ class SessionModelAssociator
   // if no page is found to be referring to the favicon anymore.
   void DecrementAndCleanFaviconForURL(const std::string& page_url);
 
+  // Helper method to build sync's tab specifics from a newly modified
+  // tab, window, and the locally stored previous tab data. After completing,
+  // |prev_tab| will be updated to reflect the current data, |sync_tab| will
+  // be filled with the tab data (preserving old timestamps as necessary), and
+  // |new_url| will be the tab's current url.
+  void AssociateTabContents(const SyncedWindowDelegate& window,
+                            const SyncedTabDelegate& new_tab,
+                            SyncedSessionTab* prev_tab,
+                            sync_pb::SessionTab* sync_tab,
+                            GURL* new_url);
+
   // Load the favicon for the tab specified by |tab_link|. Will cancel any
   // outstanding request for this tab. OnFaviconDataAvailable(..) will be called
   // when the load completes.
@@ -440,21 +456,21 @@ class SessionModelAssociator
   // Used to populate a session tab from the session specifics tab provided.
   static void PopulateSessionTabFromSpecifics(const sync_pb::SessionTab& tab,
                                               const base::Time& mtime,
-                                              SessionTab* session_tab);
+                                              SyncedSessionTab* session_tab);
 
   // Helper method to load the favicon data from the tab specifics. If the
   // favicon is valid, stores the favicon data and increments the usage counter
   // in |synced_favicons_| and updates |synced_favicon_pages_| appropriately.
   void LoadForeignTabFavicon(const sync_pb::SessionTab& tab);
 
-  // Used to populate a session tab from the session specifics tab provided.
+  // Append a new navigation from sync specifics onto |tab| navigation vectors.
   static void AppendSessionTabNavigation(
      const sync_pb::TabNavigation& navigation,
-     std::vector<TabNavigation>* navigations);
+     SyncedSessionTab* tab);
 
   // Populates the navigation portion of the session specifics.
   static void PopulateSessionSpecificsNavigation(
-     const TabNavigation* navigation,
+     const content::NavigationEntry& navigation,
      sync_pb::TabNavigation* tab_navigation);
 
   // Returns true if this tab belongs to this profile and belongs to a window,

chrome/browser/sync/glue/synced_session.cc

@@ -5,11 +5,54 @@
 #include "chrome/browser/sync/glue/synced_session.h"
 
 #include "base/stl_util.h"
-#include "chrome/browser/sessions/session_types.h"
 #include "chrome/common/url_constants.h"
+#include "content/public/browser/navigation_entry.h"
 
 namespace browser_sync {
 
+SyncedTabNavigation::SyncedTabNavigation() : unique_id_(0) {
+}
+
+SyncedTabNavigation::SyncedTabNavigation(const SyncedTabNavigation& tab)
+    : TabNavigation(tab),
+      unique_id_(tab.unique_id_),
+      timestamp_(tab.timestamp_) {
+}
+
+SyncedTabNavigation::SyncedTabNavigation(int index,
+                                         const GURL& virtual_url,
+                                         const content::Referrer& referrer,
+                                         const string16& title,
+                                         const std::string& state,
+                                         content::PageTransition transition,
+                                         int unique_id,
+                                         const base::Time& timestamp)
+    : TabNavigation(index, virtual_url, referrer, title, state, transition),
+      unique_id_(unique_id),
+      timestamp_(timestamp) {
+}
+
+SyncedTabNavigation::~SyncedTabNavigation() {
+}
+
+void SyncedTabNavigation::set_unique_id(int unique_id) {
+  unique_id_ = unique_id;
+}
+
+int SyncedTabNavigation::unique_id() const {
+  return unique_id_;
+}
+
+void SyncedTabNavigation::set_timestamp(const base::Time& timestamp) {
+  timestamp_ = timestamp;
+}
+base::Time SyncedTabNavigation::timestamp() const {
+  return timestamp_;
+}
+
+SyncedSessionTab::SyncedSessionTab() {}
+SyncedSessionTab::~SyncedSessionTab() {}
+
 SyncedSession::SyncedSession() : session_tag("invalid"),
                                  device_type(TYPE_UNSET) {
 }

chrome/browser/sync/glue/synced_session.h

@@ -11,12 +11,53 @@
 
 #include "base/time.h"
 #include "chrome/browser/sessions/session_id.h"
+#include "chrome/browser/sessions/session_types.h"
 
-struct SessionTab;
-struct SessionWindow;
+namespace content {
+class NavigationEntry;
+}
 
 namespace browser_sync {
 
+// Sync-specific wrapper around a normal TabNavigation.
+// Copy semantics supported.
+class SyncedTabNavigation : public TabNavigation {
+ public:
+  SyncedTabNavigation();
+  SyncedTabNavigation(const SyncedTabNavigation& tab);
+  SyncedTabNavigation(int index,
+                      const GURL& virtual_url,
+                      const content::Referrer& referrer,
+                      const string16& title,
+                      const std::string& state,
+                      content::PageTransition transition,
+                      int unique_id,
+                      const base::Time& timestamp);
+  virtual ~SyncedTabNavigation();
+
+  // Unique id for this navigation.
+  void set_unique_id(int unique_id);
+  int unique_id() const;
+
+  // Timestamp this navigation occurred.
+  void set_timestamp(const base::Time& timestamp);
+  base::Time timestamp() const;
+
+ private:
+  int unique_id_;
+  base::Time timestamp_;
+};
+
+// Sync-specific wrapper around a normal SessionTab to support using
+// SyncedTabNavigation.
+struct SyncedSessionTab : public SessionTab {
+ public:
+  SyncedSessionTab();
+  virtual ~SyncedSessionTab();
+
+  std::vector<SyncedTabNavigation> synced_tab_navigations;
+};
+
 // Defines a synced session for use by session sync. A synced session is a
 // list of windows along with a unique session identifer (tag) and meta-data
 // about the device being synced.

chrome/browser/sync/glue/synced_session_tracker.cc

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -67,7 +67,7 @@ bool SyncedSessionTracker::LookupSessionWindows(
 bool SyncedSessionTracker::LookupSessionTab(
     const std::string& tag,
     SessionID::id_type tab_id,
-    const SessionTab** tab) const {
+    const SyncedSessionTab** tab) const {
   DCHECK(tab);
   SyncedTabMap::const_iterator tab_map_iter = synced_tab_map_.find(tag);
   if (tab_map_iter == synced_tab_map_.end()) {
@@ -165,7 +165,7 @@ bool SyncedSessionTracker::DeleteOldSessionTabIfNecessary(
     SessionTabWrapper tab_wrapper) {
   if (!tab_wrapper.owned) {
     if (VLOG_IS_ON(1)) {
-      SessionTab* tab_ptr = tab_wrapper.tab_ptr;
+      SyncedSessionTab* tab_ptr = tab_wrapper.tab_ptr;
       std::string title;
       if (tab_ptr->navigations.size() > 0) {
         title = " (" + UTF16ToUTF8(
@@ -239,7 +239,7 @@ void SyncedSessionTracker::PutTabInWindow(const std::string& session_tag,
                                           SessionID::id_type window_id,
                                           SessionID::id_type tab_id,
                                           size_t tab_index) {
-  SessionTab* tab_ptr = GetTab(session_tag, tab_id);
+  SyncedSessionTab* tab_ptr = GetTab(session_tag, tab_id);
   unmapped_tabs_.erase(tab_ptr);
   synced_tab_map_[session_tag][tab_id].owned = true;
   tab_ptr->window_id.set_id(window_id);
@@ -251,14 +251,14 @@ void SyncedSessionTracker::PutTabInWindow(const std::string& session_tag,
   if (window_tabs.size() <= tab_index) {
     window_tabs.resize(tab_index+1, NULL);
   }
-  DCHECK_EQ((SessionTab*)NULL, window_tabs[tab_index]);
+  DCHECK_EQ((SyncedSessionTab*)NULL, window_tabs[tab_index]);
   window_tabs[tab_index] = tab_ptr;
 }
 
-SessionTab* SyncedSessionTracker::GetTab(
+SyncedSessionTab* SyncedSessionTracker::GetTab(
     const std::string& session_tag,
     SessionID::id_type tab_id) {
-  SessionTab* tab_ptr = NULL;
+  SyncedSessionTab* tab_ptr = NULL;
   IDToSessionTabMap::iterator iter =
       synced_tab_map_[session_tag].find(tab_id);
   if (iter != synced_tab_map_[session_tag].end()) {
@@ -275,7 +275,7 @@ SessionTab* SyncedSessionTracker::GetTab(
                << "'s seen tab " << tab_id  << " at " << tab_ptr << title;
     }
   } else {
-    tab_ptr = new SessionTab();
+    tab_ptr = new SyncedSessionTab();
     tab_ptr->tab_id.set_id(tab_id);
     synced_tab_map_[session_tag][tab_id] = SessionTabWrapper(tab_ptr, false);
     unmapped_tabs_.insert(tab_ptr);

chrome/browser/sync/glue/synced_session_tracker.h

@@ -20,7 +20,7 @@
 namespace browser_sync {
 
 // Class to manage synced sessions. The tracker will own all SyncedSession
-// and SessionTab objects it creates, and deletes them appropriately on
+// and SyncedSessionTab objects it creates, and deletes them appropriately on
 // destruction.
 // Note: SyncedSession objects are created for all synced sessions, including
 // the local session (whose tag we maintain separately).
@@ -57,7 +57,7 @@ class SyncedSessionTracker {
   // - Returns false, tab is set to NULL.
   bool LookupSessionTab(const std::string& session_tag,
                         SessionID::id_type tab_id,
-                        const SessionTab** tab) const;
+                        const SyncedSessionTab** tab) const;
 
   // Returns a pointer to the SyncedSession object associated with
   // |session_tag|.  If none exists, creates one. Ownership of the
@@ -104,8 +104,8 @@ class SyncedSessionTracker {
   // Returns a pointer to the SessionTab object associated with |tab_id| for
   // the session specified with |session_tag|. If none exists, creates one.
   // Ownership of the SessionTab remains within the SyncedSessionTracker.
-  SessionTab* GetTab(const std::string& session_tag,
-                     SessionID::id_type tab_id);
+  SyncedSessionTab* GetTab(const std::string& session_tag,
+                           SessionID::id_type tab_id);
 
   // Free the memory for all dynamically allocated objects and clear the
   // tracking structures.
@@ -139,10 +139,10 @@ class SyncedSessionTracker {
   // above).
   struct SessionTabWrapper {
     SessionTabWrapper() : tab_ptr(NULL), owned(false) {}
-    SessionTabWrapper(SessionTab* tab_ptr, bool owned)
+    SessionTabWrapper(SyncedSessionTab* tab_ptr, bool owned)
         : tab_ptr(tab_ptr),
           owned(owned) {}
-    SessionTab* tab_ptr;
+    SyncedSessionTab* tab_ptr;
     bool owned;
   };
   typedef std::map<SessionID::id_type, SessionTabWrapper> IDToSessionTabMap;
@@ -185,7 +185,7 @@ class SyncedSessionTracker {
   // have not yet mapped to SyncedSessions. These are temporarily orphaned
   // tabs, and won't be deleted if we delete synced_session_map_, but are still
   // owned by the SyncedSessionTracker itself (and deleted on Clear()).
-  std::set<SessionTab*> unmapped_tabs_;
+  std::set<SyncedSessionTab*> unmapped_tabs_;
 
   // The tag for this machine's local session, so we can distinguish the foreign
   // sessions.

chrome/browser/sync/glue/synced_session_tracker_unittest.cc

@@ -28,7 +28,7 @@ TEST_F(SyncedSessionTrackerTest, GetSession) {
 
 TEST_F(SyncedSessionTrackerTest, GetTabUnmapped) {
   SyncedSessionTracker tracker;
-  SessionTab* tab = tracker.GetTab("tag", 0);
+  SyncedSessionTab* tab = tracker.GetTab("tag", 0);
   ASSERT_EQ(tab, tracker.GetTab("tag", 0));
   // Should clean up memory on its own.
 }
@@ -60,7 +60,7 @@ TEST_F(SyncedSessionTrackerTest, LookupAllForeignSessions) {
   tracker.GetSession("tag2");
   tracker.PutWindowInSession("tag1", 0);
   tracker.PutTabInWindow("tag1", 0, 15, 0);
-  SessionTab* tab = tracker.GetTab("tag1", 15);
+  SyncedSessionTab* tab = tracker.GetTab("tag1", 15);
   ASSERT_TRUE(tab);
   tab->navigations.push_back(TabNavigation(
       0, GURL("bla://valid_url"), content::Referrer(GURL("bla://referrer"),
@@ -91,13 +91,13 @@ TEST_F(SyncedSessionTrackerTest, LookupSessionWindows) {
 
 TEST_F(SyncedSessionTrackerTest, LookupSessionTab) {
   SyncedSessionTracker tracker;
-  const SessionTab* tab;
+  const SyncedSessionTab* tab;
   ASSERT_FALSE(tracker.LookupSessionTab("tag1", 5, &tab));
   tracker.GetSession("tag1");
   tracker.PutWindowInSession("tag1", 0);
   tracker.PutTabInWindow("tag1", 0, 5, 0);
   ASSERT_TRUE(tracker.LookupSessionTab("tag1", 5, &tab));
-  ASSERT_NE((SessionTab*)NULL, tab);
+  ASSERT_NE((SyncedSessionTab*)NULL, tab);
 }
 
 TEST_F(SyncedSessionTrackerTest, Complex) {
@@ -105,8 +105,8 @@ TEST_F(SyncedSessionTrackerTest, Complex) {
   const std::string tag2 = "tag2";
   const std::string tag3 = "tag3";
   SyncedSessionTracker tracker;
-  std::vector<SessionTab*> tabs1, tabs2;
-  SessionTab* temp_tab;
+  std::vector<SyncedSessionTab*> tabs1, tabs2;
+  SyncedSessionTab* temp_tab;
   ASSERT_TRUE(tracker.Empty());
   ASSERT_EQ(0U, tracker.num_synced_sessions());
   ASSERT_EQ(0U, tracker.num_synced_tabs(tag1));
@@ -141,13 +141,13 @@ TEST_F(SyncedSessionTrackerTest, Complex) {
   tracker.PutTabInWindow(tag1, 0, 2, 0);    // No longer unmapped.
   ASSERT_EQ(3U, tracker.num_synced_tabs(tag1));      // Has not changed.
 
-  const SessionTab *tab_ptr;
+  const SyncedSessionTab *tab_ptr;
   ASSERT_TRUE(tracker.LookupSessionTab(tag1, 0, &tab_ptr));
   ASSERT_EQ(tab_ptr, tabs1[0]);
   ASSERT_TRUE(tracker.LookupSessionTab(tag1, 2, &tab_ptr));
   ASSERT_EQ(tab_ptr, tabs1[2]);
   ASSERT_FALSE(tracker.LookupSessionTab(tag1, 3, &tab_ptr));
-  ASSERT_EQ(static_cast<const SessionTab*>(NULL), tab_ptr);
+  ASSERT_EQ(static_cast<const SyncedSessionTab*>(NULL), tab_ptr);
 
   std::vector<const SessionWindow*> windows;
   ASSERT_TRUE(tracker.LookupSessionWindows(tag1, &windows));
@@ -177,7 +177,7 @@ TEST_F(SyncedSessionTrackerTest, ManyGetTabs) {
       // More attempts than tabs means we'll sometimes get the same tabs,
       // sometimes have to allocate new tabs.
       int rand_tab_num = base::RandInt(0, kMaxTabs);
-      SessionTab* tab = tracker.GetTab(tag, rand_tab_num);
+      SyncedSessionTab* tab = tracker.GetTab(tag, rand_tab_num);
       ASSERT_TRUE(tab);
     }
   }

chrome/browser/sync/profile_sync_service_session_unittest.cc

@@ -97,7 +97,6 @@ void BuildTabSpecifics(const std::string& tag, int window_id, int tab_id,
   tab->set_pinned(true);
   tab->set_extension_app_id("app_id");
   sync_pb::TabNavigation* navigation = tab->add_navigation();
-  navigation->set_index(12);
   navigation->set_virtual_url("http://foo/1");
   navigation->set_referrer("referrer");
   navigation->set_title("title");
@@ -140,7 +139,6 @@ void VerifySyncedSession(
       ASSERT_TRUE(tab->pinned);
       ASSERT_EQ("app_id", tab->extension_app_id);
       ASSERT_EQ(1U, tab->navigations.size());
-      ASSERT_EQ(12, tab->navigations[0].index());
       ASSERT_EQ(tab->navigations[0].virtual_url(), GURL("http://foo/1"));
       ASSERT_EQ(tab->navigations[0].referrer().url, GURL("referrer"));
       ASSERT_EQ(tab->navigations[0].title(), string16(ASCIIToUTF16("title")));

sync/protocol/proto_value_conversions.cc

@@ -157,13 +157,14 @@ DictionaryValue* SessionWindowToValue(
 DictionaryValue* TabNavigationToValue(
     const sync_pb::TabNavigation& proto) {
   DictionaryValue* value = new DictionaryValue();
-  SET_INT32(index);
   SET_STR(virtual_url);
   SET_STR(referrer);
   SET_STR(title);
   SET_STR(state);
   SET_ENUM(page_transition, GetPageTransitionString);
   SET_ENUM(navigation_qualifier, GetPageTransitionQualifierString);
+  SET_INT32(unique_id);
+  SET_INT64(timestamp);
   return value;
 }
 

sync/protocol/session_specifics.proto

@@ -91,7 +91,7 @@ message SessionTab {
 message TabNavigation {
   // The index in the NavigationController. If this is -1, it means this
   // TabNavigation is bogus.
-  optional int32 index = 1 [default = -1];
+  // optional int32 index = 1 [default = -1];  // obsolete.
   // The virtual URL, when nonempty, will override the actual URL of the page
   // when we display it to the user.
   optional string virtual_url = 2;
@@ -126,5 +126,10 @@ message TabNavigation {
   }
   optional PageTransition page_transition = 6 [default = TYPED];
   optional PageTransitionQualifier navigation_qualifier = 7;
+  // The unique navigation id (within this client).
+  optional int32 unique_id = 8;
+  // Timestamp for when this navigation last occurred (in client time).
+  // If the user goes back/foward in history the timestamp may refresh.
+  optional int64 timestamp = 9;
 }