[Sync] Fix flakiness from Sessions refactor

Fixes a flaky issue introduced in the recent tabs submenu tests due to
inspecting possibly unset timestamp values. Also fixes a pre-existing issue
where sync ids for tabs were not being set, which the refactor brought
to light.

BUG=671902, 671902
TBR=msw

Review-Url: https://codereview.chromium.org/2562853002
Cr-Commit-Position: refs/heads/master@{#437604}

chrome/browser/sync/sessions/sessions_sync_manager_unittest.cc

@@ -2051,6 +2051,14 @@ TEST_F(SessionsSyncManagerTest, OnLocalTabModified) {
   ASSERT_EQ(2, tab2_2.navigation_size());
   EXPECT_EQ(bar1.spec(), tab2_2.navigation(0).virtual_url());
   EXPECT_EQ(bar2.spec(), tab2_2.navigation(1).virtual_url());
+
+  // Verify tab delegates have Sync ids.
+  std::set<const SyncedWindowDelegate*> window_delegates =
+      get_synced_window_getter()->GetSyncedWindowDelegates();
+  // Sync ids are in reverse order because tabs are inserted at the beginning
+  // of the tab list.
+  EXPECT_EQ(1, (*window_delegates.begin())->GetTabAt(0)->GetSyncId());
+  EXPECT_EQ(0, (*window_delegates.begin())->GetTabAt(1)->GetSyncId());
 }
 
 // Check that if a tab becomes uninteresting (for example no syncable URLs),
@@ -2161,6 +2169,14 @@ TEST_F(SessionsSyncManagerTest, MergeLocalSessionExistingTabs) {
   EXPECT_TRUE(specifics2.has_header());
   const sync_pb::SessionHeader& header_s2 = specifics2.header();
   EXPECT_EQ(1, header_s2.window_size());
+
+  // Verify tab delegates have Sync ids.
+  std::set<const SyncedWindowDelegate*> window_delegates =
+      get_synced_window_getter()->GetSyncedWindowDelegates();
+  // Sync ids are in same order as tabs because the association happens after
+  // the tabs are opened (and therefore iterates through same order).
+  EXPECT_EQ(0, (*window_delegates.begin())->GetTabAt(0)->GetSyncId());
+  EXPECT_EQ(1, (*window_delegates.begin())->GetTabAt(1)->GetSyncId());
 }
 
 TEST_F(SessionsSyncManagerTest, ForeignSessionModifiedTime) {

chrome/browser/ui/toolbar/recent_tabs_sub_menu_model_unittest.cc

@@ -573,8 +573,7 @@ TEST_F(RecentTabsSubMenuModelTest, MaxTabsPerSessionAndRecency) {
     EXPECT_EQ(tab_titles[i], model.GetLabelAt(i + 5));
 }
 
-// Flaky, see crbug.com/671902.
-TEST_F(RecentTabsSubMenuModelTest, DISABLED_MaxWidth) {
+TEST_F(RecentTabsSubMenuModelTest, MaxWidth) {
   // Create 1 session with 1 window and 1 tab.
   RecentTabsBuilderTestHelper recent_tabs_builder;
   recent_tabs_builder.AddSession();

components/sync_sessions/sessions_sync_manager.cc

@@ -367,6 +367,7 @@ void SessionsSyncManager::AssociateTab(SyncedTabDelegate* const tab_delegate,
   bool existing_tab_node =
       session_tracker_.GetTabNodeForLocalTab(tab_id, &tab_node_id);
   DCHECK_NE(TabNodePool::kInvalidTabNodeID, tab_node_id);
+  tab_delegate->SetSyncId(tab_node_id);
   sessions::SessionTab* session_tab =
       session_tracker_.GetTab(current_machine_tag(), tab_id);
 
@@ -766,9 +767,10 @@ void SessionsSyncManager::UpdateTrackerWithSpecifics(
     // 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 > modification_time) {
+    if (!tab->timestamp.is_null() && tab->timestamp > modification_time) {
       DVLOG(1) << "Ignoring " << session_tag << "'s session tab " << tab_id
-               << " with earlier modification time";
+               << " with earlier modification time: " << tab->timestamp
+               << " vs " << modification_time;
       return;
     }
 

components/sync_sessions/synced_session_tracker.cc

@@ -261,7 +261,11 @@ void SyncedSessionTracker::PutTabInWindow(const std::string& session_tag,
     tab = std::move(it->second);
     unmapped_tabs_[session_tag].erase(it);
   }
-  DCHECK(tab);
+  if (!tab) {
+    LOG(ERROR) << "crbug.com/665196 Attempting to map tab " << tab_id
+               << " multiple times!";
+    return;
+  }
 
   tab->window_id.set_id(window_id);
   DVLOG(1) << "  - tab " << tab_id << " added to window " << window_id;