Commit 39804b20 authored by Marc Treib's avatar Marc Treib Committed by Commit Bot

Revert "Sync cleanup: Remove DEPRECATED_EXPERIMENTS ModelType"

This reverts commit df3250dd.

Reason for revert: Suspected of causing crashes, see crbug.com/1047879

Original change's description:
> Sync cleanup: Remove DEPRECATED_EXPERIMENTS ModelType
> 
> It has been deprecated and unused for a while.
> The specifics proto itself can't be removed yet because it's still used
> on the server.
> 
> Bug: 1009361
> Change-Id: Ic183a6e244435cfce60ccdda34bff3e22a032ff3
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2022670
> Commit-Queue: Marc Treib <treib@chromium.org>
> Reviewed-by: Jan Krcal <jkrcal@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#736397}

TBR=treib@chromium.org,jkrcal@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1009361, 1047879
Change-Id: I762fbf6f1b4b752a47b34e6a101a066d0c78aebc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2036005Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738205}
parent 7d849ef7
...@@ -55,7 +55,7 @@ class ProfileSyncServiceFactoryTest : public testing::Test { ...@@ -55,7 +55,7 @@ class ProfileSyncServiceFactoryTest : public testing::Test {
// Returns the collection of default datatypes. // Returns the collection of default datatypes.
std::vector<syncer::ModelType> DefaultDatatypes() { std::vector<syncer::ModelType> DefaultDatatypes() {
static_assert(40 == syncer::ModelType::NUM_ENTRIES, static_assert(41 == syncer::ModelType::NUM_ENTRIES,
"When adding a new type, you probably want to add it here as " "When adding a new type, you probably want to add it here as "
"well (assuming it is already enabled)."); "well (assuming it is already enabled).");
......
...@@ -24,8 +24,10 @@ EXCEPTION_MODEL_TYPES = [ ...@@ -24,8 +24,10 @@ EXCEPTION_MODEL_TYPES = [
'NIGORI', # Model type string is 'encryption keys'. 'NIGORI', # Model type string is 'encryption keys'.
'SUPERVISED_USER_SETTINGS', # Root tag and model type string replace 'SUPERVISED_USER_SETTINGS', # Root tag and model type string replace
# 'Supervised' with 'Managed' # 'Supervised' with 'Managed'
'SUPERVISED_USER_WHITELISTS' # See previous. 'SUPERVISED_USER_WHITELISTS', # See previous.
]
# Deprecated types:
'DEPRECATED_EXPERIMENTS']
# Root tags are used as prefixes when creating storage keys, so certain strings # Root tags are used as prefixes when creating storage keys, so certain strings
# are blacklisted in order to prevent prefix collision. # are blacklisted in order to prevent prefix collision.
......
...@@ -112,6 +112,12 @@ class ModelTypeInfoChangeTest(unittest.TestCase): ...@@ -112,6 +112,12 @@ class ModelTypeInfoChangeTest(unittest.TestCase):
results = self._testChange('{PROXY_TABS, "", "", "Tabs", -1, 25},') results = self._testChange('{PROXY_TABS, "", "", "Tabs", -1, 25},')
self.assertEqual(0, len(results)) self.assertEqual(0, len(results))
def testValidChangeDeprecatedEntry(self):
results = self._testChange('{DEPRECATED_EXPERIMENTS, "EXPERIMENTS",'
'"experiments", "Experiments",'
'sync_pb::EntitySpecifics::kExperimentsFieldNumber, 19},')
self.assertEqual(0, len(results))
def testInvalidChangeMismatchedNotificationType(self): def testInvalidChangeMismatchedNotificationType(self):
results = self._testChange('{AUTOFILL, "AUTOFILL_WRONG", "autofill",\n' results = self._testChange('{AUTOFILL, "AUTOFILL_WRONG", "autofill",\n'
'"Autofill",sync_pb::EntitySpecifics::kAutofillFieldNumber, 6},') '"Autofill",sync_pb::EntitySpecifics::kAutofillFieldNumber, 6},')
......
...@@ -181,16 +181,19 @@ const ModelTypeInfo kModelTypeInfoMap[] = { ...@@ -181,16 +181,19 @@ const ModelTypeInfo kModelTypeInfoMap[] = {
{NIGORI, "NIGORI", "nigori", "Encryption Keys", {NIGORI, "NIGORI", "nigori", "Encryption Keys",
sync_pb::EntitySpecifics::kNigoriFieldNumber, sync_pb::EntitySpecifics::kNigoriFieldNumber,
ModelTypeForHistograms::kNigori}, ModelTypeForHistograms::kNigori},
{DEPRECATED_EXPERIMENTS, "EXPERIMENTS", "experiments", "Experiments",
sync_pb::EntitySpecifics::kExperimentsFieldNumber,
ModelTypeForHistograms::kDeprecatedExperiments},
}; };
static_assert(base::size(kModelTypeInfoMap) == ModelType::NUM_ENTRIES, static_assert(base::size(kModelTypeInfoMap) == ModelType::NUM_ENTRIES,
"kModelTypeInfoMap should have ModelType::NUM_ENTRIES elements"); "kModelTypeInfoMap should have ModelType::NUM_ENTRIES elements");
static_assert(40 == syncer::ModelType::NUM_ENTRIES, static_assert(41 == syncer::ModelType::NUM_ENTRIES,
"When adding a new type, update enum SyncModelTypes in enums.xml " "When adding a new type, update enum SyncModelTypes in enums.xml "
"and suffix SyncModelType in histograms.xml."); "and suffix SyncModelType in histograms.xml.");
static_assert(40 == syncer::ModelType::NUM_ENTRIES, static_assert(41 == syncer::ModelType::NUM_ENTRIES,
"When adding a new type, update kAllocatorDumpNameWhitelist in " "When adding a new type, update kAllocatorDumpNameWhitelist in "
"base/trace_event/memory_infra_background_whitelist.cc."); "base/trace_event/memory_infra_background_whitelist.cc.");
...@@ -299,6 +302,9 @@ void AddDefaultFieldValue(ModelType type, sync_pb::EntitySpecifics* specifics) { ...@@ -299,6 +302,9 @@ void AddDefaultFieldValue(ModelType type, sync_pb::EntitySpecifics* specifics) {
case NIGORI: case NIGORI:
specifics->mutable_nigori(); specifics->mutable_nigori();
break; break;
case DEPRECATED_EXPERIMENTS:
specifics->mutable_experiments();
break;
case WEB_APPS: case WEB_APPS:
specifics->mutable_web_app(); specifics->mutable_web_app();
break; break;
...@@ -356,7 +362,7 @@ ModelType GetModelType(const sync_pb::SyncEntity& sync_entity) { ...@@ -356,7 +362,7 @@ ModelType GetModelType(const sync_pb::SyncEntity& sync_entity) {
} }
ModelType GetModelTypeFromSpecifics(const sync_pb::EntitySpecifics& specifics) { ModelType GetModelTypeFromSpecifics(const sync_pb::EntitySpecifics& specifics) {
static_assert(40 == ModelType::NUM_ENTRIES, static_assert(41 == ModelType::NUM_ENTRIES,
"When adding new protocol types, the following type lookup " "When adding new protocol types, the following type lookup "
"logic must be updated."); "logic must be updated.");
if (specifics.has_bookmark()) if (specifics.has_bookmark())
...@@ -419,6 +425,8 @@ ModelType GetModelTypeFromSpecifics(const sync_pb::EntitySpecifics& specifics) { ...@@ -419,6 +425,8 @@ ModelType GetModelTypeFromSpecifics(const sync_pb::EntitySpecifics& specifics) {
return USER_CONSENTS; return USER_CONSENTS;
if (specifics.has_nigori()) if (specifics.has_nigori())
return NIGORI; return NIGORI;
if (specifics.has_experiments())
return DEPRECATED_EXPERIMENTS;
if (specifics.has_send_tab_to_self()) if (specifics.has_send_tab_to_self())
return SEND_TAB_TO_SELF; return SEND_TAB_TO_SELF;
if (specifics.has_security_event()) if (specifics.has_security_event())
...@@ -438,7 +446,7 @@ ModelType GetModelTypeFromSpecifics(const sync_pb::EntitySpecifics& specifics) { ...@@ -438,7 +446,7 @@ ModelType GetModelTypeFromSpecifics(const sync_pb::EntitySpecifics& specifics) {
} }
ModelTypeSet EncryptableUserTypes() { ModelTypeSet EncryptableUserTypes() {
static_assert(40 == ModelType::NUM_ENTRIES, static_assert(41 == ModelType::NUM_ENTRIES,
"If adding an unencryptable type, remove from " "If adding an unencryptable type, remove from "
"encryptable_user_types below."); "encryptable_user_types below.");
ModelTypeSet encryptable_user_types = UserTypes(); ModelTypeSet encryptable_user_types = UserTypes();
......
...@@ -154,7 +154,8 @@ enum ModelType { ...@@ -154,7 +154,8 @@ enum ModelType {
// ---- Control Types ---- // ---- Control Types ----
// An object representing a set of Nigori keys. // An object representing a set of Nigori keys.
NIGORI, NIGORI,
LAST_REAL_MODEL_TYPE = NIGORI, DEPRECATED_EXPERIMENTS,
LAST_REAL_MODEL_TYPE = DEPRECATED_EXPERIMENTS,
NUM_ENTRIES, NUM_ENTRIES,
}; };
...@@ -199,7 +200,7 @@ enum class ModelTypeForHistograms { ...@@ -199,7 +200,7 @@ enum class ModelTypeForHistograms {
kHistoryDeleteDirectices = 16, kHistoryDeleteDirectices = 16,
kNigori = 17, kNigori = 17,
kDeviceInfo = 18, kDeviceInfo = 18,
// kDeprecatedExperiments = 19, kDeprecatedExperiments = 19,
// kDeprecatedSyncedNotifications = 20, // kDeprecatedSyncedNotifications = 20,
kPriorityPreferences = 21, kPriorityPreferences = 21,
kDictionary = 22, kDictionary = 22,
...@@ -257,9 +258,10 @@ constexpr ModelTypeSet ProtocolTypes() { ...@@ -257,9 +258,10 @@ constexpr ModelTypeSet ProtocolTypes() {
EXTENSION_SETTINGS, HISTORY_DELETE_DIRECTIVES, DICTIONARY, FAVICON_IMAGES, EXTENSION_SETTINGS, HISTORY_DELETE_DIRECTIVES, DICTIONARY, FAVICON_IMAGES,
FAVICON_TRACKING, DEVICE_INFO, PRIORITY_PREFERENCES, FAVICON_TRACKING, DEVICE_INFO, PRIORITY_PREFERENCES,
SUPERVISED_USER_SETTINGS, APP_LIST, SUPERVISED_USER_WHITELISTS, SUPERVISED_USER_SETTINGS, APP_LIST, SUPERVISED_USER_WHITELISTS,
ARC_PACKAGE, PRINTERS, READING_LIST, USER_EVENTS, NIGORI, USER_CONSENTS, ARC_PACKAGE, PRINTERS, READING_LIST, USER_EVENTS, NIGORI,
SEND_TAB_TO_SELF, SECURITY_EVENTS, WEB_APPS, WIFI_CONFIGURATIONS, DEPRECATED_EXPERIMENTS, USER_CONSENTS, SEND_TAB_TO_SELF, SECURITY_EVENTS,
OS_PREFERENCES, OS_PRIORITY_PREFERENCES, SHARING_MESSAGE); WEB_APPS, WIFI_CONFIGURATIONS, OS_PREFERENCES, OS_PRIORITY_PREFERENCES,
SHARING_MESSAGE);
} }
// These are the normal user-controlled types. This is to distinguish from // These are the normal user-controlled types. This is to distinguish from
......
...@@ -29,6 +29,7 @@ namespace { ...@@ -29,6 +29,7 @@ namespace {
static const ModelType kStartOrder[] = { static const ModelType kStartOrder[] = {
NIGORI, // Listed for completeness. NIGORI, // Listed for completeness.
DEVICE_INFO, DEVICE_INFO,
DEPRECATED_EXPERIMENTS, // Listed for completeness.
PROXY_TABS, // Listed for completeness. PROXY_TABS, // Listed for completeness.
// Kick off the association of the non-UI types first so they can associate // Kick off the association of the non-UI types first so they can associate
......
...@@ -1461,7 +1461,7 @@ ModelTypeSet ProfileSyncService::GetModelTypesForTransportOnlyMode() const { ...@@ -1461,7 +1461,7 @@ ModelTypeSet ProfileSyncService::GetModelTypesForTransportOnlyMode() const {
} }
// Outside the #if so non-Chrome OS developers will hit it before uploading. // Outside the #if so non-Chrome OS developers will hit it before uploading.
static_assert(40 == ModelType::NUM_ENTRIES, static_assert(41 == ModelType::NUM_ENTRIES,
"If a new ModelType is Chrome OS-only and uses OS sync " "If a new ModelType is Chrome OS-only and uses OS sync "
"consent, add it below."); "consent, add it below.");
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
......
...@@ -241,7 +241,7 @@ ModelTypeSet SyncUserSettingsImpl::GetPreferredDataTypes() const { ...@@ -241,7 +241,7 @@ ModelTypeSet SyncUserSettingsImpl::GetPreferredDataTypes() const {
#endif #endif
types.RetainAll(registered_model_types_); types.RetainAll(registered_model_types_);
static_assert(40 == ModelType::NUM_ENTRIES, static_assert(41 == ModelType::NUM_ENTRIES,
"If adding a new sync data type, update the list below below if" "If adding a new sync data type, update the list below below if"
" you want to disable the new data type for local sync."); " you want to disable the new data type for local sync.");
types.PutAll(ControlTypes()); types.PutAll(ControlTypes());
......
...@@ -61,7 +61,7 @@ bool EncryptKeyBag(const CryptographerImpl& cryptographer, ...@@ -61,7 +61,7 @@ bool EncryptKeyBag(const CryptographerImpl& cryptographer,
void UpdateNigoriSpecificsFromEncryptedTypes( void UpdateNigoriSpecificsFromEncryptedTypes(
ModelTypeSet encrypted_types, ModelTypeSet encrypted_types,
sync_pb::NigoriSpecifics* specifics) { sync_pb::NigoriSpecifics* specifics) {
static_assert(40 == ModelType::NUM_ENTRIES, static_assert(41 == ModelType::NUM_ENTRIES,
"If adding an encryptable type, update handling below."); "If adding an encryptable type, update handling below.");
specifics->set_encrypt_bookmarks(encrypted_types.Has(BOOKMARKS)); specifics->set_encrypt_bookmarks(encrypted_types.Has(BOOKMARKS));
specifics->set_encrypt_preferences(encrypted_types.Has(PREFERENCES)); specifics->set_encrypt_preferences(encrypted_types.Has(PREFERENCES));
......
...@@ -59,7 +59,7 @@ namespace { ...@@ -59,7 +59,7 @@ namespace {
DEFINE_SPECIFICS_TO_VALUE_TEST(encrypted) DEFINE_SPECIFICS_TO_VALUE_TEST(encrypted)
static_assert(40 == syncer::ModelType::NUM_ENTRIES, static_assert(41 == syncer::ModelType::NUM_ENTRIES,
"When adding a new field, add a DEFINE_SPECIFICS_TO_VALUE_TEST " "When adding a new field, add a DEFINE_SPECIFICS_TO_VALUE_TEST "
"for your field below, and optionally a test for the specific " "for your field below, and optionally a test for the specific "
"conversions."); "conversions.");
......
...@@ -370,7 +370,7 @@ VISIT_PROTO_FIELDS(const sync_pb::EntityMetadata& proto) { ...@@ -370,7 +370,7 @@ VISIT_PROTO_FIELDS(const sync_pb::EntityMetadata& proto) {
} }
VISIT_PROTO_FIELDS(const sync_pb::EntitySpecifics& proto) { VISIT_PROTO_FIELDS(const sync_pb::EntitySpecifics& proto) {
static_assert(40 == ModelType::NUM_ENTRIES, static_assert(41 == ModelType::NUM_ENTRIES,
"When adding a new protocol type, you will likely need to add " "When adding a new protocol type, you will likely need to add "
"it here as well."); "it here as well.");
VISIT(encrypted); VISIT(encrypted);
......
...@@ -265,7 +265,7 @@ void UpdateNigoriFromEncryptedTypes(ModelTypeSet encrypted_types, ...@@ -265,7 +265,7 @@ void UpdateNigoriFromEncryptedTypes(ModelTypeSet encrypted_types,
bool encrypt_everything, bool encrypt_everything,
sync_pb::NigoriSpecifics* nigori) { sync_pb::NigoriSpecifics* nigori) {
nigori->set_encrypt_everything(encrypt_everything); nigori->set_encrypt_everything(encrypt_everything);
static_assert(40 == ModelType::NUM_ENTRIES, static_assert(41 == ModelType::NUM_ENTRIES,
"If adding an encryptable type, update handling below."); "If adding an encryptable type, update handling below.");
nigori->set_encrypt_bookmarks(encrypted_types.Has(BOOKMARKS)); nigori->set_encrypt_bookmarks(encrypted_types.Has(BOOKMARKS));
nigori->set_encrypt_preferences(encrypted_types.Has(PREFERENCES)); nigori->set_encrypt_preferences(encrypted_types.Has(PREFERENCES));
...@@ -300,7 +300,7 @@ ModelTypeSet GetEncryptedTypesFromNigori( ...@@ -300,7 +300,7 @@ ModelTypeSet GetEncryptedTypesFromNigori(
return ModelTypeSet::All(); return ModelTypeSet::All();
ModelTypeSet encrypted_types; ModelTypeSet encrypted_types;
static_assert(40 == ModelType::NUM_ENTRIES, static_assert(41 == ModelType::NUM_ENTRIES,
"If adding an encryptable type, update handling below."); "If adding an encryptable type, update handling below.");
if (nigori.encrypt_bookmarks()) if (nigori.encrypt_bookmarks())
encrypted_types.Put(BOOKMARKS); encrypted_types.Put(BOOKMARKS);
......
...@@ -43,7 +43,7 @@ class ProfileSyncServiceFactoryTest : public PlatformTest { ...@@ -43,7 +43,7 @@ class ProfileSyncServiceFactoryTest : public PlatformTest {
protected: protected:
// Returns the collection of default datatypes. // Returns the collection of default datatypes.
std::vector<syncer::ModelType> DefaultDatatypes() { std::vector<syncer::ModelType> DefaultDatatypes() {
static_assert(40 == syncer::ModelType::NUM_ENTRIES, static_assert(41 == syncer::ModelType::NUM_ENTRIES,
"When adding a new type, you probably want to add it here as " "When adding a new type, you probably want to add it here as "
"well (assuming it is already enabled)."); "well (assuming it is already enabled).");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment