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

components/sync_sessions/sessions_sync_manager.cc

Index: components/sync_sessions/sessions_sync_manager.cc
diff --git a/components/sync_sessions/sessions_sync_manager.cc b/components/sync_sessions/sessions_sync_manager.cc
index 4fa6f7eca6a8ba055d902dbd7c5b1228bf21d9a8..b2c62df09a0721c5b8e90df659a0617b76f91b33 100644
--- a/components/sync_sessions/sessions_sync_manager.cc
+++ b/components/sync_sessions/sessions_sync_manager.cc
@@ -156,7 +156,8 @@
   syncer::SyncChangeList new_changes;
 
   // First, we iterate over sync data to update our session_tracker_.
-  if (!InitFromSyncModel(initial_sync_data, &new_changes)) {
+  syncer::SyncDataList restored_tabs;
+  if (!InitFromSyncModel(initial_sync_data, &restored_tabs, &new_changes)) {
     // The sync db didn't have a header node for us. Create one.
     sync_pb::EntitySpecifics specifics;
     sync_pb::SessionSpecifics* base_specifics = specifics.mutable_session();
@@ -178,7 +179,7 @@
 #endif
 
   // Check if anything has changed on the local client side.
-  AssociateWindows(RELOAD_TABS, &new_changes);
+  AssociateWindows(RELOAD_TABS, restored_tabs, &new_changes);
   local_tab_pool_out_of_sync_ = false;
 
   merge_result.set_error(
@@ -190,6 +191,7 @@
 
 void SessionsSyncManager::AssociateWindows(
     ReloadTabsOption option,
+    const syncer::SyncDataList& restored_tabs,
     syncer::SyncChangeList* change_output) {
   const std::string local_tag = current_machine_tag();
   sync_pb::SessionSpecifics specifics;
@@ -271,7 +273,7 @@
           if (synced_tab->GetSyncId() > TabNodePool::kInvalidTabNodeID &&
               tab_id > TabNodePool::kInvalidTabID) {
             AssociateRestoredPlaceholderTab(*synced_tab, tab_id, window_id,
-                                            change_output);
+                                            restored_tabs, change_output);
             found_tabs = true;
             window_s.add_tab(tab_id);
           }
@@ -350,10 +352,14 @@
       tab->SetSyncId(tab_node_id);
     }
     local_tab_pool_.AssociateTabNode(tab_node_id, tab_id);
-    tab_link = new TabLink(tab_node_id);
+    tab_link = new TabLink(tab_node_id, tab);
     local_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);
   }
   DCHECK(tab_link);
   DCHECK_NE(tab_link->tab_node_id(), TabNodePool::kInvalidTabNodeID);
@@ -439,7 +445,7 @@
   // "interesting" by going to a valid URL, in which case it needs to be added
   // to the window's tab information. Similarly, if a tab became
   // "uninteresting", we remove it from the window's tab information.
-  AssociateWindows(DONT_RELOAD_TABS, &changes);
+  AssociateWindows(DONT_RELOAD_TABS, syncer::SyncDataList(), &changes);
   sync_processor_->ProcessSyncChanges(FROM_HERE, changes);
 }
 
@@ -574,7 +580,7 @@
           LOG(WARNING) << "Dropping modification to local session.";
           return syncer::SyncError();
         }
