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 4b0938e0bfaa7f4c6d87d6deb8872efc3c9cb78c..c13df7cb44fb3a9571d5c537c3d814652bc32d8b 100644 |
--- a/components/sync_sessions/sessions_sync_manager.cc |
+++ b/components/sync_sessions/sessions_sync_manager.cc |
@@ -7,9 +7,12 @@ |
#include <algorithm> |
#include <utility> |
+#include "base/format_macros.h" |
+#include "base/logging.h" |
#include "base/memory/ptr_util.h" |
#include "base/metrics/field_trial.h" |
#include "base/metrics/histogram_macros.h" |
+#include "base/strings/stringprintf.h" |
#include "build/build_config.h" |
#include "components/sync/base/hash_util.h" |
#include "components/sync/device_info/local_device_info_provider.h" |
@@ -21,6 +24,7 @@ |
#include "components/sync_sessions/synced_tab_delegate.h" |
#include "components/sync_sessions/synced_window_delegate.h" |
#include "components/sync_sessions/synced_window_delegates_getter.h" |
+#include "components/sync_sessions/tab_node_pool.h" |
#include "components/variations/variations_associated_data.h" |
using sessions::SerializedNavigationEntry; |
@@ -62,17 +66,60 @@ bool SessionsRecencyComparator(const SyncedSession* s1, |
return s1->modified_time > s2->modified_time; |
} |
+std::string TabNodeIdToTag(const std::string& machine_tag, int tab_node_id) { |
+ CHECK_GT(tab_node_id, TabNodePool::kInvalidTabNodeID) << "crbug.com/673618"; |
+ return base::StringPrintf("%s %d", machine_tag.c_str(), tab_node_id); |
+} |
+ |
std::string TagFromSpecifics(const sync_pb::SessionSpecifics& specifics) { |
if (specifics.has_header()) { |
return specifics.session_tag(); |
} else if (specifics.has_tab()) { |
- return TabNodePool::TabIdToTag(specifics.session_tag(), |
- specifics.tab_node_id()); |
+ return TabNodeIdToTag(specifics.session_tag(), specifics.tab_node_id()); |
} else { |
return std::string(); |
} |
} |
+sync_pb::SessionSpecifics SessionTabToSpecifics( |
+ const sessions::SessionTab& session_tab, |
+ const std::string& local_tag, |
+ int tab_node_id) { |
+ sync_pb::SessionSpecifics specifics; |
+ specifics.mutable_tab()->CopyFrom(session_tab.ToSyncData()); |
+ specifics.set_session_tag(local_tag); |
+ specifics.set_tab_node_id(tab_node_id); |
+ return specifics; |
+} |
+ |
+void AppendDeletionsForTabNodes(const std::set<int>& tab_node_ids, |
+ const std::string& machine_tag, |
+ syncer::SyncChangeList* change_output) { |
+ for (std::set<int>::const_iterator it = tab_node_ids.begin(); |
+ it != tab_node_ids.end(); ++it) { |
+ change_output->push_back(syncer::SyncChange( |
+ FROM_HERE, SyncChange::ACTION_DELETE, |
+ SyncData::CreateLocalDelete(TabNodeIdToTag(machine_tag, *it), |
+ syncer::SESSIONS))); |
+ } |
+} |
+ |
+// Ensure that the tab id is not invalid and is not already synced (which |
+// can confuse the logic that tracks whether tabs are mapped or unmapped). |
+bool ShouldSyncTabId( |
+ SessionID::id_type tab_id, |
+ const google::protobuf::RepeatedField<int>& synced_tab_ids) { |
+ if (tab_id == TabNodePool::kInvalidTabID) |
+ return false; |
+ |
+ for (auto synced_tab_id : synced_tab_ids) { |
+ if (tab_id == synced_tab_id) |
+ return false; |
+ } |
+ |
+ return true; |
+} |
+ |
} // namespace |
// |local_device| is owned by ProfileSyncService, its lifetime exceeds |
@@ -117,7 +164,6 @@ syncer::SyncMergeResult SessionsSyncManager::MergeDataAndStartSyncing( |
std::unique_ptr<syncer::SyncErrorFactory> error_handler) { |
syncer::SyncMergeResult merge_result(type); |
DCHECK(session_tracker_.Empty()); |
- DCHECK_EQ(0U, local_tab_pool_.Capacity()); |
error_handler_ = std::move(error_handler); |
sync_processor_ = std::move(sync_processor); |
@@ -153,13 +199,12 @@ syncer::SyncMergeResult SessionsSyncManager::MergeDataAndStartSyncing( |
InitializeCurrentMachineTag(local_device_->GetLocalSyncCacheGUID()); |
} |
- session_tracker_.SetLocalSessionTag(current_machine_tag_); |
+ 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(); |
@@ -176,12 +221,12 @@ syncer::SyncMergeResult SessionsSyncManager::MergeDataAndStartSyncing( |
#if defined(OS_ANDROID) |
std::string sync_machine_tag( |
BuildMachineTag(local_device_->GetLocalSyncCacheGUID())); |
- if (current_machine_tag_.compare(sync_machine_tag) != 0) |
+ 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( |
@@ -193,7 +238,6 @@ syncer::SyncMergeResult SessionsSyncManager::MergeDataAndStartSyncing( |
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; |
@@ -261,30 +305,33 @@ void SessionsSyncManager::AssociateWindows( |
if (!synced_tab) |
continue; |
+ if (!ShouldSyncTabId(tab_id, window_s.tab())) { |
+ LOG(ERROR) << "Not syncing invalid/duplicate 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. |
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) { |
+ if (synced_tab->GetSyncId() > TabNodePool::kInvalidTabNodeID) { |
AssociateRestoredPlaceholderTab(*synced_tab, tab_id, window_id, |
- restored_tabs, change_output); |
- found_tabs = true; |
- window_s.add_tab(tab_id); |
+ change_output); |
} |
- continue; |
- } |
- |
- if (RELOAD_TABS == option) |
+ } else if (RELOAD_TABS == option) { |
AssociateTab(synced_tab, change_output); |
+ } |
- // If the tab is valid, it would have been added to the tracker either |
- // by the above AssociateTab call (at association time), or by the |
- // change processor calling AssociateTab for all modified tabs. |
- // Therefore, we can key whether this window has valid tabs based on |
- // the tab's presence in the tracker. |
+ // 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. |
const sessions::SessionTab* tab = nullptr; |
if (session_tracker_.LookupSessionTab(local_tag, tab_id, &tab)) { |
found_tabs = true; |
@@ -303,8 +350,10 @@ void SessionsSyncManager::AssociateWindows( |
} |
} |
} |
- local_tab_pool_.DeleteUnassociatedTabNodes(change_output); |
- session_tracker_.CleanupSession(local_tag); |
+ 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 |
@@ -317,73 +366,63 @@ void SessionsSyncManager::AssociateWindows( |
syncer::SyncChange(FROM_HERE, syncer::SyncChange::ACTION_UPDATE, data)); |
} |
-void SessionsSyncManager::AssociateTab(SyncedTabDelegate* const tab, |
+void SessionsSyncManager::AssociateTab(SyncedTabDelegate* const tab_delegate, |
syncer::SyncChangeList* change_output) { |
- DCHECK(!tab->IsPlaceholderTab()); |
- 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()) { |
- // We aren't tracking this tab (for example, sync setting page). |
- return; |
- } |
- local_tab_pool_.FreeTabNode(tab_iter->second->tab_node_id(), change_output); |
- local_tab_map_.erase(tab_iter); |
+ 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->ShouldSync(sessions_client_)) |
+ if (!tab_delegate->ShouldSync(sessions_client_)) |
return; |
- TabLinksMap::iterator local_tab_map_iter = local_tab_map_.find(tab_id); |
- TabLink* tab_link = nullptr; |
- |
- 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); |
- 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); |
- DVLOG(1) << "Reloading tab " << tab_id << " from window " |
- << tab->GetWindowId(); |
+ 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); |
+ 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. |
+ SetSessionTabFromDelegate(*tab_delegate, base::Time::Now(), session_tab); |
+ SetVariationIds(session_tab); |
+ session_tracker_.GetSession(current_machine_tag())->modified_time = |
+ base::Time::Now(); |
- // Write to sync model. |
+ // Write to the sync model itself. |
sync_pb::EntitySpecifics specifics; |
- LocalTabDelegateToSpecifics(*tab, specifics.mutable_session()); |
+ specifics.mutable_session()->CopyFrom( |
+ SessionTabToSpecifics(*session_tab, current_machine_tag(), tab_node_id)); |
syncer::SyncData data = syncer::SyncData::CreateLocalData( |
- TabNodePool::TabIdToTag(current_machine_tag_, tab_link->tab_node_id()), |
- current_session_name_, specifics); |
- change_output->push_back( |
- syncer::SyncChange(FROM_HERE, syncer::SyncChange::ACTION_UPDATE, data)); |
- |
- int current_index = tab->GetCurrentEntryIndex(); |
- const GURL new_url = tab->GetVirtualURLAtIndex(current_index); |
- if (new_url != tab_link->url()) { |
- tab_link->set_url(new_url); |
- favicon_cache_.OnFaviconVisited(new_url, |
- tab->GetFaviconURLAtIndex(current_index)); |
+ TabNodeIdToTag(current_machine_tag(), tab_node_id), current_session_name_, |
+ specifics); |
+ change_output->push_back(syncer::SyncChange( |
+ FROM_HERE, existing_tab_node ? syncer::SyncChange::ACTION_UPDATE |
+ : syncer::SyncChange::ACTION_ADD, |
+ data)); |
+ |
+ int current_index = tab_delegate->GetCurrentEntryIndex(); |
+ const GURL new_url = tab_delegate->GetVirtualURLAtIndex(current_index); |
+ if (new_url != old_url) { |
+ favicon_cache_.OnFaviconVisited( |
+ new_url, tab_delegate->GetFaviconURLAtIndex(current_index)); |
page_revisit_broadcaster_.OnPageVisit( |
- new_url, tab->GetTransitionAtIndex(current_index)); |
+ new_url, tab_delegate->GetTransitionAtIndex(current_index)); |
} |
- |
- session_tracker_.GetSession(current_machine_tag())->modified_time = |
- base::Time::Now(); |
} |
bool SessionsSyncManager::RebuildAssociations() { |
@@ -443,21 +482,14 @@ void SessionsSyncManager::OnLocalTabModified(SyncedTabDelegate* modified_tab) { |
// "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) { |
- if (tab_iter->second->url() == page_url) |
- favicon_cache_.OnPageFaviconUpdated(page_url); |
- } |
- } |
+ for (const GURL& page_url : page_urls) |
+ favicon_cache_.OnPageFaviconUpdated(page_url); |
} |
void SessionsSyncManager::StopSyncing(syncer::ModelType type) { |
@@ -470,8 +502,6 @@ void SessionsSyncManager::StopSyncing(syncer::ModelType type) { |
sync_processor_.reset(nullptr); |
error_handler_.reset(); |
session_tracker_.Clear(); |
- local_tab_map_.clear(); |
- local_tab_pool_.Clear(); |
current_machine_tag_.clear(); |
current_session_name_.clear(); |
local_session_header_node_id_ = TabNodePool::kInvalidTabNodeID; |
@@ -494,22 +524,17 @@ syncer::SyncDataList SessionsSyncManager::GetAllSyncData( |
current_machine_tag(), current_session_name_, header_entity); |
list.push_back(data); |
- for (auto win_iter = session->windows.begin(); |
- win_iter != session->windows.end(); ++win_iter) { |
- for (auto tabs_iter = win_iter->second->tabs.begin(); |
- tabs_iter != win_iter->second->tabs.end(); ++tabs_iter) { |
+ for (auto& win_iter : session->windows) { |
+ for (auto& tab : win_iter.second->tabs) { |
+ // TODO(zea): replace with with the correct tab node id once there's a |
+ // sync specific wrapper for SessionTab. This method is only used in |
+ // tests though, so it's fine for now. crbug.com/662597 |
+ int tab_node_id = 0; |
sync_pb::EntitySpecifics entity; |
- sync_pb::SessionSpecifics* specifics = entity.mutable_session(); |
- specifics->mutable_tab()->MergeFrom((*tabs_iter)->ToSyncData()); |
- specifics->set_session_tag(current_machine_tag_); |
- |
- TabLinksMap::const_iterator tab_map_iter = |
- local_tab_map_.find((*tabs_iter)->tab_id.id()); |
- DCHECK(tab_map_iter != local_tab_map_.end()); |
- specifics->set_tab_node_id(tab_map_iter->second->tab_node_id()); |
+ entity.mutable_session()->CopyFrom( |
+ SessionTabToSpecifics(*tab, current_machine_tag(), tab_node_id)); |
syncer::SyncData data = syncer::SyncData::CreateLocalData( |
- TabNodePool::TabIdToTag(current_machine_tag_, |
- specifics->tab_node_id()), |
+ TabNodeIdToTag(current_machine_tag(), tab_node_id), |
current_session_name_, entity); |
list.push_back(data); |
} |
@@ -518,7 +543,7 @@ syncer::SyncDataList SessionsSyncManager::GetAllSyncData( |
} |
bool SessionsSyncManager::GetLocalSession(const SyncedSession** local_session) { |
- if (current_machine_tag_.empty()) |
+ if (current_machine_tag().empty()) |
return false; |
*local_session = session_tracker_.GetSession(current_machine_tag()); |
return true; |
@@ -578,7 +603,7 @@ syncer::SyncError SessionsSyncManager::ProcessSyncChanges( |
LOG(WARNING) << "Dropping modification to local session."; |
return syncer::SyncError(); |
} |
- UpdateTrackerWithForeignSession( |
+ UpdateTrackerWithSpecifics( |
session, syncer::SyncDataRemote(it->sync_data()).GetModifiedTime()); |
break; |
default: |
@@ -600,7 +625,7 @@ syncer::SyncChange SessionsSyncManager::TombstoneTab( |
return syncer::SyncChange( |
FROM_HERE, SyncChange::ACTION_DELETE, |
SyncData::CreateLocalDelete( |
- TabNodePool::TabIdToTag(current_machine_tag(), tab.tab_node_id()), |
+ TabNodeIdToTag(current_machine_tag(), tab.tab_node_id()), |
syncer::SESSIONS)); |
} |
} |
@@ -616,7 +641,6 @@ bool SessionsSyncManager::GetAllForeignSessions( |
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; |
@@ -634,7 +658,7 @@ bool SessionsSyncManager::InitFromSyncModel( |
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 |
@@ -653,6 +677,10 @@ bool SessionsSyncManager::InitFromSyncModel( |
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,11 +688,19 @@ bool SessionsSyncManager::InitFromSyncModel( |
syncer::SyncChange tombstone(TombstoneTab(specifics)); |
if (tombstone.IsValid()) |
new_changes->push_back(tombstone); |
+ } else if (specifics.tab().tab_id() == TabNodePool::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 pool so it can be |
- // reused for reassociation. |
- local_tab_pool_.AddTabNode(specifics.tab_node_id()); |
- restored_tabs->push_back(*it); |
+ // This is a valid old tab node, add it to the tracker and associate |
+ // it. |
+ 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()); |
} |
} |
} |
@@ -676,7 +712,7 @@ bool SessionsSyncManager::InitFromSyncModel( |
session_tracker_.LookupAllForeignSessions(&sessions, |
SyncedSessionTracker::RAW); |
for (const auto* session : sessions) { |
- session_tracker_.CleanupSession(session->session_tag); |
+ session_tracker_.CleanupForeignSession(session->session_tag); |
} |
UMA_HISTOGRAM_COUNTS_100("Sync.SessionsBadForeignHashOnMergeCount", |
@@ -685,70 +721,84 @@ bool SessionsSyncManager::InitFromSyncModel( |
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) << "Populating " << 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_.CleanupForeignSession(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) && |
- 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"; |
+ 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. |
+ // |
+ // Note that TabIDs are not stable across restarts of a client. Consider |
+ // this example with two tabs: |
+ // |
+ // http://a.com TabID1 --> NodeIDA |
+ // http://b.com TabID2 --> NodeIDB |
+ // |
+ // After restart, tab ids are reallocated. e.g, one possibility: |
+ // http://a.com TabID2 --> NodeIDA |
+ // http://b.com TabID1 --> NodeIDB |
+ // |
+ // If that happened on a remote client, here we will see an update to |
+ // TabID1 with tab_node_id changing from NodeIDA to NodeIDB, and TabID2 |
+ // with tab_node_id changing from NodeIDB to NodeIDA. |
+ // |
+ // We can also wind up here if we 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 and |
+ // is currently kInvalidTabNodeID. |
+ // |
+ // In both cases, we can safely throw it into the set of node ids. |
+ session_tracker_.OnTabNodeSeen(session_tag, specifics.tab_node_id()); |
+ sessions::SessionTab* tab = session_tracker_.GetTab(session_tag, tab_id); |
+ if (!tab->timestamp.is_null() && tab->timestamp > modification_time) { |
+ DVLOG(1) << "Ignoring " << session_tag << "'s session tab " << tab_id |
+ << " with earlier modification time: " << tab->timestamp |
+ << " vs " << modification_time; |
return; |
} |
- 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); |
@@ -757,11 +807,11 @@ void SessionsSyncManager::UpdateTrackerWithForeignSession( |
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 << "."; |
} |
} |
@@ -779,8 +829,6 @@ void SessionsSyncManager::InitializeCurrentMachineTag( |
DVLOG(1) << "Creating session sync guid: " << current_machine_tag_; |
sync_prefs_->SetSyncSessionsGUID(current_machine_tag_); |
} |
- |
- local_tab_pool_.SetMachineTag(current_machine_tag_); |
} |
// static |
@@ -844,11 +892,11 @@ void SessionsSyncManager::BuildSyncedSessionFromSpecifics( |
} |
} |
session_window->timestamp = mtime; |
- session_window->tabs.resize(specifics.tab_size()); |
+ session_window->tabs.clear(); |
for (int i = 0; i < specifics.tab_size(); i++) { |
SessionID::id_type tab_id = specifics.tab(i); |
session_tracker_.PutTabInWindow(session_tag, session_window->window_id.id(), |
- tab_id, i); |
+ tab_id); |
} |
} |
@@ -890,20 +938,14 @@ void SessionsSyncManager::DeleteForeignSessionInternal( |
} |
std::set<int> tab_node_ids_to_delete; |
- session_tracker_.LookupTabNodeIds(tag, &tab_node_ids_to_delete); |
+ session_tracker_.LookupForeignTabNodeIds(tag, &tab_node_ids_to_delete); |
if (DisassociateForeignSession(tag)) { |
// Only tell sync to delete the header if there was one. |
change_output->push_back( |
syncer::SyncChange(FROM_HERE, SyncChange::ACTION_DELETE, |
SyncData::CreateLocalDelete(tag, syncer::SESSIONS))); |
} |
- for (std::set<int>::const_iterator it = tab_node_ids_to_delete.begin(); |
- it != tab_node_ids_to_delete.end(); ++it) { |
- change_output->push_back(syncer::SyncChange( |
- FROM_HERE, SyncChange::ACTION_DELETE, |
- SyncData::CreateLocalDelete(TabNodePool::TabIdToTag(tag, *it), |
- syncer::SESSIONS))); |
- } |
+ AppendDeletionsForTabNodes(tab_node_ids_to_delete, tag, change_output); |
if (!sessions_updated_callback_.is_null()) |
sessions_updated_callback_.Run(); |
} |
@@ -912,7 +954,7 @@ bool SessionsSyncManager::DisassociateForeignSession( |
const std::string& foreign_session_tag) { |
DCHECK_NE(foreign_session_tag, current_machine_tag()); |
DVLOG(1) << "Disassociating session " << foreign_session_tag; |
- return session_tracker_.DeleteSession(foreign_session_tag); |
+ return session_tracker_.DeleteForeignSession(foreign_session_tag); |
} |
bool SessionsSyncManager::GetForeignSession( |
@@ -958,66 +1000,41 @@ bool SessionsSyncManager::GetForeignTab(const std::string& tag, |
return success; |
} |
-void SessionsSyncManager::LocalTabDelegateToSpecifics( |
- const SyncedTabDelegate& tab_delegate, |
- sync_pb::SessionSpecifics* specifics) { |
- sessions::SessionTab* session_tab = nullptr; |
- 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); |
- 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."; |
+ |
+ // It's possible the placeholder tab is associated with a tab node that's |
+ // since been deleted. If that's the case, there's no way to reassociate it, |
+ // so just return now without adding the tab to the session tracker. |
+ if (!session_tracker_.IsLocalTabNodeAssociated(tab_delegate.GetSyncId())) { |
+ DVLOG(1) << "Restored placeholder tab's node " << tab_delegate.GetSyncId() |
+ << " deleted."; |
return; |
} |
- 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); |
+ // 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); |
- if (specifics->tab().tab_id() == new_tab_id && |
- specifics->tab().window_id() == new_window_id) |
- return; |
+ // 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); |
- // 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; |
- } |
+ // 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())); |
+ syncer::SyncData data = syncer::SyncData::CreateLocalData( |
+ TabNodeIdToTag(current_machine_tag(), tab_delegate.GetSyncId()), |
+ current_session_name_, entity); |
+ change_output->push_back( |
+ syncer::SyncChange(FROM_HERE, syncer::SyncChange::ACTION_UPDATE, data)); |
} |
// static |