Revert 213872 "Revert 213589 to reland 213396, which re-lands 21..."

Causing DCHECK failure
[26974:26974:0730/185024:FATAL:synced_session_tracker.cc(297)] Check failed: TabNodePool::kInvalidTabNodeID != tab_node_id (-1 vs. -1)

> Revert 213589 to reland 213396, which re-lands 212635 - Track tab node IDs in SyncedSessionTracker.
> 
> Only difference to 213396 is that this also includes: https://codereview.chromium.org/20239002, removing a DCHECK from original cl that landed as 212635.
> 
> TBR=pkasting@chromium.org
> 
> BUG=98892, 80194, 263999
> 
> Review URL: https://chromiumcodereview.appspot.com/20529002

TBR=tim@chromium.org

Review URL: https://codereview.chromium.org/21231002

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

11 files changed, 170 insertions, 358 deletions

diff --git a/chrome/browser/sync/glue/session_model_associator.cc b/chrome/browser/sync/glue/session_model_associator.cc
index a4e8f60..777fa96 100644
--- a/chrome/browser/sync/glue/session_model_associator.cc
+++ b/chrome/browser/sync/glue/session_model_associator.cc
@@ -114,7 +114,7 @@ static const int kMaxSyncFavicons = 200;
 SessionModelAssociator::SessionModelAssociator(
     ProfileSyncService* sync_service,
     DataTypeErrorHandler* error_handler)
-    : local_tab_pool_(sync_service),
+    : tab_pool_(sync_service),
       local_session_syncid_(syncer::kInvalidId),
       sync_service_(sync_service),
       stale_session_threshold_days_(kDefaultStaleSessionThresholdDays),
