[Sync] Deprecate favicon data types

This CL deprecates two data types: FAVICON_IMAGES and FAVICON_TRACKING.
The CL also removes all logic related to these data types that can be
removed at the moment. The actual data types need to stay until 2020-12
because of clean-up code.

Bug: 978775
Change-Id: I396ba5f55b97075635167d762677f51e5dc6612e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162847
Auto-Submit: Jan Krcal <jkrcal@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#763325}

components/sync/PRESUBMIT.py

@@ -27,7 +27,9 @@ EXCEPTION_MODEL_TYPES = [
   'SUPERVISED_USER_WHITELISTS',  # See previous.
 
   # Deprecated types:
-  'DEPRECATED_EXPERIMENTS']
+  'DEPRECATED_EXPERIMENTS',
+  'DEPRECATED_FAVICON_IMAGES',
+  'DEPRECATED_FAVICON_TRACKING']
 
 # Root tags are used as prefixes when creating storage keys, so certain strings
 # are blacklisted in order to prevent prefix collision.

components/sync/base/model_type.cc

@@ -113,10 +113,10 @@ const ModelTypeInfo kModelTypeInfoMap[] = {
     {DICTIONARY, "DICTIONARY", "dictionary", "Dictionary",
      sync_pb::EntitySpecifics::kDictionaryFieldNumber,
      ModelTypeForHistograms::kDictionary},
-    {FAVICON_IMAGES, "FAVICON_IMAGE", "favicon_images", "Favicon Images",
-     sync_pb::EntitySpecifics::kFaviconImageFieldNumber,
+    {DEPRECATED_FAVICON_IMAGES, "FAVICON_IMAGE", "favicon_images",
+     "Favicon Images", sync_pb::EntitySpecifics::kFaviconImageFieldNumber,
      ModelTypeForHistograms::kFaviconImages},
-    {FAVICON_TRACKING, "FAVICON_TRACKING", "favicon_tracking",
+    {DEPRECATED_FAVICON_TRACKING, "FAVICON_TRACKING", "favicon_tracking",
      "Favicon Tracking", sync_pb::EntitySpecifics::kFaviconTrackingFieldNumber,
      ModelTypeForHistograms::kFaviconTracking},
     {DEVICE_INFO, "DEVICE_INFO", "device_info", "Device Info",
@@ -254,10 +254,10 @@ void AddDefaultFieldValue(ModelType type, sync_pb::EntitySpecifics* specifics) {
     case DICTIONARY:
       specifics->mutable_dictionary();
       break;
-    case FAVICON_IMAGES:
+    case DEPRECATED_FAVICON_IMAGES:
       specifics->mutable_favicon_image();
       break;
-    case FAVICON_TRACKING:
+    case DEPRECATED_FAVICON_TRACKING:
       specifics->mutable_favicon_tracking();
       break;
     case DEVICE_INFO:
@@ -400,9 +400,9 @@ ModelType GetModelTypeFromSpecifics(const sync_pb::EntitySpecifics& specifics) {
   if (specifics.has_dictionary())
     return DICTIONARY;
   if (specifics.has_favicon_image())
-    return FAVICON_IMAGES;
+    return DEPRECATED_FAVICON_IMAGES;
   if (specifics.has_favicon_tracking())
-    return FAVICON_TRACKING;
+    return DEPRECATED_FAVICON_TRACKING;
   if (specifics.has_device_info())
     return DEVICE_INFO;
   if (specifics.has_priority_preference())

components/sync/base/model_type.h

@@ -97,11 +97,9 @@ enum ModelType {
   // Custom spelling dictionary entries.
   DICTIONARY,
   // Favicon images, including both the image URL and the actual pixels.
-  // TODO(https://crbug.com/978775): Prepend DEPRECATED to the name of favicon
-  // data types.
-  FAVICON_IMAGES,
+  DEPRECATED_FAVICON_IMAGES,
   // Favicon tracking information, i.e. metadata such as last visit date.
-  FAVICON_TRACKING,
+  DEPRECATED_FAVICON_TRACKING,
   // Client-specific metadata, synced before other user types.
   DEVICE_INFO,
   // These preferences are synced before other user types and are never
@@ -255,13 +253,13 @@ constexpr ModelTypeSet ProtocolTypes() {
       BOOKMARKS, PREFERENCES, PASSWORDS, AUTOFILL_PROFILE, AUTOFILL,
       AUTOFILL_WALLET_DATA, AUTOFILL_WALLET_METADATA, THEMES, TYPED_URLS,
       EXTENSIONS, SEARCH_ENGINES, SESSIONS, APPS, APP_SETTINGS,
-      EXTENSION_SETTINGS, HISTORY_DELETE_DIRECTIVES, DICTIONARY, FAVICON_IMAGES,
-      FAVICON_TRACKING, DEVICE_INFO, PRIORITY_PREFERENCES,
-      SUPERVISED_USER_SETTINGS, APP_LIST, SUPERVISED_USER_WHITELISTS,
-      ARC_PACKAGE, PRINTERS, READING_LIST, USER_EVENTS, NIGORI,
-      DEPRECATED_EXPERIMENTS, USER_CONSENTS, SEND_TAB_TO_SELF, SECURITY_EVENTS,
-      WEB_APPS, WIFI_CONFIGURATIONS, OS_PREFERENCES, OS_PRIORITY_PREFERENCES,
-      SHARING_MESSAGE);
+      EXTENSION_SETTINGS, HISTORY_DELETE_DIRECTIVES, DICTIONARY,
+      DEPRECATED_FAVICON_IMAGES, DEPRECATED_FAVICON_TRACKING, DEVICE_INFO,
+      PRIORITY_PREFERENCES, SUPERVISED_USER_SETTINGS, APP_LIST,
+      SUPERVISED_USER_WHITELISTS, ARC_PACKAGE, PRINTERS, READING_LIST,
+      USER_EVENTS, NIGORI, DEPRECATED_EXPERIMENTS, USER_CONSENTS,
+      SEND_TAB_TO_SELF, SECURITY_EVENTS, WEB_APPS, WIFI_CONFIGURATIONS,
+      OS_PREFERENCES, OS_PRIORITY_PREFERENCES, SHARING_MESSAGE);
 }
 
 // These are the normal user-controlled types. This is to distinguish from

components/sync/base/user_selectable_type.cc

@@ -64,8 +64,9 @@ UserSelectableTypeInfo GetUserSelectableTypeInfo(UserSelectableType type) {
     case UserSelectableType::kHistory:
       return {kTypedUrlsTypeName,
               TYPED_URLS,
-              {TYPED_URLS, HISTORY_DELETE_DIRECTIVES, SESSIONS, FAVICON_IMAGES,
-               FAVICON_TRACKING, USER_EVENTS}};
+              {TYPED_URLS, HISTORY_DELETE_DIRECTIVES, SESSIONS,
+               DEPRECATED_FAVICON_IMAGES, DEPRECATED_FAVICON_TRACKING,
+               USER_EVENTS}};
     case UserSelectableType::kExtensions:
       return {
           kExtensionsTypeName, EXTENSIONS, {EXTENSIONS, EXTENSION_SETTINGS}};
@@ -88,8 +89,8 @@ UserSelectableTypeInfo GetUserSelectableTypeInfo(UserSelectableType type) {
     case UserSelectableType::kTabs:
       return {kTabsTypeName,
               PROXY_TABS,
-              {PROXY_TABS, SESSIONS, FAVICON_IMAGES, FAVICON_TRACKING,
-               SEND_TAB_TO_SELF}};
+              {PROXY_TABS, SESSIONS, DEPRECATED_FAVICON_IMAGES,
+               DEPRECATED_FAVICON_TRACKING, SEND_TAB_TO_SELF}};
   }
   NOTREACHED();
   return {nullptr, UNSPECIFIED};

components/sync/driver/glue/sync_engine_impl.cc

@@ -303,8 +303,6 @@ void SyncEngineImpl::FinishConfigureDataTypesOnFrontendLoop(
 #if defined(OS_ANDROID)
     if (!sessions_invalidation_enabled_) {
       invalidation_enabled_types.Remove(syncer::SESSIONS);
-      invalidation_enabled_types.Remove(syncer::FAVICON_IMAGES);
-      invalidation_enabled_types.Remove(syncer::FAVICON_TRACKING);
     }
 #endif
     bool success = invalidator_->UpdateInterestedTopics(
@@ -463,8 +461,6 @@ void SyncEngineImpl::SetInvalidationsForSessionsEnabled(bool enabled) {
       Difference(last_enabled_types_, CommitOnlyTypes()));
   if (!enabled) {
     enabled_for_invalidation.Remove(syncer::SESSIONS);
-    enabled_for_invalidation.Remove(syncer::FAVICON_IMAGES);
-    enabled_for_invalidation.Remove(syncer::FAVICON_TRACKING);
   }
   bool success = invalidator_->UpdateInterestedTopics(
       this, ModelTypeSetToTopicSet(enabled_for_invalidation));

components/sync/driver/glue/sync_engine_impl_unittest.cc

@@ -734,18 +734,14 @@ TEST_F(SyncEngineImplTest,
   // Making sure that the noisy types we're interested in are in the
   // |enabled_types_|.
   enabled_types_.Put(SESSIONS);
-  enabled_types_.Put(FAVICON_IMAGES);
-  enabled_types_.Put(FAVICON_TRACKING);
 
   ModelTypeSet invalidation_enabled_types(
       Difference(enabled_types_, CommitOnlyTypes()));
 
 #if defined(OS_ANDROID)
-  // SESSIONS, FAVICON_IMAGES, FAVICON_TRACKING are noisy data types whose
-  // invalidations aren't enabled by default on Android.
+  // SESSIONS is a noisy data type whose invalidations aren't enabled by default
+  // on Android.
   invalidation_enabled_types.Remove(SESSIONS);
-  invalidation_enabled_types.Remove(FAVICON_IMAGES);
-  invalidation_enabled_types.Remove(FAVICON_TRACKING);
 #endif
 
   InitializeBackend(true);
@@ -764,8 +760,6 @@ TEST_F(SyncEngineImplTest, WhenEnabledTypesStayDisabled) {
   // they're disabled in Sync, hence removing noisy datatypes from
   // |enabled_types_|.
   enabled_types_.Remove(SESSIONS);
-  enabled_types_.Remove(FAVICON_IMAGES);
-  enabled_types_.Remove(FAVICON_TRACKING);
 
   InitializeBackend(true);
   EXPECT_CALL(invalidator_,
@@ -783,8 +777,6 @@ TEST_F(SyncEngineImplTest,
   // Making sure that the noisy types we're interested in are in the
   // |enabled_types_|.
   enabled_types_.Put(SESSIONS);
-  enabled_types_.Put(FAVICON_IMAGES);
-  enabled_types_.Put(FAVICON_TRACKING);
 
   InitializeBackend(true);
   ConfigureDataTypes();
@@ -797,8 +789,6 @@ TEST_F(SyncEngineImplTest,
 
   ModelTypeSet enabled_types(enabled_types_);
   enabled_types.Remove(SESSIONS);
-  enabled_types.Remove(FAVICON_IMAGES);
-  enabled_types.Remove(FAVICON_TRACKING);
 
   EXPECT_CALL(invalidator_,
               UpdateInterestedTopics(backend_.get(),

components/sync/driver/model_association_manager.cc

@@ -45,9 +45,10 @@ static const ModelType kStartOrder[] = {
     // UI thread data types.
     BOOKMARKS, PREFERENCES, PRIORITY_PREFERENCES, EXTENSIONS, APPS, APP_LIST,
     ARC_PACKAGE, READING_LIST, THEMES, SEARCH_ENGINES, SESSIONS, DICTIONARY,
-    FAVICON_IMAGES, FAVICON_TRACKING, PRINTERS, USER_CONSENTS, USER_EVENTS,
-    SHARING_MESSAGE, SUPERVISED_USER_SETTINGS, SUPERVISED_USER_WHITELISTS,
-    SEND_TAB_TO_SELF, SECURITY_EVENTS, WEB_APPS, WIFI_CONFIGURATIONS};
+    DEPRECATED_FAVICON_IMAGES, DEPRECATED_FAVICON_TRACKING, PRINTERS,
+    USER_CONSENTS, USER_EVENTS, SHARING_MESSAGE, SUPERVISED_USER_SETTINGS,
+    SUPERVISED_USER_WHITELISTS, SEND_TAB_TO_SELF, SECURITY_EVENTS, WEB_APPS,
+    WIFI_CONFIGURATIONS};
 
 static_assert(base::size(kStartOrder) ==
                   ModelType::NUM_ENTRIES - FIRST_REAL_MODEL_TYPE,

components/sync/engine_impl/cycle/nudge_tracker.cc

@@ -41,8 +41,6 @@ base::TimeDelta GetDefaultDelayForType(ModelType model_type,
     case BOOKMARKS:
     case PREFERENCES:
     case SESSIONS:
-    case FAVICON_IMAGES:
-    case FAVICON_TRACKING:
       // Types with sometimes automatic changes get longer delays to allow more
       // coalescing.
       return base::TimeDelta::FromMilliseconds(kSlowNudgeDelayMilliseconds);

components/sync/engine_impl/cycle/nudge_tracker_unittest.cc

@@ -826,10 +826,6 @@ TEST_F(NudgeTrackerTest, NudgeDelayTest) {
   EXPECT_EQ(nudge_tracker_.RecordLocalChange(ModelTypeSet(SESSIONS)),
             nudge_tracker_.RecordLocalChange(ModelTypeSet(BOOKMARKS)));
 
-  // Favicons have the same delay as sessions.
-  EXPECT_EQ(nudge_tracker_.RecordLocalChange(ModelTypeSet(SESSIONS)),
-            nudge_tracker_.RecordLocalChange(ModelTypeSet(FAVICON_TRACKING)));
-
   // Autofill has the longer delay of all.
   EXPECT_GT(nudge_tracker_.RecordLocalChange(ModelTypeSet(AUTOFILL)),
             nudge_tracker_.RecordLocalChange(ModelTypeSet(SESSIONS)));

components/sync/engine_impl/syncer_proto_util.cc

@@ -472,10 +472,6 @@ SyncerError SyncerProtoUtil::PostClientToServerMessage(
       std::map<ModelType, base::TimeDelta> delay_map;
       delay_map[SESSIONS] =
           base::TimeDelta::FromSeconds(command.sessions_commit_delay_seconds());
-      delay_map[FAVICON_TRACKING] =
-          base::TimeDelta::FromSeconds(command.sessions_commit_delay_seconds());
-      delay_map[FAVICON_IMAGES] =
-          base::TimeDelta::FromSeconds(command.sessions_commit_delay_seconds());
       cycle->delegate()->OnReceivedCustomNudgeDelays(delay_map);
     }
 

components/sync/model_impl/model_type_store_service_impl.cc

@@ -40,9 +40,10 @@ void InitOnBackendSequence(const base::FilePath& level_db_path,
     return;
   }
 
-  // TODO(crbug.com/978775): Remove the cleanup logic after a year.
+  // TODO(crbug.com/978775): Remove the cleanup logic after a year (in 2020-12).
   // Clean up local data from deprecated datatypes.
-  for (ModelType type : {FAVICON_IMAGES, FAVICON_TRACKING}) {
+  for (ModelType type :
+       {DEPRECATED_FAVICON_IMAGES, DEPRECATED_FAVICON_TRACKING}) {
     BlockingModelTypeStoreImpl(type, store_backend).DeleteAllDataAndMetadata();
   }
 }

components/sync/nigori/nigori_state.cc

@@ -80,9 +80,10 @@ void UpdateNigoriSpecificsFromEncryptedTypes(
   specifics->set_encrypt_extension_settings(
       encrypted_types.Has(EXTENSION_SETTINGS));
   specifics->set_encrypt_dictionary(encrypted_types.Has(DICTIONARY));
-  specifics->set_encrypt_favicon_images(encrypted_types.Has(FAVICON_IMAGES));
+  specifics->set_encrypt_favicon_images(
+      encrypted_types.Has(DEPRECATED_FAVICON_IMAGES));
   specifics->set_encrypt_favicon_tracking(
-      encrypted_types.Has(FAVICON_TRACKING));
+      encrypted_types.Has(DEPRECATED_FAVICON_TRACKING));
   specifics->set_encrypt_app_list(encrypted_types.Has(APP_LIST));
   specifics->set_encrypt_arc_package(encrypted_types.Has(ARC_PACKAGE));
   specifics->set_encrypt_printers(encrypted_types.Has(PRINTERS));

components/sync/syncable/nigori_util.cc

@@ -283,8 +283,10 @@ void UpdateNigoriFromEncryptedTypes(ModelTypeSet encrypted_types,
   nigori->set_encrypt_extension_settings(
       encrypted_types.Has(EXTENSION_SETTINGS));
   nigori->set_encrypt_dictionary(encrypted_types.Has(DICTIONARY));
-  nigori->set_encrypt_favicon_images(encrypted_types.Has(FAVICON_IMAGES));
-  nigori->set_encrypt_favicon_tracking(encrypted_types.Has(FAVICON_TRACKING));
+  nigori->set_encrypt_favicon_images(
+      encrypted_types.Has(DEPRECATED_FAVICON_IMAGES));
+  nigori->set_encrypt_favicon_tracking(
+      encrypted_types.Has(DEPRECATED_FAVICON_TRACKING));
   nigori->set_encrypt_app_list(encrypted_types.Has(APP_LIST));
   nigori->set_encrypt_arc_package(encrypted_types.Has(ARC_PACKAGE));
   nigori->set_encrypt_printers(encrypted_types.Has(PRINTERS));
@@ -331,9 +333,9 @@ ModelTypeSet GetEncryptedTypesFromNigori(
   if (nigori.encrypt_dictionary())
     encrypted_types.Put(DICTIONARY);
   if (nigori.encrypt_favicon_images())
-    encrypted_types.Put(FAVICON_IMAGES);
+    encrypted_types.Put(DEPRECATED_FAVICON_IMAGES);
   if (nigori.encrypt_favicon_tracking())
-    encrypted_types.Put(FAVICON_TRACKING);
+    encrypted_types.Put(DEPRECATED_FAVICON_TRACKING);
   if (nigori.encrypt_app_list())
     encrypted_types.Put(APP_LIST);
   if (nigori.encrypt_arc_package())