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

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/memory/ptr_util.h"
@@ skipping 138 matching lines @@
     merge_result.set_error(error_handler_->CreateAndUploadError(
         FROM_HERE, "Failed to get local device info."));
     return merge_result;
   }
 
   session_tracker_.SetLocalSessionTag(current_machine_tag_);
 
   syncer::SyncChangeList new_changes;
 
   // First, we iterate over sync data to update our session_tracker_.
-  syncer::SyncDataList restored_tabs;
-  if (!InitFromSyncModel(initial_sync_data, &restored_tabs, &new_changes)) {
+  if (!InitFromSyncModel(initial_sync_data, &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();
     base_specifics->set_session_tag(current_machine_tag());
     sync_pb::SessionHeader* header_s = base_specifics->mutable_header();
     header_s->set_client_name(current_session_name_);
     header_s->set_device_type(local_device_info->device_type());
     syncer::SyncData data = syncer::SyncData::CreateLocalData(
         current_machine_tag(), current_session_name_, specifics);
     new_changes.push_back(
         syncer::SyncChange(FROM_HERE, syncer::SyncChange::ACTION_ADD, data));
   }
 
 #if defined(OS_ANDROID)
   std::string sync_machine_tag(
       BuildMachineTag(local_device_->GetLocalSyncCacheGUID()));
   if (current_machine_tag_.compare(sync_machine_tag) != 0)
     DeleteForeignSessionInternal(sync_machine_tag, &new_changes);
 #endif
 
   // Check if anything has changed on the local client side.
-  AssociateWindows(RELOAD_TABS, restored_tabs, &new_changes);
+  AssociateWindows(RELOAD_TABS, &new_changes);
   local_tab_pool_out_of_sync_ = false;
 
   merge_result.set_error(
       sync_processor_->ProcessSyncChanges(FROM_HERE, new_changes));
 
   local_event_router_->StartRoutingTo(this);
   return merge_result;
 }
 
 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;
   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_);
   // SessionDataTypeController ensures that the local device info
   // is available before activating this datatype.
@@ skipping 61 matching lines @@
 
         if (synced_tab->IsPlaceholderTab()) {
           // For tabs without WebContents update the |tab_id| and |window_id|,
           // as it could have changed after a session restore.
           // Note: We cannot check if a tab is valid if it has no WebContents.
           // We assume any such tab is valid and leave the contents of
           // corresponding sync node unchanged.
           if (synced_tab->GetSyncId() > TabNodePool::kInvalidTabNodeID &&
               tab_id > TabNodePool::kInvalidTabID) {
             AssociateRestoredPlaceholderTab(*synced_tab, tab_id, window_id,
-                                            restored_tabs, change_output);
+                                            change_output);
             found_tabs = true;
             window_s.add_tab(tab_id);
           }
           continue;
         }
 
         if (RELOAD_TABS == option)
           AssociateTab(synced_tab, change_output);
 
         // If the tab is valid, it would have been added to the tracker either