-        UpdateTrackerWithSpecifics(
+        UpdateTrackerWithForeignSession(
             session, syncer::SyncDataRemote(it->sync_data()).GetModifiedTime());
         break;
       default:
@@ -612,6 +618,7 @@
 
 bool SessionsSyncManager::InitFromSyncModel(
     const syncer::SyncDataList& sync_data,
+    syncer::SyncDataList* restored_tabs,
     syncer::SyncChangeList* new_changes) {
   bool found_current_header = false;
   int bad_foreign_hash_count = 0;
@@ -629,7 +636,7 @@
         new_changes->push_back(tombstone);
     } else if (specifics.session_tag() != current_machine_tag()) {
       if (TagHashFromSpecifics(specifics) == remote.GetClientTagHash()) {
-        UpdateTrackerWithSpecifics(specifics, remote.GetModifiedTime());
+        UpdateTrackerWithForeignSession(specifics, remote.GetModifiedTime());
       } else {
         // In the past, like years ago, we believe that some session data was
         // created with bad tag hashes. This causes any change this client makes
@@ -648,10 +655,6 @@
         found_current_header = true;
         if (specifics.header().has_client_name())
           current_session_name_ = specifics.header().client_name();
-
-        // TODO(zea): crbug.com/639009 update the tracker with the specifics
-        // from the header node as well. This will be necessary to preserve
-        // the set of open tabs when a custom tab is opened.
       } else {
         if (specifics.has_header() || !specifics.has_tab()) {
           LOG(WARNING) << "Found more than one session header node with local "
@@ -660,14 +663,10 @@
           if (tombstone.IsValid())
             new_changes->push_back(tombstone);
         } else {
-          // This is a valid old tab node, reassociate it and add it to the
-          // tracker.
-          local_tab_map_[specifics.tab().tab_id()] =
-              make_linked_ptr<TabLink>(new TabLink(specifics.tab_node_id()));
+          // This is a valid old tab node, add it to the pool so it can be
+          // reused for reassociation.
           local_tab_pool_.AddTabNode(specifics.tab_node_id());
-          local_tab_pool_.AssociateTabNode(specifics.tab_node_id(),
-                                           specifics.tab().tab_id());
-          UpdateTrackerWithSpecifics(specifics, remote.GetModifiedTime());
+          restored_tabs->push_back(*it);
         }
       }
     }
@@ -688,64 +687,69 @@
   return found_current_header;
 }
 
