Reland v5 of Sessions Refactor

components/sync_sessions/synced_session_tracker_unittest.cc

Index: components/sync_sessions/synced_session_tracker_unittest.cc
diff --git a/components/sync_sessions/synced_session_tracker_unittest.cc b/components/sync_sessions/synced_session_tracker_unittest.cc
index 68c1f471a1fdac1bffcc7502cceeaf926c5298a5..7d8d1a6095d43c63d1fa6aa8aa38704b6c9bc65a 100644
--- a/components/sync_sessions/synced_session_tracker_unittest.cc
+++ b/components/sync_sessions/synced_session_tracker_unittest.cc
@@ -17,6 +17,15 @@ namespace {
 
 const char kValidUrl[] = "http://www.example.com";
 const char kInvalidUrl[] = "invalid.url";
+const char kTag[] = "tag";
+const char kTag2[] = "tag2";
+const char kTag3[] = "tag3";
+const char kTitle[] = "title";
+const int kWindow1 = 1;
+const int kTabNode = 0;
+const int kTab1 = 15;
+const int kTab2 = 25;
+const int kTab3 = 35;
 
 }  // namespace
 
@@ -26,6 +35,7 @@ class SyncedSessionTrackerTest : public testing::Test {
   ~SyncedSessionTrackerTest() override {}
 
   SyncedSessionTracker* GetTracker() { return &tracker_; }
+  TabNodePool* GetTabNodePool() { return &tracker_.local_tab_pool_; }
 
  private:
   FakeSyncSessionsClient sessions_client_;
@@ -33,34 +43,33 @@ class SyncedSessionTrackerTest : public testing::Test {
 };
 
 TEST_F(SyncedSessionTrackerTest, GetSession) {
-  SyncedSession* session1 = GetTracker()->GetSession("tag");
-  SyncedSession* session2 = GetTracker()->GetSession("tag2");
-  ASSERT_EQ(session1, GetTracker()->GetSession("tag"));
+  SyncedSession* session1 = GetTracker()->GetSession(kTag);
+  SyncedSession* session2 = GetTracker()->GetSession(kTag2);
+  ASSERT_EQ(session1, GetTracker()->GetSession(kTag));
   ASSERT_NE(session1, session2);
   // Should clean up memory on its own.
 }
 
 TEST_F(SyncedSessionTrackerTest, GetTabUnmapped) {
-  sessions::SessionTab* tab = GetTracker()->GetTab("tag", 0, 0);
-  ASSERT_EQ(tab, GetTracker()->GetTab("tag", 0, 0));
+  sessions::SessionTab* tab = GetTracker()->GetTab(kTag, 0);
+  ASSERT_EQ(tab, GetTracker()->GetTab(kTag, 0));
   // Should clean up memory on its own.
 }
 
 TEST_F(SyncedSessionTrackerTest, PutWindowInSession) {
-  GetTracker()->PutWindowInSession("tag", 0);
-  SyncedSession* session = GetTracker()->GetSession("tag");
+  GetTracker()->PutWindowInSession(kTag, 0);
+  SyncedSession* session = GetTracker()->GetSession(kTag);
   ASSERT_EQ(1U, session->windows.size());
   // Should clean up memory on its own.
 }
 
 TEST_F(SyncedSessionTrackerTest, PutTabInWindow) {
-  GetTracker()->PutWindowInSession("tag", 10);
-  GetTracker()->PutTabInWindow("tag", 10, 15,
-                               0);  // win id 10, tab id 15, tab ind 0.
-  SyncedSession* session = GetTracker()->GetSession("tag");
+  GetTracker()->PutWindowInSession(kTag, 10);
+  GetTracker()->PutTabInWindow(kTag, 10, 15);  // win id 10, tab id 15
+  SyncedSession* session = GetTracker()->GetSession(kTag);
   ASSERT_EQ(1U, session->windows.size());
   ASSERT_EQ(1U, session->windows[10]->tabs.size());
-  ASSERT_EQ(GetTracker()->GetTab("tag", 15, 1),
+  ASSERT_EQ(GetTracker()->GetTab(kTag, 15),
             session->windows[10]->tabs[0].get());
   // Should clean up memory on its own.
 }
@@ -69,28 +78,28 @@ TEST_F(SyncedSessionTrackerTest, LookupAllForeignSessions) {
   std::vector<const SyncedSession*> sessions;
   ASSERT_FALSE(GetTracker()->LookupAllForeignSessions(
       &sessions, SyncedSessionTracker::PRESENTABLE));
-  GetTracker()->GetSession("tag1");
-  GetTracker()->PutWindowInSession("tag1", 0);
-  GetTracker()->PutTabInWindow("tag1", 0, 15, 0);
-  sessions::SessionTab* tab = GetTracker()->GetTab("tag1", 15, 1);
+  GetTracker()->GetSession(kTag);
+  GetTracker()->PutWindowInSession(kTag, 0);
+  GetTracker()->PutTabInWindow(kTag, 0, 15);
+  sessions::SessionTab* tab = GetTracker()->GetTab(kTag, 15);
   ASSERT_TRUE(tab);
   tab->navigations.push_back(
       sessions::SerializedNavigationEntryTestHelper::CreateNavigation(kValidUrl,
-                                                                      "title"));
-  GetTracker()->GetSession("tag2");
-  GetTracker()->GetSession("tag3");
-  GetTracker()->PutWindowInSession("tag3", 0);
-  GetTracker()->PutTabInWindow("tag3", 0, 15, 0);
-  tab = GetTracker()->GetTab("tag3", 15, 1);
+                                                                      kTitle));
+  GetTracker()->GetSession(kTag2);
+  GetTracker()->GetSession(kTag3);
+  GetTracker()->PutWindowInSession(kTag3, 0);
+  GetTracker()->PutTabInWindow(kTag3, 0, 15);
+  tab = GetTracker()->GetTab(kTag3, 15);
   ASSERT_TRUE(tab);
   tab->navigations.push_back(
       sessions::SerializedNavigationEntryTestHelper::CreateNavigation(
-          kInvalidUrl, "title"));
+          kInvalidUrl, kTitle));
   ASSERT_TRUE(GetTracker()->LookupAllForeignSessions(
       &sessions, SyncedSessionTracker::PRESENTABLE));
   // Only the session with a valid window and tab gets returned.
   ASSERT_EQ(1U, sessions.size());
