Do not submit: high level overview patch.

chrome/browser/sync/glue/tab_node_pool.cc

Index: chrome/browser/sync/glue/tab_node_pool.cc
diff --git a/chrome/browser/sync/glue/tab_node_pool.cc b/chrome/browser/sync/glue/tab_node_pool.cc
index 69ed09f83d70f7746a380af96b6c0beb72d27733..2e21073757f9c23f6cfdd567f140bbb14acb8c24 100644
--- a/chrome/browser/sync/glue/tab_node_pool.cc
+++ b/chrome/browser/sync/glue/tab_node_pool.cc
@@ -4,6 +4,9 @@
 
 #include "chrome/browser/sync/glue/tab_node_pool.h"
 
+#include <algorithm>
+#include <vector>
+
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/stringprintf.h"
@@ -20,24 +23,61 @@ static const char kNoSessionsFolderError[] =
     "Server did not create the top-level sessions node. We "
     "might be running against an out-of-date server.";
 
-TabNodePool::TabNodePool(
-    ProfileSyncService* sync_service)
-    : tab_pool_fp_(-1),
-      sync_service_(sync_service) {
-}
+TabNodePool::TabNodePool(ProfileSyncService* sync_service)
+    : max_used_tab_node_id_(-1),
+      tab_pool_fp_(-1),
+      sync_service_(sync_service) {}
 
 TabNodePool::~TabNodePool() {}
 
 // Static
-std::string TabNodePool::TabIdToTag(
-    const std::string machine_tag,
-    size_t tab_node_id) {
-  return base::StringPrintf("%s %" PRIuS "", machine_tag.c_str(), tab_node_id);
+std::string TabNodePool::TabIdToTag(const std::string machine_tag,
+                                    int32 tab_node_id) {
+  return base::StringPrintf("%s %" PRId32 "", machine_tag.c_str(), tab_node_id);
 }
 
-void TabNodePool::AddTabNode(int64 sync_id) {
-  tab_syncid_pool_.resize(tab_syncid_pool_.size() + 1);
-  tab_syncid_pool_[static_cast<size_t>(++tab_pool_fp_)] = sync_id;
+int64 TabNodePool::GetOldSyncNode(const int64 sync_id) {
+  if (sync_id > 0) {
+    std::vector<int64>::iterator sync_node_iter =
+        std::lower_bound(old_sync_ids.begin(), old_sync_ids.end(), sync_id);
+    if (sync_node_iter != old_sync_ids.end() && sync_id == *sync_node_iter) {
+      old_sync_ids.erase(sync_node_iter);
+      return sync_id;
+    }
+  }
+  return -1;
+}
+
+void TabNodePool::AddTabNode(int64 sync_id, int32 tab_node_id) {
+  DCHECK_GE(sync_id, 0);
+  if (max_used_tab_node_id_ < tab_node_id) {
+    max_used_tab_node_id_ = tab_node_id;
+  }
+
+  old_sync_ids.push_back(sync_id);
+  std::sort(old_sync_ids.begin(), old_sync_ids.end());
+}
+
+void TabNodePool::PurgeOldSyncNodes(std::vector<int64>& used_sync_ids) {
+  std::sort(used_sync_ids.begin(), used_sync_ids.end());
+  std::vector<int64> remaining;
+  syncer::WriteTransaction trans(FROM_HERE, sync_service_->GetUserShare());
+  for (std::vector<int64>::iterator i = old_sync_ids.begin();
+       i != old_sync_ids.end();
+       ++i) {
+    if (std::lower_bound(used_sync_ids.begin(), used_sync_ids.end(), *i) ==
+        used_sync_ids.end()) {
+      // Delete the node.
+      syncer::WriteNode tab_node(&trans);
+      if (tab_node.InitByIdLookup(*i) == syncer::BaseNode::INIT_OK) {
+        tab_node.Tombstone();

[Nicolas Zea, 2013/05/16 22:59:03]

shouldn't these nodes be added to the tab_syncid_pool? Otherwise you'll
constantly be creating new nodes with ever increasing ids, and losing the ids of
old nodes right?

[shashi, 2013/05/17 00:29:16]

Yes, they should be returned to the free tab pool., this is a remnant from my
prototype. I was debugging and found it easier to just delete the sync node.
Btw should we limit the number of free nodes. 
Can there be scenarios like crashes, when we lose tabs and keep adding back to
the free tab pool. Should we limit adding to free tab pool, if we have say 25
free tab nodes?

On 2013/05/16 22:59:03, Nicolas Zea wrote:

+      }
+    } else {
+      remaining.push_back(*i);
+    }
+  }
+  old_sync_ids.clear();
+  old_sync_ids.swap(remaining);
 }
 
 int64 TabNodePool::GetFreeTabNode() {
@@ -51,7 +91,7 @@ int64 TabNodePool::GetFreeTabNode() {
       LOG(ERROR) << kNoSessionsFolderError;
       return syncer::kInvalidId;
     }
-    size_t tab_node_id = tab_syncid_pool_.size();
+    int32 tab_node_id = ++max_used_tab_node_id_;
     std::string tab_node_tag = TabIdToTag(machine_tag_, tab_node_id);
     syncer::WriteNode tab_node(&trans);
     syncer::WriteNode::InitUniqueByCreationResult result =
@@ -72,20 +112,23 @@ int64 TabNodePool::GetFreeTabNode() {
     // Grow the pool by 1 since we created a new node. We don't actually need
     // to put the node's id in the pool now, since the pool is still empty.
     // The id will be added when that tab is closed and the node is freed.
-    tab_syncid_pool_.resize(tab_node_id + 1);
-    DVLOG(1) << "Adding sync node "
-             << tab_node.GetId() << " to tab syncid pool";
+    tab_syncid_pool_.resize(tab_syncid_pool_.size() + 1);
+    DVLOG(1) << "Adding sync node " << tab_node.GetId()
+             << " to tab syncid pool";
     return tab_node.GetId();
   } else {
     // There are nodes available, grab next free and decrement free pointer.
-    return tab_syncid_pool_[static_cast<size_t>(tab_pool_fp_--)];
+    return tab_syncid_pool_[tab_pool_fp_--];
   }
 }
 
 void TabNodePool::FreeTabNode(int64 sync_id) {
   // Pool size should always match # of free tab nodes.
   DCHECK_LT(tab_pool_fp_, static_cast<int64>(tab_syncid_pool_.size()));
+  DCHECK_GE(max_used_tab_node_id_ + 1,
+            static_cast<int32>(tab_syncid_pool_.size()));
   tab_syncid_pool_[static_cast<size_t>(++tab_pool_fp_)] = sync_id;
+
 }
 
 }  // namespace browser_sync