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

10 files changed, 284 insertions, 43 deletions

diff --git a/chrome/browser/pref_service.cc b/chrome/browser/pref_service.cc
index 6abfd0f..d30b544 100644
--- a/chrome/browser/pref_service.cc
+++ b/chrome/browser/pref_service.cc
@@ -407,6 +407,19 @@ void PrefService::Set(const wchar_t* path, const Value& value) {
     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 @@ TEST_F(PrefServiceTest, HasPrefPath) {
   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 @@ PreferenceChangeProcessor::PreferenceChangeProcessor(
   DCHECK(error_handler);
 }
 
-PreferenceChangeProcessor::~PreferenceChangeProcessor(){
+PreferenceChangeProcessor::~PreferenceChangeProcessor() {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
 }
 
@@ -100,24 +100,32 @@ void PreferenceChangeProcessor::ApplyChangesFromSyncModel(
     }
     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 @@ bool PreferenceChangeProcessor::WritePreference(
 
 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 @@ Value* PreferenceChangeProcessor::ReadPreference(
     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 @@ class PreferenceChangeProcessor : public ChangeProcessor,
   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..169da24 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 @@ PreferenceModelAssociator::PreferenceModelAssociator(
       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 @@ bool PreferenceModelAssociator::AssociateModels() {
   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 @@ bool PreferenceModelAssociator::AssociateModels() {
       }
 
       // 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 @@ bool PreferenceModelAssociator::AssociateModels() {
       }
 
       // 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 @@ class PreferenceModelAssociator
                             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 PrefService;
 
 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..0a48364 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',