diff options
Diffstat (limited to 'chromium/components/sync_preferences')
4 files changed, 46 insertions, 66 deletions
diff --git a/chromium/components/sync_preferences/pref_model_associator.cc b/chromium/components/sync_preferences/pref_model_associator.cc index 25cf5e0b4fd..e6ae5ba302a 100644 --- a/chromium/components/sync_preferences/pref_model_associator.cc +++ b/chromium/components/sync_preferences/pref_model_associator.cc @@ -114,14 +114,15 @@ void PrefModelAssociator::InitPrefAndAssociate( if (sync_pref.IsValid()) { const sync_pb::PreferenceSpecifics& preference = GetSpecifics(sync_pref); DCHECK(pref_name == preference.name()); - base::JSONReader reader; - std::unique_ptr<base::Value> sync_value( - reader.ReadToValueDeprecated(preference.value())); - if (!sync_value.get()) { + base::JSONReader::ValueWithError parsed_json = + base::JSONReader::ReadAndReturnValueWithError(preference.value()); + if (!parsed_json.value) { LOG(ERROR) << "Failed to deserialize value of preference '" << pref_name - << "': " << reader.GetErrorMessage(); + << "': " << parsed_json.error_message; return; } + std::unique_ptr<base::Value> sync_value = + base::Value::ToUniquePtrValue(std::move(*parsed_json.value)); if (user_pref_value) { DVLOG(1) << "Found user pref value for " << pref_name; @@ -408,9 +409,9 @@ base::Optional<syncer::ModelError> PrefModelAssociator::ProcessSyncChanges( continue; } - std::unique_ptr<base::Value> new_value( + base::Optional<base::Value> new_value( ReadPreferenceSpecifics(pref_specifics)); - if (!new_value.get()) { + if (!new_value) { // Skip values we can't deserialize. // TODO(zea): consider taking some further action such as erasing the // bad data. @@ -440,18 +441,17 @@ base::Optional<syncer::ModelError> PrefModelAssociator::ProcessSyncChanges( } // static -base::Value* PrefModelAssociator::ReadPreferenceSpecifics( +base::Optional<base::Value> PrefModelAssociator::ReadPreferenceSpecifics( const sync_pb::PreferenceSpecifics& preference) { - base::JSONReader reader; - std::unique_ptr<base::Value> value( - reader.ReadToValueDeprecated(preference.value())); - if (!value.get()) { + base::JSONReader::ValueWithError parsed_json = + base::JSONReader::ReadAndReturnValueWithError(preference.value()); + if (!parsed_json.value) { std::string err = - "Failed to deserialize preference value: " + reader.GetErrorMessage(); + "Failed to deserialize preference value: " + parsed_json.error_message; LOG(ERROR) << err; - return nullptr; + return base::nullopt; } - return value.release(); + return std::move(parsed_json.value); } void PrefModelAssociator::AddSyncedPrefObserver(const std::string& name, @@ -603,7 +603,6 @@ bool PrefModelAssociator::TypeMatchesUserPrefStore( if (!local_value || local_value->type() == new_value.type()) return true; - UMA_HISTOGRAM_BOOLEAN("Sync.Preferences.RemotePrefTypeMismatch", true); DLOG(WARNING) << "Unexpected type mis-match for pref. " << "Synced value for " << pref_name << " is of type " << new_value.type() << " which doesn't match the locally " diff --git a/chromium/components/sync_preferences/pref_model_associator.h b/chromium/components/sync_preferences/pref_model_associator.h index e56d48e2706..648f33b1680 100644 --- a/chromium/components/sync_preferences/pref_model_associator.h +++ b/chromium/components/sync_preferences/pref_model_associator.h @@ -148,7 +148,7 @@ class PrefModelAssociator : public syncer::SyncableService { const base::Value& to_value); // Extract preference value from sync specifics. - static base::Value* ReadPreferenceSpecifics( + static base::Optional<base::Value> ReadPreferenceSpecifics( const sync_pb::PreferenceSpecifics& specifics); void NotifySyncedPrefObservers(const std::string& path, bool from_sync) const; diff --git a/chromium/components/sync_preferences/pref_service_syncable.cc b/chromium/components/sync_preferences/pref_service_syncable.cc index a9a11211fc0..8685a265f63 100644 --- a/chromium/components/sync_preferences/pref_service_syncable.cc +++ b/chromium/components/sync_preferences/pref_service_syncable.cc @@ -10,7 +10,6 @@ #include "base/bind_helpers.h" #include "base/callback.h" #include "base/strings/string_number_conversions.h" -#include "base/value_conversions.h" #include "components/pref_registry/pref_registry_syncable.h" #include "components/prefs/default_pref_store.h" #include "components/prefs/in_memory_pref_store.h" diff --git a/chromium/components/sync_preferences/pref_service_syncable_unittest.cc b/chromium/components/sync_preferences/pref_service_syncable_unittest.cc index 9d83153adef..24574cabbca 100644 --- a/chromium/components/sync_preferences/pref_service_syncable_unittest.cc +++ b/chromium/components/sync_preferences/pref_service_syncable_unittest.cc @@ -125,8 +125,7 @@ class TestSyncedPrefObserver : public SyncedPrefObserver { int changed_count_ = 0; }; -syncer::SyncChange MakeRemoteChange(int64_t id, - const std::string& name, +syncer::SyncChange MakeRemoteChange(const std::string& name, const base::Value& value, SyncChange::SyncChangeType change_type, syncer::ModelType model_type) { @@ -140,21 +139,18 @@ syncer::SyncChange MakeRemoteChange(int64_t id, pref->set_name(name); pref->set_value(serialized); return syncer::SyncChange(FROM_HERE, change_type, - syncer::SyncData::CreateRemoteData(id, entity)); + syncer::SyncData::CreateRemoteData(entity)); } // Creates a SyncChange for model type |PREFERENCES|. -syncer::SyncChange MakeRemoteChange(int64_t id, - const std::string& name, +syncer::SyncChange MakeRemoteChange(const std::string& name, const base::Value& value, SyncChange::SyncChangeType type) { - return MakeRemoteChange(id, name, value, type, - syncer::ModelType::PREFERENCES); + return MakeRemoteChange(name, value, type, syncer::ModelType::PREFERENCES); } // Creates SyncData for a remote pref change. -SyncData CreateRemoteSyncData(int64_t id, - const std::string& name, +SyncData CreateRemoteSyncData(const std::string& name, const base::Value& value) { std::string serialized; JSONStringValueSerializer json(&serialized); @@ -163,13 +159,12 @@ SyncData CreateRemoteSyncData(int64_t id, sync_pb::PreferenceSpecifics* pref_one = one.mutable_preference(); pref_one->set_name(name); pref_one->set_value(serialized); - return SyncData::CreateRemoteData(id, one); + return SyncData::CreateRemoteData(one); } class PrefServiceSyncableTest : public testing::Test { public: - PrefServiceSyncableTest() - : pref_sync_service_(nullptr), next_pref_remote_sync_node_id_(0) {} + PrefServiceSyncableTest() : pref_sync_service_(nullptr) {} void SetUp() override { prefs_.registry()->RegisterStringPref(kUnsyncedPreferenceName, @@ -191,8 +186,7 @@ class PrefServiceSyncableTest : public testing::Test { void AddToRemoteDataList(const std::string& name, const base::Value& value, syncer::SyncDataList* out) { - out->push_back( - CreateRemoteSyncData(++next_pref_remote_sync_node_id_, name, value)); + out->push_back(CreateRemoteSyncData(name, value)); } void InitWithSyncDataTakeOutput(const syncer::SyncDataList& initial_data, @@ -238,8 +232,6 @@ class PrefServiceSyncableTest : public testing::Test { TestingPrefServiceSyncable prefs_; PrefModelAssociator* pref_sync_service_; - - int next_pref_remote_sync_node_id_; }; TEST_F(PrefServiceSyncableTest, CreatePrefSyncData) { @@ -325,7 +317,6 @@ TEST_F(PrefServiceSyncableTest, ModelAssociationCloudHasData) { // Verifies that the implementation gracefully handles an initial remote sync // data of wrong type. The local version should not get modified in these cases. TEST_F(PrefServiceSyncableTest, ModelAssociationWithDataTypeMismatch) { - base::HistogramTester histogram_tester; prefs_.SetString(kStringPrefName, kExampleUrl0); syncer::SyncDataList in; @@ -334,8 +325,6 @@ TEST_F(PrefServiceSyncableTest, ModelAssociationWithDataTypeMismatch) { syncer::SyncChangeList out; InitWithSyncDataTakeOutput(in, &out); EXPECT_THAT(out, IsEmpty()); - histogram_tester.ExpectBucketCount("Sync.Preferences.RemotePrefTypeMismatch", - true, 1); EXPECT_THAT(prefs_.GetString(kStringPrefName), Eq(kExampleUrl0)); } @@ -388,8 +377,7 @@ class PrefServiceSyncableMergeTest : public testing::Test { &client_, /*read_error_callback=*/base::DoNothing(), /*async=*/false), - pref_sync_service_(nullptr), - next_pref_remote_sync_node_id_(0) {} + pref_sync_service_(nullptr) {} void SetUp() override { pref_registry_->RegisterStringPref(kUnsyncedPreferenceName, @@ -410,8 +398,7 @@ class PrefServiceSyncableMergeTest : public testing::Test { ASSERT_THAT(pref_sync_service_, NotNull()); } - syncer::SyncChange MakeRemoteChange(int64_t id, - const std::string& name, + syncer::SyncChange MakeRemoteChange(const std::string& name, const base::Value& value, SyncChange::SyncChangeType type) { std::string serialized; @@ -422,7 +409,7 @@ class PrefServiceSyncableMergeTest : public testing::Test { pref_one->set_name(name); pref_one->set_value(serialized); return syncer::SyncChange(FROM_HERE, type, - syncer::SyncData::CreateRemoteData(id, entity)); + syncer::SyncData::CreateRemoteData(entity)); } void AddToRemoteDataList(const std::string& name, @@ -435,8 +422,7 @@ class PrefServiceSyncableMergeTest : public testing::Test { sync_pb::PreferenceSpecifics* pref_one = one.mutable_preference(); pref_one->set_name(name); pref_one->set_value(serialized); - out->push_back( - SyncData::CreateRemoteData(++next_pref_remote_sync_node_id_, one)); + out->push_back(SyncData::CreateRemoteData(one)); } void InitWithSyncDataTakeOutput(const syncer::SyncDataList& initial_data, @@ -476,7 +462,6 @@ class PrefServiceSyncableMergeTest : public testing::Test { TestPrefModelAssociatorClient client_; PrefServiceSyncable prefs_; PrefModelAssociator* pref_sync_service_; - int next_pref_remote_sync_node_id_; }; TEST_F(PrefServiceSyncableMergeTest, ShouldMergeSelectedListValues) { @@ -546,8 +531,8 @@ TEST_F(PrefServiceSyncableMergeTest, ManagedListPreferences) { base::ListValue sync_value; sync_value.AppendString("http://crbug.com"); syncer::SyncChangeList list; - list.push_back(MakeRemoteChange(1, kListPrefName, sync_value, - SyncChange::ACTION_UPDATE)); + list.push_back( + MakeRemoteChange(kListPrefName, sync_value, SyncChange::ACTION_UPDATE)); pref_sync_service_->ProcessSyncChanges(FROM_HERE, list); const base::Value* managed_prefs_result; @@ -647,7 +632,7 @@ TEST_F(PrefServiceSyncableMergeTest, ShouldIgnoreUpdatesToNotSyncablePrefs) { syncer::SyncChangeList remote_changes; remote_changes.push_back(MakeRemoteChange( - 1, pref_name, base::Value("remote_value2"), SyncChange::ACTION_UPDATE)); + pref_name, base::Value("remote_value2"), SyncChange::ACTION_UPDATE)); pref_sync_service_->ProcessSyncChanges(FROM_HERE, remote_changes); // The pref isn't synced. EXPECT_THAT(pref_sync_service_->GetAllSyncDataForTesting(syncer::PREFERENCES), @@ -702,8 +687,8 @@ TEST_F(PrefServiceSyncableTest, UpdatedSyncNodeActionUpdate) { base::Value expected(kExampleUrl1); syncer::SyncChangeList list; - list.push_back(MakeRemoteChange(1, kStringPrefName, expected, - SyncChange::ACTION_UPDATE)); + list.push_back( + MakeRemoteChange(kStringPrefName, expected, SyncChange::ACTION_UPDATE)); pref_sync_service_->ProcessSyncChanges(FROM_HERE, list); const base::Value& actual = GetPreferenceValue(kStringPrefName); @@ -713,19 +698,16 @@ TEST_F(PrefServiceSyncableTest, UpdatedSyncNodeActionUpdate) { // Verifies that the implementation gracefully handles a remote update with the // wrong type. The local version should not get modified in these cases. TEST_F(PrefServiceSyncableTest, UpdatedSyncNodeActionUpdateTypeMismatch) { - base::HistogramTester histogram_tester; GetPrefs()->SetString(kStringPrefName, kExampleUrl0); InitWithNoSyncData(); base::Value remote_int_value(123); syncer::SyncChangeList remote_changes; - remote_changes.push_back(MakeRemoteChange( - 1, kStringPrefName, remote_int_value, SyncChange::ACTION_UPDATE)); + remote_changes.push_back(MakeRemoteChange(kStringPrefName, remote_int_value, + SyncChange::ACTION_UPDATE)); pref_sync_service_->ProcessSyncChanges(FROM_HERE, remote_changes); EXPECT_THAT(prefs_.GetString(kStringPrefName), Eq(kExampleUrl0)); - histogram_tester.ExpectBucketCount("Sync.Preferences.RemotePrefTypeMismatch", - true, 1); } TEST_F(PrefServiceSyncableTest, UpdatedSyncNodeActionAdd) { @@ -734,7 +716,7 @@ TEST_F(PrefServiceSyncableTest, UpdatedSyncNodeActionAdd) { base::Value expected(kExampleUrl0); syncer::SyncChangeList list; list.push_back( - MakeRemoteChange(1, kStringPrefName, expected, SyncChange::ACTION_ADD)); + MakeRemoteChange(kStringPrefName, expected, SyncChange::ACTION_ADD)); pref_sync_service_->ProcessSyncChanges(FROM_HERE, list); const base::Value& actual = GetPreferenceValue(kStringPrefName); @@ -746,7 +728,7 @@ TEST_F(PrefServiceSyncableTest, UpdatedSyncNodeUnknownPreference) { InitWithNoSyncData(); syncer::SyncChangeList list; base::Value expected(kExampleUrl0); - list.push_back(MakeRemoteChange(1, "unknown preference", expected, + list.push_back(MakeRemoteChange("unknown preference", expected, SyncChange::ACTION_UPDATE)); pref_sync_service_->ProcessSyncChanges(FROM_HERE, list); // Nothing interesting happens on the client when it gets an update @@ -771,8 +753,8 @@ TEST_F(PrefServiceSyncableTest, ManagedPreferences) { // value. base::Value sync_value("http://crbug.com"); syncer::SyncChangeList list; - list.push_back(MakeRemoteChange(1, kStringPrefName, sync_value, - SyncChange::ACTION_UPDATE)); + list.push_back( + MakeRemoteChange(kStringPrefName, sync_value, SyncChange::ACTION_UPDATE)); pref_sync_service_->ProcessSyncChanges(FROM_HERE, list); EXPECT_TRUE(managed_value.Equals(prefs_.GetManagedPref(kStringPrefName))); @@ -822,8 +804,8 @@ TEST_F(PrefServiceSyncableTest, DynamicManagedPreferencesWithSyncChange) { // Change the sync value. base::Value sync_value("http://example.com/sync"); syncer::SyncChangeList list; - list.push_back(MakeRemoteChange(1, kStringPrefName, sync_value, - SyncChange::ACTION_UPDATE)); + list.push_back( + MakeRemoteChange(kStringPrefName, sync_value, SyncChange::ACTION_UPDATE)); pref_sync_service_->ProcessSyncChanges(FROM_HERE, list); // The pref value should still be the one dictated by policy. @@ -873,7 +855,7 @@ TEST_F(PrefServiceSyncableTest, DeletePreference) { auto null_value = std::make_unique<base::Value>(); syncer::SyncChangeList list; - list.push_back(MakeRemoteChange(1, kStringPrefName, *null_value, + list.push_back(MakeRemoteChange(kStringPrefName, *null_value, SyncChange::ACTION_DELETE)); pref_sync_service_->ProcessSyncChanges(FROM_HERE, list); EXPECT_TRUE(pref->IsDefaultValue()); @@ -1043,7 +1025,7 @@ TEST_F(PrefServiceSyncableChromeOsTest, IsPrefSynced_OsPref) { EXPECT_FALSE(associator->IsPrefSyncedForTesting("os_pref")); syncer::SyncChangeList list; - list.push_back(MakeRemoteChange(1, "os_pref", base::Value("value"), + list.push_back(MakeRemoteChange("os_pref", base::Value("value"), SyncChange::ACTION_ADD, syncer::OS_PREFERENCES)); associator->ProcessSyncChanges(FROM_HERE, list); @@ -1059,7 +1041,7 @@ TEST_F(PrefServiceSyncableChromeOsTest, IsPrefSynced_OsPriorityPref) { EXPECT_FALSE(associator->IsPrefSyncedForTesting("os_priority_pref")); syncer::SyncChangeList list; - list.push_back(MakeRemoteChange(1, "os_priority_pref", base::Value("value"), + list.push_back(MakeRemoteChange("os_priority_pref", base::Value("value"), SyncChange::ACTION_ADD, syncer::OS_PRIORITY_PREFERENCES)); associator->ProcessSyncChanges(FROM_HERE, list); @@ -1163,7 +1145,7 @@ TEST_F(PrefServiceSyncableChromeOsTest, // Simulate an old client that has "os_pref" registered as SYNCABLE_PREF // instead of SYNCABLE_OS_PREF. syncer::SyncDataList list; - list.push_back(CreateRemoteSyncData(1, "os_pref", base::Value("new_value"))); + list.push_back(CreateRemoteSyncData("os_pref", base::Value("new_value"))); // Simulate the first sync at startup of the legacy browser prefs ModelType. auto* browser_associator = static_cast<PrefModelAssociator*>( @@ -1201,7 +1183,7 @@ TEST_F(PrefServiceSyncableChromeOsTest, syncer::SyncChangeList list; // Simulate an old client that has "os_pref" registered as SYNCABLE_PREF // instead of SYNCABLE_OS_PREF. - list.push_back(MakeRemoteChange(1, "os_pref", base::Value("new_value"), + list.push_back(MakeRemoteChange("os_pref", base::Value("new_value"), SyncChange::ACTION_ADD, syncer::PREFERENCES)); // Simulate a sync update after startup. |