Sync: Handle duplicating tab IDs in foreign session data.

This change prevents overwriting a foreign session SessionTab  with a data from another node if the SessionTab we already have has a newer timestamp. This ensures that if there are multiple tabs with the same ID, the tab that we end up associating with a window is the most recent one.

BUG=423501

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

Cr-Commit-Position: refs/heads/master@{#302674}

chrome/browser/sync/sessions/sessions_sync_manager.cc

@@ -652,6 +652,16 @@ void SessionsSyncManager::UpdateTrackerWithForeignSession(
   } else if (specifics.has_tab()) {
     const sync_pb::SessionTab& tab_s = specifics.tab();
     SessionID::id_type tab_id = tab_s.tab_id();
+
+    const SessionTab* existing_tab;
+    if (session_tracker_.LookupSessionTab(
+            foreign_session_tag, tab_id, &existing_tab) &&
+        existing_tab->timestamp > modification_time) {
+      DVLOG(1) << "Ignoring " << foreign_session_tag << "'s session tab "
+               << tab_id << " with earlier modification time";
+      return;
+    }
+
     SessionTab* tab =
         session_tracker_.GetTab(foreign_session_tag,
                                 tab_id,

chrome/browser/sync/sessions/sessions_sync_manager_unittest.cc

@@ -1969,4 +1969,81 @@ TEST_F(SessionsSyncManagerTest, ReceiveDuplicateTabInOtherWindow) {
   InitWithSyncDataTakeOutput(initial_data, &output);
 }
 
+// Tests receipt of multiple unassociated tabs and makes sure that
+// the ones with later timestamp win
+TEST_F(SessionsSyncManagerTest, ReceiveDuplicateUnassociatedTabs) {
+  std::string tag = "tag1";
+
+  SessionID::id_type n1[] = {5, 10, 17};
+  std::vector<SessionID::id_type> tab_list1(n1, n1 + arraysize(n1));
+  std::vector<sync_pb::SessionSpecifics> tabs1;
+  sync_pb::SessionSpecifics meta(
+      helper()->BuildForeignSession(tag, tab_list1, &tabs1));
+
+  // Set up initial data.
+  syncer::SyncDataList initial_data;
+  sync_pb::EntitySpecifics entity;
+  entity.mutable_session()->CopyFrom(meta);
+  initial_data.push_back(SyncData::CreateRemoteData(
+      1,
+      entity,
+      base::Time(),
+      syncer::AttachmentIdList(),
+      syncer::AttachmentServiceProxyForTest::Create()));
+
+  int node_id = 2;
+
+  for (size_t i = 0; i < tabs1.size(); i++) {
+    entity.mutable_session()->CopyFrom(tabs1[i]);
+    initial_data.push_back(SyncData::CreateRemoteData(
+        node_id++,
+        entity,
+        base::Time::FromDoubleT(2000),
+        syncer::AttachmentIdList(),
+        syncer::AttachmentServiceProxyForTest::Create()));
+  }
+
+  // Add two more tabs with duplicating IDs but with different modification
+  // times, one before and one after the tabs above.
+  // These two tabs get a different visual indices to distinguish them from the
+  // tabs above that get visual index 1 by default.
+  sync_pb::SessionSpecifics duplicating_tab1;
+  helper()->BuildTabSpecifics(tag, 0, 10, &duplicating_tab1);
+  duplicating_tab1.mutable_tab()->set_tab_visual_index(2);
+  entity.mutable_session()->CopyFrom(duplicating_tab1);
+  initial_data.push_back(SyncData::CreateRemoteData(
+      node_id++,
+      entity,
+      base::Time::FromDoubleT(1000),
+      syncer::AttachmentIdList(),
+      syncer::AttachmentServiceProxyForTest::Create()));
+
+  sync_pb::SessionSpecifics duplicating_tab2;
+  helper()->BuildTabSpecifics(tag, 0, 17, &duplicating_tab2);
+  duplicating_tab2.mutable_tab()->set_tab_visual_index(3);
+  entity.mutable_session()->CopyFrom(duplicating_tab2);
+  initial_data.push_back(SyncData::CreateRemoteData(
+      node_id++,
+      entity,
+      base::Time::FromDoubleT(3000),
+      syncer::AttachmentIdList(),
+      syncer::AttachmentServiceProxyForTest::Create()));
+
+  syncer::SyncChangeList output;
+  InitWithSyncDataTakeOutput(initial_data, &output);
+
+  std::vector<const SyncedSession*> foreign_sessions;
+  ASSERT_TRUE(manager()->GetAllForeignSessions(&foreign_sessions));
+
+  const std::vector<SessionTab*>& window_tabs =
+      foreign_sessions[0]->windows.find(0)->second->tabs;
+  ASSERT_EQ(3U, window_tabs.size());
+  // The first one is from the original set of tabs.
+  ASSERT_EQ(1, window_tabs[0]->tab_visual_index);
+  // The one from the original set of tabs wins over duplicating_tab1.
+  ASSERT_EQ(1, window_tabs[1]->tab_visual_index);
+  // duplicating_tab2 wins due to the later timestamp.
+  ASSERT_EQ(3, window_tabs[2]->tab_visual_index);
+}
+
 }  // namespace browser_sync