Improve test coverage for PutTabInWindow()

Most notably, the scenario where the tab is initially mapped was not
exercised in any test. We now cover this case as well as some other
common updates for both local and remote sessions, which also includes
test coverage for UpdateTrackerWithSpecifics() itself.

This requires some minor API changes in SyncedSessionTracker.

Bug: 814423
Change-Id: If65494ef2f505eb6b49569e4d1d1f0a132f92e08
Reviewed-on: https://chromium-review.googlesource.com/1005176
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550150}

components/sync_sessions/local_session_event_handler_impl_unittest.cc

@@ -53,9 +53,10 @@ const base::Time kTime6 = base::Time::FromInternalValue(190);
 
 const int kWindowId1 = 1000001;
 const int kWindowId2 = 1000002;
-const int kTabId1 = 1000003;
-const int kTabId2 = 1000004;
-const int kTabId3 = 1000005;
+const int kWindowId3 = 1000003;
+const int kTabId1 = 1000004;
+const int kTabId2 = 1000005;
+const int kTabId3 = 1000006;
 
 class MockWriteBatch : public LocalSessionEventHandlerImpl::WriteBatch {
  public:
@@ -114,9 +115,11 @@ class LocalSessionEventHandlerImplTest : public testing::Test {
     InitHandler(&initial_batch);
   }
 
-  TestSyncedWindowDelegate* AddWindow(int window_id) {
-    return window_getter_.AddWindow(
-        sync_pb::SessionWindow_BrowserType_TYPE_TABBED,
+  TestSyncedWindowDelegate* AddWindow(
+      int window_id,
+      sync_pb::SessionWindow_BrowserType type =
+          sync_pb::SessionWindow_BrowserType_TYPE_TABBED) {
+    return window_getter_.AddWindow(type,
                                     SessionID::FromSerializedValue(window_id));
   }
 
@@ -323,15 +326,87 @@ TEST_F(LocalSessionEventHandlerImplTest, AssociateWindowsAndTabs) {
                                      {kTabId1, kTabId2, kTabId3}))));
   EXPECT_CALL(mock_batch,
               DoAdd(Pointee(MatchesTab(kSessionTag, kWindowId1, kTabId1,
+                                       /*tab_node_id=*/_,
                                        /*urls=*/{kFoo1}))));
   EXPECT_CALL(mock_batch,
               DoAdd(Pointee(MatchesTab(kSessionTag, kWindowId2, kTabId2,
-                                       /*urls=*/{kBar1}))));
-  EXPECT_CALL(mock_batch,
+                                       /*tab_node_id=*/_, /*urls=*/{kBar1}))));
+  EXPECT_CALL(
+      mock_batch,
       DoAdd(Pointee(MatchesTab(kSessionTag, kWindowId2, kTabId3,
-                                       /*urls=*/{kBar2, kBaz1}))));
+                               /*tab_node_id=*/_, /*urls=*/{kBar2, kBaz1}))));
+
+  InitHandler(&mock_batch);
+}
+
+// Tests that calling AssociateWindowsAndTabs() reflects the open tabs in a) the
+// SyncSessionTracker and b) the delegate, for the case where a custom tab
+// exists without native data (no tabbed window).
+TEST_F(LocalSessionEventHandlerImplTest, AssociateCustomTab) {
+  const int kRegularTabNodeId = 1;
+  const int kCustomTabNodeId = 2;
+
+  // The tracker is initially restored from persisted state, containing a
+  // regular tab and a custom tab. This mimics
+  // SessionsSyncManager::InitFromSyncModel().
+  sync_pb::SessionSpecifics regular_tab;
+  regular_tab.set_session_tag(kSessionTag);
+  regular_tab.set_tab_node_id(kRegularTabNodeId);
+  regular_tab.mutable_tab()->set_window_id(kWindowId1);
+  regular_tab.mutable_tab()->set_tab_id(kTabId1);
+  session_tracker_.ReassociateLocalTab(kRegularTabNodeId,
+                                       SessionID::FromSerializedValue(kTabId1));
+  UpdateTrackerWithSpecifics(regular_tab, base::Time::Now(), &session_tracker_);
+
+  sync_pb::SessionSpecifics custom_tab;
+  custom_tab.set_session_tag(kSessionTag);
+  custom_tab.set_tab_node_id(kCustomTabNodeId);
+  custom_tab.mutable_tab()->set_window_id(kWindowId2);
+  custom_tab.mutable_tab()->set_tab_id(kTabId2);
+  session_tracker_.ReassociateLocalTab(kCustomTabNodeId,
+                                       SessionID::FromSerializedValue(kTabId2));
+  UpdateTrackerWithSpecifics(custom_tab, base::Time::Now(), &session_tracker_);
+
+  sync_pb::SessionSpecifics header;
+  header.set_session_tag(kSessionTag);
+  header.mutable_header()->add_window()->set_window_id(kWindowId1);
+  header.mutable_header()->mutable_window(0)->add_tab(kTabId1);
+  header.mutable_header()->add_window()->set_window_id(kWindowId2);
+  header.mutable_header()->mutable_window(1)->add_tab(kTabId2);
+  UpdateTrackerWithSpecifics(header, base::Time::Now(), &session_tracker_);
+
+  ASSERT_THAT(session_tracker_.LookupSession(kSessionTag),
+              MatchesSyncedSession(kSessionTag,
+                                   {{kWindowId1, std::vector<int>{kTabId1}},
+                                    {kWindowId2, std::vector<int>{kTabId2}}}));
+
+  // In the current session, all we have is a custom tab.
+  AddWindow(kWindowId3, sync_pb::SessionWindow_BrowserType_TYPE_CUSTOM_TAB);
+  AddTab(kWindowId3, kFoo1, kTabId3)->SetSyncId(kCustomTabNodeId);
 
+  EXPECT_CALL(mock_delegate_, CreateLocalSessionWriteBatch()).Times(0);
+
+  StrictMock<MockWriteBatch> mock_batch;
+  testing::InSequence seq;
+  EXPECT_CALL(mock_batch,
+              DoUpdate(Pointee(MatchesTab(kSessionTag, kWindowId1, kTabId1,
+                                          kRegularTabNodeId, /*urls=*/{}))));
+  EXPECT_CALL(mock_batch,
+              DoUpdate(Pointee(MatchesTab(kSessionTag, kWindowId2, kTabId2,
+                                          kCustomTabNodeId, /*urls=*/{}))));
+  EXPECT_CALL(mock_batch, DoUpdate(Pointee(MatchesTab(kSessionTag, kWindowId3,
+                                                      kTabId3, kCustomTabNodeId,
+                                                      /*urls=*/{kFoo1}))));
+  EXPECT_CALL(mock_batch, DoUpdate(Pointee(MatchesHeader(
+                              kSessionTag, {kWindowId1, kWindowId2, kWindowId3},
+                              {kTabId1, kTabId3}))));
   InitHandler(&mock_batch);
+
+  EXPECT_THAT(session_tracker_.LookupSession(kSessionTag),
+              MatchesSyncedSession(kSessionTag,
+                                   {{kWindowId1, std::vector<int>{kTabId1}},
+                                    {kWindowId2, std::vector<int>()},
+                                    {kWindowId3, std::vector<int>{kTabId3}}}));
 }
 
 TEST_F(LocalSessionEventHandlerImplTest, PropagateNewNavigation) {
@@ -348,6 +423,7 @@ TEST_F(LocalSessionEventHandlerImplTest, PropagateNewNavigation) {
       DoUpdate(Pointee(MatchesHeader(kSessionTag, {kWindowId1}, {kTabId1}))));
   EXPECT_CALL(*update_mock_batch,
               DoUpdate(Pointee(MatchesTab(kSessionTag, kWindowId1, kTabId1,
+                                          /*tab_node_id=*/_,
                                           /*urls=*/{kFoo1, kBar1}))));
   EXPECT_CALL(*update_mock_batch, Commit());
 
@@ -379,7 +455,7 @@ TEST_F(LocalSessionEventHandlerImplTest, PropagateNewTab) {
                                              {kTabId1, kTabId2}))));
   EXPECT_CALL(*navigation_mock_batch,
               DoAdd(Pointee(MatchesTab(kSessionTag, kWindowId1, kTabId2,
-                                       /*urls=*/{kBar1}))));
+                                       /*tab_node_id=*/_, /*urls=*/{kBar1}))));
   EXPECT_CALL(*navigation_mock_batch, Commit());
 
   EXPECT_CALL(mock_delegate_, CreateLocalSessionWriteBatch())
