[Sync] Add logic to reassociate tab nodes after restart.

chrome/browser/sync/glue/tab_node_pool.cc

 // 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.
 
 #include "chrome/browser/sync/glue/tab_node_pool.h"
 
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/read_node.h"
 #include "sync/internal_api/public/write_node.h"
 #include "sync/internal_api/public/write_transaction.h"
 
 namespace browser_sync {
 
 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) {
-}
+      max_used_tab_node_id_(0),
+      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);
 }
 
-void TabNodePool::AddTabNode(int64 sync_id) {
+void TabNodePool::AddOldTabNode(int64 sync_id, size_t tab_node_id) {
+  DCHECK_GT(sync_id, 0);
+  DCHECK(old_node_syncids_.find(sync_id) == old_node_syncids_.end());
+  if (max_used_tab_node_id_ < tab_node_id)
+    max_used_tab_node_id_ = tab_node_id;
+  // Grow the tab node pool by 1, to ensure tab pool is of correct size.
   tab_syncid_pool_.resize(tab_syncid_pool_.size() + 1);
-  tab_syncid_pool_[static_cast<size_t>(++tab_pool_fp_)] = sync_id;
+  old_node_syncids_.insert(sync_id);

[Nicolas Zea, 2013/06/06 21:27:39]

I'm not sure we really need to have the notion of "old tab nodes" at all. Could
you not track all these old syncids as just free nodes within the tab syncid
pool (although this might switching from vector to something else)

The general flow could be:
AddTabNode // adds old node to sync id pool, but doesn't track it as associated
AssociateTabNode  // marks it as associated, and therefore not free
FreeUnusuedTabNodes  // free's all non-associated nodes
GetFreeTabNode  // returns next free node, or creates one as necessary

[shashi, 2013/06/07 01:44:08]

In case of Android we have two requirements:
1. Old tab nodes must not be deleted.
2. Less obvious, old tab nodes should get reassociated with the *same* old sync
node.

Requirement 1 should be obvious, let me explain reason for requirement 2:

On Android some tabs can be without webcontents. For tabs without webcontents
(TWW) we do not have data to write tabs contents to sync model. Instead we use a
trick, we let the old sync nodes associated with those tabs to be unchanged. 

Now consider this from the point of view of a remote client.
Remote client views that the sync nodes associated with TWW are unchanged, it
will display TWW as a regular tab.
For tabs which have webcontents any free node will work, because SMA will
rewrite the tab contents to the sync node (in SMA::WriteTabContentsToSyncModel).


Keeping track of old sync ids is simply to make sure that we do not accidentally
overwrite a sync node associated with TWW.

On 2013/06/06 21:27:39, Nicolas Zea wrote:

[Nicolas Zea, 2013/06/07 14:42:28]

Right, and I agree with that approach. My point is that (I think) the API itself
doesn't need to reflect this. From an external point of view there are the
following categories of nodes:
1. Those associated with an active tab (associated nodes)
2. Those not associated with an active tab (free nodes)

All nodes have a "last associated tab node id", but start off as free on any
particular restart. The associator, at association time, tells the tab pool
which of the free nodes are in fact associated. Previously it didn't care about
associating with a particular node, so GetFreeTabNode was enough. Now, we do, so
a specific AssociateTabNode(sync_id) method is probably necessary.

But, given some sort of AssociateTabNode method, it's not clear to me that the
notion of "old" nodes really needs to be exposed.

[shashi, 2013/06/07 19:08:35]

Got it, I am going to cleanup the API as per the suggestion.
On 2013/06/07 14:42:28, Nicolas Zea wrote:

 }
 
 int64 TabNodePool::GetFreeTabNode() {
   DCHECK_GT(machine_tag_.length(), 0U);
   if (tab_pool_fp_ == -1) {
     // Tab pool has no free nodes, allocate new one.
     syncer::WriteTransaction trans(FROM_HERE, sync_service_->GetUserShare());
     syncer::ReadNode root(&trans);
     if (root.InitByTagLookup(syncer::ModelTypeToRootTag(syncer::SESSIONS)) !=
                              syncer::BaseNode::INIT_OK) {
       LOG(ERROR) << kNoSessionsFolderError;
       return syncer::kInvalidId;
     }
-    size_t tab_node_id = tab_syncid_pool_.size();
+    size_t 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 =
         tab_node.InitUniqueByCreation(syncer::SESSIONS, root, tab_node_tag);
     if (result != syncer::WriteNode::INIT_SUCCESS) {
       LOG(ERROR) << "Could not create new node with tag "
                  << tab_node_tag << "!";
       return syncer::kInvalidId;
     }
     // We fill the new node with just enough data so that in case of a crash/bug
     // we can identify the node as our own on re-association and reuse it.
     tab_node.SetTitle(UTF8ToWide(tab_node_tag));
     sync_pb::SessionSpecifics specifics;
     specifics.set_session_tag(machine_tag_);
     specifics.set_tab_node_id(tab_node_id);
     tab_node.SetSessionSpecifics(specifics);
 
     // 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_--)];
   }
 }
 
 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()));
   tab_syncid_pool_[static_cast<size_t>(++tab_pool_fp_)] = sync_id;
 }
 
+bool TabNodePool::RemoveIfOldSyncNodeExists(int64 sync_id) {
+  std::set<int64>::iterator position = old_node_syncids_.find(sync_id);
+  if (position != old_node_syncids_.end()) {
+    // If sync_id exists remove it and return.
+    old_node_syncids_.erase(position);
+    return true;
+  }
+  return false;
+}
+
+void TabNodePool::FreeUnusedOldSyncNodes(const std::set<int64>& used_sync_ids) {
+  for (std::set<int64>::iterator it = old_node_syncids_.begin();
+       it != old_node_syncids_.end();) {
+    if (used_sync_ids.find(*it) == used_sync_ids.end()) {
+      FreeTabNode(*it);
+      old_node_syncids_.erase(it++);
+    } else {
+      ++it;
+    }
+  }
+}
+
 }  // namespace browser_sync