Reland of [Sync] Put session tracker in charge of maintaining local state.

components/sync_sessions/tab_node_pool.h

Index: components/sync_sessions/tab_node_pool.h
diff --git a/components/sync_sessions/tab_node_pool.h b/components/sync_sessions/tab_node_pool.h
index caadc4e5f2ee97964b3646e73828dacb62286e28..70d02a21721ecce1a600c0b3cbe076f12f5328fd 100644
--- a/components/sync_sessions/tab_node_pool.h
+++ b/components/sync_sessions/tab_node_pool.h
@@ -13,11 +13,6 @@
 
 #include "base/macros.h"
 #include "components/sessions/core/session_id.h"
-#include "components/sync/model/sync_change_processor.h"
-
-namespace syncer {
-class SyncChangeProcessor;
-}  // namespace syncer
 
 namespace sync_sessions {
 
@@ -29,15 +24,9 @@ namespace sync_sessions {
 //   to generate the sync tab node tag through:
 //       tab_tag = StringPrintf("%s_%ui", local_session_tag, tab_node_id);

[skym, 2016/12/03 01:20:46]

I'm not very familiar with printf functionality, or what %ui does, but this is
slightly different from elsewhere.

[Nicolas Zea, 2016/12/06 01:32:55]

Huh, this comment is out of date (we don't have an underscore). Updated.

 //
-// A sync node can be in one of the three states:
+// A sync node can be in one of the two states:
 // 1. Associated   : Sync node is used and associated with a tab.
-// 2. Unassociated : Sync node is used but currently unassociated with any tab.
-//                   This is true for old nodes that remain from a session
-//                   restart. Nodes are only unassociated temporarily while the
-//                   model associator figures out which tabs belong to which
-//                   nodes. Eventually any remaining unassociated nodes are
-//                   freed.
-// 3. Free         : Sync node is unused.
+// 2. Free         : Sync node is unused.
 
 class TabNodePool {
  public:
@@ -54,65 +43,36 @@ class TabNodePool {
 
   static const int kInvalidTabNodeID;
 
-  // Build a sync tag from tab_node_id.
-  static std::string TabIdToTag(const std::string& machine_tag,
-                                int tab_node_id);
-
-  // Returns the tab_node_id for the next free tab node. If none are available,
-  // creates a new tab node and adds it to free nodes pool. The free node can
-  // then be used to associate with a tab by calling AssociateTabNode.
-  // Note: The node is considered free until it has been associated. Repeated
-  // calls to GetFreeTabNode will return the same id until node has been
-  // associated.
-  // |change_output| *must* be provided. It is the TabNodePool's link to
-  // the SyncChange pipeline that exists in the caller context. If the need
-  // to create nodes arises in the implementation, associated SyncChanges will
-  // be appended to this list for later application by the caller via the
-  // SyncChangeProcessor.
-  int GetFreeTabNode(syncer::SyncChangeList* change_output);
-
-  // Removes association for |tab_node_id| and returns it to the free node pool.
-  // |change_output| *must* be provided. It is the TabNodePool's link to
-  // the SyncChange pipeline that exists in the caller's context. If the need
-  // to delete sync nodes arises in the implementation, associated SyncChanges
-  // will be appended to this list for later application by the caller via the
-  // SyncChangeProcessor.
-  void FreeTabNode(int tab_node_id, syncer::SyncChangeList* change_output);
-
-  // Associates |tab_node_id| with |tab_id|. |tab_node_id| should either be
-  // unassociated or free. If |tab_node_id| is free, |tab_node_id| is removed
-  // from the free node pool In order to associate a non free sync node,
-  // use ReassociateTabNode.
-  void AssociateTabNode(int tab_node_id, SessionID::id_type tab_id);
-
-  // Adds |tab_node_id| as an unassociated sync node.
-  // Note: this should only be called when we discover tab sync nodes from
-  // previous sessions, not for freeing tab nodes we created through
-  // GetFreeTabNode (use FreeTabNode below for that).
-  void AddTabNode(int tab_node_id);
+  // Fills |tab_node_id| with a tab node associated with |tab_id|.
+  // If tab_id is already associated with a tab_node_id, reuses the existing
+  // association. Otherwise attempts to get the next free tab node and
+  // associate it with |tab_id|. If none are available, will create a new tab
+  // node.
+  // Returns true if a pre-existing tab node could be reused, false if a new one
+  // had to be created.
+  bool GetTabNodeForTab(SessionID::id_type tab_id, int* tab_node_id);
 
   // Returns the tab_id for |tab_node_id| if it is associated else returns
   // kInvalidTabID.
   SessionID::id_type GetTabIdFromTabNodeId(int tab_node_id) const;
 
-  // Reassociates |tab_node_id| with |tab_id|. |tab_node_id| must be either
-  // associated with a tab or in the set of unassociated nodes.
+  // Reassociates |tab_node_id| with |tab_id|. If |tab_node_id| is not already
+  // known, it is added to the tab node pool before being associated.
   void ReassociateTabNode(int tab_node_id, SessionID::id_type tab_id);
 
-  // Returns true if |tab_node_id| is an unassociated tab node.
-  bool IsUnassociatedTabNode(int tab_node_id);
+  // Removes association for |tab_id| and returns it to the free node pool.

[skym, 2016/12/03 01:20:46]

... and returns its tab node id to the free pool.

[Nicolas Zea, 2016/12/06 01:32:55]

Done.

+  void FreeTab(int tab_id);
 
-  // Returns any unassociated nodes to the free node pool.
-  // |change_output| *must* be provided. It is the TabNodePool's link to
-  // the SyncChange pipeline that exists in the caller's context.
-  // See FreeTabNode for more detail.
-  void DeleteUnassociatedTabNodes(syncer::SyncChangeList* change_output);
+  // Fills |deleted_node_ids| with any free nodes to be deleted as proscribed
+  // by the free node low/high watermarks, in order to ensure the free node pool
+  // does not grow too large.
+  void CleanupTabNodes(std::set<int>* deleted_node_ids);
 
   // Clear tab pool.
   void Clear();
 
   // Return the number of tab nodes this client currently has allocated
-  // (including both free, unassociated and associated nodes)
+  // (including both free and associated nodes).
   size_t Capacity() const;
 
   // Return empty status (all tab nodes are in use).
@@ -121,41 +81,36 @@ class TabNodePool {
   // Return full status (no tab nodes are in use).
   bool Full();
 
-  void SetMachineTag(const std::string& machine_tag);
-
  private:
   friend class SyncTabNodePoolTest;
   typedef std::map<int, SessionID::id_type> TabNodeIDToTabIDMap;
+  typedef std::map<SessionID::id_type, int> TabIDToTabNodeIDMap;
 
-  // Adds |tab_node_id| to free node pool.
-  // |change_output| *must* be provided. It is the TabNodePool's link to
-  // the SyncChange pipeline that exists in the caller's context.
-  // See FreeTabNode for more detail.
-  void FreeTabNodeInternal(int tab_node_id,
-                           syncer::SyncChangeList* change_output);
+  // Adds |tab_node_id| to the tab node pool.
+  // Note: this should only be called when we discover tab sync nodes from
+  // previous sessions, not for freeing tab nodes we created through
+  // GetTabNodeForTab (use FreeTab for that).
+  void AddTabNode(int tab_node_id);
+
+  // Associates |tab_node_id| with |tab_id|. |tab_node_id| must be free. In
+  // order to associated a non-free tab node, ReassociateTabNode must be
+  // used.
+  void AssociateTabNode(int tab_node_id, SessionID::id_type tab_id);
 
   // Stores mapping of node ids associated with tab_ids, these are the used
   // nodes of tab node pool.
   // The nodes in the map can be returned to free tab node pool by calling
-  // FreeTabNode(tab_node_id).
+  // FreeTab(..).
   TabNodeIDToTabIDMap nodeid_tabid_map_;
+  TabIDToTabNodeIDMap tabid_nodeid_map_;
 
   // The node ids for the set of free sync nodes.
   std::set<int> free_nodes_pool_;
 
-  // The node ids that are added to pool using AddTabNode and are currently
-  // not associated with any tab. They can be reassociated using
-  // ReassociateTabNode.
-  std::set<int> unassociated_nodes_;
-
   // The maximum used tab_node id for a sync node. A new sync node will always
   // be created with max_used_tab_node_id_ + 1.
   int max_used_tab_node_id_;
 
-  // The machine tag associated with this tab pool. Used in the title of new
-  // sync nodes.
-  std::string machine_tag_;
-
   DISALLOW_COPY_AND_ASSIGN(TabNodePool);
 };