Get translate prefs syncing

Add a ScopedPrefUpdate in various places as well as fix a bug that prevented the live update of the main translate pref in the options dialog.

Review URL: http://codereview.chromium.org/1547025

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@43894 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 61 insertions, 15 deletions

diff --git a/chrome/browser/browser_prefs.cc b/chrome/browser/browser_prefs.cc
index 12d626d..4f08d71 100644
--- a/chrome/browser/browser_prefs.cc
+++ b/chrome/browser/browser_prefs.cc
@@ -38,6 +38,7 @@
 #include "chrome/browser/tabs/pinned_tab_codec.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/task_manager.h"
+#include "chrome/browser/translate/translate_prefs.h"
 
 #if defined(TOOLKIT_VIEWS)  // TODO(port): whittle this down as we port
 #include "chrome/browser/views/browser_actions_container.h"
@@ -106,6 +107,7 @@ void RegisterUserPrefs(PrefService* user_prefs) {
   PinnedTabCodec::RegisterUserPrefs(user_prefs);
   ExtensionPrefs::RegisterUserPrefs(user_prefs);
   GeolocationContentSettingsMap::RegisterUserPrefs(user_prefs);
+  TranslatePrefs::RegisterUserPrefs(user_prefs);
 #if defined(TOOLKIT_VIEWS)
   BrowserActionsContainer::RegisterUserPrefs(user_prefs);
 #elif defined(TOOLKIT_GTK)
diff --git a/chrome/browser/gtk/options/advanced_contents_gtk.cc b/chrome/browser/gtk/options/advanced_contents_gtk.cc
index e3eaa79..e11ca2b 100644
--- a/chrome/browser/gtk/options/advanced_contents_gtk.cc
+++ b/chrome/browser/gtk/options/advanced_contents_gtk.cc
@@ -543,7 +543,7 @@ TranslateSection::TranslateSection(Profile* profile)
 
 void TranslateSection::NotifyPrefChanged(const std::wstring* pref_name) {
   pref_changing_ = true;
-  if (!pref_name || *pref_name == prefs::kAlternateErrorPagesEnabled) {
+  if (!pref_name || *pref_name == prefs::kEnableTranslate) {
     gtk_toggle_button_set_active(
         GTK_TOGGLE_BUTTON(translate_checkbox_), enable_translate_.GetValue());
   }
diff --git a/chrome/browser/translate/translate_manager_unittest.cc b/chrome/browser/translate/translate_manager_unittest.cc
index e543af0..45432c5 100644
--- a/chrome/browser/translate/translate_manager_unittest.cc
+++ b/chrome/browser/translate/translate_manager_unittest.cc
@@ -9,15 +9,24 @@
 #include "chrome/browser/tab_contents/render_view_context_menu.h"
 #include "chrome/browser/translate/translate_infobars_delegates.h"
 #include "chrome/browser/translate/translate_manager.h"
+#include "chrome/browser/translate/translate_prefs.h"
 #include "chrome/common/ipc_test_sink.h"
+#include "chrome/common/notification_details.h"
+#include "chrome/common/notification_observer_mock.h"
 #include "chrome/common/notification_registrar.h"
 #include "chrome/common/notification_service.h"
+#include "chrome/common/notification_type.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/test/testing_browser_process.h"
 #include "grit/generated_resources.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "third_party/cld/languages/public/languages.h"
 
+using testing::_;
+using testing::Pointee;
+using testing::Property;
+
 class TestTranslateManager : public TranslateManager {
  public:
   TestTranslateManager() {}
@@ -157,6 +166,16 @@ class TranslateManagerTest : public RenderViewHostTestHarness,
                                             std::string());
   }
 
+  void SetPrefObserverExpectation(const wchar_t* path) {
+    EXPECT_CALL(
+        pref_observer_,
+        Observe(NotificationType(NotificationType::PREF_CHANGED),
+                _,
+                Property(&Details<std::wstring>::ptr, Pointee(path))));
+  }
+
+  NotificationObserverMock pref_observer_;
+
  private:
   NotificationRegistrar notification_registrar_;
   scoped_ptr<TestTranslateManager> translate_manager_;
@@ -677,9 +696,12 @@ TEST_F(TranslateManagerTest, NeverTranslateLanguagePref) {
 
   // Select never translate this language.
   PrefService* prefs = contents()->profile()->GetPrefs();
-  TranslatePrefs translate_prefs(contents()->profile()->GetPrefs());
+  prefs->AddPrefObserver(TranslatePrefs::kPrefTranslateLanguageBlacklist,
+                         &pref_observer_);
+  TranslatePrefs translate_prefs(prefs);
   EXPECT_FALSE(translate_prefs.IsLanguageBlacklisted("fr"));
   EXPECT_TRUE(translate_prefs.CanTranslate(prefs, "fr", url));
+  SetPrefObserverExpectation(TranslatePrefs::kPrefTranslateLanguageBlacklist);
   translate_prefs.BlacklistLanguage("fr");
   EXPECT_TRUE(translate_prefs.IsLanguageBlacklisted("fr"));
   EXPECT_FALSE(translate_prefs.CanTranslate(prefs, "fr", url));
@@ -694,6 +716,7 @@ TEST_F(TranslateManagerTest, NeverTranslateLanguagePref) {
   EXPECT_TRUE(GetTranslateInfoBar() == NULL);
 
   // Remove the language from the blacklist.
+  SetPrefObserverExpectation(TranslatePrefs::kPrefTranslateLanguageBlacklist);
   translate_prefs.RemoveLanguageFromBlacklist("fr");
   EXPECT_FALSE(translate_prefs.IsLanguageBlacklisted("fr"));
   EXPECT_TRUE(translate_prefs.CanTranslate(prefs, "fr", url));
@@ -703,6 +726,8 @@ TEST_F(TranslateManagerTest, NeverTranslateLanguagePref) {
 
   // There should be a translate infobar.
   EXPECT_TRUE(GetTranslateInfoBar() != NULL);
+  prefs->RemovePrefObserver(TranslatePrefs::kPrefTranslateLanguageBlacklist,
+                            &pref_observer_);
 }
 
 // Tests the "Never translate this site" pref.
@@ -717,9 +742,12 @@ TEST_F(TranslateManagerTest, NeverTranslateSitePref) {
 
   // Select never translate this site.
   PrefService* prefs = contents()->profile()->GetPrefs();
-  TranslatePrefs translate_prefs(contents()->profile()->GetPrefs());
+  prefs->AddPrefObserver(TranslatePrefs::kPrefTranslateSiteBlacklist,
+                         &pref_observer_);
+  TranslatePrefs translate_prefs(prefs);
   EXPECT_FALSE(translate_prefs.IsSiteBlacklisted(host));
   EXPECT_TRUE(translate_prefs.CanTranslate(prefs, "fr", url));
+  SetPrefObserverExpectation(TranslatePrefs::kPrefTranslateSiteBlacklist);
   translate_prefs.BlacklistSite(host);
   EXPECT_TRUE(translate_prefs.IsSiteBlacklisted(host));
   EXPECT_FALSE(translate_prefs.CanTranslate(prefs, "fr", url));
@@ -734,6 +762,7 @@ TEST_F(TranslateManagerTest, NeverTranslateSitePref) {
   EXPECT_TRUE(GetTranslateInfoBar() == NULL);
 
   // Remove the site from the blacklist.
+  SetPrefObserverExpectation(TranslatePrefs::kPrefTranslateSiteBlacklist);
   translate_prefs.RemoveSiteFromBlacklist(host);
   EXPECT_FALSE(translate_prefs.IsSiteBlacklisted(host));
   EXPECT_TRUE(translate_prefs.CanTranslate(prefs, "fr", url));
@@ -743,12 +772,18 @@ TEST_F(TranslateManagerTest, NeverTranslateSitePref) {
 
   // There should be a translate infobar.
   EXPECT_TRUE(GetTranslateInfoBar() != NULL);
+  prefs->RemovePrefObserver(TranslatePrefs::kPrefTranslateSiteBlacklist,
+                            &pref_observer_);
 }
 
 // Tests the "Always translate this language" pref.
 TEST_F(TranslateManagerTest, AlwaysTranslateLanguagePref) {
   // Select always translate French to English.
-  TranslatePrefs translate_prefs(contents()->profile()->GetPrefs());
+  PrefService* prefs = contents()->profile()->GetPrefs();
+  prefs->AddPrefObserver(TranslatePrefs::kPrefTranslateWhitelists,
+                         &pref_observer_);
+  TranslatePrefs translate_prefs(prefs);
+  SetPrefObserverExpectation(TranslatePrefs::kPrefTranslateWhitelists);
   translate_prefs.WhitelistLanguagePair("fr", "en");
 
   // Load a page in French.
@@ -786,10 +821,13 @@ TEST_F(TranslateManagerTest, AlwaysTranslateLanguagePref) {
 
   // Now revert the always translate pref and make sure we go back to expected
   // behavior, which is show an infobar.
+  SetPrefObserverExpectation(TranslatePrefs::kPrefTranslateWhitelists);
   translate_prefs.RemoveLanguagePairFromWhitelist("fr", "en");
   SimulateNavigation(GURL("http://www.google.fr"), 3, L"Le Google", "fr");
   EXPECT_FALSE(GetTranslateMessage(&page_id, &original_lang, &target_lang));
   EXPECT_TRUE(GetTranslateInfoBar() != NULL);
+  prefs->RemovePrefObserver(TranslatePrefs::kPrefTranslateWhitelists,
+                            &pref_observer_);
 }
 
 // Context menu.
diff --git a/chrome/browser/translate/translate_prefs.cc b/chrome/browser/translate/translate_prefs.cc
index 1beb0ee..bfbf23c 100644
--- a/chrome/browser/translate/translate_prefs.cc
+++ b/chrome/browser/translate/translate_prefs.cc
@@ -6,6 +6,7 @@
 
 #include "base/string_util.h"
 #include "chrome/browser/pref_service.h"
+#include "chrome/browser/scoped_pref_update.h"
 
 const wchar_t TranslatePrefs::kPrefTranslateLanguageBlacklist[] =
     L"translate_language_blacklist";
@@ -18,7 +19,6 @@ const wchar_t TranslatePrefs::kPrefTranslateWhitelists[] =
 
 TranslatePrefs::TranslatePrefs(PrefService* user_prefs)
     : prefs_(user_prefs) {
-  Register();
 }
 
 bool TranslatePrefs::IsLanguageBlacklisted(
@@ -27,11 +27,13 @@ bool TranslatePrefs::IsLanguageBlacklisted(
 }
 
 void TranslatePrefs::BlacklistLanguage(const std::string& original_language) {
+  ScopedPrefUpdate update(prefs_, kPrefTranslateLanguageBlacklist);
   BlacklistValue(kPrefTranslateLanguageBlacklist, original_language);
 }
 
 void TranslatePrefs::RemoveLanguageFromBlacklist(
     const std::string& original_language) {
+  ScopedPrefUpdate update(prefs_, kPrefTranslateLanguageBlacklist);
   RemoveValueFromBlacklist(kPrefTranslateLanguageBlacklist, original_language);
 }
 
@@ -40,10 +42,12 @@ bool TranslatePrefs::IsSiteBlacklisted(const std::string& site) {
 }
 
 void TranslatePrefs::BlacklistSite(const std::string& site) {
+  ScopedPrefUpdate update(prefs_, kPrefTranslateSiteBlacklist);
   BlacklistValue(kPrefTranslateSiteBlacklist, site);
 }
 
 void TranslatePrefs::RemoveSiteFromBlacklist(const std::string& site) {
+  ScopedPrefUpdate update(prefs_, kPrefTranslateSiteBlacklist);
   RemoveValueFromBlacklist(kPrefTranslateSiteBlacklist, site);
 }
 
@@ -70,6 +74,7 @@ void TranslatePrefs::WhitelistLanguagePair(
     NOTREACHED() << "Unregistered translate whitelist pref";
     return;
   }
+  ScopedPrefUpdate update(prefs_, kPrefTranslateWhitelists);
   std::wstring wide_original(ASCIIToWide(original_language));
   StringValue* language = new StringValue(target_language);
   ListValue* whitelist = NULL;
@@ -92,6 +97,7 @@ void TranslatePrefs::RemoveLanguagePairFromWhitelist(
     NOTREACHED() << "Unregistered translate whitelist pref";
     return;
   }
+  ScopedPrefUpdate update(prefs_, kPrefTranslateWhitelists);
   ListValue* whitelist = NULL;
   std::wstring wide_original(ASCIIToWide(original_language));
   if (dict->GetList(wide_original, &whitelist) && whitelist) {
@@ -121,17 +127,17 @@ bool TranslatePrefs::ShouldAutoTranslate(PrefService* user_prefs,
   return prefs.IsLanguagePairWhitelisted(original_language, target_language);
 }
 
-// TranslatePrefs: private: ----------------------------------------------------
-
-void TranslatePrefs::Register() {
-  if (!prefs_->FindPreference(kPrefTranslateLanguageBlacklist))
-    prefs_->RegisterListPref(kPrefTranslateLanguageBlacklist);
-  if (!prefs_->FindPreference(kPrefTranslateSiteBlacklist))
-    prefs_->RegisterListPref(kPrefTranslateSiteBlacklist);
-  if (!prefs_->FindPreference(kPrefTranslateWhitelists))
-    prefs_->RegisterDictionaryPref(kPrefTranslateWhitelists);
+void TranslatePrefs::RegisterUserPrefs(PrefService* user_prefs) {
+  if (!user_prefs->FindPreference(kPrefTranslateLanguageBlacklist))
+    user_prefs->RegisterListPref(kPrefTranslateLanguageBlacklist);
+  if (!user_prefs->FindPreference(kPrefTranslateSiteBlacklist))
+    user_prefs->RegisterListPref(kPrefTranslateSiteBlacklist);
+  if (!user_prefs->FindPreference(kPrefTranslateWhitelists))
+    user_prefs->RegisterDictionaryPref(kPrefTranslateWhitelists);
 }
 
+// TranslatePrefs: private: ----------------------------------------------------
+
 bool TranslatePrefs::IsValueInList(const ListValue* list,
     const std::string& in_value) {
   for (size_t i = 0; i < list->GetSize(); ++i) {
diff --git a/chrome/browser/translate/translate_prefs.h b/chrome/browser/translate/translate_prefs.h
index 01a93c1..5cd334d 100644
--- a/chrome/browser/translate/translate_prefs.h
+++ b/chrome/browser/translate/translate_prefs.h
@@ -41,9 +41,9 @@ class TranslatePrefs {
   static bool ShouldAutoTranslate(PrefService* user_prefs,
       const std::string& original_language,
       const std::string& target_language);
+  static void RegisterUserPrefs(PrefService* user_prefs);
 
  private:
-  void Register();
   bool IsValueBlacklisted(const wchar_t* pref_id, const std::string& value);
   void BlacklistValue(const wchar_t* pref_id, const std::string& value);
   void RemoveValueFromBlacklist(const wchar_t* pref_id,