Factor out implementation of GlobalIdMapper to dedicated class

This makes SessionsSyncManager smaller, which I believe justifies the
change. But actually, it is motivated by upcoming changes in sessions
sync code, where a new implementation will be introduced for USS. We
anticipate the need for the very same logic to track global ID mappings
for user events.

Bug: 681921
Change-Id: I0dcd1858c602a2cc167ca3ac0e2e9d55d072c291
Reviewed-on: https://chromium-review.googlesource.com/916763
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Nicolas Zea <zea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536983}

components/browser_sync/profile_sync_service.cc

@@ -2091,7 +2091,7 @@ void ProfileSyncService::GetAllNodes(
 }
 
 syncer::GlobalIdMapper* ProfileSyncService::GetGlobalIdMapper() const {
-  return sessions_sync_manager_.get();
+  return sessions_sync_manager_->GetGlobalIdMapper();
 }
 
 base::WeakPtr<syncer::JsController> ProfileSyncService::GetJsController() {

components/sync_sessions/BUILD.gn

@@ -31,6 +31,8 @@ static_library("sync_sessions") {
     "revisit/typed_url_page_revisit_task.h",
     "session_data_type_controller.cc",
     "session_data_type_controller.h",
+    "sessions_global_id_mapper.cc",
+    "sessions_global_id_mapper.h",
     "sessions_sync_manager.cc",
     "sessions_sync_manager.h",
     "sync_sessions_client.cc",
@@ -102,6 +104,7 @@ source_set("unit_tests") {
     "revisit/sessions_page_revisit_observer_unittest.cc",
     "revisit/typed_url_page_revisit_task_unittest.cc",
     "session_data_type_controller_unittest.cc",
+    "sessions_global_id_mapper_unittest.cc",
     "sessions_sync_manager_unittest.cc",
     "sync_sessions_metrics_unittest.cc",
     "synced_session_tracker_unittest.cc",

components/sync_sessions/sessions_global_id_mapper.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/sync_sessions/sessions_global_id_mapper.h"
+
+#include <utility>
+
+#include "base/metrics/histogram_macros.h"
+#include "base/stl_util.h"
+
+namespace sync_sessions {
+namespace {
+
+// Clean up navigation tracking when we have over this many global_ids.
+const size_t kNavigationTrackingCleanupThreshold = 100;
+
+// When we clean up navigation tracking, delete this many global_ids.
+const int kNavigationTrackingCleanupAmount = 10;
+
+// Used to record conflict information into histogram Sync.GlobalIdConflict.
+enum SyncGlobalIdConflict {
+  CONFLICT = 0,
+  NO_CONFLICT_NEW_ID,
+  NO_CONFLICT_SAME_IDS,
+  CONFLICT_MAX,
+};
+
+}  // namespace
+
+SessionsGlobalIdMapper::SessionsGlobalIdMapper() = default;
+
+SessionsGlobalIdMapper::~SessionsGlobalIdMapper() = default;
+
+void SessionsGlobalIdMapper::AddGlobalIdChangeObserver(
+    syncer::GlobalIdChange callback) {
+  global_id_change_observers_.push_back(std::move(callback));
+}
+
+int64_t SessionsGlobalIdMapper::GetLatestGlobalId(int64_t global_id) {
+  auto g2u_iter = global_to_unique_.find(global_id);
+  if (g2u_iter != global_to_unique_.end()) {
+    auto u2g_iter = unique_to_current_global_.find(g2u_iter->second);
+    if (u2g_iter != unique_to_current_global_.end()) {
+      return u2g_iter->second;
+    }
+  }
+  return global_id;
+}
+
+void SessionsGlobalIdMapper::TrackNavigationIds(const base::Time& timestamp,
+                                                int unique_id) {
+  // The expectation is that global_id will update for a given unique_id, which
+  // should accurately and uniquely represent a single navigation. It is
+  // theoretically possible for two unique_ids to map to the same global_id, but
+  // hopefully rare enough that it doesn't cause much harm. Lets record metrics
+  // verify this theory.
+  int64_t global_id = timestamp.ToInternalValue();
+  // It is possible that the global_id has not been set yet for this navigation.
+  // In this case there's nothing here for us to track yet.
+  if (global_id == 0) {
+    return;
+  }
+
+  DCHECK_NE(0, unique_id);
+
+  auto g2u_iter = global_to_unique_.find(global_id);
+  if (g2u_iter == global_to_unique_.end()) {
+    global_to_unique_.insert(g2u_iter, std::make_pair(global_id, unique_id));
+    UMA_HISTOGRAM_ENUMERATION("Sync.GlobalIdConflict", NO_CONFLICT_NEW_ID,
+                              CONFLICT_MAX);
+  } else if (g2u_iter->second != unique_id) {
+    UMA_HISTOGRAM_ENUMERATION("Sync.GlobalIdConflict", CONFLICT, CONFLICT_MAX);
+  } else {
+    UMA_HISTOGRAM_ENUMERATION("Sync.GlobalIdConflict", NO_CONFLICT_SAME_IDS,
+                              CONFLICT_MAX);
+  }
+
+  auto u2g_iter = unique_to_current_global_.find(unique_id);
+  if (u2g_iter == unique_to_current_global_.end()) {
+    unique_to_current_global_.insert(u2g_iter,
+                                     std::make_pair(unique_id, global_id));
+  } else if (u2g_iter->second != global_id) {
+    // Remember the old_global_id before we insert and invalidate out iter.
+    int64_t old_global_id = u2g_iter->second;
+
+    // TODO(skym): Use insert_or_assign with hint once on C++17.
+    unique_to_current_global_[unique_id] = global_id;
+
+    // This should be done after updating unique_to_current_global_ in case one
+    // of our observers calls into GetLatestGlobalId().
+    for (auto& observer : global_id_change_observers_) {
+      observer.Run(old_global_id, global_id);
+    }
+  }
+
+  CleanupNavigationTracking();
+}
+
+void SessionsGlobalIdMapper::CleanupNavigationTracking() {
+  DCHECK(kNavigationTrackingCleanupThreshold >
+         kNavigationTrackingCleanupAmount);
+
+  // |global_to_unique_| is implicitly ordered by least recently created, which
+  //  means we can drop from the beginning.
+  if (global_to_unique_.size() > kNavigationTrackingCleanupThreshold) {
+    auto iter = global_to_unique_.begin();
+    std::advance(iter, kNavigationTrackingCleanupAmount);
+    global_to_unique_.erase(global_to_unique_.begin(), iter);
+
+    // While |unique_id|s do get bigger for the most part, this isn't a great
+    // thing to make assumptions about, and an old tab may get refreshed often
+    // and still be very important. So instead just delete anything that's
+    // orphaned from |global_to_unique_|.
+    base::EraseIf(unique_to_current_global_,
+                  [this](const std::pair<int, int64_t> kv) {
+                    return !base::ContainsKey(global_to_unique_, kv.second);
+                  });
+  }
+}
+
+}  // namespace sync_sessions

components/sync_sessions/sessions_global_id_mapper.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_SYNC_SESSIONS_SESSIONS_GLOBAL_ID_MAPPER_H_
+#define COMPONENTS_SYNC_SESSIONS_SESSIONS_GLOBAL_ID_MAPPER_H_
+
+#include <map>
+#include <vector>
+
+#include "base/time/time.h"
+#include "components/sync/user_events/global_id_mapper.h"
+
+namespace sync_sessions {
+
+// Actual implementation of GlobalIdMapper used by sessions.
+class SessionsGlobalIdMapper : public syncer::GlobalIdMapper {
+ public:
+  SessionsGlobalIdMapper();
+  ~SessionsGlobalIdMapper();
+
+  // GlobalIdMapper implementation.
+  void AddGlobalIdChangeObserver(syncer::GlobalIdChange callback) override;
+  int64_t GetLatestGlobalId(int64_t global_id) override;
+
+  void TrackNavigationIds(const base::Time& timestamp, int unique_id);
+
+ private:
+  void CleanupNavigationTracking();
+
+  std::map<int64_t, int> global_to_unique_;
+  std::map<int, int64_t> unique_to_current_global_;
+  std::vector<syncer::GlobalIdChange> global_id_change_observers_;
+
+  DISALLOW_COPY_AND_ASSIGN(SessionsGlobalIdMapper);
+};
+
+}  // namespace sync_sessions
+
+#endif  // COMPONENTS_SYNC_SESSIONS_SESSIONS_GLOBAL_ID_MAPPER_H_

components/sync_sessions/sessions_global_id_mapper_unittest.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/sync_sessions/sessions_global_id_mapper.h"
+
+#include "base/test/mock_callback.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace sync_sessions {
+namespace {
+
+using testing::_;
+
+const base::Time kTime1 = base::Time::FromInternalValue(110);
+const base::Time kTime2 = base::Time::FromInternalValue(120);
+const base::Time kTime3 = base::Time::FromInternalValue(130);
+const base::Time kTime4 = base::Time::FromInternalValue(140);
+const base::Time kTime5 = base::Time::FromInternalValue(150);
+
+// Tests that GetLatestGlobalId returns correct mappings for updated global_ids.
+TEST(SessionsGlobalIdMapperTest, GetLatestGlobalId) {
+  SessionsGlobalIdMapper mapper;
+
+  mapper.TrackNavigationIds(kTime1, /*unique_id=*/1);
+  mapper.TrackNavigationIds(kTime2, /*unique_id=*/2);
+  mapper.TrackNavigationIds(kTime3, /*unique_id=*/2);
+  mapper.TrackNavigationIds(kTime4, /*unique_id=*/2);
+
+  EXPECT_EQ(kTime1.ToInternalValue(),
+            mapper.GetLatestGlobalId(kTime1.ToInternalValue()));
+  EXPECT_EQ(kTime4.ToInternalValue(),
+            mapper.GetLatestGlobalId(kTime2.ToInternalValue()));
+  EXPECT_EQ(kTime4.ToInternalValue(),
+            mapper.GetLatestGlobalId(kTime3.ToInternalValue()));
+  EXPECT_EQ(kTime4.ToInternalValue(),
+            mapper.GetLatestGlobalId(kTime4.ToInternalValue()));
+  // kTime5 is not mapped, so itself should be returned.
+  EXPECT_EQ(kTime5.ToInternalValue(),
+            mapper.GetLatestGlobalId(kTime5.ToInternalValue()));
+}
+
+// Tests that the global_id mapping is eventually dropped after we reach our
+// threshold for the amount to remember.
+TEST(SessionsGlobalIdMapperTest, Cleanup) {
+  SessionsGlobalIdMapper mapper;
+
+  base::Time current_time = kTime1;
+  mapper.TrackNavigationIds(current_time, /*unique_id=*/1);
+
+  for (int i = 0; i < 105; i++) {
+    current_time =
+        base::Time::FromInternalValue(current_time.ToInternalValue() + 1);
+    mapper.TrackNavigationIds(current_time, /*unique_id=*/1);
+  }
+
+  // Threshold is 100, kTime1 should be dropped, kTime1+10 should not.
+  EXPECT_EQ(kTime1.ToInternalValue(),
+            mapper.GetLatestGlobalId(kTime1.ToInternalValue()));
+  EXPECT_EQ(current_time.ToInternalValue(),
+            mapper.GetLatestGlobalId(10 + kTime1.ToInternalValue()));
+}
+
+// Tests that subscribers to AddGlobalIdChangeObserver are notified when a
+// global_id is noticed to have been changed.
+TEST(SessionsGlobalIdMapperTest, AddObserver) {
+  SessionsGlobalIdMapper mapper;
+
+  mapper.TrackNavigationIds(kTime1, /*unique_id=*/1);
+
+  base::MockCallback<syncer::GlobalIdChange> mock_callback;
+  EXPECT_CALL(mock_callback, Run(_, _)).Times(0);
+
+  mapper.AddGlobalIdChangeObserver(mock_callback.Get());
+
+  EXPECT_CALL(mock_callback,
+              Run(kTime1.ToInternalValue(), kTime2.ToInternalValue()))
+      .Times(0);
+}
+
+}  // namespace
+}  // namespace sync_sessions

components/sync_sessions/sessions_sync_manager.cc

@@ -50,20 +50,6 @@ const char kNTPOpenTabSyncURL[] = "chrome://newtab/#open_tabs";
 // stale and becomes a candidate for garbage collection.
 const int kDefaultStaleSessionThresholdDays = 14;  // 2 weeks.
 
-// Clean up navigation tracking when we have over this many global_ids.
-const size_t kNavigationTrackingCleanupThreshold = 100;
-
-// When we clean up navigation tracking, delete this many global_ids.
-const int kNavigationTrackingCleanupAmount = 10;
-
-// Used to record conflict information into histogram Sync.GlobalIdConflict.
-enum SyncGlobalIdConflict {
-  CONFLICT = 0,
-  NO_CONFLICT_NEW_ID,
-  NO_CONFLICT_SAME_IDS,
-  CONFLICT_MAX,
-};
-
 // Comparator function for use with std::sort that will sort tabs by
 // descending timestamp (i.e., most recent first).
 bool TabsRecencyComparator(const sessions::SessionTab* t1,
@@ -651,7 +637,8 @@ void SessionsSyncManager::OnLocalTabModified(SyncedTabDelegate* modified_tab) {
   sessions::SerializedNavigationEntry current;
   modified_tab->GetSerializedNavigationAtIndex(
       modified_tab->GetCurrentEntryIndex(), &current);
-  TrackNavigationIds(current);
+  global_id_mapper_.TrackNavigationIds(current.timestamp(),
+                                       current.unique_id());
 
   if (local_tab_pool_out_of_sync_) {
     // If our tab pool is corrupt, pay the price of a full re-association to
@@ -1334,6 +1321,10 @@ FaviconCache* SessionsSyncManager::GetFaviconCache() {
   return &favicon_cache_;
 }
 
+SessionsGlobalIdMapper* SessionsSyncManager::GetGlobalIdMapper() {
+  return &global_id_mapper_;
+}
+
 SyncedWindowDelegatesGetter*
 SessionsSyncManager::synced_window_delegates_getter() const {
   return sessions_client_->GetSyncedWindowDelegatesGetter();
@@ -1363,22 +1354,6 @@ void SessionsSyncManager::DoGarbageCollection() {
     sync_processor_->ProcessSyncChanges(FROM_HERE, changes);
 }
 
-void SessionsSyncManager::AddGlobalIdChangeObserver(
-    syncer::GlobalIdChange callback) {
-  global_id_change_observers_.push_back(std::move(callback));
-}
-
-int64_t SessionsSyncManager::GetLatestGlobalId(int64_t global_id) {
-  auto g2u_iter = global_to_unique_.find(global_id);
-  if (g2u_iter != global_to_unique_.end()) {
-    auto u2g_iter = unique_to_current_global_.find(g2u_iter->second);
-    if (u2g_iter != unique_to_current_global_.end()) {
-      return u2g_iter->second;
-    }
-  }
-  return global_id;
-}
-
 // static
 std::string SessionsSyncManager::TagHashFromSpecifics(
     const sync_pb::SessionSpecifics& specifics) {
@@ -1386,78 +1361,6 @@ std::string SessionsSyncManager::TagHashFromSpecifics(
                                       TagFromSpecifics(specifics));
 }
 
-void SessionsSyncManager::TrackNavigationIds(
-    const sessions::SerializedNavigationEntry& current) {
-  // The expectation is that global_id will update for a given unique_id, which
-  // should accurately and uniquely represent a single navigation. It is
-  // theoretically possible for two unique_ids to map to the same global_id, but
-  // hopefully rare enough that it doesn't cause much harm. Lets record metrics
-  // verify this theory.
-  int64_t global_id = current.timestamp().ToInternalValue();
-  // It is possible that the global_id has not been set yet for this navigation.
-  // In this case there's nothing here for us to track yet.
-  if (global_id == 0) {
-    return;
-  }
-
-  int unique_id = current.unique_id();
-  DCHECK_NE(0, unique_id);
-
-  auto g2u_iter = global_to_unique_.find(global_id);
-  if (g2u_iter == global_to_unique_.end()) {
-    global_to_unique_.insert(g2u_iter, std::make_pair(global_id, unique_id));
-    UMA_HISTOGRAM_ENUMERATION("Sync.GlobalIdConflict", NO_CONFLICT_NEW_ID,
-                              CONFLICT_MAX);
-  } else if (g2u_iter->second != unique_id) {
-    UMA_HISTOGRAM_ENUMERATION("Sync.GlobalIdConflict", CONFLICT, CONFLICT_MAX);
-  } else {
-    UMA_HISTOGRAM_ENUMERATION("Sync.GlobalIdConflict", NO_CONFLICT_SAME_IDS,
-                              CONFLICT_MAX);
-  }
-
-  auto u2g_iter = unique_to_current_global_.find(unique_id);
-  if (u2g_iter == unique_to_current_global_.end()) {
-    unique_to_current_global_.insert(u2g_iter,
-                                     std::make_pair(unique_id, global_id));
-  } else if (u2g_iter->second != global_id) {
-    // Remember the old_global_id before we insert and invalidate out iter.
-    int64_t old_global_id = u2g_iter->second;
-
-    // TODO(skym): Use insert_or_assign with hint once on C++17.
-    unique_to_current_global_[unique_id] = global_id;
-
-    // This should be done after updating unique_to_current_global_ in case one
-    // of our observers calls into GetLatestGlobalId().
-    for (auto& observer : global_id_change_observers_) {
-      observer.Run(old_global_id, global_id);
-    }
-  }
-
-  CleanupNavigationTracking();
-}
-
-void SessionsSyncManager::CleanupNavigationTracking() {
-  DCHECK(kNavigationTrackingCleanupThreshold >
-         kNavigationTrackingCleanupAmount);
-
-  // |global_to_unique_| is implicitly ordered by least recently created, which
-  //  means we can drop from the beginning.
-  if (global_to_unique_.size() > kNavigationTrackingCleanupThreshold) {
-    auto iter = global_to_unique_.begin();
-    std::advance(iter, kNavigationTrackingCleanupAmount);
-    global_to_unique_.erase(global_to_unique_.begin(), iter);
-
-    // While |unique_id|s do get bigger for the most part, this isn't a great
-    // thing to make assumptions about, and an old tab may get refreshed often
-    // and still be very important. So instead just delete anything that's
-    // orphaned from |global_to_unique_|.
-    base::EraseIf(unique_to_current_global_,
-                  [this](const std::pair<int, int64_t> kv) {
-                    return !base::ContainsKey(global_to_unique_, kv.second);
-                  });
-  }
-}
-
 bool SessionsSyncManager::ScanForTabbedWindow() {
   for (auto& window_iter_pair :
        synced_window_delegates_getter()->GetSyncedWindowDelegates()) {
@@ -1478,4 +1381,4 @@ bool SessionsSyncManager::ScanForTabbedWindow() {
   return false;
 }
 
-};  // namespace sync_sessions
+}  // namespace sync_sessions

components/sync_sessions/sessions_sync_manager.h

@@ -24,12 +24,12 @@
 #include "components/sync/base/sync_prefs.h"
 #include "components/sync/device_info/device_info.h"
 #include "components/sync/model/syncable_service.h"
-#include "components/sync/user_events/global_id_mapper.h"
 #include "components/sync_sessions/favicon_cache.h"
 #include "components/sync_sessions/local_session_event_router.h"
 #include "components/sync_sessions/lost_navigations_recorder.h"
 #include "components/sync_sessions/open_tabs_ui_delegate.h"
 #include "components/sync_sessions/revisit/page_revisit_broadcaster.h"
+#include "components/sync_sessions/sessions_global_id_mapper.h"
 #include "components/sync_sessions/synced_session.h"
 #include "components/sync_sessions/synced_session_tracker.h"
 #include "components/sync_sessions/task_tracker.h"
@@ -53,6 +53,7 @@ class ExtensionSessionsTest;
 
 namespace sync_sessions {
 
+class SessionsGlobalIdMapper;
 class SyncedTabDelegate;
 class SyncedWindowDelegatesGetter;
 
@@ -60,8 +61,7 @@ class SyncedWindowDelegatesGetter;
 // the sync sessions model.
 class SessionsSyncManager : public syncer::SyncableService,
                             public OpenTabsUIDelegate,
-                            public LocalSessionEventHandler,
-                            public syncer::GlobalIdMapper {
+                            public LocalSessionEventHandler {
  public:
   SessionsSyncManager(SyncSessionsClient* sessions_client,
                       syncer::SyncPrefs* sync_prefs,
@@ -120,9 +120,7 @@ class SessionsSyncManager : public syncer::SyncableService,
   // sessions data downloaded (sync cycles complete).
   void DoGarbageCollection();
 
-  // GlobalIdMapper implementation.
-  void AddGlobalIdChangeObserver(syncer::GlobalIdChange callback) override;
-  int64_t GetLatestGlobalId(int64_t global_id) override;
+  SessionsGlobalIdMapper* GetGlobalIdMapper();
 
  private:
   friend class extensions::ExtensionSessionsTest;
@@ -291,10 +289,6 @@ class SessionsSyncManager : public syncer::SyncableService,
 
   SyncedWindowDelegatesGetter* synced_window_delegates_getter() const;
 
-  void TrackNavigationIds(const sessions::SerializedNavigationEntry& current);
-
-  void CleanupNavigationTracking();
-
   // On Android, it's possible to not have any tabbed windows when only custom
   // tabs are currently open. This means that there is tab data that will be
   // restored later, but we cannot access it. This method is an elaborate way to
@@ -306,6 +300,7 @@ class SessionsSyncManager : public syncer::SyncableService,
 
   SyncedSessionTracker session_tracker_;
   FaviconCache favicon_cache_;
+  SessionsGlobalIdMapper global_id_mapper_;
 
   // Tracks whether our local representation of which sync nodes map to what
   // tabs (belonging to the current local session) is inconsistent.  This can
@@ -357,12 +352,6 @@ class SessionsSyncManager : public syncer::SyncableService,
   // changes of current session.
   std::unique_ptr<TaskTracker> task_tracker_;
 
-  // Used to track global_ids that should be used when referencing various
-  // pieces of sessions data, and notify observer when things have changed.
-  std::map<int64_t, int> global_to_unique_;
-  std::map<int, int64_t> unique_to_current_global_;
-  std::vector<syncer::GlobalIdChange> global_id_change_observers_;
-
   DISALLOW_COPY_AND_ASSIGN(SessionsSyncManager);
 };
 

components/sync_sessions/sessions_sync_manager_unittest.cc

@@ -2699,80 +2699,4 @@ TEST_F(SessionsSyncManagerTest, TrackTasksOnLocalTabModified) {
   EXPECT_EQ(tab.navigation(1).ancestor_task_id(0), tab.navigation(0).task_id());
 }
 
-void CaptureGlobalIdChange(int64_t* old_ptr,
-                           int64_t* new_ptr,
-                           int64_t old_id,
-                           int64_t new_id) {
-  *old_ptr = old_id;
-  *new_ptr = new_id;
-}
-
-// Tests that subscribers to AddGlobalIdChangeObserver are notified when a
-// global_id is noticed to have been changed.
-TEST_F(SessionsSyncManagerTest, AddGlobalIdChangeObserver) {
-  TestSyncedWindowDelegate* window = AddWindow();
-  SessionID::id_type window_id = window->GetSessionId();
-  SyncChangeList out;
-  InitWithSyncDataTakeOutput(SyncDataList(), &out);
-
-  int64_t old_id = -1;
-  int64_t new_id = -1;
-  manager()->AddGlobalIdChangeObserver(
-      base::Bind(&CaptureGlobalIdChange, &old_id, &new_id));
-
-  TestSyncedTabDelegate* tab = AddTab(window_id, kFoo1, kTime1);
-  EXPECT_EQ(-1, old_id);
-  EXPECT_EQ(-1, new_id);
-
-  ReloadTab(tab, kTime2);
-  EXPECT_EQ(kTime1.ToInternalValue(), old_id);
-  EXPECT_EQ(kTime2.ToInternalValue(), new_id);
-}
-
-// Tests that GetLatestGlobalId returns correct mappings for updated global_ids.
-TEST_F(SessionsSyncManagerTest, GetLatestGlobalId) {
-  TestSyncedWindowDelegate* window = AddWindow();
-  SessionID::id_type window_id = window->GetSessionId();
-  SyncChangeList out;
-  InitWithSyncDataTakeOutput(SyncDataList(), &out);
-
-  TestSyncedTabDelegate* tab = AddTab(window_id, kFoo1, kTime1);
-  ReloadTab(tab, kTime2);
-  ReloadTab(tab, kTime3);
-
-  EXPECT_EQ(kTime3.ToInternalValue(),
-            manager()->GetLatestGlobalId(kTime1.ToInternalValue()));
-  EXPECT_EQ(kTime3.ToInternalValue(),
-            manager()->GetLatestGlobalId(kTime2.ToInternalValue()));
-  EXPECT_EQ(kTime3.ToInternalValue(),
-            manager()->GetLatestGlobalId(kTime3.ToInternalValue()));
-  // kTime4 is not mapped, so itself should be returned.
-  EXPECT_EQ(kTime4.ToInternalValue(),
-            manager()->GetLatestGlobalId(kTime4.ToInternalValue()));
-}
-
-// Tests that the global_id mapping is eventually dropped after we reach out
-// threshold for the amount to remember.
-TEST_F(SessionsSyncManagerTest, GlobalIdMapperCleanup) {
-  TestSyncedWindowDelegate* window = AddWindow();
-  SessionID::id_type window_id = window->GetSessionId();
-  SyncChangeList out;
-  InitWithSyncDataTakeOutput(SyncDataList(), &out);
-
-  base::Time current_time = kTime1;
-  TestSyncedTabDelegate* tab = AddTab(window_id, kFoo1, current_time);
-
-  for (int i = 0; i < 105; i++) {
-    current_time =
-        base::Time::FromInternalValue(current_time.ToInternalValue() + 1);
-    ReloadTab(tab, current_time);
-  }
-
-  // Threshold is 100, kTime1 should be dropped, kTime1+10 should not.
-  EXPECT_EQ(kTime1.ToInternalValue(),
-            manager()->GetLatestGlobalId(kTime1.ToInternalValue()));
-  EXPECT_EQ(current_time.ToInternalValue(),
-            manager()->GetLatestGlobalId(10 + kTime1.ToInternalValue()));
-}
-
 }  // namespace sync_sessions