Add a bunch of preferences to sync.
This includes a fix for not crashing when we encounter a preference that is not registered.
BUG=39958
Review URL: http://codereview.chromium.org/1571006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@43497 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/pref_service.cc b/chrome/browser/pref_service.cc
index 6abfd0f1..d30b544 100644
--- a/chrome/browser/pref_service.cc
+++ b/chrome/browser/pref_service.cc
@@ -407,6 +407,19 @@
NOTREACHED() << "Trying to write an unregistered pref: " << path;
return;
}
+
+ // Allow dictionary and list types to be set to null.
+ if (value.GetType() == Value::TYPE_NULL &&
+ (pref->type() == Value::TYPE_DICTIONARY ||
+ pref->type() == Value::TYPE_LIST)) {
+ scoped_ptr<Value> old_value(GetPrefCopy(path));
+ if (!old_value->Equals(&value)) {
+ persistent_->Remove(path, NULL);
+ FireObservers(path);
+ }
+ return;
+ }
+
if (pref->type() != value.GetType()) {
NOTREACHED() << "Wrong type for Set: " << path;
}
diff --git a/chrome/browser/pref_service_unittest.cc b/chrome/browser/pref_service_unittest.cc
index aac27b9..e643b8c 100644
--- a/chrome/browser/pref_service_unittest.cc
+++ b/chrome/browser/pref_service_unittest.cc
@@ -2,19 +2,30 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <string>
+
#include "app/test/data/resource.h"
#include "base/file_util.h"
#include "base/message_loop.h"
#include "base/path_service.h"
+#include "base/scoped_ptr.h"
+#include "base/values.h"
#include "chrome/browser/chrome_thread.h"
#include "chrome/browser/pref_service.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/json_value_serializer.h"
+#include "chrome/common/notification_observer_mock.h"
#include "chrome/common/notification_service.h"
#include "chrome/common/notification_type.h"
#include "chrome/common/pref_names.h"
+#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
+using testing::_;
+using testing::Mock;
+using testing::Pointee;
+using testing::Property;
+
class PrefServiceTest : public testing::Test {
protected:
virtual void SetUp() {
@@ -268,3 +279,113 @@
prefs.SetString(path, L"blah");
EXPECT_TRUE(prefs.HasPrefPath(path));
}
+
+class PrefServiceSetValueTest : public testing::Test {
+ protected:
+ static const wchar_t name_[];
+ static const wchar_t value_[];
+
+ PrefServiceSetValueTest()
+ : prefs_(FilePath()),
+ name_string_(name_),
+ null_value_(Value::CreateNullValue()) {}
+
+ void SetExpectNoNotification() {
+ EXPECT_CALL(observer_, Observe(_, _, _)).Times(0);
+ }
+
+ void SetExpectPrefChanged() {
+ EXPECT_CALL(observer_,
+ Observe(NotificationType(NotificationType::PREF_CHANGED), _,
+ Property(&Details<std::wstring>::ptr,
+ Pointee(name_string_))));
+ }
+
+ PrefService prefs_;
+ std::wstring name_string_;
+ scoped_ptr<Value> null_value_;
+ NotificationObserverMock observer_;
+};
+const wchar_t PrefServiceSetValueTest::name_[] = L"name";
+const wchar_t PrefServiceSetValueTest::value_[] = L"value";
+
+TEST_F(PrefServiceSetValueTest, SetStringValue) {
+ const wchar_t default_string[] = L"default";
+ scoped_ptr<Value> default_value(Value::CreateStringValue(default_string));
+ prefs_.RegisterStringPref(name_, default_string);
+ prefs_.AddPrefObserver(name_, &observer_);
+ SetExpectNoNotification();
+ prefs_.Set(name_, *default_value);
+ Mock::VerifyAndClearExpectations(&observer_);
+
+ scoped_ptr<Value> new_value(Value::CreateStringValue(value_));
+ SetExpectPrefChanged();
+ prefs_.Set(name_, *new_value);
+ EXPECT_EQ(value_, prefs_.GetString(name_));
+
+ prefs_.RemovePrefObserver(name_, &observer_);
+}
+
+TEST_F(PrefServiceSetValueTest, SetDictionaryValue) {
+ prefs_.RegisterDictionaryPref(name_);
+ prefs_.AddPrefObserver(name_, &observer_);
+
+ SetExpectNoNotification();
+ prefs_.Set(name_, *null_value_);
+ Mock::VerifyAndClearExpectations(&observer_);
+
+ DictionaryValue new_value;
+ new_value.SetString(name_, value_);
+ SetExpectPrefChanged();
+ prefs_.Set(name_, new_value);
+ Mock::VerifyAndClearExpectations(&observer_);
+ DictionaryValue* dict = prefs_.GetMutableDictionary(name_);
+ EXPECT_EQ(1U, dict->size());
+ std::wstring out_value;
+ dict->GetString(name_, &out_value);
+ EXPECT_EQ(value_, out_value);
+
+ SetExpectNoNotification();
+ prefs_.Set(name_, new_value);
+ Mock::VerifyAndClearExpectations(&observer_);
+
+ SetExpectPrefChanged();
+ prefs_.Set(name_, *null_value_);
+ Mock::VerifyAndClearExpectations(&observer_);
+ dict = prefs_.GetMutableDictionary(name_);
+ EXPECT_EQ(0U, dict->size());
+
+ prefs_.RemovePrefObserver(name_, &observer_);
+}
+
+TEST_F(PrefServiceSetValueTest, SetListValue) {
+ prefs_.RegisterListPref(name_);
+ prefs_.AddPrefObserver(name_, &observer_);
+
+ SetExpectNoNotification();
+ prefs_.Set(name_, *null_value_);
+ Mock::VerifyAndClearExpectations(&observer_);
+
+ ListValue new_value;
+ new_value.Append(Value::CreateStringValue(value_));
+ SetExpectPrefChanged();
+ prefs_.Set(name_, new_value);
+ Mock::VerifyAndClearExpectations(&observer_);
+ ListValue* list = prefs_.GetMutableList(name_);
+ ASSERT_EQ(1U, list->GetSize());
+ std::wstring out_value;
+ list->GetString(0, &out_value);
+ EXPECT_EQ(value_, out_value);
+
+ SetExpectNoNotification();
+ prefs_.Set(name_, new_value);
+ Mock::VerifyAndClearExpectations(&observer_);
+
+ SetExpectPrefChanged();
+ prefs_.Set(name_, *null_value_);
+ Mock::VerifyAndClearExpectations(&observer_);
+ list = prefs_.GetMutableList(name_);
+ EXPECT_EQ(0U, list->GetSize());
+
+ prefs_.RemovePrefObserver(name_, &observer_);
+}
diff --git a/chrome/browser/sync/glue/preference_change_processor.cc b/chrome/browser/sync/glue/preference_change_processor.cc
index 3229655..921b30b 100644
--- a/chrome/browser/sync/glue/preference_change_processor.cc
+++ b/chrome/browser/sync/glue/preference_change_processor.cc
@@ -31,7 +31,7 @@
DCHECK(error_handler);
}
-PreferenceChangeProcessor::~PreferenceChangeProcessor(){
+PreferenceChangeProcessor::~PreferenceChangeProcessor() {
DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
}
@@ -100,24 +100,32 @@
}
DCHECK(syncable::PREFERENCES == node.GetModelType());
- std::string name;
+ std::wstring name;
scoped_ptr<Value> value(ReadPreference(&node, &name));
+ // Skip values we can't deserialize.
+ if (!value.get())
+ continue;
+
+ // It is possible that we may receive a change to a preference we
+ // do not want to sync. For example if the user is syncing a Mac
+ // client and a Windows client, the Windows client does not
+ // support kShowPageOptionsButtons. Ignore updates from these
+ // preferences.
+ const wchar_t* pref_name = name.c_str();
+ if (model_associator_->synced_preferences().count(pref_name) == 0)
+ continue;
+
if (sync_api::SyncManager::ChangeRecord::ACTION_DELETE ==
changes[i].action) {
- pref_service_->ClearPref(UTF8ToWide(name).c_str());
+ pref_service_->ClearPref(pref_name);
} else {
- std::wstring pref_name(UTF8ToWide(name));
- const PrefService::Preference* preference =
- pref_service_->FindPreference(pref_name.c_str());
- if (value.get() && preference) {
- pref_service_->Set(pref_name.c_str(), *value);
- if (pref_name == prefs::kShowBookmarkBar) {
- // If it was the bookmark bar, send an additional notification.
- NotificationService::current()->Notify(
- NotificationType::BOOKMARK_BAR_VISIBILITY_PREF_CHANGED,
- Source<PreferenceChangeProcessor>(this),
- NotificationService::NoDetails());
- }
+ pref_service_->Set(pref_name, *value);
+ if (pref_name == prefs::kShowBookmarkBar) {
+ // If it was the bookmark bar, send an additional notification.
+ NotificationService::current()->Notify(
+ NotificationType::BOOKMARK_BAR_VISIBILITY_PREF_CHANGED,
+ Source<PreferenceChangeProcessor>(this),
+ NotificationService::NoDetails());
}
}
}
@@ -146,7 +154,7 @@
Value* PreferenceChangeProcessor::ReadPreference(
sync_api::ReadNode* node,
- std::string* name) {
+ std::wstring* name) {
const sync_pb::PreferenceSpecifics& preference(
node->GetPreferenceSpecifics());
base::JSONReader reader;
@@ -157,7 +165,7 @@
error_handler()->OnUnrecoverableError();
return NULL;
}
- *name = preference.name();
+ *name = UTF8ToWide(preference.name());
return value.release();
}
diff --git a/chrome/browser/sync/glue/preference_change_processor.h b/chrome/browser/sync/glue/preference_change_processor.h
index f00ca2e..4820b33 100644
--- a/chrome/browser/sync/glue/preference_change_processor.h
+++ b/chrome/browser/sync/glue/preference_change_processor.h
@@ -47,7 +47,7 @@
bool WritePreference(sync_api::WriteNode* node,
const std::wstring& name,
const Value* value);
- Value* ReadPreference(sync_api::ReadNode* node, std::string* name);
+ Value* ReadPreference(sync_api::ReadNode* node, std::wstring* name);
void StartObserving();
void StopObserving();
diff --git a/chrome/browser/sync/glue/preference_model_associator.cc b/chrome/browser/sync/glue/preference_model_associator.cc
index 28ba96f..169da24c 100644
--- a/chrome/browser/sync/glue/preference_model_associator.cc
+++ b/chrome/browser/sync/glue/preference_model_associator.cc
@@ -12,10 +12,10 @@
#include "chrome/browser/pref_service.h"
#include "chrome/browser/profile.h"
#include "chrome/browser/sync/engine/syncapi.h"
+#include "chrome/browser/sync/glue/synchronized_preferences.h"
#include "chrome/browser/sync/profile_sync_service.h"
#include "chrome/browser/sync/protocol/preference_specifics.pb.h"
#include "chrome/common/json_value_serializer.h"
-#include "chrome/common/pref_names.h"
namespace browser_sync {
@@ -27,12 +27,16 @@
preferences_node_id_(sync_api::kInvalidId) {
DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
DCHECK(sync_service_);
- synced_preferences_.insert(prefs::kHomePageIsNewTabPage);
- synced_preferences_.insert(prefs::kHomePage);
- synced_preferences_.insert(prefs::kRestoreOnStartup);
- synced_preferences_.insert(prefs::kURLsToRestoreOnStartup);
- synced_preferences_.insert(prefs::kShowBookmarkBar);
- synced_preferences_.insert(prefs::kShowHomeButton);
+
+ // Add the list of kSynchronizedPreferences to our local
+ // synced_preferences set, taking care to filter out any preferences
+ // that are not registered.
+ PrefService* pref_service = sync_service_->profile()->GetPrefs();
+ for (size_t i = 0;
+ i < static_cast<size_t>(arraysize(kSynchronizedPreferences)); ++i) {
+ if (pref_service->FindPreference(kSynchronizedPreferences[i]))
+ synced_preferences_.insert(kSynchronizedPreferences[i]);
+ }
}
PreferenceModelAssociator::~PreferenceModelAssociator() {
@@ -65,6 +69,9 @@
for (std::set<std::wstring>::iterator it = synced_preferences_.begin();
it != synced_preferences_.end(); ++it) {
std::string tag = WideToUTF8(*it);
+ const PrefService::Preference* pref =
+ pref_service->FindPreference((*it).c_str());
+ DCHECK(pref);
sync_api::ReadNode node(&trans);
if (node.InitByClientTagLookup(syncable::PREFERENCES, tag)) {
@@ -83,13 +90,6 @@
}
// Update the local preference based on what we got from the sync server.
- const PrefService::Preference* pref =
- pref_service->FindPreference((*it).c_str());
- if (!pref) {
- LOG(ERROR) << "Unrecognized preference -- ignoring.";
- continue;
- }
-
pref_service->Set(pref_name.c_str(), *value);
Associate(pref, node.GetId());
} else {
@@ -101,13 +101,6 @@
}
// Update the sync node with the local value for this preference.
- const PrefService::Preference* pref =
- pref_service->FindPreference((*it).c_str());
- if (!pref) {
- LOG(ERROR) << "Unrecognized preference -- ignoring.";
- continue;
- }
-
std::string serialized;
JSONStringValueSerializer json(&serialized);
if (!json.Serialize(*(pref->GetValue()))) {
diff --git a/chrome/browser/sync/glue/preference_model_associator.h b/chrome/browser/sync/glue/preference_model_associator.h
index ca66e30..7495b59 100644
--- a/chrome/browser/sync/glue/preference_model_associator.h
+++ b/chrome/browser/sync/glue/preference_model_associator.h
@@ -34,7 +34,9 @@
UnrecoverableErrorHandler* error_handler);
virtual ~PreferenceModelAssociator();
- // Returns the list of preference names that should be monitored for changes.
+ // Returns the list of preference names that should be monitored for
+ // changes. Only preferences that are registered will be in this
+ // list.
const std::set<std::wstring>& synced_preferences() {
return synced_preferences_;
}
diff --git a/chrome/browser/sync/glue/synchronized_preferences.h b/chrome/browser/sync/glue/synchronized_preferences.h
new file mode 100644
index 0000000..ecc16bf
--- /dev/null
+++ b/chrome/browser/sync/glue/synchronized_preferences.h
@@ -0,0 +1,97 @@
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Defines a list of the preferences that the
+// PreferencesChangeProcessor should process changes for.
+
+#ifndef CHROME_BROWSER_SYNC_GLUE_SYNCHRONIZED_PREFERENCES_H_
+#define CHROME_BROWSER_SYNC_GLUE_SYNCHRONIZED_PREFERENCES_H_
+
+#include "chrome/browser/translate/translate_prefs.h"
+#include "chrome/common/pref_names.h"
+
+namespace browser_sync {
+
+static const wchar_t* kSynchronizedPreferences[] = {
+ // General profile preferences.
+ prefs::kAcceptLanguages,
+ prefs::kAlternateErrorPagesEnabled,
+ prefs::kApplicationLocale,
+ prefs::kAutoFillAuxiliaryProfilesEnabled,
+ prefs::kAutoFillEnabled,
+ prefs::kAutoFillInfoBarShown,
+ prefs::kBlockThirdPartyCookies,
+ prefs::kClearSiteDataOnExit,
+ prefs::kCookiePromptExpanded,
+ prefs::kDefaultCharset,
+ prefs::kDefaultContentSettings,
+ prefs::kDefaultSearchProviderID,
+ prefs::kDefaultSearchProviderName,
+ prefs::kDefaultSearchProviderSearchURL,
+ prefs::kDefaultSearchProviderSuggestURL,
+ prefs::kDeleteBrowsingHistory,
+ prefs::kDeleteCache,
+ prefs::kDeleteCookies,
+ prefs::kDeleteDownloadHistory,
+ prefs::kDeleteFormData,
+ prefs::kDeletePasswords,
+ prefs::kDeleteTimePeriod,
+ prefs::kDesktopNotificationAllowedOrigins,
+ prefs::kDesktopNotificationDeniedOrigins,
+ prefs::kDnsPrefetchingEnabled,
+ prefs::kEnableAutoSpellCorrect,
+ prefs::kEnableSpellCheck,
+ prefs::kEnableTranslate,
+ prefs::kExtensionsUIDeveloperMode,
+ prefs::kGeolocationContentSettings,
+ prefs::kGeolocationDefaultContentSetting,
+ prefs::kHomePage,
+ prefs::kHomePageIsNewTabPage,
+ prefs::kMixedContentFiltering,
+ prefs::kNTPPromoImageRemaining,
+ prefs::kNTPPromoLineRemaining,
+ prefs::kPasswordManagerEnabled,
+ prefs::kPerHostContentSettings,
+ prefs::kPerHostZoomLevels,
+ prefs::kPinnedTabs,
+ prefs::kPrintingPageFooterCenter,
+ prefs::kPrintingPageFooterLeft,
+ prefs::kPrintingPageFooterRight,
+ prefs::kPrintingPageHeaderCenter,
+ prefs::kPrintingPageHeaderLeft,
+ prefs::kPrintingPageHeaderRight,
+ prefs::kPrivacyFilterRules,
+ prefs::kPromptForDownload,
+ prefs::kRecentlySelectedEncoding,
+ prefs::kRestoreOnStartup,
+ prefs::kSafeBrowsingEnabled,
+ prefs::kSearchSuggestEnabled,
+ prefs::kShowBookmarkBar,
+ prefs::kShowHomeButton,
+ prefs::kShowOmniboxSearchHint,
+ prefs::kShowPageOptionsButtons,
+ prefs::kStaticEncodings,
+ prefs::kURLsToRestoreOnStartup,
+ prefs::kURLsToRestoreOnStartup,
+ prefs::kWebKitDomPasteEnabled,
+ prefs::kWebKitInspectorSettings,
+ prefs::kWebKitJavaEnabled,
+ prefs::kWebKitJavascriptCanOpenWindowsAutomatically,
+ prefs::kWebKitJavascriptEnabled,
+ prefs::kWebKitLoadsImagesAutomatically,
+ prefs::kWebKitPluginsEnabled,
+ prefs::kWebKitShrinksStandaloneImagesToFit,
+ prefs::kWebKitTextAreasAreResizable,
+ prefs::kWebKitUsesUniversalDetector,
+ prefs::kWebKitWebSecurityEnabled,
+
+ // Translate preferences.
+ TranslatePrefs::kPrefTranslateLanguageBlacklist,
+ TranslatePrefs::kPrefTranslateSiteBlacklist,
+ TranslatePrefs::kPrefTranslateWhitelists,
+};
+
+} // namespace browser_sync
+
+#endif // CHROME_BROWSER_SYNC_GLUE_SYNCHRONIZED_PREFERENCES_H_
diff --git a/chrome/browser/translate/translate_prefs.cc b/chrome/browser/translate/translate_prefs.cc
index 3077cb0..1beb0ee 100644
--- a/chrome/browser/translate/translate_prefs.cc
+++ b/chrome/browser/translate/translate_prefs.cc
@@ -7,11 +7,12 @@
#include "base/string_util.h"
#include "chrome/browser/pref_service.h"
-static const wchar_t kPrefTranslateLanguageBlacklist[] =
+const wchar_t TranslatePrefs::kPrefTranslateLanguageBlacklist[] =
L"translate_language_blacklist";
-static const wchar_t kPrefTranslateSiteBlacklist[] =
+const wchar_t TranslatePrefs::kPrefTranslateSiteBlacklist[] =
L"translate_site_blacklist";
-static const wchar_t kPrefTranslateWhitelists[] = L"translate_whitelists";
+const wchar_t TranslatePrefs::kPrefTranslateWhitelists[] =
+ L"translate_whitelists";
// TranslatePrefs: public: -----------------------------------------------------
diff --git a/chrome/browser/translate/translate_prefs.h b/chrome/browser/translate/translate_prefs.h
index 542dacc..01a93c1 100644
--- a/chrome/browser/translate/translate_prefs.h
+++ b/chrome/browser/translate/translate_prefs.h
@@ -5,6 +5,7 @@
#ifndef CHROME_BROWSER_TRANSLATE_TRANSLATE_PREFS_H_
#define CHROME_BROWSER_TRANSLATE_TRANSLATE_PREFS_H_
+#include <string>
#include <vector>
#include "googleurl/src/gurl.h"
@@ -14,6 +15,10 @@
class TranslatePrefs {
public:
+ static const wchar_t kPrefTranslateLanguageBlacklist[];
+ static const wchar_t kPrefTranslateSiteBlacklist[];
+ static const wchar_t kPrefTranslateWhitelists[];
+
explicit TranslatePrefs(PrefService* user_prefs);
bool IsLanguageBlacklisted(const std::string& original_language);
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index e2e8de1..0a483644 100755
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -1960,6 +1960,7 @@
'browser/sync/glue/preference_model_associator.h',
'browser/sync/glue/sync_backend_host.cc',
'browser/sync/glue/sync_backend_host.h',
+ 'browser/sync/glue/synchronized_preferences.h',
'browser/sync/glue/theme_change_processor.cc',
'browser/sync/glue/theme_change_processor.h',
'browser/sync/glue/theme_data_type_controller.cc',