@@ -413,7 +489,7 @@ TEST_F(LocalSessionEventHandlerImplTest, PropagateNewWindow) {
                                      {kTabId1, kTabId2, kTabId3}))));
   EXPECT_CALL(*navigation_mock_batch,
               DoAdd(Pointee(MatchesTab(kSessionTag, kWindowId2, kTabId3,
-                                       /*urls=*/{kBaz1}))));
+                                       /*tab_node_id=*/_, /*urls=*/{kBaz1}))));
   EXPECT_CALL(*navigation_mock_batch, Commit());
 
   EXPECT_CALL(mock_delegate_, CreateLocalSessionWriteBatch())

components/sync_sessions/sessions_sync_manager.cc

@@ -558,8 +558,8 @@ void SessionsSyncManager::DeleteForeignSessionInternal(
     return;
   }
 
-  std::set<int> tab_node_ids_to_delete;
-  session_tracker_.LookupForeignTabNodeIds(tag, &tab_node_ids_to_delete);
+  const std::set<int> tab_node_ids_to_delete =
+      session_tracker_.LookupTabNodeIds(tag);
   if (DisassociateForeignSession(tag)) {
     // Only tell sync to delete the header if there was one.
     change_output->push_back(

components/sync_sessions/sessions_sync_manager_unittest.cc

@@ -29,6 +29,7 @@ using syncer::SyncData;
 using syncer::SyncDataList;
 using syncer::SyncDataLocal;
 using syncer::SyncError;
+using testing::ElementsAre;
 using testing::Eq;
 using testing::IsNull;
 
@@ -1164,16 +1165,10 @@ TEST_F(SessionsSyncManagerTest, ProcessForeignDeleteTabsWithShadowing) {
       manager()->session_tracker_.LookupSessionTab(session_tag, kTabIds1[1]),
       IsNull());
 
-  std::set<int> tab_node_ids;
-  manager()->session_tracker_.LookupForeignTabNodeIds(session_tag,
-                                                      &tab_node_ids);
-  EXPECT_EQ(6U, tab_node_ids.size());
-  EXPECT_TRUE(tab_node_ids.find(tab1A.tab_node_id()) != tab_node_ids.end());
-  EXPECT_TRUE(tab_node_ids.find(tab1B.tab_node_id()) != tab_node_ids.end());
-  EXPECT_TRUE(tab_node_ids.find(tab1C.tab_node_id()) != tab_node_ids.end());
-  EXPECT_TRUE(tab_node_ids.find(tab2A.tab_node_id()) != tab_node_ids.end());
-  EXPECT_TRUE(tab_node_ids.find(tab2B.tab_node_id()) != tab_node_ids.end());
-  EXPECT_TRUE(tab_node_ids.find(tab2C.tab_node_id()) != tab_node_ids.end());
+  EXPECT_THAT(manager()->session_tracker_.LookupTabNodeIds(session_tag),
+              ElementsAre(tab1A.tab_node_id(), tab1B.tab_node_id(),
+                          tab1C.tab_node_id(), tab2A.tab_node_id(),
+                          tab2B.tab_node_id(), tab2C.tab_node_id()));
 
   SyncChangeList changes;
   changes.push_back(MakeRemoteChange(tab1A, SyncChange::ACTION_DELETE));
@@ -1181,13 +1176,9 @@ TEST_F(SessionsSyncManagerTest, ProcessForeignDeleteTabsWithShadowing) {
   changes.push_back(MakeRemoteChange(tab2C, SyncChange::ACTION_DELETE));
   manager()->ProcessSyncChanges(FROM_HERE, changes);
 
-  tab_node_ids.clear();
-  manager()->session_tracker_.LookupForeignTabNodeIds(session_tag,
-                                                      &tab_node_ids);
-  EXPECT_EQ(3U, tab_node_ids.size());
-  EXPECT_TRUE(tab_node_ids.find(tab1C.tab_node_id()) != tab_node_ids.end());
-  EXPECT_TRUE(tab_node_ids.find(tab2A.tab_node_id()) != tab_node_ids.end());
-  EXPECT_TRUE(tab_node_ids.find(tab2B.tab_node_id()) != tab_node_ids.end());
+  EXPECT_THAT(manager()->session_tracker_.LookupTabNodeIds(session_tag),
+              ElementsAre(tab1C.tab_node_id(), tab2A.tab_node_id(),
+                          tab2B.tab_node_id()));
 
   manager()->DoGarbageCollection();
   ASSERT_EQ(3U, output.size());
@@ -1224,22 +1215,15 @@ TEST_F(SessionsSyncManagerTest, ProcessForeignDeleteTabsWithReusedNodeIds) {
   ASSERT_EQ(2U, output.size());
   output.clear();
 
-  std::set<int> tab_node_ids;
-  manager()->session_tracker_.LookupForeignTabNodeIds(session_tag,
-                                                      &tab_node_ids);
-  EXPECT_EQ(2U, tab_node_ids.size());
-  EXPECT_TRUE(tab_node_ids.find(tab_node_id_shared) != tab_node_ids.end());
-  EXPECT_TRUE(tab_node_ids.find(tab_node_id_unique) != tab_node_ids.end());
+  EXPECT_THAT(manager()->session_tracker_.LookupTabNodeIds(session_tag),
+              ElementsAre(tab_node_id_shared, tab_node_id_unique));
 
   SyncChangeList changes;
   changes.push_back(MakeRemoteChange(tab1A, SyncChange::ACTION_DELETE));
   manager()->ProcessSyncChanges(FROM_HERE, changes);
 
-  tab_node_ids.clear();
-  manager()->session_tracker_.LookupForeignTabNodeIds(session_tag,
-                                                      &tab_node_ids);
-  EXPECT_EQ(1U, tab_node_ids.size());
-  EXPECT_TRUE(tab_node_ids.find(tab_node_id_unique) != tab_node_ids.end());
+  EXPECT_THAT(manager()->session_tracker_.LookupTabNodeIds(session_tag),
+              ElementsAre(tab_node_id_unique));
 
   manager()->DoGarbageCollection();
   EXPECT_EQ(1U, output.size());

components/sync_sessions/synced_session_tracker.cc

@@ -199,21 +199,19 @@ const sessions::SessionTab* SyncedSessionTracker::LookupSessionTab(
   return tab_iter->second;
 }
 
-void SyncedSessionTracker::LookupForeignTabNodeIds(
-    const std::string& session_tag,
-    std::set<int>* tab_node_ids) const {
-  tab_node_ids->clear();
+std::set<int> SyncedSessionTracker::LookupTabNodeIds(
+    const std::string& session_tag) const {
   const TrackedSession* session = LookupTrackedSession(session_tag);
-  if (session) {
-    tab_node_ids->insert(session->tab_node_ids.begin(),
-                         session->tab_node_ids.end());
-  }
-  // In case an invalid node id was included, remove it.
-  tab_node_ids->erase(TabNodePool::kInvalidTabNodeID);
+  return session ? session->tab_node_ids : std::set<int>();
 }
 
 const SyncedSession* SyncedSessionTracker::LookupLocalSession() const {
-  const TrackedSession* session = LookupTrackedSession(local_session_tag_);
+  return LookupSession(local_session_tag_);
+}
+
+const SyncedSession* SyncedSessionTracker::LookupSession(
+    const std::string& session_tag) const {
+  const TrackedSession* session = LookupTrackedSession(session_tag);
   return session ? &session->synced_session : nullptr;
 }
 
@@ -319,12 +317,6 @@ bool SyncedSessionTracker::IsTabUnmappedForTesting(SessionID tab_id) {
   return session->unmapped_tabs.count(tab_id) != 0;
 }
 
-std::set<int> SyncedSessionTracker::GetTabNodeIdsForTesting(
-    const std::string& session_tag) const {
-  const TrackedSession* session = LookupTrackedSession(session_tag);
-  return session ? session->tab_node_ids : std::set<int>();
-}
-
 void SyncedSessionTracker::PutWindowInSession(const std::string& session_tag,
                                               SessionID window_id) {
   TrackedSession* session = GetTrackedSession(session_tag);
@@ -420,7 +412,9 @@ void SyncedSessionTracker::PutTabInWindow(const std::string& session_tag,
 
 void SyncedSessionTracker::OnTabNodeSeen(const std::string& session_tag,
                                          int tab_node_id) {
+  if (tab_node_id != TabNodePool::kInvalidTabNodeID) {
     GetTrackedSession(session_tag)->tab_node_ids.insert(tab_node_id);
+  }
 }
 
 sessions::SessionTab* SyncedSessionTracker::GetTab(

components/sync_sessions/synced_session_tracker.h

@@ -52,10 +52,9 @@ class SyncedSessionTracker {
   std::vector<const SyncedSession*> LookupAllForeignSessions(
       SessionLookup lookup) const;
 
-  // Fills |tab_node_ids| with the tab node ids (see GetTab) for all the tabs*
-  // associated with the session having tag |session_tag|.
-  void LookupForeignTabNodeIds(const std::string& session_tag,
-                               std::set<int>* tab_node_ids) const;
+  // Returns the tab node ids (see GetTab) for all the tabs* associated with the
+  // session having tag |session_tag|.
+  std::set<int> LookupTabNodeIds(const std::string& session_tag) const;
 
   // Attempts to look up the session windows associatd with the session given
   // by |session_tag|. Ownership of SessionWindows stays within the
@@ -80,6 +79,11 @@ class SyncedSessionTracker {
 
   // **** Methods for manipulating synced sessions and tabs. ****
 
+  // Returns a pointer to the SyncedSession object associated with
+  // |session_tag|. If none exists, returns nullptr. Ownership of the
+  // SyncedSession remains within the SyncedSessionTracker.
+  const SyncedSession* LookupSession(const std::string& session_tag) const;
+
   // Returns a pointer to the SyncedSession object associated with
   // |session_tag|. If none exists, creates one. Ownership of the
   // SyncedSession remains within the SyncedSessionTracker.
@@ -205,8 +209,6 @@ class SyncedSessionTracker {
   // Returns whether a tab is unmapped or not.
   bool IsTabUnmappedForTesting(SessionID tab_id);
 
-  std::set<int> GetTabNodeIdsForTesting(const std::string& session_tag) const;
-
  private:
   friend class SessionsSyncManagerTest;
   friend class SyncedSessionTrackerTest;

components/sync_sessions/test_matchers.cc

@@ -12,10 +12,12 @@
 namespace sync_sessions {
 namespace {
 
+using testing::ContainerEq;
 using testing::ElementsAreArray;
 using testing::Matcher;
 using testing::MatcherInterface;
 using testing::MatchResultListener;
+using testing::PrintToString;
 
 class MatchesHeaderMatcher
     : public MatcherInterface<const sync_pb::SessionSpecifics&> {
@@ -28,11 +30,15 @@ class MatchesHeaderMatcher
   bool MatchAndExplain(const sync_pb::SessionSpecifics& actual,
                        MatchResultListener* listener) const override {
     if (!actual.has_header()) {
-      *listener << "which is not a header entity";
+      *listener << " which is not a header entity";
+      return false;
+    }
+    if (actual.tab_node_id() != -1) {
+      *listener << " which has a valid tab node ID: " << actual.tab_node_id();
       return false;
     }
     if (!session_tag_.MatchAndExplain(actual.session_tag(), listener)) {
-      *listener << "which contains an unexpected session tag: "
+      *listener << " which contains an unexpected session tag: "
                 << actual.session_tag();
       return false;
     }
@@ -44,11 +50,13 @@ class MatchesHeaderMatcher
                             window.tab().end());
     }
     if (!window_ids_.MatchAndExplain(actual_window_ids, listener)) {
-      *listener << "which contains an unexpected windows";
+      *listener << " which contains unexpected windows: "
+                << PrintToString(actual_window_ids);
       return false;
     }
     if (!tab_ids_.MatchAndExplain(actual_tab_ids, listener)) {
-      *listener << "which contains an unexpected tabs";
+      *listener << " which contains unexpected tabs: "
+                << PrintToString(actual_tab_ids);
       return false;
     }
     return true;
@@ -74,39 +82,46 @@ class MatchesTabMatcher
   MatchesTabMatcher(Matcher<std::string> session_tag,
                     Matcher<int> window_id,
                     Matcher<int> tab_id,
+                    Matcher<int> tab_node_id,
                     Matcher<std::vector<std::string>> urls)
       : session_tag_(session_tag),
         window_id_(window_id),
         tab_id_(tab_id),
+        tab_node_id_(tab_node_id),
         urls_(urls) {}
 
   bool MatchAndExplain(const sync_pb::SessionSpecifics& actual,
                        MatchResultListener* listener) const override {
     if (!actual.has_tab()) {
-      *listener << "which is not a tab entity";
+      *listener << " which is not a tab entity";
       return false;
     }
     if (!session_tag_.MatchAndExplain(actual.session_tag(), listener)) {
-      *listener << "which contains an unexpected session tag: "
+      *listener << " which contains an unexpected session tag: "
                 << actual.session_tag();
       return false;
     }
     if (!window_id_.MatchAndExplain(actual.tab().window_id(), listener)) {
-      *listener << "which contains an unexpected window ID: "
+      *listener << " which contains an unexpected window ID: "
                 << actual.tab().window_id();
       return false;
     }
     if (!tab_id_.MatchAndExplain(actual.tab().tab_id(), listener)) {
-      *listener << "which contains an unexpected tab ID: "
+      *listener << " which contains an unexpected tab ID: "
                 << actual.tab().tab_id();
       return false;
     }
+    if (!tab_node_id_.MatchAndExplain(actual.tab_node_id(), listener)) {
+      *listener << " which contains an unexpected tab node ID: "
+                << actual.tab_node_id();
+      return false;
+    }
     std::vector<std::string> actual_urls;
     for (const sync_pb::TabNavigation& navigation : actual.tab().navigation()) {
       actual_urls.push_back(navigation.virtual_url());
     }
     if (!urls_.MatchAndExplain(actual_urls, listener)) {
-      *listener << "which contains unexpected navigation URLs";
+      *listener << " which contains unexpected navigation URLs";
       return false;
     }
     return true;
@@ -124,43 +139,47 @@ class MatchesTabMatcher
   Matcher<std::string> session_tag_;
   Matcher<int> window_id_;
   Matcher<int> tab_id_;
+  Matcher<int> tab_node_id_;
   Matcher<std::vector<std::string>> urls_;
 };
 
 class MatchesSyncedSessionMatcher
     : public MatcherInterface<const SyncedSession*> {
  public:
-  MatchesSyncedSessionMatcher(Matcher<std::string> session_tag,
-                              Matcher<std::vector<int>> window_ids,
-                              Matcher<std::vector<int>> tab_ids)
-      : session_tag_(session_tag), window_ids_(window_ids), tab_ids_(tab_ids) {}
+  MatchesSyncedSessionMatcher(
+      Matcher<std::string> session_tag,
+      Matcher<std::map<int, std::vector<int>>> window_id_to_tabs)
+      : session_tag_(session_tag), window_id_to_tabs_(window_id_to_tabs) {}
 
   bool MatchAndExplain(const SyncedSession* actual,
                        MatchResultListener* listener) const override {
     if (!actual) {
-      *listener << "which is null";
+      *listener << " which is null";
       return false;
     }
     if (!session_tag_.MatchAndExplain(actual->session_tag, listener)) {
-      *listener << "which contains an unexpected session tag: "
+      *listener << " which contains an unexpected session tag: "
                 << actual->session_tag;
       return false;
     }
-    std::vector<int> actual_window_ids;
-    std::vector<int> actual_tab_ids;
+
+    std::map<int, std::vector<int>> actual_window_id_to_tabs;
     for (const auto& id_and_window : actual->windows) {
-      actual_window_ids.push_back(
-          id_and_window.second->wrapped_window.window_id.id());
-      for (const auto& id : id_and_window.second->wrapped_window.tabs) {
-        actual_tab_ids.push_back(id->tab_id.id());
+      const SessionID actual_window_id = id_and_window.first;
+      if (actual_window_id != id_and_window.second->wrapped_window.window_id) {
+        *listener << " which has an inconsistent window representation";
+        return false;
       }
+      actual_window_id_to_tabs.emplace(actual_window_id.id(),
+                                       std::vector<int>());
+      for (const auto& tab : id_and_window.second->wrapped_window.tabs) {
+        actual_window_id_to_tabs[actual_window_id.id()].push_back(
+            tab->tab_id.id());
       }
-    if (!window_ids_.MatchAndExplain(actual_window_ids, listener)) {
-      *listener << "which contains an unexpected window set";
-      return false;
     }
-    if (!tab_ids_.MatchAndExplain(actual_tab_ids, listener)) {
-      *listener << "which contains an unexpected tab set";
+
+    if (!window_id_to_tabs_.MatchAndExplain(actual_window_id_to_tabs,
+                                            listener)) {
       return false;
     }
     return true;
@@ -176,8 +195,7 @@ class MatchesSyncedSessionMatcher
 
  private:
   Matcher<std::string> session_tag_;
-  Matcher<std::vector<int>> window_ids_;
-  Matcher<std::vector<int>> tab_ids_;
+  Matcher<std::map<int, std::vector<int>>> window_id_to_tabs_;
 };
 
 }  // namespace
@@ -190,8 +208,8 @@ Matcher<const sync_pb::SessionSpecifics&> MatchesHeader(
       new MatchesHeaderMatcher(session_tag, window_ids, tab_ids));
 }
 
-testing::Matcher<const sync_pb::SessionSpecifics&> MatchesHeader(
-    testing::Matcher<std::string> session_tag,
+Matcher<const sync_pb::SessionSpecifics&> MatchesHeader(
+    Matcher<std::string> session_tag,
     const std::vector<int>& window_ids,
     const std::vector<int>& tab_ids) {
   return MatchesHeader(session_tag, ElementsAreArray(window_ids),
@@ -202,33 +220,33 @@ Matcher<const sync_pb::SessionSpecifics&> MatchesTab(
     Matcher<std::string> session_tag,
     Matcher<int> window_id,
     Matcher<int> tab_id,
+    Matcher<int> tab_node_id,
     Matcher<std::vector<std::string>> urls) {
   return testing::MakeMatcher(
-      new MatchesTabMatcher(session_tag, window_id, tab_id, urls));
+      new MatchesTabMatcher(session_tag, window_id, tab_id, tab_node_id, urls));
 }
 
-testing::Matcher<const sync_pb::SessionSpecifics&> MatchesTab(
-    testing::Matcher<std::string> session_tag,
-    testing::Matcher<int> window_id,
-    testing::Matcher<int> tab_id,
+Matcher<const sync_pb::SessionSpecifics&> MatchesTab(
+    Matcher<std::string> session_tag,
+    Matcher<int> window_id,
+    Matcher<int> tab_id,
+    Matcher<int> tab_node_id,
     const std::vector<std::string>& urls) {
-  return MatchesTab(session_tag, window_id, tab_id, ElementsAreArray(urls));
+  return MatchesTab(session_tag, window_id, tab_id, tab_node_id,
+                    ElementsAreArray(urls));
 }
 
 Matcher<const SyncedSession*> MatchesSyncedSession(
     Matcher<std::string> session_tag,
-    Matcher<std::vector<int>> window_ids,
-    Matcher<std::vector<int>> tab_ids) {
+    Matcher<std::map<int, std::vector<int>>> window_id_to_tabs) {
   return testing::MakeMatcher(
-      new MatchesSyncedSessionMatcher(session_tag, window_ids, tab_ids));
+      new MatchesSyncedSessionMatcher(session_tag, window_id_to_tabs));
 }
 
-testing::Matcher<const SyncedSession*> MatchesSyncedSession(
-    testing::Matcher<std::string> session_tag,
-    const std::vector<int>& window_ids,
-    const std::vector<int>& tab_ids) {
-  return MatchesSyncedSession(session_tag, ElementsAreArray(window_ids),
-                              ElementsAreArray(tab_ids));
+Matcher<const SyncedSession*> MatchesSyncedSession(
+    Matcher<std::string> session_tag,
+    const std::map<int, std::vector<int>>& window_id_to_tabs) {
+  return MatchesSyncedSession(session_tag, ContainerEq(window_id_to_tabs));
 }
 
 }  // namespace sync_sessions

components/sync_sessions/test_matchers.h

@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_SYNC_SESSIONS_TEST_MATCHERS_H_
 #define COMPONENTS_SYNC_SESSIONS_TEST_MATCHERS_H_
 
+#include <map>
 #include <string>
 #include <vector>
 
@@ -30,6 +31,7 @@ testing::Matcher<const sync_pb::SessionSpecifics&> MatchesTab(
     testing::Matcher<std::string> session_tag,
     testing::Matcher<int> window_id,
     testing::Matcher<int> tab_id,
+    testing::Matcher<int> tab_node_id,
     testing::Matcher<std::vector<std::string>> urls);
 
 // Convenience overload.
@@ -37,18 +39,17 @@ testing::Matcher<const sync_pb::SessionSpecifics&> MatchesTab(
     testing::Matcher<std::string> session_tag,
     testing::Matcher<int> window_id,
     testing::Matcher<int> tab_id,
+    testing::Matcher<int> tab_node_id,
     const std::vector<std::string>& urls);
 
 testing::Matcher<const SyncedSession*> MatchesSyncedSession(
     testing::Matcher<std::string> session_tag,
-    testing::Matcher<std::vector<int>> window_ids,
-    testing::Matcher<std::vector<int>> tab_ids);
+    testing::Matcher<std::map<int, std::vector<int>>> window_id_to_tabs);
 
 // Convenience overload.
 testing::Matcher<const SyncedSession*> MatchesSyncedSession(
     testing::Matcher<std::string> session_tag,
-    const std::vector<int>& window_ids,
-    const std::vector<int>& tab_ids);
+    const std::map<int, std::vector<int>>& window_id_to_tabs);
 
 }  // namespace sync_sessions