diff options
| author | kalman@chromium.org <kalman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-27 03:12:58 +0000 |
|---|---|---|
| committer | kalman@chromium.org <kalman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-27 03:12:58 +0000 |
| commit | 38871709e598bf8829218643d4b1f4ff3d74fcb9 (patch) | |
| tree | 1decaa288960c419b4b10162db13cf9c56b10fd6 | |
| parent | 4f5cd50ad11505a0171ec5f58ccc070893f3bf72 (diff) | |
| download | chromium_src-38871709e598bf8829218643d4b1f4ff3d74fcb9.zip chromium_src-38871709e598bf8829218643d4b1f4ff3d74fcb9.tar.gz chromium_src-38871709e598bf8829218643d4b1f4ff3d74fcb9.tar.bz2 | |
Extension settings API: send ACTION_UPDATE rather than ACTION_ADD when a
setting that was already in sync (from MergeDataAndStartSyncing) is changed.
BUG=101635
TEST=ExtensionSettingsSyncTest
Review URL: http://codereview.chromium.org/8341043
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@107524 0039d316-1c4b-4281-b951-d872f2087c98
| -rw-r--r-- | chrome/browser/extensions/extension_settings_sync_unittest.cc | 14 | ||||
| -rw-r--r-- | chrome/browser/extensions/syncable_extension_settings_storage.cc | 5 |
2 files changed, 17 insertions, 2 deletions
diff --git a/chrome/browser/extensions/extension_settings_sync_unittest.cc b/chrome/browser/extensions/extension_settings_sync_unittest.cc index 7073475..43a50c0 100644 --- a/chrome/browser/extensions/extension_settings_sync_unittest.cc +++ b/chrome/browser/extensions/extension_settings_sync_unittest.cc @@ -226,10 +226,22 @@ TEST_F(ExtensionSettingsSyncTest, InSyncDataDoesNotInvokeSync) { backend_->MergeDataAndStartSyncing( syncable::EXTENSION_SETTINGS, sync_data, &sync_); - backend_->StopSyncing(syncable::EXTENSION_SETTINGS); // Already in sync, so no changes. ASSERT_EQ(0u, sync_.changes().size()); + + // Regression test: not-changing the synced value shouldn't result in a sync + // change, and changing the synced value should result in an update. + storage1->Set("foo", value1); + ASSERT_EQ(0u, sync_.changes().size()); + + storage1->Set("foo", value2); + ASSERT_EQ(1u, sync_.changes().size()); + ExtensionSettingSyncData change = sync_.GetOnlyChange("s1", "foo"); + ASSERT_EQ(SyncChange::ACTION_UPDATE, change.change_type()); + ASSERT_TRUE(value2.Equals(&change.value())); + + backend_->StopSyncing(syncable::EXTENSION_SETTINGS); } TEST_F(ExtensionSettingsSyncTest, LocalDataWithNoSyncDataIsPushedToSync) { diff --git a/chrome/browser/extensions/syncable_extension_settings_storage.cc b/chrome/browser/extensions/syncable_extension_settings_storage.cc index 8ec5d17..cf57766 100644 --- a/chrome/browser/extensions/syncable_extension_settings_storage.cc +++ b/chrome/browser/extensions/syncable_extension_settings_storage.cc @@ -174,7 +174,10 @@ SyncError SyncableExtensionSettingsStorage::OverwriteLocalSettingsWithSync( scoped_ptr<Value> sync_value(orphaned_sync_value); Value* local_value = NULL; settings.GetWithoutPathExpansion(*it, &local_value); - if (!sync_value->Equals(local_value)) { + if (sync_value->Equals(local_value)) { + // Sync and local values are the same, no changes to send. + synced_keys_.insert(*it); + } else { // Sync value is different, update local setting with new value. changes.push_back( ExtensionSettingSyncData( |