[SYNC] Sessions datatype refactor. Most things related to sessions under-the-...

chrome/browser/sync/glue/session_model_associator.h

Index: chrome/browser/sync/glue/session_model_associator.h
===================================================================
--- chrome/browser/sync/glue/session_model_associator.h	(revision 68844)
+++ chrome/browser/sync/glue/session_model_associator.h	(working copy)
@@ -6,6 +6,8 @@
 #define CHROME_BROWSER_SYNC_GLUE_SESSION_MODEL_ASSOCIATOR_H_
 #pragma once
 
+#include <map>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -13,12 +15,17 @@
 #include "base/gtest_prod_util.h"
 #include "base/observer_list.h"
 #include "base/scoped_vector.h"
+#include "base/string_util.h"
+#include "base/utf_string_conversions.h"
+#include "chrome/browser/browser_window.h"
+#include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/sessions/session_service.h"
 #include "chrome/browser/sessions/session_types.h"
 #include "chrome/browser/sync/engine/syncapi.h"
 #include "chrome/browser/sync/glue/model_associator.h"
 #include "chrome/browser/sync/protocol/session_specifics.pb.h"
 #include "chrome/browser/sync/syncable/model_type.h"
+#include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/common/notification_registrar.h"
 
 class Profile;
@@ -40,172 +47,334 @@
 
 // Contains all logic for associating the Chrome sessions model and
 // the sync sessions model.