@@ -132,7 +132,7 @@ SessionModelAssociator::SessionModelAssociator(
 
 SessionModelAssociator::SessionModelAssociator(ProfileSyncService* sync_service,
                                                bool setup_for_test)
-    : local_tab_pool_(sync_service),
+    : tab_pool_(sync_service),
       local_session_syncid_(syncer::kInvalidId),
       sync_service_(sync_service),
       stale_session_threshold_days_(kDefaultStaleSessionThresholdDays),
@@ -286,7 +286,7 @@ bool SessionModelAssociator::AssociateWindows(bool reload_tabs,
   }
 
   // Free old sync nodes.
-  local_tab_pool_.FreeUnassociatedTabNodes();
+  tab_pool_.FreeUnassociatedTabNodes();
   // Free memory for closed windows and tabs.
   synced_session_tracker_.CleanupSession(local_tag);
 
@@ -337,27 +337,27 @@ bool SessionModelAssociator::AssociateTab(SyncedTabDelegate* const tab,
   SessionID::id_type tab_id = tab->GetSessionId();
   if (tab->IsBeingDestroyed()) {
     // This tab is closing.
-    TabLinksMap::iterator tab_iter = local_tab_map_.find(tab_id);
-    if (tab_iter == local_tab_map_.end()) {
+    TabLinksMap::iterator tab_iter = tab_map_.find(tab_id);
+    if (tab_iter == tab_map_.end()) {
       // We aren't tracking this tab (for example, sync setting page).
       return true;
     }
-    local_tab_pool_.FreeTabNode(tab_iter->second->sync_id());
-    local_tab_map_.erase(tab_iter);
+    tab_pool_.FreeTabNode(tab_iter->second->sync_id());
+    tab_map_.erase(tab_iter);
     return true;
   }
 
   if (!ShouldSyncTab(*tab))
     return true;
 
-  TabLinksMap::iterator local_tab_map_iter = local_tab_map_.find(tab_id);
+  TabLinksMap::iterator tab_map_iter = tab_map_.find(tab_id);
   TabLink* tab_link = NULL;
-  if (local_tab_map_iter == local_tab_map_.end()) {
+  if (tab_map_iter == tab_map_.end()) {
     sync_id = tab->GetSyncId();
     // if there is an old sync node for the tab, reuse it.
-    if (!local_tab_pool_.IsUnassociatedTabNode(sync_id)) {
+    if (!tab_pool_.IsUnassociatedTabNode(sync_id)) {
       // This is a new tab, get a sync node for it.
-      sync_id = local_tab_pool_.GetFreeTabNode();
+      sync_id = tab_pool_.GetFreeTabNode();
       if (sync_id == syncer::kInvalidId) {
         if (error) {
           *error = error_handler_->CreateAndUploadError(
@@ -369,15 +369,15 @@ bool SessionModelAssociator::AssociateTab(SyncedTabDelegate* const tab,
       }
       tab->SetSyncId(sync_id);
     }
-    local_tab_pool_.AssociateTabNode(sync_id, tab_id);
+    tab_pool_.AssociateTabNode(sync_id, tab_id);
     tab_link = new TabLink(sync_id, tab);
-    local_tab_map_[tab_id] = make_linked_ptr<TabLink>(tab_link);
+    tab_map_[tab_id] = make_linked_ptr<TabLink>(tab_link);
   } else {
     // This tab is already associated with a sync node, reuse it.
     // Note: on some platforms the tab object may have changed, so we ensure
     // the tab link is up to date.
-    tab_link = local_tab_map_iter->second.get();
-    local_tab_map_iter->second->set_tab(tab);
+    tab_link = tab_map_iter->second.get();
+    tab_map_iter->second->set_tab(tab);
   }
   DCHECK(tab_link);
   DCHECK_NE(tab_link->sync_id(), syncer::kInvalidId);
@@ -420,50 +420,20 @@ bool SessionModelAssociator::WriteTabContentsToSyncModel(
   const SyncedTabDelegate& tab_delegate = *(tab_link->tab());
   int64 sync_id = tab_link->sync_id();
   GURL old_tab_url = tab_link->url();
-  const GURL new_url = GetCurrentVirtualURL(tab_delegate);
-  DVLOG(1) << "Local tab " << tab_delegate.GetSessionId()
-           << " now has URL " << new_url.spec();
 
-  SessionTab* session_tab = NULL;
-  {
-    syncer::WriteTransaction trans(FROM_HERE, sync_service_->GetUserShare());
-    syncer::WriteNode tab_node(&trans);
-    if (tab_node.InitByIdLookup(sync_id) != syncer::BaseNode::INIT_OK) {
-      if (error) {
-        *error = error_handler_->CreateAndUploadError(
-            FROM_HERE,
-            "Failed to look up local tab node",
-            model_type());
-      }
-      return false;
-    }
+  // Load the last stored version of this tab so we can compare changes. If this
+  // is a new tab, session_tab will be a blank/newly created SessionTab object.
+  SessionTab* session_tab =
+      synced_session_tracker_.GetTab(GetCurrentMachineTag(),
+                                     tab_delegate.GetSessionId());
 
-    // Load the last stored version of this tab so we can compare changes. If
-    // this is a new tab, session_tab will be a new, blank SessionTab object.
-    sync_pb::SessionSpecifics specifics = tab_node.GetSessionSpecifics();
-    session_tab =
-        synced_session_tracker_.GetTab(GetCurrentMachineTag(),
-                                       tab_delegate.GetSessionId(),
-                                       specifics.tab_node_id());
-    SetSessionTabFromDelegate(tab_delegate, base::Time::Now(), session_tab);
-    sync_pb::SessionTab tab_s = session_tab->ToSyncData();
-
-    if (new_url == old_tab_url) {
-      // Load the old specifics and copy over the favicon data if needed.
-      // TODO(zea): remove this once favicon sync is enabled as a separate type.
-      tab_s.set_favicon(specifics.tab().favicon());
-      tab_s.set_favicon_source(specifics.tab().favicon_source());
-      tab_s.set_favicon_type(specifics.tab().favicon_type());
-    }
-    // Retain the base SessionSpecifics data (tag, tab_node_id, etc.), and just
-    // write the new SessionTabSpecifics.
-    specifics.mutable_tab()->CopyFrom(tab_s);
+  SetSessionTabFromDelegate(tab_delegate, base::Time::Now(), session_tab);
 
-    // Write into the actual sync model.
-    tab_node.SetSessionSpecifics(specifics);
-  }
+  const GURL new_url = GetCurrentVirtualURL(tab_delegate);
+  DVLOG(1) << "Local tab " << tab_delegate.GetSessionId()
+           << " now has URL " << new_url.spec();
 
-  // Trigger the favicon load if needed. We do this outside the write
+  // Trigger the favicon load if needed. We do this before opening the write
   // transaction to avoid jank.
   tab_link->set_url(new_url);
   if (new_url != old_tab_url) {
@@ -475,6 +445,34 @@ bool SessionModelAssociator::WriteTabContentsToSyncModel(
   synced_session_tracker_.GetSession(GetCurrentMachineTag())->modified_time =
       base::Time::Now();
 
+  syncer::WriteTransaction trans(FROM_HERE, sync_service_->GetUserShare());
+  syncer::WriteNode tab_node(&trans);
+  if (tab_node.InitByIdLookup(sync_id) != syncer::BaseNode::INIT_OK) {
+    if (error) {
+      *error = error_handler_->CreateAndUploadError(
+          FROM_HERE,
+          "Failed to look up local tab node",
+          model_type());
+    }
+    return false;
+  }
+
+  sync_pb::SessionTab tab_s = session_tab->ToSyncData();
+  sync_pb::SessionSpecifics specifics = tab_node.GetSessionSpecifics();
+  if (new_url == old_tab_url) {
+    // Load the old specifics and copy over the favicon data if needed.
+    // TODO(zea): remove this once favicon sync is enabled as a separate type.
+    tab_s.set_favicon(specifics.tab().favicon());
+    tab_s.set_favicon_source(specifics.tab().favicon_source());
+    tab_s.set_favicon_type(specifics.tab().favicon_type());
+  }
+  // Retain the base SessionSpecifics data (tag, tab_node_id, etc.), and just
+  // write the new SessionTabSpecifics.
+  specifics.mutable_tab()->CopyFrom(tab_s);
+
+  // Write into the actual sync model.
+  tab_node.SetSessionSpecifics(specifics);
+
   return true;
 }
 
@@ -535,8 +533,8 @@ void SessionModelAssociator::FaviconsUpdated(
   // TODO(zea): consider a separate container for tabs with outstanding favicon
   // loads so we don't have to iterate through all tabs comparing urls.
   for (std::set<GURL>::const_iterator i = urls.begin(); i != urls.end(); ++i) {
-    for (TabLinksMap::iterator tab_iter = local_tab_map_.begin();
-         tab_iter != local_tab_map_.end();
+    for (TabLinksMap::iterator tab_iter = tab_map_.begin();
+         tab_iter != tab_map_.end();
          ++tab_iter) {
       if (tab_iter->second->url() == *i)
         favicon_cache_.OnPageFaviconUpdated(*i);
@@ -552,7 +550,7 @@ syncer::SyncError SessionModelAssociator::AssociateModels(
 
   // Ensure that we disassociated properly, otherwise memory might leak.
   DCHECK(synced_session_tracker_.Empty());
-  DCHECK_EQ(0U, local_tab_pool_.Capacity());
+  DCHECK_EQ(0U, tab_pool_.Capacity());
 
   local_session_syncid_ = syncer::kInvalidId;
 
@@ -646,8 +644,8 @@ syncer::SyncError SessionModelAssociator::DisassociateModels() {
   DCHECK(CalledOnValidThread());
   DVLOG(1) << "Disassociating local session " << GetCurrentMachineTag();
   synced_session_tracker_.Clear();
-  local_tab_map_.clear();
-  local_tab_pool_.Clear();
+  tab_map_.clear();
+  tab_pool_.Clear();
   local_session_syncid_ = syncer::kInvalidId;
   current_machine_tag_ = "";
   current_session_name_ = "";
@@ -678,7 +676,7 @@ void SessionModelAssociator::InitializeCurrentMachineTag(
     prefs.SetSyncSessionsGUID(current_machine_tag_);
   }
 
-  local_tab_pool_.SetMachineTag(current_machine_tag_);
+  tab_pool_.SetMachineTag(current_machine_tag_);
 }
 
 bool SessionModelAssociator::GetSyncedFaviconForPageURL(
@@ -699,7 +697,7 @@ bool SessionModelAssociator::UpdateAssociationsFromSyncModel(
     syncer::WriteTransaction* trans,
     syncer::SyncError* error) {
   DCHECK(CalledOnValidThread());
-  DCHECK(local_tab_pool_.Empty());
+  DCHECK(tab_pool_.Empty());
   DCHECK_EQ(local_session_syncid_, syncer::kInvalidId);
 
   // Iterate through the nodes and associate any foreign sessions.
@@ -745,7 +743,7 @@ bool SessionModelAssociator::UpdateAssociationsFromSyncModel(
           // reused for reassociation.
           SessionID tab_id;
           tab_id.set_id(specifics.tab().tab_id());
-          local_tab_pool_.AddTabNode(id, tab_id, specifics.tab_node_id());
+          tab_pool_.AddTabNode(id, tab_id, specifics.tab_node_id());
         }
       }
     }
@@ -803,9 +801,7 @@ void SessionModelAssociator::AssociateForeignSpecifics(
     const sync_pb::SessionTab& tab_s = specifics.tab();
     SessionID::id_type tab_id = tab_s.tab_id();
     SessionTab* tab =
-        synced_session_tracker_.GetTab(foreign_session_tag,
-                                       tab_id,
-                                       specifics.tab_node_id());
+        synced_session_tracker_.GetTab(foreign_session_tag, tab_id);
 
     // Update SessionTab based on protobuf.
     tab->SetFromSyncData(tab_s, modification_time);
@@ -1157,7 +1153,7 @@ void SessionModelAssociator::UpdateTabIdIfNecessary(
     int64 sync_id,
     SessionID::id_type new_tab_id) {
   DCHECK_NE(sync_id, syncer::kInvalidId);
-  SessionID::id_type old_tab_id = local_tab_pool_.GetTabIdFromSyncId(sync_id);
+  SessionID::id_type old_tab_id = tab_pool_.GetTabIdFromSyncId(sync_id);
   if (old_tab_id != new_tab_id) {
     // Rewrite tab id if required.
     syncer::WriteTransaction trans(FROM_HERE, sync_service_->GetUserShare());
@@ -1171,7 +1167,7 @@ void SessionModelAssociator::UpdateTabIdIfNecessary(
         tab_s->set_tab_id(new_tab_id);
         tab_node.SetSessionSpecifics(session_specifics);
         // Update tab node pool with the new association.
-        local_tab_pool_.ReassociateTabNode(sync_id, new_tab_id);
+        tab_pool_.ReassociateTabNode(sync_id, new_tab_id);
       }
     }
   }
diff --git a/chrome/browser/sync/glue/session_model_associator.h b/chrome/browser/sync/glue/session_model_associator.h
index d8b1ff5..5f46168 100644
--- a/chrome/browser/sync/glue/session_model_associator.h
+++ b/chrome/browser/sync/glue/session_model_associator.h
@@ -182,7 +182,7 @@ class SessionModelAssociator
   // entries.
   bool ShouldSyncTab(const SyncedTabDelegate& tab) const;
 
-  // Compare |urls| against |local_tab_map_|'s urls to see if any tabs with
+  // Compare |urls| against |tab_map_|'s urls to see if any tabs with
   // outstanding favicon loads can be fulfilled.
   void FaviconsUpdated(const std::set<GURL>& urls);
 
@@ -343,15 +343,15 @@ class SessionModelAssociator
   // User-visible machine name.
   std::string current_session_name_;
 
-  // Pool of all used/available sync nodes associated with local tabs.
-  TabNodePool local_tab_pool_;
+  // Pool of all used/available sync nodes associated with tabs.
+  TabNodePool tab_pool_;
 
   // 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 local_tab_map_;
+  TabLinksMap tab_map_;
 
   SyncedSessionTracker synced_session_tracker_;
 
diff --git a/chrome/browser/sync/glue/synced_session_tracker.cc b/chrome/browser/sync/glue/synced_session_tracker.cc
index 402800c..7499adb 100644
--- a/chrome/browser/sync/glue/synced_session_tracker.cc
+++ b/chrome/browser/sync/glue/synced_session_tracker.cc
@@ -87,23 +87,6 @@ bool SyncedSessionTracker::LookupSessionTab(
   return true;
 }
 
-bool SyncedSessionTracker::LookupTabNodeIds(
-    const std::string& session_tag, std::set<int>* tab_node_ids) {
-  tab_node_ids->clear();
-  SyncedTabMap::const_iterator tab_map_iter =
-      synced_tab_map_.find(session_tag);
-  if (tab_map_iter == synced_tab_map_.end())
-    return false;
-
-  IDToSessionTabMap::const_iterator tab_iter = tab_map_iter->second.begin();
-  while (tab_iter != tab_map_iter->second.end()) {
-    if (tab_iter->second.tab_node_id != TabNodePool::kInvalidTabNodeID)
-      tab_node_ids->insert(tab_iter->second.tab_node_id);
-    ++tab_iter;
-  }
-  return true;
-}
-
 SyncedSession* SyncedSessionTracker::GetSession(
     const std::string& session_tag) {
   SyncedSession* synced_session = NULL;
@@ -242,7 +225,7 @@ void SyncedSessionTracker::PutWindowInSession(const std::string& session_tag,
     window_ptr = new SessionWindow();
     window_ptr->window_id.set_id(window_id);
     synced_window_map_[session_tag][window_id] =
-        SessionWindowWrapper(window_ptr, IS_OWNED);
+        SessionWindowWrapper(window_ptr, true);
     DVLOG(1) << "Putting new window " << window_id  << " at " << window_ptr
              << "in " << (session_tag == local_session_tag_ ?
                           "local session" : session_tag);
@@ -258,23 +241,7 @@ void SyncedSessionTracker::PutTabInWindow(const std::string& session_tag,
                                           SessionID::id_type window_id,
                                           SessionID::id_type tab_id,
                                           size_t tab_index) {
-  // We're called here for two reasons. 1) We've received an update to the
-  // SessionWindow information of a SessionHeader node for a foreign session,
-  // and 2) The SessionHeader node for our local session changed. In both cases
-  // we need to update our tracking state to reflect the change.
-  //
-  // Because the SessionHeader nodes are separate from the individual tab nodes
-  // and we don't store tab_node_ids in the header / SessionWindow specifics,
-  // the tab_node_ids are not always available when processing headers.
-  // We know that we will eventually process (via GetTab) every single tab node
-  // in the system, so we permit ourselves to use kInvalidTabNodeID here and
-  // rely on the later update to build the mapping (or a restart).
-  // TODO(tim): Bug 98892. Update comment when Sync API conversion finishes to
-  // mention that in the meantime, the only ill effect is that we may not be
-  // able to fully clean up a stale foreign session, but it will get garbage
-  // collected eventually.
-  SessionTab* tab_ptr = GetTabImpl(
-      session_tag, tab_id, TabNodePool::kInvalidTabNodeID);
+  SessionTab* 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);
@@ -292,47 +259,12 @@ void SyncedSessionTracker::PutTabInWindow(const std::string& session_tag,
 
 SessionTab* SyncedSessionTracker::GetTab(
     const std::string& session_tag,
-    SessionID::id_type tab_id,
-    int tab_node_id) {
-  DCHECK_NE(TabNodePool::kInvalidTabNodeID, tab_node_id);
-  return GetTabImpl(session_tag, tab_id, tab_node_id);
-}
-
-SessionTab* SyncedSessionTracker::GetTabImpl(
-    const std::string& session_tag,
-    SessionID::id_type tab_id,
-    int tab_node_id) {
+    SessionID::id_type tab_id) {
   SessionTab* tab_ptr = NULL;
   IDToSessionTabMap::iterator iter =
       synced_tab_map_[session_tag].find(tab_id);
   if (iter != synced_tab_map_[session_tag].end()) {
     tab_ptr = iter->second.tab_ptr;
-    if (tab_node_id != TabNodePool::kInvalidTabNodeID &&
-        tab_id != TabNodePool::kInvalidTabID) {
-      // We likely created this tab as an out-of-order update to the header
-      // node for this session before actually associating the tab itself, so
-      // the tab node id wasn't available at the time.  Update it.
-
-      // TODO(shashishekhar): Make sure the following constraint works.
-      // All nodes either have unique tab_ids or have kInvalidTabID as their
-      // tab_id. There can be multiple nodes associated with kInvalidTabID but
-      // there is always one-to-one mapping between valid tab_ids and
-      // tab_node_ids.
-
-      if (iter->second.tab_node_id != tab_node_id &&
-          iter->second.tab_node_id != TabNodePool::kInvalidTabNodeID) {
-        // We are updating tab_node_id for a valid tab_id, ideally this should
-        // never happen, but there are a few existing foreign sessions that
-        // may violate this constraint.
-        // TODO(shashishekhar): Introduce a DCHECK here to enforce this
-        // constraint in future.
-        DLOG(ERROR)
-            << "Updating tab_node_id for " << session_tag << " tab: " << tab_id
-            << " from: " << iter->second.tab_node_id << " to: " << tab_node_id;
-      }
-      iter->second.tab_node_id = tab_node_id;
-    }
-
     if (VLOG_IS_ON(1)) {
       std::string title;
       if (tab_ptr->navigations.size() > 0) {
@@ -347,9 +279,7 @@ SessionTab* SyncedSessionTracker::GetTabImpl(
   } else {
     tab_ptr = new SessionTab();
     tab_ptr->tab_id.set_id(tab_id);
-    synced_tab_map_[session_tag][tab_id] = SessionTabWrapper(tab_ptr,
-                                                             NOT_OWNED,
-                                                             tab_node_id);
+    synced_tab_map_[session_tag][tab_id] = SessionTabWrapper(tab_ptr, false);
     unmapped_tabs_.insert(tab_ptr);
     DVLOG(1) << "Getting "
              << (session_tag == local_session_tag_ ?
diff --git a/chrome/browser/sync/glue/synced_session_tracker.h b/chrome/browser/sync/glue/synced_session_tracker.h
index 4ef7db2..782a257 100644
--- a/chrome/browser/sync/glue/synced_session_tracker.h
+++ b/chrome/browser/sync/glue/synced_session_tracker.h
@@ -15,7 +15,6 @@
 #include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/sessions/session_types.h"
 #include "chrome/browser/sync/glue/synced_session.h"
-#include "chrome/browser/sync/glue/tab_node_pool.h"
 
 namespace browser_sync {
 
@@ -104,23 +103,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.
-  // |tab_node_id| must be a valid node id for the node backing this tab.
   SessionTab* GetTab(const std::string& session_tag,
-                     SessionID::id_type tab_id,
-                     int tab_node_id);
-
-  // Fills |tab_node_ids| with the tab node ids (see GetTab) for all the tabs*
-  // associated with the session having tag |session_tag|.
-  // Returns false if we don't have any record of the session.  If no tabs were
-  // found as part of the session, the return value will be true but
-  // |tab_node_ids| will be empty.
-  //
-  // * - note that this only returns the ids we're aware of; it's possible we
-  // don't have the latest tab state from a foreign session and it's also
-  // possible we just haven't updated the tab_node_id for a tab yet, so the
-  // result list should not be treated as authoritative.
-  bool LookupTabNodeIds(const std::string& session_tag,
-                        std::set<int>* tab_node_ids);
+                     SessionID::id_type tab_id);
 
   // Free the memory for all dynamically allocated objects and clear the
   // tracking structures.
@@ -152,42 +136,22 @@ class SyncedSessionTracker {
   // Note, we pair pointers with bools so that we can track what is owned and
   // what can be deleted (see ResetSessionTracking(..) and CleanupSession(..)
   // above).
-  // The wrappers also serve as a convenient place to augment state stored in
-  // SessionTab for sync purposes, such as |tab_node_id|.
-  // IsOwned is used as a wrapper constructor parameter for readability.
-  enum OwnedState {
-    IS_OWNED,
-    NOT_OWNED
-  };
   struct SessionTabWrapper {
-    SessionTabWrapper() : tab_ptr(NULL),
-                          owned(false),
-                          tab_node_id(TabNodePool::kInvalidTabNodeID) {}
-    SessionTabWrapper(SessionTab* tab_ptr, OwnedState owned, int tab_node_id)
+    SessionTabWrapper() : tab_ptr(NULL), owned(false) {}
+    SessionTabWrapper(SessionTab* tab_ptr, bool owned)
         : tab_ptr(tab_ptr),
-          owned(owned == IS_OWNED),
-          tab_node_id(tab_node_id) {}
+          owned(owned) {}
     SessionTab* tab_ptr;
-
-    // This is used as part of a mark-and-sweep approach to garbage
-    // collection for closed tabs that are no longer "in use", or "owned".
-    // ResetSessionTracking will clear |owned| bits, and if it is not claimed
-    // by a window by the time CleanupSession is called it will be deleted.
     bool owned;
-
-    // This lets us identify the sync node that is "backing" this tab in the
-    // sync model, whether it is part of a local or foreign session. The
-    // "tab node id" is described in session_specifics.proto.
-    int tab_node_id;
   };
   typedef std::map<SessionID::id_type, SessionTabWrapper> IDToSessionTabMap;
   typedef std::map<std::string, IDToSessionTabMap> SyncedTabMap;
 
   struct SessionWindowWrapper {
     SessionWindowWrapper() : window_ptr(NULL), owned(false) {}
-    SessionWindowWrapper(SessionWindow* window_ptr, OwnedState owned)
+    SessionWindowWrapper(SessionWindow* window_ptr, bool owned)
         : window_ptr(window_ptr),
-          owned(owned == IS_OWNED) {}
+          owned(owned) {}
     SessionWindow* window_ptr;
     bool owned;
   };
@@ -201,11 +165,6 @@ class SyncedSessionTracker {
   bool DeleteOldSessionWindowIfNecessary(SessionWindowWrapper window_wrapper);
   bool DeleteOldSessionTabIfNecessary(SessionTabWrapper tab_wrapper);
 
-  // Implementation for GetTab(...) above, permits invalid tab_node_id.
-  SessionTab* GetTabImpl(const std::string& session_tag,
-                         SessionID::id_type tab_id,
-                         int tab_node_id);
-
   // Per client mapping of tab id's to their SessionTab objects.
   // Key: session tag.
   // Value: Tab id to SessionTabWrapper map.
diff --git a/chrome/browser/sync/glue/synced_session_tracker_unittest.cc b/chrome/browser/sync/glue/synced_session_tracker_unittest.cc
index 7b50cbd..64187dc 100644
--- a/chrome/browser/sync/glue/synced_session_tracker_unittest.cc
+++ b/chrome/browser/sync/glue/synced_session_tracker_unittest.cc
@@ -29,8 +29,8 @@ TEST_F(SyncedSessionTrackerTest, GetSession) {
 
 TEST_F(SyncedSessionTrackerTest, GetTabUnmapped) {
   SyncedSessionTracker tracker;
-  SessionTab* tab = tracker.GetTab("tag", 0, 0);
-  ASSERT_EQ(tab, tracker.GetTab("tag", 0, 0));
+  SessionTab* tab = tracker.GetTab("tag", 0);
+  ASSERT_EQ(tab, tracker.GetTab("tag", 0));
   // Should clean up memory on its own.
 }
 
@@ -49,7 +49,7 @@ TEST_F(SyncedSessionTrackerTest, PutTabInWindow) {
   SyncedSession* session = tracker.GetSession("tag");
   ASSERT_EQ(1U, session->windows.size());
   ASSERT_EQ(1U, session->windows[10]->tabs.size());
-  ASSERT_EQ(tracker.GetTab("tag", 15, 1), session->windows[10]->tabs[0]);
+  ASSERT_EQ(tracker.GetTab("tag", 15), session->windows[10]->tabs[0]);
   // Should clean up memory on its own.
 }
 
@@ -61,7 +61,7 @@ TEST_F(SyncedSessionTrackerTest, LookupAllForeignSessions) {
   tracker.GetSession("tag2");
   tracker.PutWindowInSession("tag1", 0);
   tracker.PutTabInWindow("tag1", 0, 15, 0);
-  SessionTab* tab = tracker.GetTab("tag1", 15, 1);
+  SessionTab* tab = tracker.GetTab("tag1", 15);
   ASSERT_TRUE(tab);
   tab->navigations.push_back(
       sessions::SerializedNavigationEntryTestHelper::CreateNavigation(
@@ -110,16 +110,16 @@ TEST_F(SyncedSessionTrackerTest, Complex) {
   ASSERT_TRUE(tracker.Empty());
   ASSERT_EQ(0U, tracker.num_synced_sessions());
   ASSERT_EQ(0U, tracker.num_synced_tabs(tag1));
-  tabs1.push_back(tracker.GetTab(tag1, 0, 0));
-  tabs1.push_back(tracker.GetTab(tag1, 1, 1));
-  tabs1.push_back(tracker.GetTab(tag1, 2, 2));
+  tabs1.push_back(tracker.GetTab(tag1, 0));
+  tabs1.push_back(tracker.GetTab(tag1, 1));
+  tabs1.push_back(tracker.GetTab(tag1, 2));
   ASSERT_EQ(3U, tracker.num_synced_tabs(tag1));
   ASSERT_EQ(0U, tracker.num_synced_sessions());
-  temp_tab = tracker.GetTab(tag1, 0, 0);  // Already created.
+  temp_tab = tracker.GetTab(tag1, 0);  // Already created.
   ASSERT_EQ(3U, tracker.num_synced_tabs(tag1));
   ASSERT_EQ(0U, tracker.num_synced_sessions());
   ASSERT_EQ(tabs1[0], temp_tab);
-  tabs2.push_back(tracker.GetTab(tag2, 0, 0));
+  tabs2.push_back(tracker.GetTab(tag2, 0));
   ASSERT_EQ(1U, tracker.num_synced_tabs(tag2));
   ASSERT_EQ(0U, tracker.num_synced_sessions());
   ASSERT_FALSE(tracker.DeleteSession(tag3));
@@ -177,66 +177,12 @@ 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, rand_tab_num + 1);
+      SessionTab* tab = tracker.GetTab(tag, rand_tab_num);
       ASSERT_TRUE(tab);
     }
   }
 }
 
-TEST_F(SyncedSessionTrackerTest, LookupTabNodeIds) {
-  SyncedSessionTracker tracker;
-  std::set<int> result;
-  std::string tag1 = "session1";
-  std::string tag2 = "session2";
-  std::string tag3 = "session3";
-
-  tracker.GetTab(tag1, 1, 1);
-  tracker.GetTab(tag1, 2, 2);
-  EXPECT_TRUE(tracker.LookupTabNodeIds(tag1, &result));
-  EXPECT_EQ(2U, result.size());
-  EXPECT_FALSE(result.end() == result.find(1));
-  EXPECT_FALSE(result.end() == result.find(2));
-  EXPECT_FALSE(tracker.LookupTabNodeIds(tag2, &result));
-
-  tracker.PutWindowInSession(tag1, 0);
-  tracker.PutTabInWindow(tag1, 0, 3, 0);
-  EXPECT_TRUE(tracker.LookupTabNodeIds(tag1, &result));
-  EXPECT_EQ(2U, result.size());
-
-  tracker.GetTab(tag1, 3, 3);
-  EXPECT_TRUE(tracker.LookupTabNodeIds(tag1, &result));
-  EXPECT_EQ(3U, result.size());
-  EXPECT_FALSE(result.end() == result.find(3));
-
-  tracker.GetTab(tag2, 1, 21);
-  tracker.GetTab(tag2, 2, 22);
-  EXPECT_TRUE(tracker.LookupTabNodeIds(tag2, &result));
-  EXPECT_EQ(2U, result.size());
-  EXPECT_FALSE(result.end() == result.find(21));
-  EXPECT_FALSE(result.end() == result.find(22));
-  EXPECT_TRUE(tracker.LookupTabNodeIds(tag1, &result));
-  EXPECT_EQ(3U, result.size());
-  EXPECT_FALSE(result.end() == result.find(1));
-  EXPECT_FALSE(result.end() == result.find(2));
-
-  EXPECT_FALSE(tracker.LookupTabNodeIds(tag3, &result));
-  tracker.PutWindowInSession(tag3, 1);
-  tracker.PutTabInWindow(tag3, 1, 5, 0);
-  EXPECT_TRUE(tracker.LookupTabNodeIds(tag3, &result));
-  EXPECT_TRUE(result.empty());
-  EXPECT_TRUE(tracker.DeleteSession(tag3));
-  EXPECT_FALSE(tracker.LookupTabNodeIds(tag3, &result));
-
-  EXPECT_TRUE(tracker.DeleteSession(tag1));
-  EXPECT_FALSE(tracker.LookupTabNodeIds(tag1, &result));
-  EXPECT_TRUE(tracker.LookupTabNodeIds(tag2, &result));
-  EXPECT_EQ(2U, result.size());
-  EXPECT_FALSE(result.end() == result.find(21));
-  EXPECT_FALSE(result.end() == result.find(22));
-  EXPECT_TRUE(tracker.DeleteSession(tag2));
-  EXPECT_FALSE(tracker.LookupTabNodeIds(tag2, &result));
-}
-
 TEST_F(SyncedSessionTrackerTest, SessionTracking) {
   SyncedSessionTracker tracker;
   ASSERT_TRUE(tracker.Empty());
@@ -248,8 +194,8 @@ TEST_F(SyncedSessionTrackerTest, SessionTracking) {
   tracker.PutWindowInSession(tag1, 0);
   tracker.PutTabInWindow(tag1, 0, 0, 0);
   tracker.PutTabInWindow(tag1, 0, 1, 1);
-  tracker.GetTab(tag1, 2, 3U)->window_id.set_id(0);  // Will be unmapped.
-  tracker.GetTab(tag1, 3, 4U)->window_id.set_id(0);  // Will be unmapped.
+  tracker.GetTab(tag1, 2)->window_id.set_id(0);  // Will be an unmapped tab.
+  tracker.GetTab(tag1, 3)->window_id.set_id(0);  // Will be an unmapped tab.
   tracker.PutWindowInSession(tag1, 1);
   tracker.PutTabInWindow(tag1, 1, 4, 0);
   tracker.PutTabInWindow(tag1, 1, 5, 1);
@@ -267,12 +213,10 @@ TEST_F(SyncedSessionTrackerTest, SessionTracking) {
   ASSERT_EQ(1U, tracker.num_synced_tabs(tag2));
 
   // Reset tracking and get the current windows/tabs.
-  // We simulate moving a tab from one window to another, then closing the
-  // first window (including its one remaining tab), and opening a new tab
-  // on the remaining window.
-
-  // New tab, arrived before meta node so unmapped.
-  tracker.GetTab(tag1, 6, 7U);
+  // We simulate moving a tab from one window to another, then closing the first
+  // window (including its one remaining tab), and opening a new tab on the
+  // remaining window.
+  tracker.GetTab(tag1, 6);  // New tab, arrived before meta node so unmapped.
   tracker.ResetSessionTracking(tag1);
   tracker.PutWindowInSession(tag1, 0);
   tracker.PutTabInWindow(tag1, 0, 0, 0);
diff --git a/chrome/browser/sync/glue/tab_node_pool.cc b/chrome/browser/sync/glue/tab_node_pool.cc
index 43c9f54..2b41851 100644
--- a/chrome/browser/sync/glue/tab_node_pool.cc
+++ b/chrome/browser/sync/glue/tab_node_pool.cc
@@ -24,24 +24,20 @@ const size_t TabNodePool::kFreeNodesLowWatermark = 25;
 const size_t TabNodePool::kFreeNodesHighWatermark = 100;
 
 TabNodePool::TabNodePool(ProfileSyncService* sync_service)
-    : max_used_tab_node_id_(kInvalidTabNodeID),
-      sync_service_(sync_service) {}
-
-// static
-// We start vending tab node IDs at 0.
-const int TabNodePool::kInvalidTabNodeID = -1;
+    : max_used_tab_node_id_(0), sync_service_(sync_service) {}
 
 TabNodePool::~TabNodePool() {}
 
 // Static
 std::string TabNodePool::TabIdToTag(
-    const std::string machine_tag, int tab_node_id) {
-  return base::StringPrintf("%s %d", machine_tag.c_str(), tab_node_id);
+    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,
                              const SessionID& tab_id,
-                             int tab_node_id) {
+                             size_t tab_node_id) {
   DCHECK_GT(sync_id, syncer::kInvalidId);
   DCHECK_GT(tab_id.id(), kInvalidTabID);
   DCHECK(syncid_tabid_map_.find(sync_id) == syncid_tabid_map_.end());
@@ -78,7 +74,7 @@ int64 TabNodePool::GetFreeTabNode() {
       LOG(ERROR) << kNoSessionsFolderError;
       return syncer::kInvalidId;
     }
-    int tab_node_id = ++max_used_tab_node_id_;
+    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 =
@@ -178,7 +174,7 @@ void TabNodePool::Clear() {
   unassociated_nodes_.clear();
   free_nodes_pool_.clear();
   syncid_tabid_map_.clear();
-  max_used_tab_node_id_ = kInvalidTabNodeID;
+  max_used_tab_node_id_ = 0;
 }
 
 size_t TabNodePool::Capacity() const {
diff --git a/chrome/browser/sync/glue/tab_node_pool.h b/chrome/browser/sync/glue/tab_node_pool.h
index 2ef09c7..7727ba3 100644
--- a/chrome/browser/sync/glue/tab_node_pool.h
+++ b/chrome/browser/sync/glue/tab_node_pool.h
@@ -17,8 +17,8 @@ class ProfileSyncService;
 
 namespace browser_sync {
 
-// A pool for managing free/used tab sync nodes for the *local* session.
-// Performs lazy creation of sync nodes when necessary.
+// A pool for managing free/used tab sync nodes. Performs lazy creation
+// of sync nodes when necessary.
 // Note: We make use of the following "id's"
 // - a sync_id: an int64 used in |syncer::InitByIdLookup|
 // - a tab_id: created by session service, unique to this client
@@ -55,11 +55,9 @@ class TabNodePool {
   // Maximum limit of FreeNodes allowed on the client.
   static const size_t kFreeNodesHighWatermark;
 
-  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);
+                                size_t tab_node_id);
 
   // Returns the sync_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
@@ -81,7 +79,7 @@ class TabNodePool {
   // 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(int64 sync_id, const SessionID& tab_id, int tab_node_id);
+  void AddTabNode(int64 sync_id, const SessionID& tab_id, size_t tab_node_id);
 
   // Returns the tab_id for |sync_id| if it is associated else returns
   // kInvalidTabID.
@@ -135,7 +133,7 @@ class TabNodePool {
 
   // 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_;
+  size_t max_used_tab_node_id_;
 
   // The machine tag associated with this tab pool. Used in the title of new
   // sync nodes.
diff --git a/chrome/browser/sync/glue/tab_node_pool_unittest.cc b/chrome/browser/sync/glue/tab_node_pool_unittest.cc
index 6e8a079..4487df6 100644
--- a/chrome/browser/sync/glue/tab_node_pool_unittest.cc
+++ b/chrome/browser/sync/glue/tab_node_pool_unittest.cc
@@ -12,7 +12,7 @@ class SyncTabNodePoolTest : public testing::Test {
  protected:
   SyncTabNodePoolTest() : pool_(NULL) { pool_.SetMachineTag("tag"); }
 
-  int GetMaxUsedTabNodeId() const { return pool_.max_used_tab_node_id_; }
+  size_t GetMaxUsedTabNodeId() const { return pool_.max_used_tab_node_id_; }
 
   TabNodePool pool_;
 };
@@ -24,24 +24,24 @@ TEST_F(SyncTabNodePoolTest, TabNodeIdIncreases) {
   SessionID session_id;
   session_id.set_id(1);
   pool_.AddTabNode(4, session_id, 10);
-  EXPECT_EQ(10, GetMaxUsedTabNodeId());
+  EXPECT_EQ(10u, GetMaxUsedTabNodeId());
   session_id.set_id(2);
   pool_.AddTabNode(5, session_id, 1);
-  EXPECT_EQ(10, GetMaxUsedTabNodeId());
+  EXPECT_EQ(10u, GetMaxUsedTabNodeId());
   session_id.set_id(3);
   pool_.AddTabNode(6, session_id, 1000);
-  EXPECT_EQ(1000, GetMaxUsedTabNodeId());
+  EXPECT_EQ(1000u, GetMaxUsedTabNodeId());
   pool_.ReassociateTabNode(6, 500);
   // Freeing a tab node does not change max_used_tab_node_id_.
   pool_.FreeTabNode(6);
   pool_.FreeUnassociatedTabNodes();
-  EXPECT_EQ(1000, GetMaxUsedTabNodeId());
+  EXPECT_EQ(1000u, GetMaxUsedTabNodeId());
   for (int i = 0; i < 3; ++i) {
     pool_.AssociateTabNode(pool_.GetFreeTabNode(), i + 1);
-    EXPECT_EQ(1000, GetMaxUsedTabNodeId());
+    EXPECT_EQ(1000u, GetMaxUsedTabNodeId());
   }
 
-  EXPECT_EQ(1000, GetMaxUsedTabNodeId());
+  EXPECT_EQ(1000u, GetMaxUsedTabNodeId());
 }
 
 TEST_F(SyncTabNodePoolTest, OldTabNodesAddAndRemove) {
diff --git a/chrome/browser/sync/profile_sync_service_session_unittest.cc b/chrome/browser/sync/profile_sync_service_session_unittest.cc
index 2319d45..aec8539 100644
--- a/chrome/browser/sync/profile_sync_service_session_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_session_unittest.cc
@@ -123,6 +123,22 @@ void AddWindowSpecifics(int window_id,
   }
 }
 
+void BuildTabSpecifics(const std::string& tag, int window_id, int tab_id,
+                       sync_pb::SessionSpecifics* tab_base) {
+  tab_base->set_session_tag(tag);
+  sync_pb::SessionTab* tab = tab_base->mutable_tab();
+  tab->set_tab_id(tab_id);
+  tab->set_tab_visual_index(1);
+  tab->set_current_navigation_index(0);
+  tab->set_pinned(true);
+  tab->set_extension_app_id("app_id");
+  sync_pb::TabNavigation* navigation = tab->add_navigation();
+  navigation->set_virtual_url("http://foo/1");
+  navigation->set_referrer("referrer");
+  navigation->set_title("title");
+  navigation->set_page_transition(sync_pb::SyncEnums_PageTransition_TYPED);
+}
+
 // Verifies number of windows, number of tabs, and basic fields.
 void VerifySyncedSession(
     const std::string& tag,
@@ -189,27 +205,9 @@ class ProfileSyncServiceSessionTest
   ProfileSyncServiceSessionTest()
       : window_bounds_(0, 1, 2, 3),
         notified_of_update_(false),
-        notified_of_refresh_(false),
-        max_tab_node_id_(0) {}
+        notified_of_refresh_(false) {}
   ProfileSyncService* sync_service() { return sync_service_.get(); }
 
-  void BuildTabSpecifics(const std::string& tag, int window_id, int tab_id,
-                         sync_pb::SessionSpecifics* tab_base) {
-    tab_base->set_session_tag(tag);
-    tab_base->set_tab_node_id(++max_tab_node_id_);
-    sync_pb::SessionTab* tab = tab_base->mutable_tab();
-    tab->set_tab_id(tab_id);
-    tab->set_tab_visual_index(1);
-    tab->set_current_navigation_index(0);
-    tab->set_pinned(true);
-    tab->set_extension_app_id("app_id");
-    sync_pb::TabNavigation* navigation = tab->add_navigation();
-    navigation->set_virtual_url("http://foo/1");
-    navigation->set_referrer("referrer");
-    navigation->set_title("title");
-    navigation->set_page_transition(sync_pb::SyncEnums_PageTransition_TYPED);
-  }
-
  protected:
   virtual TestingProfile* CreateProfile() OVERRIDE {
     TestingProfile* profile = new TestingProfile();
@@ -248,7 +246,6 @@ class ProfileSyncServiceSessionTest
   }
 
   virtual void TearDown() {
-    max_tab_node_id_ = 0;
     sync_service_->Shutdown();
     sync_service_.reset();
 
@@ -321,7 +318,6 @@ class ProfileSyncServiceSessionTest
   bool notified_of_refresh_;
   content::NotificationRegistrar registrar_;
   net::TestURLFetcherFactory fetcher_factory_;
-  int max_tab_node_id_;
 };
 
 class CreateRootHelper {
@@ -412,8 +408,7 @@ TEST_F(ProfileSyncServiceSessionTest, MAYBE_WriteFilledSessionToNode) {
 
   // Get the tabs for this machine from the node and check that they were
   // filled.
-  SessionModelAssociator::TabLinksMap tab_map =
-      model_associator_->local_tab_map_;
+  SessionModelAssociator::TabLinksMap tab_map = model_associator_->tab_map_;
   ASSERT_EQ(2U, tab_map.size());
   // Tabs are ordered by sessionid in tab_map, so should be able to traverse
   // the tree based on order of tabs created
@@ -763,26 +758,26 @@ TEST_F(ProfileSyncServiceSessionTest, TabNodePoolEmpty) {
   ASSERT_TRUE(create_root.success());
 
   std::vector<int64> node_ids;
-  ASSERT_EQ(0U, model_associator_->local_tab_pool_.Capacity());
-  ASSERT_TRUE(model_associator_->local_tab_pool_.Empty());
-  ASSERT_TRUE(model_associator_->local_tab_pool_.Full());
+  ASSERT_EQ(0U, model_associator_->tab_pool_.Capacity());
+  ASSERT_TRUE(model_associator_->tab_pool_.Empty());
+  ASSERT_TRUE(model_associator_->tab_pool_.Full());
   const size_t num_ids = 10;
   for (size_t i = 0; i < num_ids; ++i) {
-    int64 id = model_associator_->local_tab_pool_.GetFreeTabNode();
+    int64 id = model_associator_->tab_pool_.GetFreeTabNode();
     ASSERT_GT(id, -1);
     node_ids.push_back(id);
     // Associate with a tab node.
-    model_associator_->local_tab_pool_.AssociateTabNode(id, i + 1);
+    model_associator_->tab_pool_.AssociateTabNode(id, i + 1);
   }
-  ASSERT_EQ(num_ids, model_associator_->local_tab_pool_.Capacity());
-  ASSERT_TRUE(model_associator_->local_tab_pool_.Empty());
-  ASSERT_FALSE(model_associator_->local_tab_pool_.Full());
+  ASSERT_EQ(num_ids, model_associator_->tab_pool_.Capacity());
+  ASSERT_TRUE(model_associator_->tab_pool_.Empty());
+  ASSERT_FALSE(model_associator_->tab_pool_.Full());
   for (size_t i = 0; i < num_ids; ++i) {
-    model_associator_->local_tab_pool_.FreeTabNode(node_ids[i]);
+    model_associator_->tab_pool_.FreeTabNode(node_ids[i]);
   }
-  ASSERT_EQ(num_ids, model_associator_->local_tab_pool_.Capacity());
-  ASSERT_FALSE(model_associator_->local_tab_pool_.Empty());
-  ASSERT_TRUE(model_associator_->local_tab_pool_.Full());
+  ASSERT_EQ(num_ids, model_associator_->tab_pool_.Capacity());
+  ASSERT_FALSE(model_associator_->tab_pool_.Empty());
+  ASSERT_TRUE(model_associator_->tab_pool_.Full());
 }
 
 // TODO(jhorwich): Re-enable when crbug.com/121487 addressed
@@ -795,31 +790,31 @@ TEST_F(ProfileSyncServiceSessionTest, TabNodePoolNonEmpty) {
   SessionID session_id;
   for (size_t i = 0; i < num_starting_nodes; ++i) {
     session_id.set_id(i + 1);
-    model_associator_->local_tab_pool_.AddTabNode(i + 1, session_id, i);
+    model_associator_->tab_pool_.AddTabNode(i + 1, session_id, i);
   }
 
-  model_associator_->local_tab_pool_.FreeUnassociatedTabNodes();
+  model_associator_->tab_pool_.FreeUnassociatedTabNodes();
   std::vector<int64> node_ids;
-  ASSERT_EQ(num_starting_nodes, model_associator_->local_tab_pool_.Capacity());
-  ASSERT_FALSE(model_associator_->local_tab_pool_.Empty());
-  ASSERT_TRUE(model_associator_->local_tab_pool_.Full());
+  ASSERT_EQ(num_starting_nodes, model_associator_->tab_pool_.Capacity());
+  ASSERT_FALSE(model_associator_->tab_pool_.Empty());
+  ASSERT_TRUE(model_associator_->tab_pool_.Full());
   const size_t num_ids = 10;
   for (size_t i = 0; i < num_ids; ++i) {
-    int64 id = model_associator_->local_tab_pool_.GetFreeTabNode();
+    int64 id = model_associator_->tab_pool_.GetFreeTabNode();
     ASSERT_GT(id, -1);
     node_ids.push_back(id);
     // Associate with a tab node.
-    model_associator_->local_tab_pool_.AssociateTabNode(id, i + 1);
+    model_associator_->tab_pool_.AssociateTabNode(id, i + 1);
   }
-  ASSERT_EQ(num_ids, model_associator_->local_tab_pool_.Capacity());
-  ASSERT_TRUE(model_associator_->local_tab_pool_.Empty());
-  ASSERT_FALSE(model_associator_->local_tab_pool_.Full());
+  ASSERT_EQ(num_ids, model_associator_->tab_pool_.Capacity());
+  ASSERT_TRUE(model_associator_->tab_pool_.Empty());
+  ASSERT_FALSE(model_associator_->tab_pool_.Full());
   for (size_t i = 0; i < num_ids; ++i) {
-    model_associator_->local_tab_pool_.FreeTabNode(node_ids[i]);
+    model_associator_->tab_pool_.FreeTabNode(node_ids[i]);
   }
-  ASSERT_EQ(num_ids, model_associator_->local_tab_pool_.Capacity());
-  ASSERT_FALSE(model_associator_->local_tab_pool_.Empty());
-  ASSERT_TRUE(model_associator_->local_tab_pool_.Full());
+  ASSERT_EQ(num_ids, model_associator_->tab_pool_.Capacity());
+  ASSERT_FALSE(model_associator_->tab_pool_.Empty());
+  ASSERT_TRUE(model_associator_->tab_pool_.Full());
 }
 
 // Write a foreign session to a node, and then delete it.
@@ -1005,8 +1000,7 @@ TEST_F(ProfileSyncServiceSessionTest, MAYBE_ValidTabs) {
   // Note: chrome://newtab has special handling which crashes in unit tests.
 
   // Get the tabs for this machine. Only the bla:// url should be synced.
-  SessionModelAssociator::TabLinksMap tab_map =
-      model_associator_->local_tab_map_;
+  SessionModelAssociator::TabLinksMap tab_map = model_associator_->tab_map_;
   ASSERT_EQ(1U, tab_map.size());
   SessionModelAssociator::TabLinksMap::iterator iter = tab_map.begin();
   ASSERT_EQ(1, iter->second->tab()->GetEntryCount());
@@ -1060,8 +1054,7 @@ TEST_F(ProfileSyncServiceSessionTest, ExistingTabs) {
 
   // Get the tabs for this machine from the node and check that they were
   // filled.
-  SessionModelAssociator::TabLinksMap tab_map =
-      model_associator_->local_tab_map_;
+  SessionModelAssociator::TabLinksMap tab_map = model_associator_->tab_map_;
   ASSERT_EQ(2U, tab_map.size());
   // Tabs are ordered by sessionid in tab_map, so should be able to traverse
   // the tree based on order of tabs created
@@ -1343,8 +1336,8 @@ TEST_F(ProfileSyncServiceSessionTest, TabPoolFreeNodeLimits) {
   std::vector<int64> used_sync_ids;
   for (size_t i = 1; i <= TabNodePool::kFreeNodesHighWatermark + 1; ++i) {
     session_id.set_id(i);
-    int64 sync_id = model_associator_->local_tab_pool_.GetFreeTabNode();
-    model_associator_->local_tab_pool_.AssociateTabNode(sync_id, i);
+    int64 sync_id = model_associator_->tab_pool_.GetFreeTabNode();
+    model_associator_->tab_pool_.AssociateTabNode(sync_id, i);
     used_sync_ids.push_back(sync_id);
   }
 
@@ -1353,23 +1346,23 @@ TEST_F(ProfileSyncServiceSessionTest, TabPoolFreeNodeLimits) {
   used_sync_ids.pop_back();
 
   for (size_t i = 0; i < used_sync_ids.size(); ++i) {
-    model_associator_->local_tab_pool_.FreeTabNode(used_sync_ids[i]);
+    model_associator_->tab_pool_.FreeTabNode(used_sync_ids[i]);
   }
 
   // Except one node all nodes should be in FreeNode pool.
-  EXPECT_FALSE(model_associator_->local_tab_pool_.Full());
-  EXPECT_FALSE(model_associator_->local_tab_pool_.Empty());
+  EXPECT_FALSE(model_associator_->tab_pool_.Full());
+  EXPECT_FALSE(model_associator_->tab_pool_.Empty());
   // Total capacity = 1 Associated Node + kFreeNodesHighWatermark free node.
   EXPECT_EQ(TabNodePool::kFreeNodesHighWatermark + 1,
-            model_associator_->local_tab_pool_.Capacity());
+            model_associator_->tab_pool_.Capacity());
 
   // Freeing the last sync node should drop the free nodes to
   // kFreeNodesLowWatermark.
-  model_associator_->local_tab_pool_.FreeTabNode(last_sync_id);
-  EXPECT_FALSE(model_associator_->local_tab_pool_.Empty());
-  EXPECT_TRUE(model_associator_->local_tab_pool_.Full());
+  model_associator_->tab_pool_.FreeTabNode(last_sync_id);
+  EXPECT_FALSE(model_associator_->tab_pool_.Empty());
+  EXPECT_TRUE(model_associator_->tab_pool_.Full());
   EXPECT_EQ(TabNodePool::kFreeNodesLowWatermark,
-            model_associator_->local_tab_pool_.Capacity());
+            model_associator_->tab_pool_.Capacity());
 }
 
 }  // namespace browser_sync
diff --git a/chrome/browser/ui/toolbar/recent_tabs_builder_test_helper.cc b/chrome/browser/ui/toolbar/recent_tabs_builder_test_helper.cc
index 1fa9d5c..3fe29a4 100644
--- a/chrome/browser/ui/toolbar/recent_tabs_builder_test_helper.cc
+++ b/chrome/browser/ui/toolbar/recent_tabs_builder_test_helper.cc
@@ -76,8 +76,7 @@ struct RecentTabsBuilderTestHelper::SessionInfo {
   std::vector<WindowInfo> windows;
 };
 
-RecentTabsBuilderTestHelper::RecentTabsBuilderTestHelper()
-    : max_tab_node_id_(0) {
+RecentTabsBuilderTestHelper::RecentTabsBuilderTestHelper() {
   start_time_ = base::Time::Now();
 }
 
@@ -263,7 +262,6 @@ void RecentTabsBuilderTestHelper::BuildTabSpecifics(
   SessionID::id_type tab_id = GetTabID(session_index, window_index, tab_index);
 
   tab_base->set_session_tag(ToSessionTag(session_id));
-  tab_base->set_tab_node_id(++max_tab_node_id_);
   sync_pb::SessionTab* tab = tab_base->mutable_tab();
   tab->set_window_id(window_id);
   tab->set_tab_id(tab_id);
diff --git a/chrome/browser/ui/toolbar/recent_tabs_builder_test_helper.h b/chrome/browser/ui/toolbar/recent_tabs_builder_test_helper.h
index ac7492b..63f4761 100644
--- a/chrome/browser/ui/toolbar/recent_tabs_builder_test_helper.h
+++ b/chrome/browser/ui/toolbar/recent_tabs_builder_test_helper.h
@@ -71,8 +71,6 @@ class RecentTabsBuilderTestHelper {
   std::vector<SessionInfo> sessions_;
   base::Time start_time_;
 
-  int max_tab_node_id_;
-
   DISALLOW_COPY_AND_ASSIGN(RecentTabsBuilderTestHelper);
 };