-  ASSERT_EQ("tag1", sessions[0]->session_tag);
+  ASSERT_EQ(kTag, sessions[0]->session_tag);
 
   ASSERT_TRUE(GetTracker()->LookupAllForeignSessions(
       &sessions, SyncedSessionTracker::RAW));
@@ -99,15 +108,15 @@ TEST_F(SyncedSessionTrackerTest, LookupAllForeignSessions) {
 
 TEST_F(SyncedSessionTrackerTest, LookupSessionWindows) {
   std::vector<const sessions::SessionWindow*> windows;
-  ASSERT_FALSE(GetTracker()->LookupSessionWindows("tag1", &windows));
-  GetTracker()->GetSession("tag1");
-  GetTracker()->PutWindowInSession("tag1", 0);
-  GetTracker()->PutWindowInSession("tag1", 2);
-  GetTracker()->GetSession("tag2");
-  GetTracker()->PutWindowInSession("tag2", 0);
-  GetTracker()->PutWindowInSession("tag2", 2);
-  ASSERT_TRUE(GetTracker()->LookupSessionWindows("tag1", &windows));
-  ASSERT_EQ(2U, windows.size());  // Only windows from tag1 session.
+  ASSERT_FALSE(GetTracker()->LookupSessionWindows(kTag, &windows));
+  GetTracker()->GetSession(kTag);
+  GetTracker()->PutWindowInSession(kTag, 0);
+  GetTracker()->PutWindowInSession(kTag, 2);
+  GetTracker()->GetSession(kTag2);
+  GetTracker()->PutWindowInSession(kTag2, 0);
+  GetTracker()->PutWindowInSession(kTag2, 2);
+  ASSERT_TRUE(GetTracker()->LookupSessionWindows(kTag, &windows));
+  ASSERT_EQ(2U, windows.size());  // Only windows from kTag session.
   ASSERT_NE((sessions::SessionWindow*)nullptr, windows[0]);
   ASSERT_NE((sessions::SessionWindow*)nullptr, windows[1]);
   ASSERT_NE(windows[1], windows[0]);
@@ -115,40 +124,39 @@ TEST_F(SyncedSessionTrackerTest, LookupSessionWindows) {
 
 TEST_F(SyncedSessionTrackerTest, LookupSessionTab) {
   const sessions::SessionTab* tab;
-  ASSERT_FALSE(GetTracker()->LookupSessionTab("tag1", 5, &tab));
-  GetTracker()->GetSession("tag1");
-  GetTracker()->PutWindowInSession("tag1", 0);
-  GetTracker()->PutTabInWindow("tag1", 0, 5, 0);
-  ASSERT_TRUE(GetTracker()->LookupSessionTab("tag1", 5, &tab));
+  ASSERT_FALSE(
+      GetTracker()->LookupSessionTab(kTag, TabNodePool::kInvalidTabID, &tab));
+  ASSERT_FALSE(GetTracker()->LookupSessionTab(kTag, 5, &tab));
+  GetTracker()->GetSession(kTag);
+  GetTracker()->PutWindowInSession(kTag, 0);
+  GetTracker()->PutTabInWindow(kTag, 0, 5);
+  ASSERT_TRUE(GetTracker()->LookupSessionTab(kTag, 5, &tab));
   ASSERT_NE((sessions::SessionTab*)nullptr, tab);
 }
 
 TEST_F(SyncedSessionTrackerTest, Complex) {
-  const std::string tag1 = "tag";
-  const std::string tag2 = "tag2";
-  const std::string tag3 = "tag3";
   std::vector<sessions::SessionTab *> tabs1, tabs2;
   sessions::SessionTab* temp_tab;
   ASSERT_TRUE(GetTracker()->Empty());
   ASSERT_EQ(0U, GetTracker()->num_synced_sessions());
-  ASSERT_EQ(0U, GetTracker()->num_synced_tabs(tag1));
-  tabs1.push_back(GetTracker()->GetTab(tag1, 0, 0));
-  tabs1.push_back(GetTracker()->GetTab(tag1, 1, 1));
-  tabs1.push_back(GetTracker()->GetTab(tag1, 2, 2));
-  ASSERT_EQ(3U, GetTracker()->num_synced_tabs(tag1));
-  ASSERT_EQ(1U, GetTracker()->num_synced_sessions());
-  temp_tab = GetTracker()->GetTab(tag1, 0, 0);  // Already created.
-  ASSERT_EQ(3U, GetTracker()->num_synced_tabs(tag1));
-  ASSERT_EQ(1U, GetTracker()->num_synced_sessions());
+  ASSERT_EQ(0U, GetTracker()->num_synced_tabs(kTag));
+  tabs1.push_back(GetTracker()->GetTab(kTag, 0));
+  tabs1.push_back(GetTracker()->GetTab(kTag, 1));
+  tabs1.push_back(GetTracker()->GetTab(kTag, 2));
+  ASSERT_EQ(3U, GetTracker()->num_synced_tabs(kTag));
+  ASSERT_EQ(0U, GetTracker()->num_synced_sessions());
+  temp_tab = GetTracker()->GetTab(kTag, 0);  // Already created.
+  ASSERT_EQ(3U, GetTracker()->num_synced_tabs(kTag));
+  ASSERT_EQ(0U, GetTracker()->num_synced_sessions());
   ASSERT_EQ(tabs1[0], temp_tab);
-  tabs2.push_back(GetTracker()->GetTab(tag2, 0, 0));
-  ASSERT_EQ(1U, GetTracker()->num_synced_tabs(tag2));
-  ASSERT_EQ(2U, GetTracker()->num_synced_sessions());
-  ASSERT_FALSE(GetTracker()->DeleteSession(tag3));
+  tabs2.push_back(GetTracker()->GetTab(kTag2, 0));
+  ASSERT_EQ(1U, GetTracker()->num_synced_tabs(kTag2));
+  ASSERT_EQ(0U, GetTracker()->num_synced_sessions());
+  ASSERT_FALSE(GetTracker()->DeleteForeignSession(kTag3));
 
-  SyncedSession* session = GetTracker()->GetSession(tag1);
-  SyncedSession* session2 = GetTracker()->GetSession(tag2);
-  SyncedSession* session3 = GetTracker()->GetSession(tag3);
+  SyncedSession* session = GetTracker()->GetSession(kTag);
+  SyncedSession* session2 = GetTracker()->GetSession(kTag2);
+  SyncedSession* session3 = GetTracker()->GetSession(kTag3);
   session3->device_type = SyncedSession::TYPE_OTHER;
   ASSERT_EQ(3U, GetTracker()->num_synced_sessions());
 
@@ -157,25 +165,25 @@ TEST_F(SyncedSessionTrackerTest, Complex) {
   ASSERT_TRUE(session3);
   ASSERT_NE(session, session2);
   ASSERT_NE(session2, session3);
-  ASSERT_TRUE(GetTracker()->DeleteSession(tag3));
+  ASSERT_TRUE(GetTracker()->DeleteForeignSession(kTag3));
   ASSERT_EQ(2U, GetTracker()->num_synced_sessions());
 
-  GetTracker()->PutWindowInSession(tag1, 0);           // Create a window.
-  GetTracker()->PutTabInWindow(tag1, 0, 2, 0);         // No longer unmapped.
-  ASSERT_EQ(3U, GetTracker()->num_synced_tabs(tag1));  // Has not changed.
+  GetTracker()->PutWindowInSession(kTag, 0);           // Create a window.
+  GetTracker()->PutTabInWindow(kTag, 0, 2);            // No longer unmapped.
+  ASSERT_EQ(3U, GetTracker()->num_synced_tabs(kTag));  // Has not changed.
 
   const sessions::SessionTab* tab_ptr;
-  ASSERT_TRUE(GetTracker()->LookupSessionTab(tag1, 0, &tab_ptr));
+  ASSERT_TRUE(GetTracker()->LookupSessionTab(kTag, 0, &tab_ptr));
   ASSERT_EQ(tab_ptr, tabs1[0]);
-  ASSERT_TRUE(GetTracker()->LookupSessionTab(tag1, 2, &tab_ptr));
+  ASSERT_TRUE(GetTracker()->LookupSessionTab(kTag, 2, &tab_ptr));
   ASSERT_EQ(tab_ptr, tabs1[2]);
-  ASSERT_FALSE(GetTracker()->LookupSessionTab(tag1, 3, &tab_ptr));
+  ASSERT_FALSE(GetTracker()->LookupSessionTab(kTag, 3, &tab_ptr));
   ASSERT_FALSE(tab_ptr);
 
   std::vector<const sessions::SessionWindow*> windows;
-  ASSERT_TRUE(GetTracker()->LookupSessionWindows(tag1, &windows));
+  ASSERT_TRUE(GetTracker()->LookupSessionWindows(kTag, &windows));
   ASSERT_EQ(1U, windows.size());
-  ASSERT_TRUE(GetTracker()->LookupSessionWindows(tag2, &windows));
+  ASSERT_TRUE(GetTracker()->LookupSessionWindows(kTag2, &windows));
   ASSERT_EQ(0U, windows.size());
 
   // The sessions don't have valid tabs, lookup should not succeed.
@@ -187,8 +195,8 @@ TEST_F(SyncedSessionTrackerTest, Complex) {
   ASSERT_EQ(2U, sessions.size());
 
   GetTracker()->Clear();
-  ASSERT_EQ(0U, GetTracker()->num_synced_tabs(tag1));
-  ASSERT_EQ(0U, GetTracker()->num_synced_tabs(tag2));
+  ASSERT_EQ(0U, GetTracker()->num_synced_tabs(kTag));
+  ASSERT_EQ(0U, GetTracker()->num_synced_tabs(kTag2));
   ASSERT_EQ(0U, GetTracker()->num_synced_sessions());
 }
 
@@ -203,107 +211,101 @@ TEST_F(SyncedSessionTrackerTest, ManyGetTabs) {
       // More attempts than tabs means we'll sometimes get the same tabs,
       // sometimes have to allocate new tabs.
       int rand_tab_num = base::RandInt(0, kMaxTabs);
-      sessions::SessionTab* tab =
-          GetTracker()->GetTab(tag, rand_tab_num, rand_tab_num + 1);
+      sessions::SessionTab* tab = GetTracker()->GetTab(tag, rand_tab_num + 1);
       ASSERT_TRUE(tab);
     }
   }
 }
 
-TEST_F(SyncedSessionTrackerTest, LookupTabNodeIds) {
+TEST_F(SyncedSessionTrackerTest, LookupForeignTabNodeIds) {
   std::set<int> result;
-  std::string tag1 = "session1";
-  std::string tag2 = "session2";
-  std::string tag3 = "session3";
 
-  GetTracker()->GetTab(tag1, 1, 1);
-  GetTracker()->GetTab(tag1, 2, 2);
-  GetTracker()->LookupTabNodeIds(tag1, &result);
+  GetTracker()->OnTabNodeSeen(kTag, 1);
+  GetTracker()->OnTabNodeSeen(kTag, 2);
+  GetTracker()->LookupForeignTabNodeIds(kTag, &result);
   EXPECT_EQ(2U, result.size());
   EXPECT_FALSE(result.end() == result.find(1));
   EXPECT_FALSE(result.end() == result.find(2));
-  GetTracker()->LookupTabNodeIds(tag2, &result);
+  GetTracker()->LookupForeignTabNodeIds(kTag2, &result);
   EXPECT_TRUE(result.empty());
 
-  GetTracker()->PutWindowInSession(tag1, 0);
-  GetTracker()->PutTabInWindow(tag1, 0, 3, 0);
-  GetTracker()->LookupTabNodeIds(tag1, &result);
+  GetTracker()->PutWindowInSession(kTag, 0);
+  GetTracker()->PutTabInWindow(kTag, 0, 3);
+  GetTracker()->LookupForeignTabNodeIds(kTag, &result);
   EXPECT_EQ(2U, result.size());
 
-  GetTracker()->GetTab(tag1, 3, 3);
-  GetTracker()->LookupTabNodeIds(tag1, &result);
+  GetTracker()->OnTabNodeSeen(kTag, 3);
+  GetTracker()->LookupForeignTabNodeIds(kTag, &result);
   EXPECT_EQ(3U, result.size());
   EXPECT_FALSE(result.end() == result.find(3));
 
-  GetTracker()->GetTab(tag2, 1, 21);
-  GetTracker()->GetTab(tag2, 2, 22);
-  GetTracker()->LookupTabNodeIds(tag2, &result);
+  GetTracker()->OnTabNodeSeen(kTag2, 21);
+  GetTracker()->OnTabNodeSeen(kTag2, 22);
+  GetTracker()->LookupForeignTabNodeIds(kTag2, &result);
   EXPECT_EQ(2U, result.size());
   EXPECT_FALSE(result.end() == result.find(21));
   EXPECT_FALSE(result.end() == result.find(22));
-  GetTracker()->LookupTabNodeIds(tag1, &result);
+  GetTracker()->LookupForeignTabNodeIds(kTag, &result);
   EXPECT_EQ(3U, result.size());
   EXPECT_FALSE(result.end() == result.find(1));
   EXPECT_FALSE(result.end() == result.find(2));
 
-  GetTracker()->LookupTabNodeIds(tag3, &result);
+  GetTracker()->LookupForeignTabNodeIds(kTag3, &result);
   EXPECT_TRUE(result.empty());
-  GetTracker()->PutWindowInSession(tag3, 1);
-  GetTracker()->PutTabInWindow(tag3, 1, 5, 0);
-  GetTracker()->LookupTabNodeIds(tag3, &result);
+  GetTracker()->PutWindowInSession(kTag3, 1);
+  GetTracker()->PutTabInWindow(kTag3, 1, 5);
+  GetTracker()->LookupForeignTabNodeIds(kTag3, &result);
   EXPECT_TRUE(result.empty());
-  EXPECT_FALSE(GetTracker()->DeleteSession(tag3));
-  GetTracker()->LookupTabNodeIds(tag3, &result);
+  EXPECT_FALSE(GetTracker()->DeleteForeignSession(kTag3));
+  GetTracker()->LookupForeignTabNodeIds(kTag3, &result);
   EXPECT_TRUE(result.empty());
 
-  EXPECT_FALSE(GetTracker()->DeleteSession(tag1));
-  GetTracker()->LookupTabNodeIds(tag1, &result);
+  EXPECT_FALSE(GetTracker()->DeleteForeignSession(kTag));
+  GetTracker()->LookupForeignTabNodeIds(kTag, &result);
   EXPECT_TRUE(result.empty());
-  GetTracker()->LookupTabNodeIds(tag2, &result);
+  GetTracker()->LookupForeignTabNodeIds(kTag2, &result);
   EXPECT_EQ(2U, result.size());
   EXPECT_FALSE(result.end() == result.find(21));
   EXPECT_FALSE(result.end() == result.find(22));
 
-  GetTracker()->GetTab(tag2, 1, 21);
-  GetTracker()->GetTab(tag2, 2, 23);
-  GetTracker()->LookupTabNodeIds(tag2, &result);
+  GetTracker()->OnTabNodeSeen(kTag2, 21);
+  GetTracker()->OnTabNodeSeen(kTag2, 23);
+  GetTracker()->LookupForeignTabNodeIds(kTag2, &result);
   EXPECT_EQ(3U, result.size());
   EXPECT_FALSE(result.end() == result.find(21));
   EXPECT_FALSE(result.end() == result.find(22));
   EXPECT_FALSE(result.end() == result.find(23));
 
-  EXPECT_FALSE(GetTracker()->DeleteSession(tag2));
-  GetTracker()->LookupTabNodeIds(tag2, &result);
+  EXPECT_FALSE(GetTracker()->DeleteForeignSession(kTag2));
+  GetTracker()->LookupForeignTabNodeIds(kTag2, &result);
   EXPECT_TRUE(result.empty());
 }
 
 TEST_F(SyncedSessionTrackerTest, SessionTracking) {
   ASSERT_TRUE(GetTracker()->Empty());
-  std::string tag1 = "tag1";
-  std::string tag2 = "tag2";
 
   // Create some session information that is stale.
-  SyncedSession* session1 = GetTracker()->GetSession(tag1);
-  GetTracker()->PutWindowInSession(tag1, 0);
-  GetTracker()->PutTabInWindow(tag1, 0, 0, 0);
-  GetTracker()->PutTabInWindow(tag1, 0, 1, 1);
-  GetTracker()->GetTab(tag1, 2, 3U)->window_id.set_id(0);  // Will be unmapped.
-  GetTracker()->GetTab(tag1, 3, 4U)->window_id.set_id(0);  // Will be unmapped.
-  GetTracker()->PutWindowInSession(tag1, 1);
-  GetTracker()->PutTabInWindow(tag1, 1, 4, 0);
-  GetTracker()->PutTabInWindow(tag1, 1, 5, 1);
+  SyncedSession* session1 = GetTracker()->GetSession(kTag);
+  GetTracker()->PutWindowInSession(kTag, 0);
+  GetTracker()->PutTabInWindow(kTag, 0, 0);
+  GetTracker()->PutTabInWindow(kTag, 0, 1);
+  GetTracker()->GetTab(kTag, 2)->window_id.set_id(0);  // Will be unmapped.
+  GetTracker()->GetTab(kTag, 3)->window_id.set_id(0);  // Will be unmapped.
+  GetTracker()->PutWindowInSession(kTag, 1);
+  GetTracker()->PutTabInWindow(kTag, 1, 4);
+  GetTracker()->PutTabInWindow(kTag, 1, 5);
   ASSERT_EQ(2U, session1->windows.size());
   ASSERT_EQ(2U, session1->windows[0]->tabs.size());
   ASSERT_EQ(2U, session1->windows[1]->tabs.size());
-  ASSERT_EQ(6U, GetTracker()->num_synced_tabs(tag1));
+  ASSERT_EQ(6U, GetTracker()->num_synced_tabs(kTag));
 
   // Create a session that should not be affected.
-  SyncedSession* session2 = GetTracker()->GetSession(tag2);
-  GetTracker()->PutWindowInSession(tag2, 2);
-  GetTracker()->PutTabInWindow(tag2, 2, 1, 0);
+  SyncedSession* session2 = GetTracker()->GetSession(kTag2);
+  GetTracker()->PutWindowInSession(kTag2, 2);
+  GetTracker()->PutTabInWindow(kTag2, 2, 1);
   ASSERT_EQ(1U, session2->windows.size());
   ASSERT_EQ(1U, session2->windows[2]->tabs.size());
-  ASSERT_EQ(1U, GetTracker()->num_synced_tabs(tag2));
+  ASSERT_EQ(1U, GetTracker()->num_synced_tabs(kTag2));
 
   // Reset tracking and get the current windows/tabs.
   // We simulate moving a tab from one window to another, then closing the
@@ -311,18 +313,18 @@ TEST_F(SyncedSessionTrackerTest, SessionTracking) {
   // on the remaining window.
 
   // New tab, arrived before meta node so unmapped.
-  GetTracker()->GetTab(tag1, 6, 7U);
-  GetTracker()->ResetSessionTracking(tag1);
-  GetTracker()->PutWindowInSession(tag1, 0);
-  GetTracker()->PutTabInWindow(tag1, 0, 0, 0);
+  GetTracker()->GetTab(kTag, 6);
+  GetTracker()->ResetSessionTracking(kTag);
+  GetTracker()->PutWindowInSession(kTag, 0);
+  GetTracker()->PutTabInWindow(kTag, 0, 0);
   // Tab 1 is closed.
-  GetTracker()->PutTabInWindow(tag1, 0, 2, 1);  // No longer unmapped.
+  GetTracker()->PutTabInWindow(kTag, 0, 2);  // No longer unmapped.
   // Tab 3 was unmapped and does not get used.
-  GetTracker()->PutTabInWindow(tag1, 0, 4, 2);  // Moved from window 1.
+  GetTracker()->PutTabInWindow(kTag, 0, 4);  // Moved from window 1.
   // Window 1 was closed, along with tab 5.
-  GetTracker()->PutTabInWindow(tag1, 0, 6, 3);  // No longer unmapped.
+  GetTracker()->PutTabInWindow(kTag, 0, 6);  // No longer unmapped.
   // Session 2 should not be affected.
-  GetTracker()->CleanupSession(tag1);
+  GetTracker()->CleanupForeignSession(kTag);
 
   // Verify that only those parts of the session not owned have been removed.
   ASSERT_EQ(1U, session1->windows.size());
@@ -330,39 +332,224 @@ TEST_F(SyncedSessionTrackerTest, SessionTracking) {
   ASSERT_EQ(1U, session2->windows.size());
   ASSERT_EQ(1U, session2->windows[2]->tabs.size());
   ASSERT_EQ(2U, GetTracker()->num_synced_sessions());
-  ASSERT_EQ(4U, GetTracker()->num_synced_tabs(tag1));
-  ASSERT_EQ(1U, GetTracker()->num_synced_tabs(tag2));
+  ASSERT_EQ(4U, GetTracker()->num_synced_tabs(kTag));
+  ASSERT_EQ(1U, GetTracker()->num_synced_tabs(kTag2));
 
   // All memory should be properly deallocated by destructor for the
   // SyncedSessionTracker.
 }
 
 TEST_F(SyncedSessionTrackerTest, DeleteForeignTab) {
-  std::string session_tag = "session_tag";
-  int tab_id_1 = 1;
-  int tab_id_2 = 2;
-  int tab_node_id_3 = 3;
-  int tab_node_id_4 = 4;
+  int tab_node_id_1 = 1;
+  int tab_node_id_2 = 2;
   std::set<int> result;
 
-  GetTracker()->GetTab(session_tag, tab_id_1, tab_node_id_3);
-  GetTracker()->GetTab(session_tag, tab_id_1, tab_node_id_4);
-  GetTracker()->GetTab(session_tag, tab_id_2, tab_node_id_3);
-  GetTracker()->GetTab(session_tag, tab_id_2, tab_node_id_4);
+  GetTracker()->OnTabNodeSeen(kTag, tab_node_id_1);
+  GetTracker()->OnTabNodeSeen(kTag, tab_node_id_2);
 
-  GetTracker()->LookupTabNodeIds(session_tag, &result);
+  GetTracker()->LookupForeignTabNodeIds(kTag, &result);
   EXPECT_EQ(2U, result.size());
-  EXPECT_TRUE(result.find(tab_node_id_3) != result.end());
-  EXPECT_TRUE(result.find(tab_node_id_4) != result.end());
+  EXPECT_TRUE(result.find(tab_node_id_1) != result.end());
+  EXPECT_TRUE(result.find(tab_node_id_2) != result.end());
 
-  GetTracker()->DeleteForeignTab(session_tag, tab_node_id_3);
-  GetTracker()->LookupTabNodeIds(session_tag, &result);
+  GetTracker()->DeleteForeignTab(kTag, tab_node_id_1);
+  GetTracker()->LookupForeignTabNodeIds(kTag, &result);
   EXPECT_EQ(1U, result.size());
-  EXPECT_TRUE(result.find(tab_node_id_4) != result.end());
+  EXPECT_TRUE(result.find(tab_node_id_2) != result.end());
 
-  GetTracker()->DeleteForeignTab(session_tag, tab_node_id_4);
-  GetTracker()->LookupTabNodeIds(session_tag, &result);
+  GetTracker()->DeleteForeignTab(kTag, tab_node_id_2);
+  GetTracker()->LookupForeignTabNodeIds(kTag, &result);
   EXPECT_TRUE(result.empty());
 }
 
+TEST_F(SyncedSessionTrackerTest, CleanupLocalTabs) {
+  std::set<int> free_node_ids;
+  int tab_node_id = TabNodePool::kInvalidTabNodeID;
+  const int kTabNode1 = 1;
+  const int kTabNode2 = 2;
+  const int kTabNode3 = 3;
+
+  GetTracker()->SetLocalSessionTag(kTag);
+
+  // Start with two restored tab nodes.
+  GetTracker()->ReassociateLocalTab(kTabNode1, kTab1);
+  GetTracker()->ReassociateLocalTab(kTabNode2, kTab2);
+  EXPECT_TRUE(GetTabNodePool()->Empty());
+  EXPECT_FALSE(GetTabNodePool()->Full());
+  EXPECT_EQ(2U, GetTabNodePool()->Capacity());
+
+  // Associate with no tabs. The tab pool should now be full.
+  GetTracker()->ResetSessionTracking(kTag);
+  GetTracker()->CleanupLocalTabs(&free_node_ids);
+  EXPECT_TRUE(free_node_ids.empty());
+  EXPECT_TRUE(GetTabNodePool()->Full());
+
+  // Associate with only 1 tab open. A tab node should be reused.
+  GetTracker()->ResetSessionTracking(kTag);
+  GetTracker()->PutWindowInSession(kTag, kWindow1);
+  GetTracker()->PutTabInWindow(kTag, kWindow1, kTab1);
+  EXPECT_TRUE(GetTracker()->GetTabNodeFromLocalTabId(kTab1, &tab_node_id));
+  GetTracker()->CleanupLocalTabs(&free_node_ids);
+  EXPECT_TRUE(free_node_ids.empty());
+
+  // TabNodePool should have one free tab node and one used.
+  EXPECT_EQ(2U, GetTabNodePool()->Capacity());
+  EXPECT_FALSE(GetTabNodePool()->Empty());
+  EXPECT_FALSE(GetTabNodePool()->Full());
+
+  // Simulate a tab opening, which should use the last free tab node.
+  EXPECT_TRUE(GetTracker()->GetTabNodeFromLocalTabId(kTab2, &tab_node_id));
+  EXPECT_TRUE(GetTabNodePool()->Empty());
+
+  // Simulate another tab opening, which should create a new associated tab
+  // node.
+  EXPECT_FALSE(GetTracker()->GetTabNodeFromLocalTabId(kTab3, &tab_node_id));
+  EXPECT_EQ(kTabNode3, tab_node_id);
+  EXPECT_EQ(3U, GetTabNodePool()->Capacity());
+  EXPECT_TRUE(GetTabNodePool()->Empty());
+
+  // Fetching the same tab should return the same tab node id.
+  EXPECT_TRUE(GetTracker()->GetTabNodeFromLocalTabId(kTab3, &tab_node_id));
+  EXPECT_EQ(kTabNode3, tab_node_id);
+  EXPECT_TRUE(GetTabNodePool()->Empty());
+
+  // Associate with no tabs. All tabs should be freed again, and the pool
+  // should now be full.
+  GetTracker()->ResetSessionTracking(kTag);
+  GetTracker()->CleanupLocalTabs(&free_node_ids);
+  EXPECT_TRUE(free_node_ids.empty());
+  EXPECT_TRUE(GetTabNodePool()->Full());
+  EXPECT_FALSE(GetTabNodePool()->Empty());
+}
+
+TEST_F(SyncedSessionTrackerTest, ReassociateTabMapped) {
+  std::set<int> free_node_ids;
+
+  // First create the tab normally.
+  GetTracker()->SetLocalSessionTag(kTag);
+  EXPECT_FALSE(GetTracker()->IsLocalTabNodeAssociated(kTabNode));
+  GetTracker()->ReassociateLocalTab(kTabNode, kTab1);
+  EXPECT_TRUE(GetTracker()->IsLocalTabNodeAssociated(kTabNode));
+  EXPECT_TRUE(GetTracker()->IsTabUnmappedForTesting(kTab1));
+
+  // Map it to a window with the same tab id as it was created with.
+  GetTracker()->ResetSessionTracking(kTag);
+  GetTracker()->PutWindowInSession(kTag, kWindow1);
+  GetTracker()->PutTabInWindow(kTag, kWindow1, kTab1);
+  GetTracker()->CleanupLocalTabs(&free_node_ids);
+  EXPECT_FALSE(GetTracker()->IsTabUnmappedForTesting(kTab1));
+  SyncedSession* session = GetTracker()->GetSession(kTag);
+  ASSERT_EQ(1U, session->windows.size());
+  ASSERT_EQ(1U, session->windows[kWindow1]->tabs.size());
+  ASSERT_EQ(GetTracker()->GetTab(kTag, kTab1),
+            session->windows[kWindow1]->tabs[0].get());
+
+  // Then reassociate with a new tab id.
+  GetTracker()->ReassociateLocalTab(kTabNode, kTab2);
+  EXPECT_TRUE(GetTracker()->IsLocalTabNodeAssociated(kTabNode));
+  EXPECT_FALSE(GetTracker()->IsTabUnmappedForTesting(kTab2));
+  EXPECT_FALSE(GetTracker()->IsTabUnmappedForTesting(kTab1));
+
+  // Reset tracking, and put the new tab id into the window.
+  GetTracker()->ResetSessionTracking(kTag);
+  EXPECT_TRUE(GetTracker()->IsTabUnmappedForTesting(kTab2));
+  GetTracker()->PutWindowInSession(kTag, kWindow1);
+  GetTracker()->PutTabInWindow(kTag, kWindow1, kTab2);
+  GetTracker()->CleanupLocalTabs(&free_node_ids);
+  EXPECT_TRUE(free_node_ids.empty());
+  EXPECT_FALSE(GetTracker()->IsTabUnmappedForTesting(kTab2));
+
+  // Now that it's been mapped, it should be accessible both via the
+  // GetSession as well as the GetTab.
+  ASSERT_EQ(GetTracker()->GetTab(kTag, kTab2),
+            session->windows[kWindow1]->tabs[0].get());
+  ASSERT_EQ(session->tab_node_ids.size(),
+            session->tab_node_ids.count(kTabNode));
+  ASSERT_EQ(1U, GetTabNodePool()->Capacity());

[skym, 2017/02/27 18:22:38]

Don't do this in this CL, but these lines would be smaller if these accessors
were just pool() and tracker(), and I think they'd be equally clear.

[Nicolas Zea, 2017/02/28 00:09:01]

Acknowledged.

+}
+
+TEST_F(SyncedSessionTrackerTest, ReassociateTabMappedTwice) {
+  std::set<int> free_node_ids;
+
+  // First create the tab normally.
+  GetTracker()->SetLocalSessionTag(kTag);
+  EXPECT_FALSE(GetTracker()->IsLocalTabNodeAssociated(kTabNode));
+  GetTracker()->ReassociateLocalTab(kTabNode, kTab1);
+  EXPECT_TRUE(GetTracker()->IsLocalTabNodeAssociated(kTabNode));
+  EXPECT_TRUE(GetTracker()->IsTabUnmappedForTesting(kTab1));
+
+  // Map it to a window with the same tab id as it was created with.
+  GetTracker()->ResetSessionTracking(kTag);
+  GetTracker()->PutWindowInSession(kTag, kWindow1);
+  GetTracker()->PutTabInWindow(kTag, kWindow1, kTab1);
+  GetTracker()->CleanupLocalTabs(&free_node_ids);

[skym, 2017/02/27 18:22:38]

Should deleted_node_ids.empty() be checked here as well?

[Nicolas Zea, 2017/02/28 00:09:01]

Do you mean free_node_ids? Sure, done.

+  EXPECT_FALSE(GetTracker()->IsTabUnmappedForTesting(kTab1));
+  SyncedSession* session = GetTracker()->GetSession(kTag);
+  ASSERT_EQ(1U, session->windows.size());
+  ASSERT_EQ(1U, session->windows[kWindow1]->tabs.size());
+  ASSERT_EQ(GetTracker()->GetTab(kTag, kTab1),
+            session->windows[kWindow1]->tabs[0].get());
+
+  // Then reassociate with a new tab id.
+  GetTracker()->ReassociateLocalTab(kTabNode, kTab2);
+  EXPECT_TRUE(GetTracker()->IsLocalTabNodeAssociated(kTabNode));
+  EXPECT_FALSE(GetTracker()->IsTabUnmappedForTesting(kTab2));
+  EXPECT_FALSE(GetTracker()->IsTabUnmappedForTesting(kTab1));
+
+  // Reset tracking and add back both the old tab and the new tab (both of which
+  // refer to the same tab node id).
+  GetTracker()->ResetSessionTracking(kTag);
+  EXPECT_TRUE(GetTracker()->IsTabUnmappedForTesting(kTab2));

[skym, 2017/02/27 18:22:38]

Shouldn't kTab1 be unmapped as well?

[Nicolas Zea, 2017/02/28 00:09:01]

kTab1 doesn't exist at this point (the SessionTab object was reused for kTab2,
so it's no longer associated with anything). So it's neither mapped nor unmapped
(and IsTabUnmappedForTesting will actually return false).

Adding a LookupSessionTab check to verify that.

+  GetTracker()->PutWindowInSession(kTag, kWindow1);
+  GetTracker()->PutTabInWindow(kTag, kWindow1, kTab1);
+  GetTracker()->PutTabInWindow(kTag, kWindow1, kTab2);
+  GetTracker()->CleanupLocalTabs(&free_node_ids);
+  EXPECT_TRUE(free_node_ids.empty());
+  EXPECT_FALSE(GetTracker()->IsTabUnmappedForTesting(kTab2));

[skym, 2017/02/27 18:22:38]

..is kTab1 unmapped?

[Nicolas Zea, 2017/02/28 00:09:01]

Same as above. Since it doesn't exist anymore, IsTabUnmapped would return false.

+
+  // Now that it's been mapped, it should be accessible both via the
+  // GetSession as well as the GetTab.

[skym, 2017/02/27 18:22:38]

Did you mean to re-assign |session| via GetSession()? Or is the non-owned
SyncedSession object being updated a feature we want our tests to verify?

[Nicolas Zea, 2017/02/28 00:09:01]

GetSession is meant to give you access to a Session object that will live
indefinitely, since the caller will actually be modifying the session object as
well. I'd say it's fine to rely on that behavior here.

+  ASSERT_EQ(GetTracker()->GetTab(kTag, kTab2),
+            session->windows[kWindow1]->tabs[1].get());
+  ASSERT_EQ(session->tab_node_ids.size(),
+            session->tab_node_ids.count(kTabNode));
+  ASSERT_EQ(1U, GetTabNodePool()->Capacity());

[skym, 2017/02/27 18:22:38]

Do all these really need to be assert and not expect?

[Nicolas Zea, 2017/02/28 00:09:01]

Done.

+
+  // Attempting to access the original tab will create a new SessionTab object.
+  ASSERT_NE(GetTracker()->GetTab(kTag, kTab1),

[skym, 2017/02/27 18:22:38]

What about making sure that kTab1's tab node is isn't kTabNode as well?

[Nicolas Zea, 2017/02/28 00:09:01]

See above. This GetTab method is actually creating a new placeholder SessionTab
that has no data, and isn't associated with any tab node.

Added a GetTabNodeFromLocalTabId to verify that.

+            GetTracker()->GetTab(kTag, kTab2));
+}
+
+TEST_F(SyncedSessionTrackerTest, ReassociateTabUnmapped) {
+  std::set<int> free_node_ids;
+
+  // First create the old tab in an unmapped state.
+  GetTracker()->SetLocalSessionTag(kTag);
+  EXPECT_FALSE(GetTracker()->IsLocalTabNodeAssociated(kTabNode));
+  GetTracker()->ReassociateLocalTab(kTabNode, kTab1);
+  EXPECT_TRUE(GetTracker()->IsLocalTabNodeAssociated(kTabNode));
+  EXPECT_TRUE(GetTracker()->IsTabUnmappedForTesting(kTab1));
+
+  // Map it to a window, but reassociated with a new tab id.
+  GetTracker()->ResetSessionTracking(kTag);
+  GetTracker()->ReassociateLocalTab(kTabNode, kTab2);
+  EXPECT_TRUE(GetTracker()->IsLocalTabNodeAssociated(kTabNode));
+  EXPECT_TRUE(GetTracker()->IsTabUnmappedForTesting(kTab2));
+  EXPECT_FALSE(GetTracker()->IsTabUnmappedForTesting(kTab1));
+  GetTracker()->PutWindowInSession(kTag, kWindow1);
+  GetTracker()->PutTabInWindow(kTag, kWindow1, kTab2);
+  GetTracker()->CleanupLocalTabs(&free_node_ids);
+  EXPECT_TRUE(free_node_ids.empty());
+  EXPECT_FALSE(GetTracker()->IsTabUnmappedForTesting(kTab2));
+
+  // Now that it's been mapped, it should be accessible both via the
+  // GetSession as well as GetTab.
+  SyncedSession* session = GetTracker()->GetSession(kTag);
+  ASSERT_EQ(GetTracker()->GetTab(kTag, kTab2),
+            session->windows[kWindow1]->tabs[0].get());
+  ASSERT_EQ(session->tab_node_ids.size(),
+            session->tab_node_ids.count(kTabNode));
+  ASSERT_EQ(1U, GetTabNodePool()->Capacity());
+}
+
 }  // namespace sync_sessions