@@ skipping 58 matching lines @@
 
   if (local_tab_map_iter == local_tab_map_.end()) {
     int tab_node_id = tab->GetSyncId();
     // If there is an old sync node for the tab, reuse it.  If this is a new
     // tab, get a sync node for it.
     if (!local_tab_pool_.IsUnassociatedTabNode(tab_node_id)) {
       tab_node_id = local_tab_pool_.GetFreeTabNode(change_output);
       tab->SetSyncId(tab_node_id);
     }
     local_tab_pool_.AssociateTabNode(tab_node_id, tab_id);
-    tab_link = new TabLink(tab_node_id, tab);
+    tab_link = new TabLink(tab_node_id);
     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

[maxbogue, 2016/11/10 06:43:25]

This note is now obsolete.

[Nicolas Zea, 2016/11/10 18:25:36]

Done.

     // 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);
   DVLOG(1) << "Reloading tab " << tab_id << " from window "
            << tab->GetWindowId();
 
   // Write to sync model.
   sync_pb::EntitySpecifics specifics;
   LocalTabDelegateToSpecifics(*tab, specifics.mutable_session());
   syncer::SyncData data = syncer::SyncData::CreateLocalData(
@@ skipping 65 matching lines @@
     DCHECK(!local_tab_pool_out_of_sync_);
     return;
   }
 
   syncer::SyncChangeList changes;
   AssociateTab(modified_tab, &changes);
   // Note, we always associate windows because it's possible a tab became
   // "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, syncer::SyncDataList(), &changes);
+  AssociateWindows(DONT_RELOAD_TABS, &changes);
   sync_processor_->ProcessSyncChanges(FROM_HERE, changes);
 }
 
 void SessionsSyncManager::OnFaviconsChanged(const std::set<GURL>& page_urls,
                                             const GURL& /* icon_url */) {
   // 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 (const GURL& page_url : page_urls) {
     for (TabLinksMap::iterator tab_iter = local_tab_map_.begin();
          tab_iter != local_tab_map_.end(); ++tab_iter) {
@@ skipping 114 matching lines @@
         break;
       case syncer::SyncChange::ACTION_ADD:
       case syncer::SyncChange::ACTION_UPDATE:
         if (current_machine_tag() == session.session_tag()) {
           // We should only ever receive a change to our own machine's session
           // info if encryption was turned on. In that case, the data is still
           // the same, so we can ignore.
           LOG(WARNING) << "Dropping modification to local session.";
           return syncer::SyncError();
         }
-        UpdateTrackerWithForeignSession(
+        UpdateTrackerWithSpecifics(
             session, syncer::SyncDataRemote(it->sync_data()).GetModifiedTime());
         break;
       default:
         NOTREACHED() << "Processing sync changes failed, unknown change type.";
     }
   }
 
   if (!sessions_updated_callback_.is_null())
     sessions_updated_callback_.Run();
   return syncer::SyncError();
@@ skipping 17 matching lines @@
     std::vector<const SyncedSession*>* sessions) {
   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::SyncDataList* restored_tabs,
     syncer::SyncChangeList* new_changes) {
   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() &&
          (!specifics.has_tab_node_id() || !specifics.tab().has_tab_id()))) {
       syncer::SyncChange tombstone(TombstoneTab(specifics));
       if (tombstone.IsValid())
         new_changes->push_back(tombstone);
     } else if (specifics.session_tag() != current_machine_tag()) {
       if (TagHashFromSpecifics(specifics) == remote.GetClientTagHash()) {
-        UpdateTrackerWithForeignSession(specifics, remote.GetModifiedTime());
+        UpdateTrackerWithSpecifics(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
         // to that foreign data (like deletion through garbage collection) to
         // trigger a data type error because the tag looking mechanism fails. So
         // look for these and delete via remote SyncData, which uses a server id
         // lookup mechanism instead, see crbug.com/604657.
         bad_foreign_hash_count++;
         new_changes->push_back(
             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.
       } 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 {
-          // This is a valid old tab node, add it to the pool so it can be
-          // reused for reassociation.
+          // 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()));
           local_tab_pool_.AddTabNode(specifics.tab_node_id());
-          restored_tabs->push_back(*it);
+          local_tab_pool_.AssociateTabNode(specifics.tab_node_id(),
+                                           specifics.tab().tab_id());
+          UpdateTrackerWithSpecifics(specifics, remote.GetModifiedTime());
         }
       }
     }
   }
 
   // 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_.CleanupSession(session->session_tag);
   }
 
   UMA_HISTOGRAM_COUNTS_100("Sync.SessionsBadForeignHashOnMergeCount",
                            bad_foreign_hash_count);
 
   return found_current_header;
 }
 
-void SessionsSyncManager::UpdateTrackerWithForeignSession(
+void SessionsSyncManager::UpdateTrackerWithSpecifics(
     const sync_pb::SessionSpecifics& specifics,
     const base::Time& modification_time) {
-  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);
+  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 foreign session. Header data is
+    // 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 foreign session node with invalid header "
-                   << "and tag " << foreign_session_tag << ".";
+      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,
-                                       foreign_session);
+    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(foreign_session_tag);
+    session_tracker_.ResetSessionTracking(session_tag);
 
     // Process all the windows and their tab information.
     int num_windows = header.window_size();