-// In the case of sessions, our local model is nothing but a buffer (specifics_)
-// that gets overwritten everytime there is an update. From it, we build a new
-// foreign session windows list each time |GetSessionData| is called by the
-// ForeignSessionHandler.
-class SessionModelAssociator : public PerDataTypeAssociatorInterface<
-    sync_pb::SessionSpecifics, std::string>, public NonThreadSafe {
+class SessionModelAssociator
+    : public PerDataTypeAssociatorInterface<TabContents, size_t>,
+      public NonThreadSafe {
  public:
-
   // Does not take ownership of sync_service.
   explicit SessionModelAssociator(ProfileSyncService* sync_service);
   virtual ~SessionModelAssociator();
 
+  // The has_nodes out parameter is set to true if the sync model has
+  // nodes other than the permanent tagged nodes.  The method may
+  // return false if an error occurred.
+  virtual bool SyncModelHasUserCreatedNodes(bool* has_nodes);
 
   // AssociatorInterface and PerDataTypeAssociator Interface implementation.
   virtual void AbortAssociation() {
     return;
-    // No implementation needed, this associator runs on the main
-    // thread.
+    // No implementation needed, this associator runs on the main thread.
   }
 
-  // Dummy method, we do everything all-at-once in UpdateFromSyncModel.
-  virtual void Associate(const sync_pb::SessionSpecifics* specifics,
-    int64 sync_id) {
-  }
+  // Returns sync id for the given chrome model id.
+  // Returns sync_api::kInvalidId if the sync node is not found for the given
+  // chrome id.
+  virtual int64 GetSyncIdFromChromeId(const size_t& id);
 
-  // Updates the sync model with the local client data. (calls
-  // UpdateFromSyncModel)
-  virtual bool AssociateModels();
+  // Returns sync id for the given session tag.
+  // Returns sync_api::kInvalidId if the sync node is not found for the given
+  // tag
+  virtual int64 GetSyncIdFromSessionTag(const std::string& tag);
 
-  // The has_nodes out parameter is set to true if the chrome model
-  // has user-created nodes.  The method may return false if an error
-  // occurred.
-  virtual bool ChromeModelHasUserCreatedNodes(bool* has_nodes);
+  // Not used.
+  virtual const TabContents* GetChromeNodeFromSyncId(int64 sync_id) {
+    NOTREACHED();
+    return NULL;
+  }
 
-  // Dummy method, we do everything all-at-once in UpdateFromSyncModel.
-  virtual void Disassociate(int64 sync_id) {
+  // Not used.
+  bool InitSyncNodeFromChromeId(const size_t& id,
+                                sync_api::BaseNode* sync_node) {
+    NOTREACHED();
+    return false;
   }
 
-  // Clear specifics_ buffer and notify foreign session handlers.
-  virtual bool DisassociateModels();
+  // Resync local window information. Updates the local sessions header node
+  // with the status of open windows and the order of tabs they contain. Should
+  // only be called for changes that affect a window, not a change within a
+  // single tab.
+  //
+  // If |reload_tabs| is true, will also resync all tabs (same as calling
+  // ReassociateTabs with a vector of all tabs).
+  void ReassociateWindows(bool reload_tabs);
 
-  // Returns the chrome session specifics for the given sync id.
-  // Returns NULL if no specifics are found for the given sync id.
-  virtual const sync_pb::SessionSpecifics* GetChromeNodeFromSyncId(
-    int64 sync_id);
+  // Loads and reassociates the local tabs referenced in |tabs|.
+  void ReassociateTabs(const std::vector<TabContents*>& tabs);
 
-  // Returns whether a node with the given permanent tag was found and update
-  // |sync_id| with that node's id.
-  virtual bool GetSyncIdForTaggedNode(const std::string* tag, int64* sync_id);
+  // Reassociates a single tab with the sync model. Will check if the tab
+  // already is associated with a sync node and allocate one if necessary.
+  void ReassociateTab(const TabContents& tab);
 
-  // Returns sync id for the given chrome model id.
-  // Returns sync_api::kInvalidId if the sync node is not found for the given
-  // chrome id.
-  virtual int64 GetSyncIdFromChromeId(const std::string& id);
+  // Associate a local tab and it's sync node. Will overwrite the contents of
+  // the sync node with new specifics built from the tab.
+  virtual void Associate(const TabContents* tab, int64 sync_id);
 
+  // Looks up the specified sync node, and marks that tab as closed, then marks
+  // the node as free and deletes association.
+  virtual void Disassociate(int64 sync_id);
 
-  // Initializes the given sync node from the given chrome node id.
-  // Returns false if no sync node was found for the given chrome node id or
-  // if the initialization of sync node fails.
-  virtual bool InitSyncNodeFromChromeId(const std::string& id,
-                                        sync_api::BaseNode* sync_node) {
-      return false;
-  }
+  // Load any ƒoreign session info stored in sync db and update the sync db
+  // with local client data. Processes/reuses any sync nodes owned by this
+  // client and creates any further sync nodes needed to store local header and
+  // tab info.
+  virtual bool AssociateModels();
 
-  // The has_nodes out parameter is set to true if the sync model has
-  // nodes other than the permanent tagged nodes.  The method may
-  // return false if an error occurred.
-  virtual bool SyncModelHasUserCreatedNodes(bool* has_nodes);
+  // Clear local sync data buffers. Does not delete sync nodes to avoid
+  // tombstones. TODO(zea): way to eventually delete orphaned nodes.
+  virtual bool DisassociateModels();
 
   // Returns the tag used to uniquely identify this machine's session in the
   // sync model.
-  std::string GetCurrentMachineTag();
+  inline const std::string& GetCurrentMachineTag() {
+    DCHECK(!current_machine_tag_.empty());
+    return current_machine_tag_;
+  }
 
-  // Updates the server data based upon the current client session.  If no node
-  // corresponding to this machine exists in the sync model, one is created.
-  void UpdateSyncModelDataFromClient();
+  // Load and associate window and tab data for a foreign session
+  bool AssociateForeignSpecifics(const sync_pb::SessionSpecifics& specifics,
+                                 int64 modification_time);
 
-  // Pulls the current sync model from the sync database and returns true upon
-  // update of the client model. Called by SessionChangeProcessor.
-  // Note that the specifics_ vector is reset and built from scratch each time.
-  bool UpdateFromSyncModel(const sync_api::BaseTransaction* trans);
+  // Removes a foreign session from our internal bookkeeping.
+  void DisassociateForeignSession(const std::string& foreign_session_tag);
 
-  // Helper for UpdateFromSyncModel. Appends sync data to a vector of specifics.
-  bool QuerySyncModel(const sync_api::BaseTransaction* trans,
-      std::vector<const sync_pb::SessionSpecifics*>& specifics);
+  // Builds a list of all foreign sessions.
+  // Caller does NOT own ForeignSession objects.
+  bool GetAllForeignSessions(std::vector<const ForeignSession*>* sessions);
 
-  // Builds sessions from buffered specifics data
-  bool GetSessionData(std::vector<ForeignSession*>* sessions);
+  // Loads all windows for foreign session with session tag |tag|.
+  // Caller does NOT own ForeignSession objects.
+  bool GetForeignSession(const std::string& tag,
+                         std::vector<SessionWindow*>* windows);
 
-  // Helper method to generate session specifics from session windows.
-  void FillSpecificsFromSessions(std::vector<SessionWindow*>* windows,
-      sync_pb::SessionSpecifics* session);
+  // Looks up the foreign tab identified by |tab_id| and belonging to foreign
+  // session |tag|.
+  // Caller does NOT own the SessionTab object.
+  bool GetForeignTab(const std::string& tag,
+                     const SessionID::id_type tab_id,
+                     const SessionTab** tab);
 
-  // Helper method for converting session specifics to windows.
-  void AppendForeignSessionFromSpecifics(
-      const sync_pb::SessionSpecifics* specifics,
-      std::vector<ForeignSession*>* session);
-
-  // Fills the given empty vector with foreign session windows to restore.
-  // If the vector is returned empty, then the session data could not be
-  // converted back into windows.
-  void AppendForeignSessionWithID(int64 id,
-      std::vector<ForeignSession*>* session,
-      sync_api::BaseTransaction* trans);
-
   // Returns the syncable model type.
   static syncable::ModelType model_type() { return syncable::SESSIONS; }
 
  private:
   FRIEND_TEST_ALL_PREFIXES(ProfileSyncServiceSessionTest, WriteSessionToNode);
   FRIEND_TEST_ALL_PREFIXES(ProfileSyncServiceSessionTest,
+                           WriteFilledSessionToNode);
+  FRIEND_TEST_ALL_PREFIXES(ProfileSyncServiceSessionTest,
                            WriteForeignSessionToNode);
 
-  // Returns the session service from |sync_service_|.
-  SessionService* GetSessionService();
+  // Keep all the links to local tab data in one place.
+  class TabLinks {
+   public:
+    // To support usage as second value in maps we need default and copy
+    // constructors.
+    explicit TabLinks()

[tim (not reviewing), 2010/12/15 21:14:00]

remove explicit

[Nicolas Zea, 2010/12/16 18:09:18]

Done.

+      : sync_id_(0),

[tim (not reviewing), 2010/12/15 21:14:00]

nit - line above or 4 space indent.

[Nicolas Zea, 2010/12/16 18:09:18]

Done.

+        session_tab_(NULL),
+        tab_(NULL) {}
 
+    // We only ever have either a SessionTab (for foreign tabs), or a
+    // TabContents (for local tabs).
+    TabLinks(int64 sync_id, const TabContents* tab)
+      : sync_id_(sync_id),
+        session_tab_(NULL) {
+      tab_ = const_cast<TabContents*>(tab);
+    }
+    TabLinks(int64 sync_id, const SessionTab* session_tab)
+      : sync_id_(sync_id),
+        tab_(NULL) {
+      session_tab_ = const_cast<SessionTab*>(session_tab);
+    }
+
+    // Getters.
+    inline const int64 sync_id() const { return sync_id_; }
+    inline const SessionTab* session_tab() const { return session_tab_; }
+    inline const TabContents* tab() const { return tab_; }
+   private:
+    int64 sync_id_;
+    SessionTab* session_tab_;
+    TabContents* tab_;
+  };
+
+  // Functor for comparing SessionID's.
+  class SessionIDLessThan {
+   public:
+    bool operator()(const SessionID& lhs, const SessionID& rhs) const {
+      return lhs.id() < rhs.id();
+    }
+  };
+
+  // Datatypes for accessing foreign tab data.
+  typedef std::map<SessionID, SessionTab*, SessionIDLessThan> IDToSessTabMap;

[tim (not reviewing), 2010/12/15 21:14:00]

we should really use IDToSessionTabMap and TabToSessionMap

[Nicolas Zea, 2010/12/16 18:09:18]

Done.

+  typedef std::map<std::string, IDToSessTabMap*> ForeignTabMap;
+  typedef std::map<std::string, ForeignSession*> ForeignSessionMap;
+
+  // Datatypes for accessing local tab data.
+  typedef std::map<SessionID, TabLinks, SessionIDLessThan> TabLinksMap;
+
+  // Delete all foreign session/window/tab objects allocated dynamically.
+  // This is comprised of ForeignSession*, TabToSessMap*, and any orphaned
+  // SessionTab*'s.
+  void DeleteForeignSessions();
+
+  // Determine if a window is of a type we're interested in syncing.
+  bool ShouldSyncWindowType(Browser::Type type);

[tim (not reviewing), 2010/12/15 21:14:00]

const? or static

[Nicolas Zea, 2010/12/16 18:09:18]

Done.

+
+  // Build a sync tag from tab_node_id.
+  inline std::string TabIdToTag(size_t tab_node_id) {
+    return StringPrintf("%s %zu",
+        GetCurrentMachineTag().c_str(), tab_node_id);
+  }
+
+  // Extract tab node id from sync tag.
+  inline size_t TabTagToId(const std::string& tag) {
+    std::istringstream iss(tag);

[tim (not reviewing), 2010/12/15 21:14:00]

we try not to use streams in chromium.  this also seems a bit brittle, although
probably faster than using a delimiter with SplitString.  But if we know the
first n chars are GetCurrentMachineTag, we could do something similar I think:

client_tag = GetCurrentMachineTag();
DCHECK(base::StartsWith(tag, client_tag, true));
StringToInt(tag.substr(client_tag.length, npos), &tab_node_id);

[Nicolas Zea, 2010/12/16 18:09:18]

See comment response below.

On 2010/12/15 21:14:00, timsteele wrote:

+    std::string client_tag;
+    size_t tab_node_id;
+    iss >> client_tag >> tab_node_id;
+    iss.peek();
+    DCHECK(iss.eof());
+    DCHECK_EQ(client_tag, GetCurrentMachineTag());
+    return tab_node_id;
+  }
+
+  // Support wide char strings.

[tim (not reviewing), 2010/12/15 21:14:00]

where's this used? we can't use string16 can we?

[Nicolas Zea, 2010/12/16 18:09:18]

Originally I was parsing the title from syncapi (GetTitle()), which returned a
wstring. At this point, I'm not longer using that functionality, so we don't
need to bother with either of these methods.

On 2010/12/15 21:14:00, timsteele wrote:

+  inline size_t TabTagToId(const std::wstring& tag) {
+    return TabTagToId(WideToUTF8(tag));
+  }
+
   // Initializes the tag corresponding to this machine.
   // Note: creates a syncable::BaseTransaction.

[tim (not reviewing), 2010/12/15 21:14:00]

I'd say something like 'Precondition: do not call this within a transaction.'
Alternatively you can require the transaction be passed in to force the caller
to know.

[Nicolas Zea, 2010/12/16 18:09:18]

Done.

   void InitializeCurrentMachineTag();
 
+  // Updates the server data based upon the current client session.  If no node
+  // corresponding to this machine exists in the sync model, one is created.
+  void UpdateSyncModelDataFromClient();
+
+  // Pulls the current sync model from the sync database and returns true upon
+  // update of the client model. Will associate any foreign sessions as well as
+  // keep track of any local tab nodes, adding them to our free tab node pool.
+  bool UpdateAssociationsFromSyncModel(const sync_api::ReadNode& root,
+                                       const sync_api::BaseTransaction* trans);
+
+  // Used to populate a session window from the session specifics window
+  // provided.
+  void PopulateSessionWindowFromSpecifics(std::string foreign_session_tag,
+                                          const sync_pb::SessionWindow& window,
+                                          const int64 mtime,
+                                          SessionWindow* session_window);
+
+  // Used to populate a session tab from the session specifics tab provided.
+  void PopulateSessionTabFromSpecifics(const sync_pb::SessionTab& tab,
+                                       const int64 mtime,
+                                       SessionTab* session_tab);
+
+  // Used to populate a session tab from the session specifics tab provided.
+  void AppendSessionTabNavigation(const sync_pb::TabNavigation& navigation,
+                                  std::vector<TabNavigation>* navigations);
+
+  // Returns the sync_id for the next available tab node. If none are available,
+  // creates a new tab node.
+  // Note: We make use of the following "id's"
+  // - sync_id: an int64 used in |sync_api::InitByIdLookup|
+  // - a tab_id: created by session service, unique to this client
+  // - a tab_node_id: the id for a particular sync tab node. This is used
+  //   to generate the sync tab node tag through:
+  //       tab_tag = StringPrintf("%s_%ui", local_session_tag, tab_node_id);
+  // tab_node_id and sync_id are both unique to a particular sync node. The
+  // difference is that tab_node_id is controlled by the model associator and is
+  // used when creating a new sync node, which returns the sync_id, created by
+  // the sync db.
+  int64 GetFreeTabNode();
+
+  // Adds a sync_id to our pool of free tab nodes.
+  void FreeTabNode(int64 sync_id);
+
+  // Fills a tab sync node with data from a TabContents object.
+  // (from a local navigation event)
+  bool WriteTabContentsToSyncModel(const TabContents& tab,
+                                   const int64 sync_id,
+                                   sync_api::WriteTransaction* trans);
+
   // Populates the navigation portion of the session specifics.
-  void PopulateSessionSpecificsNavigation(const TabNavigation* navigation,
-      sync_pb::TabNavigation* tab_navigation);
+  void PopulateSessionSpecificsNavigation(
+     const TabNavigation* navigation,
+     sync_pb::TabNavigation* tab_navigation);
 
-  // Populates the tab portion of the session specifics.
-  void PopulateSessionSpecificsTab(const SessionTab* tab,
-      sync_pb::SessionTab* session_tab);
+  // Specifies whether the window has tabs to sync. The new tab page does not
+  // count. If no tabs to sync, it returns true, otherwise false;
+  bool WindowHasNoTabsToSync(const SessionWindow& window);
 
-  // Populates the window portion of the session specifics.
-  void PopulateSessionSpecificsWindow(const SessionWindow* window,
-      sync_pb::SessionWindow* session_window);
+  // Control which local tabs we're interested in syncing.
+  // Ensures the profile matches sync's profile and that the tab has at least
+  // one navigation entry and is not an empty tab.
+  bool IsValidTab(const TabContents& tab);
 
-  // Specifies whether the window has tabs to sync.  The new tab page does not
-  // count.  If no tabs to sync, it returns true, otherwise false;
-  bool WindowHasNoTabsToSync(const SessionWindow* window);
+  // Control which foreign tabs we're interested in displaying.
+  // Checks that the tab has navigations and is not a new tab.
+  // Note: a new tab page with back/forward history is valid.
+  bool IsValidSessionTab(const SessionTab& tab);
 
+  // Returns the session service from |sync_service_|.
+  SessionService* GetSessionService();
+
   // Internal method used in the callback to obtain the current session.
   // We don't own |windows|.
   void OnGotSession(int handle, std::vector<SessionWindow*>* windows);
 
-  // Used to populate a session tab from the session specifics tab provided.
-  void AppendSessionTabNavigation(std::vector<TabNavigation>* navigations,
-      const sync_pb::TabNavigation* navigation);
+  // Populate a session specifics header from a list of SessionWindows
+  void PopulateSessionSpecificsHeader(
+      const std::vector<SessionWindow*>& windows,
+      sync_pb::SessionHeader* header_s);
 
-  // Used to populate a session tab from the session specifics tab provided.
-  void PopulateSessionTabFromSpecifics(SessionTab* session_tab,
-      const sync_pb::SessionTab* tab, SessionID id);
+  // Populates the window portion of the session specifics.
+  void PopulateSessionSpecificsWindow(const SessionWindow& window,
+                                      sync_pb::SessionWindow* session_window);
 
-  // Used to populate a session window from the session specifics window
-  // provided.
-  void PopulateSessionWindowFromSpecifics(SessionWindow* session_window,
-      const sync_pb::SessionWindow* window);
+  // Syncs all the tabs in |window| with the local sync db. Will allocate tab
+  // nodes if needed.
+  bool SyncLocalWindowToSyncModel(const SessionWindow& window);
 
-  // Updates the current session on the server.  Creates a node for this machine
-  // if there is not one already.
-  bool UpdateSyncModel(sync_pb::SessionSpecifics* session_data,
-                    sync_api::WriteTransaction* trans,
-                    const sync_api::ReadNode* root);
-  // Stores the machine tag.
+  // Fills a tab sync node with data from a SessionTab object.
+  // (from ReadCurrentSessions)
+  bool WriteSessionTabToSyncModel(const SessionTab& tab,
+                                  const int64 sync_id,
+                                  sync_api::WriteTransaction* trans);
+
+  // Populates the tab portion of the session specifics.
+  void PopulateSessionSpecificsTab(const SessionTab& tab,
+                                   sync_pb::SessionTab* session_tab);
+
+  // Local client name.
   std::string current_machine_tag_;
 
-  // Stores the current set of foreign session specifics.
-  // Used by ForeignSessionHandler through |GetSessionData|.
-  // Built by |QuerySyncModel| via |UpdateFromSyncModel|.
-  std::vector<const sync_pb::SessionSpecifics*> specifics_;
+  // Pool of all available syncid's for tab's we have created.

[tim (not reviewing), 2010/12/15 21:14:00]

what's 'available' mean? the comment next to tab_pool_fp_ confuses me...
Is order important here?

[Nicolas Zea, 2010/12/16 18:09:18]

Done.

+  std::vector<int64> tab_syncid_pool_;
 
+  // Free pointer for tab pool. Only those node id's, up to and including the
+  // one indexed by the free pointer, are valid and free. The rest of the
+  // |tab_syncid_pool_| is invalid because the nodes are in use.
+  // To get the next free node, use tab_syncid_pool_[tab_pool_fp_--].

[tim (not reviewing), 2010/12/15 21:14:00]

I actually would consider pulling all this pool stuff into it's own little class
to prevent dumb mistakes trying to combine the various variables.  the
Free/GetFree methods above could call in, perhaps, and we could test it
separately.

[Nicolas Zea, 2010/12/16 18:09:18]

Done.

+  int64 tab_pool_fp_;
+
+  // SyncID for the sync node containing all the window information for this
+  // client.
+  int64 local_session_syncid_;
+
+  // Mapping of current open (local) tabs to their sync identifiers.
+  TabLinksMap tab_map_;
+
+  // Per foreign client mapping of their tab id's to their SessionTab objects.
+  ForeignTabMap foreign_tab_map_;
+
+  // Map of foreign sessions, accessed by the foreign client id.
+  ForeignSessionMap foreign_session_map_;
+
+  // The set of foreign tabs that we have seen, and created SessionTab objects
+  // for, but have not yet mapped to ForeignSessions. These are temporarily
+  // orphaned tabs, and won't be deleted if we delete foreign_session_map_.
+  std::set<SessionTab*> unmapped_tabs_;
+
   // Weak pointer.
   ProfileSyncService* sync_service_;