[Favicon cache] Remove favicon sync functionality

This CL removes any code in favicon_cache that related to favicon sync
or that is not needed by the only remaining purpose of the class.

This CL does not change behavior w.r.t. the remaining public API.

Removing the class completely is left for follow-ups.

Bug: 978775
Change-Id: Ibb69eb0258a78f056d762bf01b27a94b638a04d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144137
Auto-Submit: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760025}

components/sync_sessions/session_store.cc

@@ -449,10 +449,10 @@ SessionStore::SessionStore(
     if (specifics.session_tag() != local_session_info_.session_tag) {
       UpdateTrackerWithSpecifics(specifics, mtime, &session_tracker_);
 
-      // Notify listeners. In practice, this has the goal to load the URLs and
-      // visit times into the in-memory favicon cache.
+      // Notify listeners. In practice, this has the goal to load the URLs into
+      // the in-memory favicon cache.
       if (specifics.has_tab()) {
-        restored_foreign_tab_callback_.Run(specifics.tab(), mtime);
+        restored_foreign_tab_callback_.Run(specifics.tab());
       }
     } else if (specifics.has_header()) {
       // This is previously stored local header information. Restoring the local

components/sync_sessions/session_store.h

@@ -40,7 +40,7 @@ class SessionStore {
       std::unique_ptr<syncer::MetadataBatch> metadata_batch)>;
   // Mimics signature of FaviconCache::UpdateMappingsFromForeignTab().
   using RestoredForeignTabCallback =
-      base::RepeatingCallback<void(const sync_pb::SessionTab&, base::Time)>;
+      base::RepeatingCallback<void(const sync_pb::SessionTab&)>;
 
   // Opens a SessionStore instance, which involves IO to load previous state
   // from disk. |sessions_client| must not be null and must outlive the

components/sync_sessions/session_store_unittest.cc

@@ -349,7 +349,7 @@ TEST_F(SessionStoreTest, ShouldUpdateTrackerWithForeignData) {
   const int kTabNodeId1 = 2;
   const int kTabNodeId2 = 3;
 
-  EXPECT_CALL(mock_restored_foreign_tab_callback_, Run(_, _)).Times(0);
+  EXPECT_CALL(mock_restored_foreign_tab_callback_, Run(_)).Times(0);
 
   ASSERT_THAT(session_store()->tracker()->LookupAllForeignSessions(
                   SyncedSessionTracker::RAW),
@@ -422,7 +422,7 @@ TEST_F(SessionStoreTest, ShouldWriteAndRestoreForeignData) {
   const int kTabId1 = 7;
   const int kTabNodeId1 = 2;
 
-  EXPECT_CALL(mock_restored_foreign_tab_callback_, Run(_, _)).Times(0);
+  EXPECT_CALL(mock_restored_foreign_tab_callback_, Run(_)).Times(0);
 
   const std::string local_header_storage_key =
       SessionStore::GetHeaderStorageKey(kLocalSessionTag);
@@ -492,7 +492,7 @@ TEST_F(SessionStoreTest, ShouldWriteAndRestoreForeignData) {
   // Creation of a second session store should trigger a callback for the
   // restored tab.
   EXPECT_CALL(mock_restored_foreign_tab_callback_,
-              Run(testing::Property(&sync_pb::SessionTab::tab_id, kTabId1), _));
+              Run(testing::Property(&sync_pb::SessionTab::tab_id, kTabId1)));
 
   // Create second session store to verify that the persisted state is restored,
   // by mimicing a Chrome restart and using |underlying_store_| (in-memory) as a
@@ -539,7 +539,7 @@ TEST_F(SessionStoreTest, ShouldDeleteForeignData) {
   const int kTabNodeId1 = 1;
   const int kTabNodeId2 = 2;
 
-  EXPECT_CALL(mock_restored_foreign_tab_callback_, Run(_, _)).Times(0);
+  EXPECT_CALL(mock_restored_foreign_tab_callback_, Run(_)).Times(0);
 
   const std::string local_header_storage_key =
       SessionStore::GetHeaderStorageKey(kLocalSessionTag);

components/sync_sessions/session_sync_bridge.cc

@@ -38,8 +38,8 @@ using syncer::MetadataChangeList;
 using syncer::ModelTypeStore;
 using syncer::ModelTypeSyncBridge;
 
-// Maximum number of favicons to sync.
-const int kMaxSyncFavicons = 200;
+// Maximum number of favicon mappings to keep in memory.
+const int kMaxFaviconMappings = 200;
 
 // Default time without activity after which a session is considered stale and
 // becomes a candidate for garbage collection.
@@ -111,7 +111,7 @@ SessionSyncBridge::SessionSyncBridge(
           sessions_client->GetLocalSessionEventRouter()),
       favicon_cache_(sessions_client->GetFaviconService(),
                      sessions_client->GetHistoryService(),
-                     kMaxSyncFavicons) {
+                     kMaxFaviconMappings) {
   DCHECK(sessions_client_);
   DCHECK(local_session_event_router_);
 }
@@ -239,11 +239,10 @@ base::Optional<syncer::ModelError> SessionSyncBridge::ApplySyncChanges(
                       syncer::SESSIONS, SessionStore::GetClientTag(specifics)));
 
         batch->PutAndUpdateTracker(specifics, change->data().modification_time);
-        // If a favicon or favicon urls are present, load the URLs and visit
-        // times into the in-memory favicon cache.
+        // If a favicon or favicon urls are present, load the URLs into the
+        // in-memory favicon cache.
         if (specifics.has_tab()) {
-          favicon_cache_.UpdateMappingsFromForeignTab(
-              specifics.tab(), change->data().modification_time);
+          favicon_cache_.UpdateMappingsFromForeignTab(specifics.tab());
         }
         break;
       }
@@ -344,7 +343,7 @@ void SessionSyncBridge::TrackLocalNavigationId(base::Time timestamp,
 }
 
 void SessionSyncBridge::OnPageFaviconUpdated(const GURL& page_url) {
-  favicon_cache_.OnPageFaviconUpdated(page_url, base::Time::Now());
+  favicon_cache_.OnPageFaviconUpdated(page_url);
 }
 
 void SessionSyncBridge::OnFaviconVisited(const GURL& page_url,

components/sync_sessions/session_sync_bridge_unittest.cc

@@ -167,7 +167,7 @@ class SessionSyncBridgeTest : public ::testing::Test {
         session_sync_prefs_(&pref_service_),
         favicon_cache_(/*favicon_service=*/nullptr,
                        /*history_service=*/nullptr,
-                       /*max_sync_favicon_limit=*/0) {
+                       /*max_mappings_limit=*/0) {
     SessionSyncPrefs::RegisterProfilePrefs(pref_service_.registry());
 
     ON_CALL(mock_sync_sessions_client_, GetSessionSyncPrefs())