-    DVLOG(1) << "Associating " << foreign_session_tag << " with " << num_windows
+    DVLOG(1) << "Associating " << 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(foreign_session_tag, window_id);
-      BuildSyncedSessionFromSpecifics(
-          foreign_session_tag, window_s, modification_time,
-          foreign_session->windows[window_id].get());
+      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_.CleanupSession(foreign_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();
 
     const sessions::SessionTab* existing_tab;
-    if (session_tracker_.LookupSessionTab(foreign_session_tag, tab_id,
-                                          &existing_tab) &&
+    if (session_tracker_.LookupSessionTab(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(foreign_session_tag, tab_id,
-                              specifics.tab_node_id());
-      DVLOG(1) << "Ignoring " << foreign_session_tag << "'s session tab "
-               << tab_id << " with earlier modification time";
+      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";
       return;
     }
 
-    sessions::SessionTab* tab = session_tracker_.GetTab(
-        foreign_session_tag, tab_id, specifics.tab_node_id());
+    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());
 
     // Update SessionTab based on protobuf.
     tab->SetFromSyncData(tab_s, modification_time);
 
     // If a favicon or favicon urls are present, load the URLs and visit
     // times into the in-memory favicon cache.
     RefreshFaviconVisitTimesFromForeignTab(tab_s, modification_time);
 
     // Update the last modified time.
-    if (foreign_session->modified_time < modification_time)
-      foreign_session->modified_time = modification_time;
+    if (session->modified_time < modification_time)
+      session->modified_time = modification_time;
   } else {
-    LOG(WARNING) << "Ignoring foreign session node with missing header/tab "
-                 << "fields and tag " << foreign_session_tag << ".";
+    LOG(WARNING) << "Ignoring session node with missing header/tab "
+                 << "fields and tag " << session_tag << ".";
   }
 }
 
 void SessionsSyncManager::InitializeCurrentMachineTag() {
   DCHECK(current_machine_tag_.empty());
   std::string persisted_guid;
   persisted_guid = sync_prefs_->GetSyncSessionsGUID();
   if (!persisted_guid.empty()) {
     current_machine_tag_ = persisted_guid;
     DVLOG(1) << "Restoring persisted session sync guid: " << persisted_guid;
@@ skipping 180 matching lines @@
   const sessions::SessionTab* synced_tab = nullptr;
   bool success = session_tracker_.LookupSessionTab(tag, tab_id, &synced_tab);
   if (success)
     *tab = synced_tab;
   return success;
 }
 
 void SessionsSyncManager::LocalTabDelegateToSpecifics(
     const SyncedTabDelegate& tab_delegate,
     sync_pb::SessionSpecifics* specifics) {
   sessions::SessionTab* session_tab = nullptr;

[maxbogue, 2016/11/10 06:43:25]

Is there any particular reason this is a separate line before the GetTab line?
Seems unnecessary.

[Nicolas Zea, 2016/11/10 18:25:36]

Done.

-  session_tab = session_tracker_.GetTab(current_machine_tag(),
-                                        tab_delegate.GetSessionId(),
+
+  SessionID::id_type tab_id = tab_delegate.GetSessionId();
+  session_tab = session_tracker_.GetTab(current_machine_tag(), tab_id,
                                         tab_delegate.GetSyncId());
   SetSessionTabFromDelegate(tab_delegate, base::Time::Now(), session_tab);
   SetVariationIds(session_tab);
   sync_pb::SessionTab tab_s = session_tab->ToSyncData();
   specifics->set_session_tag(current_machine_tag_);
   specifics->set_tab_node_id(tab_delegate.GetSyncId());
   specifics->mutable_tab()->CopyFrom(tab_s);
 }
 
 void SessionsSyncManager::AssociateRestoredPlaceholderTab(
     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);
-  // Rewrite the tab using |restored_tabs| to retrieve the specifics.
-  if (restored_tabs.empty()) {
-    DLOG(WARNING) << "Can't Update tab ID.";
+
+  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 restore tab " << new_tab_id;

[maxbogue, 2016/11/10 06:43:26]

Maybe "Failed to reassociate tab "?

[Nicolas Zea, 2016/11/10 18:25:36]

Done.

     return;
   }
+  local_tab_pool_.ReassociateTabNode(tab_delegate.GetSyncId(), 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;
+  // 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 restore tab " << new_tab_id;

[maxbogue, 2016/11/10 06:43:25]

Maybe "Failed to update local tab map for "?

[Nicolas Zea, 2016/11/10 18:25:36]

Done.

+      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();
-    specifics->CopyFrom(it->GetSpecifics().session());
-    DCHECK(specifics->has_tab());
+  sync_pb::EntitySpecifics entity;
+  sync_pb::SessionSpecifics* specifics = entity.mutable_session();
 
-    // 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);
+  // 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());
 
-    if (specifics->tab().tab_id() == new_tab_id &&
-        specifics->tab().window_id() == new_window_id)
-      return;
-
-    // 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;
-  }
+  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));
 }
 
 // static
 void SessionsSyncManager::SetSessionTabFromDelegate(
     const SyncedTabDelegate& tab_delegate,
     base::Time mtime,
     sessions::SessionTab* session_tab) {
   DCHECK(session_tab);
   session_tab->window_id.set_id(tab_delegate.GetWindowId());
   session_tab->tab_id.set_id(tab_delegate.GetSessionId());
@@ skipping 98 matching lines @@
 }
 
 // static
 std::string SessionsSyncManager::TagHashFromSpecifics(
     const sync_pb::SessionSpecifics& specifics) {
   return syncer::syncable::GenerateSyncableHash(syncer::SESSIONS,
                                                 TagFromSpecifics(specifics));
 }
 
 };  // namespace sync_sessions