[Sync] Restore previous session if no tabbed windows present

components/sync_sessions/sessions_sync_manager.cc

 // Copyright 2014 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 "components/sync_sessions/sessions_sync_manager.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/format_macros.h"
@@ skipping 92 matching lines @@
   }
 }
 
 // Ensure that the tab id is not invalid.
 bool ShouldSyncTabId(SessionID::id_type tab_id) {
   if (tab_id == kInvalidTabID)
     return false;
   return true;
 }
 
+SyncedSession::DeviceType ProtoDeviceTypeToSyncedSessionDeviceType(
+    sync_pb::SyncEnums::DeviceType proto_device_type) {
+  switch (proto_device_type) {
+    case sync_pb::SyncEnums_DeviceType_TYPE_WIN:
+      return SyncedSession::TYPE_WIN;
+    case sync_pb::SyncEnums_DeviceType_TYPE_MAC:
+      return SyncedSession::TYPE_MACOSX;
+    case sync_pb::SyncEnums_DeviceType_TYPE_LINUX:
+      return SyncedSession::TYPE_LINUX;
+    case sync_pb::SyncEnums_DeviceType_TYPE_CROS:
+      return SyncedSession::TYPE_CHROMEOS;
+    case sync_pb::SyncEnums_DeviceType_TYPE_PHONE:
+      return SyncedSession::TYPE_PHONE;
+    case sync_pb::SyncEnums_DeviceType_TYPE_TABLET:
+      return SyncedSession::TYPE_TABLET;
+    case sync_pb::SyncEnums_DeviceType_TYPE_OTHER:
+    // Intentionally fall-through

[skym, 2017/04/17 20:37:02]

Why allow a fall through? Why not have a compile error when someone adds a new
type?

[Nicolas Zea, 2017/04/17 21:24:54]

Good call, done.

+    default:
+      return SyncedSession::TYPE_OTHER;
+  }
+}
+
 }  // namespace
 
 // |local_device| is owned by ProfileSyncService, its lifetime exceeds
 // lifetime of SessionSyncManager.
 SessionsSyncManager::SessionsSyncManager(
     sync_sessions::SyncSessionsClient* sessions_client,
     syncer::SyncPrefs* sync_prefs,
     LocalDeviceInfoProvider* local_device,
     LocalSessionEventRouter* router,
     const base::Closure& sessions_updated_callback,
@@ skipping 99 matching lines @@
   merge_result.set_error(
       sync_processor_->ProcessSyncChanges(FROM_HERE, new_changes));
 
   local_event_router_->StartRoutingTo(this);
   return merge_result;
 }
 
 void SessionsSyncManager::AssociateWindows(
     ReloadTabsOption option,
     syncer::SyncChangeList* change_output) {
-  const std::string local_tag = current_machine_tag();
-  sync_pb::SessionSpecifics specifics;
-  specifics.set_session_tag(local_tag);
-  sync_pb::SessionHeader* header_s = specifics.mutable_header();
-  SyncedSession* current_session = session_tracker_.GetSession(local_tag);
-  current_session->modified_time = base::Time::Now();
-  header_s->set_client_name(current_session_name_);
-  header_s->set_device_type(current_device_type_);
+  // Note that |current_session| is a pointer owned by |session_tracker_|.
+  // |session_tracker_| will continue to update |current_session| under
+  // the hood, e.g. during ResetSessionTracking(..), so care must be taken

[skym, 2017/04/17 20:37:02]

I don't understand what this means and how you're taking care in this function.

[Nicolas Zea, 2017/04/17 21:24:54]

Expanded the comment to clarify. ResetSessionTracking will e.g. delete tabs from
the SyncedSession object. As such, you shouldn't expect any data previously
within the SyncedSession object (like SessionTab pointers) to remain valid after
interactions with the SyncedSessionTracker.

+  // accessing it.
+  SyncedSession* current_session =
+      session_tracker_.GetSession(current_machine_tag());
+  current_session->session_name = current_session_name_;
+  current_session->device_type =
+      ProtoDeviceTypeToSyncedSessionDeviceType(current_device_type_);
+  current_session->session_tag = current_machine_tag();
 
-  session_tracker_.ResetSessionTracking(local_tag);
   SyncedWindowDelegatesGetter::SyncedWindowDelegateMap windows =
       synced_window_delegates_getter()->GetSyncedWindowDelegates();
 
-  if (option == RELOAD_TABS) {
-    UMA_HISTOGRAM_COUNTS("Sync.SessionWindows", windows.size());
+  // On Android, it's possible to not have any tabbed windows if this is a cold
+  // start triggered for a custom tab. In that case, the previous session must
+  // be restored, otherwise it will be lost. On the other hand, if there is at
+  // least one tabbed window open, it's safe to overwrite the previous session
+  // entirely. See crbug.com/639009 for more info.
+  bool found_tabbed_window = false;
+  for (auto& window_iter_pair : windows) {
+    if (window_iter_pair.second->IsTypeTabbed())
+      found_tabbed_window = true;
   }
-  if (windows.size() == 0) {
-    // Assume that the window hasn't loaded. Attempting to associate now would
-    // clobber any old windows, so just return.
-    LOG(ERROR) << "No windows present, see crbug.com/639009";
-    return;
+
+  if (found_tabbed_window) {
+    // Just reset the session tracking. No need to worry about the previous
+    // session; the current tabbed windows are now the source of truth.
+    session_tracker_.ResetSessionTracking(current_machine_tag());
+    current_session->modified_time = base::Time::Now();
+  } else {
+    DVLOG(1) << "Found no tabbed windows. Reloading "
+             << current_session->windows.size()
+             << " windows from previous session.";
+
+    // A copy of the specifics must be made because |current_session| will be
+    // updated in place and therefore can't be relied on as the source of truth.
+    sync_pb::SessionHeader header_specifics;
+    header_specifics.CopyFrom(current_session->ToSessionHeaderProto());
+    session_tracker_.ResetSessionTracking(current_machine_tag());
+    PopulateSyncedSessionFromSpecifics(current_machine_tag(), header_specifics,
+                                       base::Time::Now(), current_session);
+
+    // The tab entities stored in sync have outdated SessionId values. Go
+    // through and update them to the new SessionIds.
+    for (auto& win_iter : current_session->windows) {
+      for (auto& tab : win_iter.second->wrapped_window.tabs) {
+        int sync_id = TabNodePool::kInvalidTabNodeID;
+        if (!session_tracker_.GetTabNodeFromLocalTabId(tab->tab_id.id(),
+                                                       &sync_id) ||
+            sync_id == TabNodePool::kInvalidTabNodeID) {
+          continue;
+        }
+        DVLOG(1) << "Rewriting tab node " << sync_id << " with tab id "
+                 << tab->tab_id.id();
+        UpdateTabSpecifics(sync_id, *tab, change_output);
+      }
+    }
   }
-  for (auto window_iter_pair : windows) {
+
+  for (auto& window_iter_pair : windows) {
     const SyncedWindowDelegate* window_delegate = window_iter_pair.second;
     if (option == RELOAD_TABS) {
       UMA_HISTOGRAM_COUNTS("Sync.SessionTabs", window_delegate->GetTabCount());
     }
 
-    // Make sure the window has tabs and a viewable window. The viewable window
-    // check is necessary because, for example, when a browser is closed the
-    // destructor is not necessarily run immediately. This means its possible
-    // for us to get a handle to a browser that is about to be removed. If
-    // the tab count is 0 or the window is null, the browser is about to be
-    // deleted, so we ignore it.
+    // Make sure the window has tabs and a viewable window. The viewable
+    // window check is necessary because, for example, when a browser is
+    // closed the destructor is not necessarily run immediately. This means
+    // its possible for us to get a handle to a browser that is about to be
+    // removed. If the tab count is 0 or the window is null, the browser is
+    // about to be deleted, so we ignore it.
     if (window_delegate->ShouldSync() && window_delegate->GetTabCount() &&
         window_delegate->HasWindow()) {
       sync_pb::SessionWindow window_s;
       SessionID::id_type window_id = window_delegate->GetSessionId();
       DVLOG(1) << "Associating window " << window_id << " with "
                << window_delegate->GetTabCount() << " tabs.";
-      window_s.set_window_id(window_id);
-      // Note: We don't bother to set selected tab index anymore. We still
-      // consume it when receiving foreign sessions, as reading it is free, but
-      // it triggers too many sync cycles with too little value to make setting
-      // it worthwhile.
-      if (window_delegate->IsTypeTabbed()) {
-        window_s.set_browser_type(
-            sync_pb::SessionWindow_BrowserType_TYPE_TABBED);
-      } else if (window_delegate->IsTypePopup()) {
-        window_s.set_browser_type(
-            sync_pb::SessionWindow_BrowserType_TYPE_POPUP);
-      } else {
-        // This is a custom tab within an app. These will not be restored on
-        // startup if not present.
-        window_s.set_browser_type(
-            sync_pb::SessionWindow_BrowserType_TYPE_CUSTOM_TAB);
-      }
 
       bool found_tabs = false;
       for (int j = 0; j < window_delegate->GetTabCount(); ++j) {
         SessionID::id_type tab_id = window_delegate->GetTabIdAt(j);
         SyncedTabDelegate* synced_tab = window_delegate->GetTabAt(j);
 
-        // GetTabAt can return a null tab; in that case just skip it.
-        if (!synced_tab)
+        // GetTabAt can return a null tab; in that case just skip it. Similarly,
+        // if for some reason the tab id is invalid, skip it.
+        if (!synced_tab || !ShouldSyncTabId(tab_id))
           continue;
 
-        if (!ShouldSyncTabId(tab_id)) {
-          LOG(ERROR) << "Not syncing invalid tab with id " << tab_id;
-          continue;
-        }
-
         // Placeholder tabs are those without WebContents, either because they
         // were never loaded into memory or they were evicted from memory
-        // (typically only on Android devices). They only have a tab id, window
-        // id, and a saved synced id (corresponding to the tab node id). Note
-        // that only placeholders have this sync id, as it's necessary to
-        // properly reassociate the tab with the entity that was backing it.
+        // (typically only on Android devices). They only have a tab id,
+        // window id, and a saved synced id (corresponding to the tab node
+        // id). Note that only placeholders have this sync id, as it's
+        // necessary to properly reassociate the tab with the entity that was
+        // backing it.
         if (synced_tab->IsPlaceholderTab()) {
           // For tabs without WebContents update the |tab_id| and |window_id|,
           // as it could have changed after a session restore.
           if (synced_tab->GetSyncId() > TabNodePool::kInvalidTabNodeID) {
             AssociateRestoredPlaceholderTab(*synced_tab, tab_id, window_id,
                                             change_output);
+          } else {
+            DVLOG(1) << "Placeholder tab " << tab_id << " has no sync id.";
           }
         } else if (RELOAD_TABS == option) {
           AssociateTab(synced_tab, change_output);
         }
 
         // If the tab was syncable, it would have been added to the tracker
-        // either by the above Associate[RestoredPlaceholder]Tab call or by the
-        // OnLocalTabModified method invoking AssociateTab directly. Therefore,
-        // we can key whether this window has valid tabs based on the tab's
-        // presence in the tracker.
+        // either by the above Associate[RestoredPlaceholder]Tab call or by
+        // the OnLocalTabModified method invoking AssociateTab directly.
+        // Therefore, we can key whether this window has valid tabs based on
+        // the tab's presence in the tracker.
         const sessions::SessionTab* tab = nullptr;
-        if (session_tracker_.LookupSessionTab(local_tag, tab_id, &tab)) {
+        if (session_tracker_.LookupSessionTab(current_machine_tag(), tab_id,
+                                              &tab)) {
           found_tabs = true;
-          window_s.add_tab(tab_id);
+
+          // Update this window's representation in the synced session tracker.
+          // This is a no-op if called multiple times.
+          session_tracker_.PutWindowInSession(current_machine_tag(), window_id);
+
+          // Put the tab in the window (must happen after the window is added
+          // to the session).
+          session_tracker_.PutTabInWindow(current_machine_tag(), window_id,
+                                          tab_id);
         }
       }
       if (found_tabs) {
-        sync_pb::SessionWindow* header_window = header_s->add_window();
-        *header_window = window_s;
-
-        // Update this window's representation in the synced session tracker.
-        session_tracker_.PutWindowInSession(local_tag, window_id);
-        BuildSyncedSessionFromSpecifics(
-            local_tag, window_s, current_session->modified_time,
-            current_session->windows[window_id].get());
+        SyncedSessionWindow* synced_session_window =
+            current_session->windows[window_id].get();
+        // Note: We don't bother to set selected tab index anymore. We still

[skym, 2017/04/17 20:37:02]

Seems like we should ideally be able to stop committing headers that have just a
selected index, like avoid including this field in the hash or something. Maybe
this could be a TODO instead until we have that ability?

[Nicolas Zea, 2017/04/17 21:24:54]

This comment is out of date actually (was copied over). We're not doing anything
with selected tab index. We never commit it.

+        // consume it when receiving foreign sessions, as reading it is free,
+        // but it triggers too many sync cycles with too little value to make
+        // setting it worthwhile.
+        if (window_delegate->IsTypeTabbed()) {
+          synced_session_window->window_type =
+              sync_pb::SessionWindow_BrowserType_TYPE_TABBED;
+        } else if (window_delegate->IsTypePopup()) {
+          synced_session_window->window_type =
+              sync_pb::SessionWindow_BrowserType_TYPE_POPUP;
+        } else {
+          // This is a custom tab within an app. These will not be restored on
+          // startup if not present.
+          synced_session_window->window_type =
+              sync_pb::SessionWindow_BrowserType_TYPE_CUSTOM_TAB;
+        }
       }
     }
   }
   std::set<int> deleted_tab_node_ids;
   session_tracker_.CleanupLocalTabs(&deleted_tab_node_ids);
   AppendDeletionsForTabNodes(deleted_tab_node_ids, current_machine_tag(),
                              change_output);
 
   // Always update the header.  Sync takes care of dropping this update
   // if the entity specifics are identical (i.e windows, client name did
   // not change).
   sync_pb::EntitySpecifics entity;
-  entity.mutable_session()->CopyFrom(specifics);
+  entity.mutable_session()->set_session_tag(current_machine_tag());
+  entity.mutable_session()->mutable_header()->CopyFrom(
+      current_session->ToSessionHeaderProto());
   syncer::SyncData data = syncer::SyncData::CreateLocalData(
       current_machine_tag(), current_session_name_, entity);
   change_output->push_back(
       syncer::SyncChange(FROM_HERE, syncer::SyncChange::ACTION_UPDATE, data));
 }
 
 void SessionsSyncManager::AssociateTab(SyncedTabDelegate* const tab_delegate,
                                        syncer::SyncChangeList* change_output) {
   DCHECK(!tab_delegate->IsPlaceholderTab());
 
   if (tab_delegate->IsBeingDestroyed()) {
     // Do nothing. By not proactively adding the tab to the session, it will be
     // removed if necessary during subsequent cleanup.
     return;
   }
 
   if (!tab_delegate->ShouldSync(sessions_client_))
     return;
 
   SessionID::id_type tab_id = tab_delegate->GetSessionId();
   DVLOG(1) << "Syncing tab " << tab_id << " from window "
            << tab_delegate->GetWindowId();
 
   int tab_node_id = TabNodePool::kInvalidTabNodeID;
-  bool existing_tab_node =
-      session_tracker_.GetTabNodeFromLocalTabId(tab_id, &tab_node_id);
-  CHECK_NE(TabNodePool::kInvalidTabNodeID, tab_node_id) << "crbug.com/673618";
-  tab_delegate->SetSyncId(tab_node_id);
+  bool existing_tab_node = true;
+  if (session_tracker_.IsLocalTabNodeAssociated(tab_delegate->GetSyncId())) {
+    tab_node_id = tab_delegate->GetSyncId();
+    session_tracker_.ReassociateLocalTab(tab_node_id, tab_id);
+  } else {
+    existing_tab_node =
+        session_tracker_.GetTabNodeFromLocalTabId(tab_id, &tab_node_id);
+    CHECK_NE(TabNodePool::kInvalidTabNodeID, tab_node_id) << "crbug.com/673618";
+    tab_delegate->SetSyncId(tab_node_id);
+  }
+
   sessions::SessionTab* session_tab =
       session_tracker_.GetTab(current_machine_tag(), tab_id);
 
   // Get the previously synced url.
   int old_index = session_tab->normalized_navigation_index();
   GURL old_url;
   if (session_tab->navigations.size() > static_cast<size_t>(old_index))
     old_url = session_tab->navigations[old_index].virtual_url();
 
   // Update the tracker's session representation.
@@ skipping 234 matching lines @@
   if (!session_tracker_.LookupAllForeignSessions(
           sessions, SyncedSessionTracker::PRESENTABLE))
     return false;
   std::sort(sessions->begin(), sessions->end(), SessionsRecencyComparator);
   return true;
 }
 
 bool SessionsSyncManager::InitFromSyncModel(
     const syncer::SyncDataList& sync_data,
     syncer::SyncChangeList* new_changes) {
+  // Map of all rewritten local ids. Because ids are reset on each restart,
+  // and id generation happens outside of Sync, all ids from a previous local
+  // session must be rewritten in order to be valid.
+  // Key: previous session id. Value: new session id.
+  std::map<SessionID::id_type, SessionID::id_type> session_id_map;
+
   bool found_current_header = false;
   int bad_foreign_hash_count = 0;
   for (syncer::SyncDataList::const_iterator it = sync_data.begin();
        it != sync_data.end(); ++it) {
     const syncer::SyncData& data = *it;
     DCHECK(data.GetSpecifics().has_session());
     syncer::SyncDataRemote remote(data);
     const sync_pb::SessionSpecifics& specifics = data.GetSpecifics().session();
     if (specifics.session_tag().empty() ||
         (specifics.has_tab() &&
@@ skipping 16 matching lines @@
             syncer::SyncChange(FROM_HERE, SyncChange::ACTION_DELETE, remote));
       }
     } else {
       // This is previously stored local session information.
       if (specifics.has_header() && !found_current_header) {
         // This is our previous header node, reuse it.
         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.
+        // The specifics from the SyncData are immutable. Create a mutable copy
+        // to hold the rewritten ids.
+        sync_pb::SessionSpecifics rewritten_specifics(specifics);
+
+        // Go through and generate new tab and window ids as necessary, updating
+        // the specifics in place.
+        for (auto& window :
+             *rewritten_specifics.mutable_header()->mutable_window()) {
+          session_id_map[window.window_id()] = SessionID().id();
+          window.set_window_id(session_id_map[window.window_id()]);
+
+          google::protobuf::RepeatedField<int>* tab_ids = window.mutable_tab();
+          for (int i = 0; i < tab_ids->size(); i++) {
+            auto tab_iter = session_id_map.find(tab_ids->Get(i));
+            if (tab_iter == session_id_map.end()) {
+              session_id_map[tab_ids->Get(i)] = SessionID().id();

[skym, 2017/04/17 20:37:02]

I was really confused by this SessionID call until I looked up the code. I wish
SessionID::SessionID() was private and you had to go through a static function
like

private SessionID::SessionID(id_type id) : id_(id) {}

public static SessionID SessionID::GenerateUnique() {
  return SessionID(next_id++);
}

session_id_map[tab_ids->Get(i)] = SessionID::GenerateUnique().id();

[Nicolas Zea, 2017/04/17 21:24:54]

Yeah, I know what you mean. Added a comment to make clear the intent here.

+            }
+            *(tab_ids->Mutable(i)) = session_id_map[tab_ids->Get(i)];
+            // Note: the tab id of the SessionTab will be updated when the tab
+            // node itself is processed.
+          }
+        }
+
+        UpdateTrackerWithSpecifics(rewritten_specifics,
+                                   remote.GetModifiedTime());
+
+        DVLOG(1) << "Loaded local header and rewrote " << session_id_map.size()
+                 << " ids.";
+
       } else {
         if (specifics.has_header() || !specifics.has_tab()) {
           LOG(WARNING) << "Found more than one session header node with local "
                        << "tag.";
           syncer::SyncChange tombstone(TombstoneTab(specifics));
           if (tombstone.IsValid())
             new_changes->push_back(tombstone);
         } else if (specifics.tab().tab_id() == kInvalidTabID) {
           LOG(WARNING) << "Found tab node with invalid tab id.";
           syncer::SyncChange tombstone(TombstoneTab(specifics));
           if (tombstone.IsValid())
             new_changes->push_back(tombstone);
         } else {
           // This is a valid old tab node, add it to the tracker and associate
-          // it.
+          // it (using the new tab id).
           DVLOG(1) << "Associating local tab " << specifics.tab().tab_id()
                    << " with node " << specifics.tab_node_id();
-          session_tracker_.ReassociateLocalTab(specifics.tab_node_id(),
-                                               specifics.tab().tab_id());
-          UpdateTrackerWithSpecifics(specifics, remote.GetModifiedTime());
+
+          // Now file the tab under the new tab id.
+          SessionID::id_type new_tab_id = kInvalidTabID;
+          auto iter = session_id_map.find(specifics.tab().tab_id());
+          if (iter != session_id_map.end()) {
+            new_tab_id = iter->second;
+          } else {
+            session_id_map[specifics.tab().tab_id()] = SessionID().id();
+            new_tab_id = session_id_map[specifics.tab().tab_id()];
+          }
+          DVLOG(1) << "Remapping tab " << specifics.tab().tab_id() << " to "
+                   << new_tab_id;
+
+          // The specifics from the SyncData are immutable. Create a mutable
+          // copy to hold the rewritten ids.
+          sync_pb::SessionSpecifics rewritten_specifics(specifics);
+          rewritten_specifics.mutable_tab()->set_tab_id(new_tab_id);
+          session_tracker_.ReassociateLocalTab(
+              rewritten_specifics.tab_node_id(), new_tab_id);
+          UpdateTrackerWithSpecifics(rewritten_specifics,
+                                     remote.GetModifiedTime());
+
+          session_tracker_.ReassociateLocalTab(
+              rewritten_specifics.tab_node_id(), new_tab_id);
         }
       }
     }
   }
 
   // Cleanup all foreign sessions, since orphaned tabs may have been added after
   // the header.
   std::vector<const SyncedSession*> sessions;
   session_tracker_.LookupAllForeignSessions(&sessions,
                                             SyncedSessionTracker::RAW);
   for (const auto* session : sessions) {
-    session_tracker_.CleanupForeignSession(session->session_tag);
+    session_tracker_.CleanupSession(session->session_tag);
   }
 
   UMA_HISTOGRAM_COUNTS_100("Sync.SessionsBadForeignHashOnMergeCount",
                            bad_foreign_hash_count);
 
   return found_current_header;
 }
 
 void SessionsSyncManager::UpdateTrackerWithSpecifics(
     const sync_pb::SessionSpecifics& specifics,
     const base::Time& modification_time) {
   std::string session_tag = specifics.session_tag();
   SyncedSession* session = session_tracker_.GetSession(session_tag);
   if (specifics.has_header()) {
     // Read in the header data for this 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 << ".";
       return;
     }
 
     // Load (or create) the SyncedSession object for this client.
     const sync_pb::SessionHeader& header = specifics.header();
-    PopulateSessionHeaderFromSpecifics(header, modification_time, 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);
 
-    // Process all the windows and their tab information.
-    int num_windows = header.window_size();
-    DVLOG(1) << "Populating " << session_tag << " with " << num_windows
-             << " windows.";
+    PopulateSyncedSessionFromSpecifics(session_tag, header, modification_time,
+                                       session);
 
-    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());
-    }
     // Delete any closed windows and unused tabs as necessary.
-    session_tracker_.CleanupForeignSession(session_tag);
+    session_tracker_.CleanupSession(session_tag);
   } else if (specifics.has_tab()) {
     const sync_pb::SessionTab& tab_s = specifics.tab();
     SessionID::id_type tab_id = tab_s.tab_id();
     DVLOG(1) << "Populating " << session_tag << "'s tab id " << tab_id
              << " from node " << specifics.tab_node_id();
 
     // Ensure the tracker is aware of the tab node id. Deleting foreign sessions
     // requires deleting all relevant tab nodes, and it's easier to track the
     // tab node ids themselves separately from the tab ids.
     //
@@ skipping 51 matching lines @@
     current_machine_tag_ = persisted_guid;
     DVLOG(1) << "Restoring persisted session sync guid: " << persisted_guid;
   } else {
     DCHECK(!cache_guid.empty());
     current_machine_tag_ = BuildMachineTag(cache_guid);
     DVLOG(1) << "Creating session sync guid: " << current_machine_tag_;
     sync_prefs_->SetSyncSessionsGUID(current_machine_tag_);
   }
 }
 