-void SessionsSyncManager::UpdateTrackerWithSpecifics(
+void SessionsSyncManager::UpdateTrackerWithForeignSession(
     const sync_pb::SessionSpecifics& specifics,
     const base::Time& modification_time) {
-  std::string session_tag = specifics.session_tag();
-  SyncedSession* session = session_tracker_.GetSession(session_tag);
+  std::string foreign_session_tag = specifics.session_tag();
+  DCHECK_NE(foreign_session_tag, current_machine_tag());
+
+  SyncedSession* foreign_session =
+      session_tracker_.GetSession(foreign_session_tag);
   if (specifics.has_header()) {
-    // Read in the header data for this session. Header data is
+    // Read in the header data for this foreign session. Header data is
     // essentially a collection of windows, each of which has an ordered id list
     // for their tabs.
 
     if (!IsValidSessionHeader(specifics.header())) {
-      LOG(WARNING) << "Ignoring session node with invalid header "
-                   << "and tag " << session_tag << ".";
+      LOG(WARNING) << "Ignoring foreign session node with invalid header "
+                   << "and tag " << foreign_session_tag << ".";
       return;
     }
 
     // Load (or create) the SyncedSession object for this client.
     const sync_pb::SessionHeader& header = specifics.header();
-    PopulateSessionHeaderFromSpecifics(header, modification_time, session);
+    PopulateSessionHeaderFromSpecifics(header, modification_time,
+                                       foreign_session);
 
     // Reset the tab/window tracking for this session (must do this before
     // we start calling PutWindowInSession and PutTabInWindow so that all
     // unused tabs/windows get cleared by the CleanupSession(...) call).
-    session_tracker_.ResetSessionTracking(session_tag);
+    session_tracker_.ResetSessionTracking(foreign_session_tag);
 
     // Process all the windows and their tab information.
     int num_windows = header.window_size();
-    DVLOG(1) << "Associating " << session_tag << " with " << num_windows
+    DVLOG(1) << "Associating " << foreign_session_tag << " with " << num_windows
              << " windows.";
 
     for (int i = 0; i < num_windows; ++i) {
       const sync_pb::SessionWindow& window_s = header.window(i);
       SessionID::id_type window_id = window_s.window_id();
-      session_tracker_.PutWindowInSession(session_tag, window_id);
-      BuildSyncedSessionFromSpecifics(session_tag, window_s, modification_time,
-                                      session->windows[window_id].get());
+      session_tracker_.PutWindowInSession(foreign_session_tag, window_id);
+      BuildSyncedSessionFromSpecifics(
+          foreign_session_tag, window_s, modification_time,
+          foreign_session->windows[window_id].get());
     }
     // Delete any closed windows and unused tabs as necessary.
-    session_tracker_.CleanupSession(session_tag);
+    session_tracker_.CleanupSession(foreign_session_tag);
   } else if (specifics.has_tab()) {
     const sync_pb::SessionTab& tab_s = specifics.tab();
     SessionID::id_type tab_id = tab_s.tab_id();
 
     const sessions::SessionTab* existing_tab;
-    if (session_tracker_.LookupSessionTab(session_tag, tab_id, &existing_tab) &&
+    if (session_tracker_.LookupSessionTab(foreign_session_tag, tab_id,
+                                          &existing_tab) &&
         existing_tab->timestamp > modification_time) {
       // Force the tracker to remember this tab node id, even if it isn't
       // currently being used.
-      session_tracker_.GetTab(session_tag, tab_id, specifics.tab_node_id());
-      DVLOG(1) << "Ignoring " << session_tag << "'s session tab " << tab_id
-               << " with earlier modification time";
+      session_tracker_.GetTab(foreign_session_tag, tab_id,
+                              specifics.tab_node_id());
+      DVLOG(1) << "Ignoring " << foreign_session_tag << "'s session tab "
+               << tab_id << " with earlier modification time";
       return;
     }
 
-    DVLOG(1) << "Associating tab " << tab_id << " with node "
-             << specifics.tab_node_id();
-    sessions::SessionTab* tab =
-        session_tracker_.GetTab(session_tag, tab_id, specifics.tab_node_id());
+    sessions::SessionTab* tab = session_tracker_.GetTab(
+        foreign_session_tag, tab_id, specifics.tab_node_id());
 
     // Update SessionTab based on protobuf.
     tab->SetFromSyncData(tab_s, modification_time);
@@ -755,11 +759,11 @@
     RefreshFaviconVisitTimesFromForeignTab(tab_s, modification_time);
 
     // Update the last modified time.
-    if (session->modified_time < modification_time)
-      session->modified_time = modification_time;
+    if (foreign_session->modified_time < modification_time)
+      foreign_session->modified_time = modification_time;
   } else {
-    LOG(WARNING) << "Ignoring session node with missing header/tab "
-                 << "fields and tag " << session_tag << ".";
+    LOG(WARNING) << "Ignoring foreign session node with missing header/tab "
+                 << "fields and tag " << foreign_session_tag << ".";
   }
 }
 
@@ -961,8 +965,8 @@
     const SyncedTabDelegate& tab_delegate,
     sync_pb::SessionSpecifics* specifics) {
   sessions::SessionTab* session_tab = nullptr;
-  SessionID::id_type tab_id = tab_delegate.GetSessionId();
-  session_tab = session_tracker_.GetTab(current_machine_tag(), tab_id,
+  session_tab = session_tracker_.GetTab(current_machine_tag(),
+                                        tab_delegate.GetSessionId(),
                                         tab_delegate.GetSyncId());
   SetSessionTabFromDelegate(tab_delegate, base::Time::Now(), session_tab);
   SetVariationIds(session_tab);
@@ -976,52 +980,47 @@
     const SyncedTabDelegate& tab_delegate,
     SessionID::id_type new_tab_id,
     SessionID::id_type new_window_id,
+    const syncer::SyncDataList& restored_tabs,
     syncer::SyncChangeList* change_output) {
   DCHECK_NE(tab_delegate.GetSyncId(), TabNodePool::kInvalidTabNodeID);
-
-  SessionID::id_type old_tab_id =
-      local_tab_pool_.GetTabIdFromTabNodeId(tab_delegate.GetSyncId());
-  DVLOG(1) << "Restoring tab id " << new_tab_id << " from sync node "
-           << tab_delegate.GetSyncId() << " (old tab id was " << old_tab_id
-           << ").";
-
-  // Update tab node pool and tracker with the new association (note that
-  // reassociating the tracker depends on the old tab pool data, so it must be
-  // reassociated first).
-  if (!session_tracker_.ReassociateTab(current_machine_tag(), old_tab_id,
-                                       new_tab_id)) {
-    LOG(ERROR) << "Failed to reassociate tab " << new_tab_id;
+  // Rewrite the tab using |restored_tabs| to retrieve the specifics.
+  if (restored_tabs.empty()) {
+    DLOG(WARNING) << "Can't Update tab ID.";
     return;
   }
-  local_tab_pool_.ReassociateTabNode(tab_delegate.GetSyncId(), new_tab_id);
-
-  // If the tab id hasn't changed, the tab map doesn't need to be updated. But
-  // the window id may have changed, so the specifics still need to be
-  // rewritten.
-  if (old_tab_id != new_tab_id) {
-    auto iter = local_tab_map_.find(old_tab_id);
-    if (iter == local_tab_map_.end()) {
-      LOG(ERROR) << "Failed to update local tab map for " << new_tab_id;
+
+  for (syncer::SyncDataList::const_iterator it = restored_tabs.begin();
+       it != restored_tabs.end(); ++it) {
+    if (it->GetSpecifics().session().tab_node_id() !=
+        tab_delegate.GetSyncId()) {
+      continue;
+    }
+
+    sync_pb::EntitySpecifics entity;
+    sync_pb::SessionSpecifics* specifics = entity.mutable_session();
+    specifics->CopyFrom(it->GetSpecifics().session());
+    DCHECK(specifics->has_tab());
+
+    // Update tab node pool with the new association.
+    local_tab_pool_.ReassociateTabNode(tab_delegate.GetSyncId(), new_tab_id);
+    TabLink* tab_link = new TabLink(tab_delegate.GetSyncId(), &tab_delegate);
+    local_tab_map_[new_tab_id] = make_linked_ptr<TabLink>(tab_link);
+
+    if (specifics->tab().tab_id() == new_tab_id &&
+        specifics->tab().window_id() == new_window_id)
       return;
-    }
-    local_tab_map_[new_tab_id] = iter->second;
-    local_tab_map_.erase(iter);
-  }
-
-  sync_pb::EntitySpecifics entity;
-  sync_pb::SessionSpecifics* specifics = entity.mutable_session();
-
-  // This will rewrite the tab id and window id based on the delegate. Note
-  // that it won't update the SessionWindow that holds the tab (which happens
-  // separately when the header node is associated).
-  LocalTabDelegateToSpecifics(tab_delegate, specifics);
-  DCHECK(specifics->has_tab());
-
-  syncer::SyncData data = syncer::SyncData::CreateLocalData(
-      TabNodePool::TabIdToTag(current_machine_tag_, specifics->tab_node_id()),
-      current_session_name_, entity);
-  change_output->push_back(
-      syncer::SyncChange(FROM_HERE, syncer::SyncChange::ACTION_UPDATE, data));
+
+    // Either the tab_id or window_id changed (e.g due to session restore), so
+    // update the sync node.
+    specifics->mutable_tab()->set_tab_id(new_tab_id);
+    specifics->mutable_tab()->set_window_id(new_window_id);
+    syncer::SyncData data = syncer::SyncData::CreateLocalData(
+        TabNodePool::TabIdToTag(current_machine_tag_, specifics->tab_node_id()),
+        current_session_name_, entity);
+    change_output->push_back(
+        syncer::SyncChange(FROM_HERE, syncer::SyncChange::ACTION_UPDATE, data));
+    return;
+  }
 }
 
 // static