+void SessionsSyncManager::PopulateSyncedSessionFromSpecifics(
+    const std::string& session_tag,
+    const sync_pb::SessionHeader& header_specifics,
+    base::Time mtime,
+    SyncedSession* synced_session) {
+  if (header_specifics.has_client_name())
+    synced_session->session_name = header_specifics.client_name();
+  if (header_specifics.has_device_type()) {
+    synced_session->device_type = ProtoDeviceTypeToSyncedSessionDeviceType(
+        header_specifics.device_type());
+  }
+  synced_session->modified_time =
+      std::max(mtime, synced_session->modified_time);
+
+  // Process all the windows and their tab information.
+  int num_windows = header_specifics.window_size();
+  DVLOG(1) << "Populating " << session_tag << " with " << num_windows
+           << " windows.";
+
+  for (int i = 0; i < num_windows; ++i) {
+    const sync_pb::SessionWindow& window_s = header_specifics.window(i);
+    SessionID::id_type window_id = window_s.window_id();
+    session_tracker_.PutWindowInSession(session_tag, window_id);
+    PopulateSyncedSessionWindowFromSpecifics(
+        session_tag, window_s, synced_session->modified_time,
+        synced_session->windows[window_id].get());
+  }
+}
+
 // static
-void SessionsSyncManager::PopulateSessionHeaderFromSpecifics(
-    const sync_pb::SessionHeader& header_specifics,
-    base::Time mtime,
-    SyncedSession* session_header) {
-  if (header_specifics.has_client_name())
-    session_header->session_name = header_specifics.client_name();
-  if (header_specifics.has_device_type()) {
-    switch (header_specifics.device_type()) {
-      case sync_pb::SyncEnums_DeviceType_TYPE_WIN:
-        session_header->device_type = SyncedSession::TYPE_WIN;
-        break;
-      case sync_pb::SyncEnums_DeviceType_TYPE_MAC:
-        session_header->device_type = SyncedSession::TYPE_MACOSX;
-        break;
-      case sync_pb::SyncEnums_DeviceType_TYPE_LINUX:
-        session_header->device_type = SyncedSession::TYPE_LINUX;
-        break;
-      case sync_pb::SyncEnums_DeviceType_TYPE_CROS:
-        session_header->device_type = SyncedSession::TYPE_CHROMEOS;
-        break;
-      case sync_pb::SyncEnums_DeviceType_TYPE_PHONE:
-        session_header->device_type = SyncedSession::TYPE_PHONE;
-        break;
-      case sync_pb::SyncEnums_DeviceType_TYPE_TABLET:
-        session_header->device_type = SyncedSession::TYPE_TABLET;
-        break;
-      case sync_pb::SyncEnums_DeviceType_TYPE_OTHER:
-      // Intentionally fall-through
-      default:
-        session_header->device_type = SyncedSession::TYPE_OTHER;
-        break;
-    }
-  }
-  session_header->modified_time =
-      std::max(mtime, session_header->modified_time);
-}
-
-// static
-void SessionsSyncManager::BuildSyncedSessionFromSpecifics(
+void SessionsSyncManager::PopulateSyncedSessionWindowFromSpecifics(
     const std::string& session_tag,
     const sync_pb::SessionWindow& specifics,
     base::Time mtime,
     SyncedSessionWindow* synced_session_window) {
   sessions::SessionWindow* session_window =
       &synced_session_window->wrapped_window;
   if (specifics.has_window_id())
     session_window->window_id.set_id(specifics.window_id());
   if (specifics.has_selected_tab_index())
     session_window->selected_tab_index = specifics.selected_tab_index();
@@ skipping 135 matching lines @@
 
   // Update tracker with the new association (and inform it of the tab node
   // in the process).
   session_tracker_.ReassociateLocalTab(tab_delegate.GetSyncId(), new_tab_id);
 
   // Update the window id on the SessionTab itself.
   sessions::SessionTab* local_tab =
       session_tracker_.GetTab(current_machine_tag(), new_tab_id);
   local_tab->window_id.set_id(new_window_id);
 
+  UpdateTabSpecifics(tab_delegate.GetSyncId(), *local_tab, change_output);
+}
+
+void SessionsSyncManager::UpdateTabSpecifics(
+    int sync_id,
+    const sessions::SessionTab& tab,
+    syncer::SyncChangeList* change_output) {
   // Rewrite the specifics based on the reassociated SessionTab to preserve
   // the new tab and window ids.
   sync_pb::EntitySpecifics entity;
-  entity.mutable_session()->CopyFrom(SessionTabToSpecifics(
-      *local_tab, current_machine_tag(), tab_delegate.GetSyncId()));
+  entity.mutable_session()->CopyFrom(
+      SessionTabToSpecifics(tab, current_machine_tag(), sync_id));
   syncer::SyncData data = syncer::SyncData::CreateLocalData(
-      TabNodeIdToTag(current_machine_tag(), tab_delegate.GetSyncId()),
-      current_session_name_, entity);
+      TabNodeIdToTag(current_machine_tag(), sync_id), current_session_name_,
+      entity);
   change_output->push_back(
       syncer::SyncChange(FROM_HERE, syncer::SyncChange::ACTION_UPDATE, data));
 }
 
 // static
 void SessionsSyncManager::SetSessionTabFromDelegate(
     const SyncedTabDelegate& tab_delegate,
     base::Time mtime,
     sessions::SessionTab* session_tab) {
   DCHECK(session_tab);
@@ skipping 88 matching lines @@
 }
 
 // static
 std::string SessionsSyncManager::TagHashFromSpecifics(
     const sync_pb::SessionSpecifics& specifics) {
   return syncer::GenerateSyncableHash(syncer::SESSIONS,
                                       TagFromSpecifics(specifics));
 }
 
 };  // namespace sync_sessions