Do not use local override for language settings: always store it into synchronized preference.

BUG=chromium-os:11147
TEST=Manual

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

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

chrome/browser/chromeos/locale_change_guard.cc

@@ -66,10 +66,8 @@ void LocaleChangeGuard::RevertLocaleChange(const ListValue* list) {
 
   reverted_ = true;
   UserMetrics::RecordAction(UserMetricsAction("LanguageChange_Revert"));
-  tab_contents_->profile()->ChangeApplicationLocale(from_locale_, true);
-  prefs->SetString(prefs::kApplicationLocaleBackup, from_locale_);
-  prefs->ClearPref(prefs::kApplicationLocaleAccepted);
-  prefs->ScheduleSavePersistentPrefs();
+  tab_contents_->profile()->ChangeAppLocale(
+      from_locale_, Profile::APP_LOCALE_CHANGED_VIA_REVERT);
 
   Browser* browser = Browser::GetBrowserForController(
       &tab_contents_->controller(), NULL);
@@ -88,10 +86,8 @@ void LocaleChangeGuard::CheckLocaleChange(TabContents* tab_contents) {
   // (1) we want to exit fast in common case when nothing should be done.
   // (2) on ChromeOS this guard may be invoked for a dummy profile first time.
   ProfileId cur_profile_id = tab_contents->profile()->GetRuntimeId();
-  if (cur_profile_id == profile_id_) {
-    // We have already checked this profile, exiting fast.
-    return;
-  }
+  if (cur_profile_id == profile_id_)
+    return;  // We have already checked this profile, exiting fast.
   profile_id_ = cur_profile_id;
 
   std::string cur_locale = g_browser_process->GetApplicationLocale();
@@ -104,13 +100,7 @@ void LocaleChangeGuard::CheckLocaleChange(TabContents* tab_contents) {
   if (prefs == NULL)
     return;
 
-  std::string to_locale = prefs->GetString(prefs::kApplicationLocaleOverride);
-  if (!to_locale.empty()) {
-    DCHECK(to_locale == cur_locale);
-    return;
-  }
-
-  to_locale = prefs->GetString(prefs::kApplicationLocale);
+  std::string to_locale = prefs->GetString(prefs::kApplicationLocale);
   if (to_locale != cur_locale) {
     // This conditional branch can occur in case kApplicationLocale
     // preference was modified by synchronization.
@@ -118,10 +108,11 @@ void LocaleChangeGuard::CheckLocaleChange(TabContents* tab_contents) {
   }
 
   std::string from_locale = prefs->GetString(prefs::kApplicationLocaleBackup);
-  if (from_locale.empty() || from_locale == to_locale) {
-    // No locale change was detected, just exit.
-    return;
-  }
+  if (from_locale.empty() || from_locale == to_locale)
+    return;  // No locale change was detected, just exit.
+
+  if (prefs->GetString(prefs::kApplicationLocaleAccepted) == to_locale)
+    return;  // Already accepted.
 
   // Locale change detected, showing notification.
   from_locale_ = from_locale;
@@ -160,10 +151,6 @@ void LocaleChangeGuard::AcceptLocaleChange() {
   PrefService* prefs = tab_contents_->profile()->GetPrefs();
   if (prefs == NULL)
     return;
-  std::string override_locale =
-      prefs->GetString(prefs::kApplicationLocaleOverride);
-  if (!override_locale.empty())
-    return;
   if (prefs->GetString(prefs::kApplicationLocale) != to_locale_)
     return;
   UserMetrics::RecordAction(UserMetricsAction("LanguageChange_Accept"));

chrome/browser/chromeos/login/login_utils.cc

@@ -155,7 +155,7 @@ class LoginUtilsImpl : public LoginUtils {
 
  private:
   // Check user's profile for kApplicationLocale setting.
-  void RespectLocalePreference(PrefService* pref);
+  void RespectLocalePreference(Profile* pref);
 
   // Indicates if DoBrowserLaunch will actually launch the browser or not.
   bool browser_launch_enabled_;
@@ -309,7 +309,7 @@ void LoginUtilsImpl::CompleteLogin(
   }
   btl->AddLoginTimeMarker("TPMOwned", false);
 
-  RespectLocalePreference(profile->GetPrefs());
+  RespectLocalePreference(profile);
 
   if (first_login) {
     SetFirstLoginPrefs(profile->GetPrefs());
@@ -354,53 +354,21 @@ void LoginUtilsImpl::FetchTokens(
   }
 }
 
-void LoginUtilsImpl::RespectLocalePreference(PrefService* prefs) {
+void LoginUtilsImpl::RespectLocalePreference(Profile* profile) {
+  DCHECK(profile != NULL);
+  PrefService* prefs = profile->GetPrefs();
   DCHECK(prefs != NULL);
-  std::string pref_locale_override =
-      prefs->GetString(prefs::kApplicationLocaleOverride);
-  if (!pref_locale_override.empty()) {
-    LanguageSwitchMenu::SwitchLanguage(pref_locale_override);
-    return;
-  }
-
   if (g_browser_process == NULL)
     return;
-  std::string cur_locale = g_browser_process->GetApplicationLocale();
 
   std::string pref_locale = prefs->GetString(prefs::kApplicationLocale);
-  if (!pref_locale.empty()) {
-    if (prefs->GetString(prefs::kApplicationLocaleAccepted) == pref_locale) {
-      // If locale is accepted then we do not want to show LocaleChange
-      // notification.  This notification is triggered by different values of
-      // kApplicationLocaleBackup and kApplicationLocale preferences,
-      // so make them identical.
-      prefs->SetString(prefs::kApplicationLocaleBackup, pref_locale);
-    } else {
-      std::string pref_locale_backup =
-          prefs->GetString(prefs::kApplicationLocaleBackup);
-      if (pref_locale_backup != cur_locale) {
-        if (pref_locale_backup == pref_locale || pref_locale_backup.empty()) {
-          prefs->SetString(prefs::kApplicationLocaleBackup, cur_locale);
-        }
-      }
-    }
-    LanguageSwitchMenu::SwitchLanguage(pref_locale);
-    return;
-  }
-  // Profile synchronization takes time and is not completed at that moment
-  // at first login.  So we initialize locale preference in steps:
-  // (1) first save it to temporary backup;
-  // (2) on next login we assume that synchronization is already completed
-  //     and we may finalize initialization.
-  std::string pref_locale_backup =
-      prefs->GetString(prefs::kApplicationLocaleBackup);
-  prefs->SetString(prefs::kApplicationLocaleBackup, cur_locale);
-  prefs->ScheduleSavePersistentPrefs();
-  if (!pref_locale_backup.empty()) {
-    prefs->SetString(prefs::kApplicationLocale, pref_locale_backup);
-    prefs->ScheduleSavePersistentPrefs();
-    LanguageSwitchMenu::SwitchLanguage(pref_locale_backup);
-  }
+  if (pref_locale.empty())
+    pref_locale = prefs->GetString(prefs::kApplicationLocaleBackup);
+  if (pref_locale.empty())
+    pref_locale = g_browser_process->GetApplicationLocale();
+  DCHECK(!pref_locale.empty());
+  profile->ChangeAppLocale(pref_locale, Profile::APP_LOCALE_CHANGED_VIA_LOGIN);
+  LanguageSwitchMenu::SwitchLanguage(pref_locale);
 }
 
 void LoginUtilsImpl::CompleteOffTheRecordLogin(const GURL& start_url) {

chrome/browser/dom_ui/options/language_options_handler.cc

@@ -383,7 +383,8 @@ void LanguageOptionsHandler::UiLanguageChangeCallback(
       "LanguageOptions_UiLanguageChange_%s", language_code.c_str());
   UserMetrics::RecordComputedAction(action);
 #if defined(OS_CHROMEOS)
-  dom_ui_->GetProfile()->ChangeApplicationLocale(language_code, false);
+  dom_ui_->GetProfile()->ChangeAppLocale(
+      language_code, Profile::APP_LOCALE_CHANGED_VIA_SETTINGS);
 #else
   PrefService* pref_service = g_browser_process->local_state();
   pref_service->SetString(prefs::kApplicationLocale, language_code);

chrome/browser/profiles/profile.cc

@@ -114,7 +114,6 @@ void Profile::RegisterUserPrefs(PrefService* prefs) {
   // in user's profile for other platforms as well.
   prefs->RegisterStringPref(prefs::kApplicationLocale, "");
   prefs->RegisterStringPref(prefs::kApplicationLocaleBackup, "");
-  prefs->RegisterStringPref(prefs::kApplicationLocaleOverride, "");
   prefs->RegisterStringPref(prefs::kApplicationLocaleAccepted, "");
 #endif
 }
@@ -614,8 +613,7 @@ class OffTheRecordProfileImpl : public Profile,
   }
 
 #if defined(OS_CHROMEOS)
-  virtual void ChangeApplicationLocale(
-      const std::string& locale, bool keep_local) {
+  virtual void ChangeAppLocale(const std::string& locale, AppLocaleChangedVia) {
   }
 #endif  // defined(OS_CHROMEOS)
 

chrome/browser/profiles/profile.h

@@ -486,10 +486,20 @@ class Profile {
   virtual policy::ProfilePolicyContext* GetPolicyContext() = 0;
 
 #if defined(OS_CHROMEOS)
-  // Changes application locale.
-  // "Keep local" means that changes should not be propagated to other devices.
-  virtual void ChangeApplicationLocale(
-      const std::string& locale, bool keep_local) = 0;
+  enum AppLocaleChangedVia {
+    // Caused by chrome://settings change.
+    APP_LOCALE_CHANGED_VIA_SETTINGS,
+    // Locale has been reverted via LocaleChangeGuard.
+    APP_LOCALE_CHANGED_VIA_REVERT,
+    // From login screen.
+    APP_LOCALE_CHANGED_VIA_LOGIN,
+    // Source unknown.
+    APP_LOCALE_CHANGED_VIA_UNKNOWN
+  };
+
+  // Changes application locale for a profile.
+  virtual void ChangeAppLocale(
+      const std::string& locale, AppLocaleChangedVia via) = 0;
 
   // Returns ChromeOS's ProxyConfigServiceImpl, creating if not yet created.
   virtual chromeos::ProxyConfigServiceImpl*

chrome/browser/profiles/profile_impl.cc

@@ -1372,28 +1372,75 @@ PromoCounter* ProfileImpl::GetInstantPromoCounter() {
 }
 
 #if defined(OS_CHROMEOS)
-void ProfileImpl::ChangeApplicationLocale(
-    const std::string& locale, bool keep_local) {
-  if (locale.empty()) {
+void ProfileImpl::ChangeAppLocale(
+    const std::string& new_locale, AppLocaleChangedVia via) {
+  if (new_locale.empty()) {
     NOTREACHED();
     return;
   }
-  if (keep_local) {
-    GetPrefs()->SetString(prefs::kApplicationLocaleOverride, locale);
-  } else {
-    GetPrefs()->SetString(prefs::kApplicationLocale, locale);
-    GetPrefs()->ClearPref(prefs::kApplicationLocaleOverride);
+  std::string pref_locale = GetPrefs()->GetString(prefs::kApplicationLocale);
+  bool do_update_pref = true;
+  switch (via) {
+    case APP_LOCALE_CHANGED_VIA_SETTINGS:
+    case APP_LOCALE_CHANGED_VIA_REVERT: {
+      // We keep kApplicationLocaleBackup value as a reference.  In case value
+      // of kApplicationLocale preference would change due to sync from other
+      // device then kApplicationLocaleBackup value will trigger and allow us to
+      // show notification about automatic locale change in LocaleChangeGuard.
+      GetPrefs()->SetString(prefs::kApplicationLocaleBackup, new_locale);
+      GetPrefs()->ClearPref(prefs::kApplicationLocaleAccepted);
+      // We maintain kApplicationLocale property in both a global storage
+      // and user's profile.  Global property determines locale of login screen,
+      // while user's profile determines his personal locale preference.
+      // In case of APP_LOCALE_CHANGED_VIA_LOGIN we won't touch local state
+      // because login screen code is active and takes care of it.
+      g_browser_process->local_state()->SetString(
+          prefs::kApplicationLocale, new_locale);
+      break;
+    }
+    case APP_LOCALE_CHANGED_VIA_LOGIN: {
+      if (!pref_locale.empty()) {
+        DCHECK(pref_locale == new_locale);
+        std::string accepted_locale =
+            GetPrefs()->GetString(prefs::kApplicationLocaleAccepted);
+        if (accepted_locale == new_locale) {
+          // If locale is accepted then we do not want to show LocaleChange
+          // notification.  This notification is triggered by different values
+          // of kApplicationLocaleBackup and kApplicationLocale preferences,
+          // so make them identical.
+          GetPrefs()->SetString(prefs::kApplicationLocaleBackup, new_locale);
+        } else {
+          // Back up locale of login screen.
+          GetPrefs()->SetString(prefs::kApplicationLocaleBackup,
+                                g_browser_process->GetApplicationLocale());
+        }
+      } else {
+        std::string cur_locale = g_browser_process->GetApplicationLocale();
+        std::string backup_locale =
+            GetPrefs()->GetString(prefs::kApplicationLocaleBackup);
+        // Profile synchronization takes time and is not completed at that
+        // moment at first login.  So we initialize locale preference in steps:
+        // (1) first save it to temporary backup;
+        // (2) on next login we assume that synchronization is already completed
+        //     and we may finalize initialization.
+        GetPrefs()->SetString(prefs::kApplicationLocaleBackup, cur_locale);
+        if (!backup_locale.empty())
+          GetPrefs()->SetString(prefs::kApplicationLocale, backup_locale);
+        do_update_pref = false;
+      }
+      break;
+    }
+    case APP_LOCALE_CHANGED_VIA_UNKNOWN:
+    default: {
+      NOTREACHED();
+      break;
+    }
   }
-  GetPrefs()->SetString(prefs::kApplicationLocaleBackup, locale);
-  GetPrefs()->ClearPref(prefs::kApplicationLocaleAccepted);
-  // We maintain kApplicationLocale property in both a global storage
-  // and user's profile.  Global property determines locale of login screen,
-  // while user's profile determines his personal locale preference.
-  g_browser_process->local_state()->SetString(
-      prefs::kApplicationLocale, locale);
-
-  GetPrefs()->SavePersistentPrefs();
-  g_browser_process->local_state()->SavePersistentPrefs();
+  if (do_update_pref)
+    GetPrefs()->SetString(prefs::kApplicationLocale, new_locale);
+
+  GetPrefs()->ScheduleSavePersistentPrefs();
+  g_browser_process->local_state()->ScheduleSavePersistentPrefs();
 }
 
 chromeos::ProxyConfigServiceImpl*

chrome/browser/profiles/profile_impl.h

@@ -130,8 +130,7 @@ class ProfileImpl : public Profile,
   virtual policy::ProfilePolicyContext* GetPolicyContext();
 
 #if defined(OS_CHROMEOS)
-  virtual void ChangeApplicationLocale(
-      const std::string& locale, bool keep_local);
+  virtual void ChangeAppLocale(const std::string& locale, AppLocaleChangedVia);
   virtual chromeos::ProxyConfigServiceImpl* GetChromeOSProxyConfigServiceImpl();
   virtual void SetupChromeOSEnterpriseExtensionObserver();
 #endif  // defined(OS_CHROMEOS)

chrome/browser/sync/glue/preference_model_associator.cc

@@ -32,8 +32,7 @@ PreferenceModelAssociator::PreferenceModelAssociator(
   // 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) {
+  for (size_t i = 0; i < arraysize(kSynchronizedPreferences); ++i) {
     if (pref_service->FindPreference(kSynchronizedPreferences[i]))
       synced_preferences_.insert(kSynchronizedPreferences[i]);
   }

chrome/common/pref_names.cc

@@ -46,16 +46,36 @@ const char kURLsToRestoreOnStartup[] = "session.urls_to_restore_on_startup";
 // while user's profile determines his personal locale preference.
 const char kApplicationLocale[] = "intl.app_locale";
 #if defined(OS_CHROMEOS)
-// Non-syncable item.  Used to detect locale change.
-// Used for two-step initialization of locale in ChromeOS
-// because synchronization of kApplicationLocale is not instant.
-const char kApplicationLocaleBackup[] = "intl.app_locale_backup";
-// Non-syncable item.
-// Used to locally override synchronized kApplicationLocale preference.
-const char kApplicationLocaleOverride[] = "intl.app_locale_override";
 // Locale accepted by user.  Non-syncable.
 // Used to determine whether we need to show Locale Change notification.
 const char kApplicationLocaleAccepted[] = "intl.app_locale_accepted";
+// Non-syncable item.
+// It is used in two distinct ways.
+// (1) Used for two-step initialization of locale in ChromeOS
+//     because synchronization of kApplicationLocale is not instant.
+// (2) Used to detect locale change.  Locale change is detected by
+//     LocaleChangeGuard in case values of kApplicationLocaleBackup and
+//     kApplicationLocale are both non-empty and differ.
+// Following is a table showing how state of those prefs may change upon
+// common real-life use cases:
+//                                  AppLocale Backup Accepted
+// Initial login                       -        A       -
+// Sync                                B        A       -
+// Accept (B)                          B        B       B
+// -----------------------------------------------------------
+// Initial login                       -        A       -
+// No sync and second login            A        A       -
+// Change options                      B        B       -
+// -----------------------------------------------------------
+// Initial login                       -        A       -
+// Sync                                A        A       -
+// Locale changed on login screen      A        C       -
+// Accept (A)                          A        A       A
+// -----------------------------------------------------------
+// Initial login                       -        A       -
+// Sync                                B        A       -
+// Revert                              A        A       -
+const char kApplicationLocaleBackup[] = "intl.app_locale_backup";
 #endif
 
 // The default character encoding to assume for a web page in the

chrome/common/pref_names.h

@@ -27,7 +27,6 @@ extern const char kURLsToRestoreOnStartup[];
 extern const char kApplicationLocale[];
 #if defined(OS_CHROMEOS)
 extern const char kApplicationLocaleBackup[];
-extern const char kApplicationLocaleOverride[];
 extern const char kApplicationLocaleAccepted[];
 #endif
 

chrome/test/testing_profile.h

@@ -298,7 +298,7 @@ class TestingProfile : public Profile {
   }
   virtual void SetupChromeOSEnterpriseExtensionObserver() {
   }
-  virtual void ChangeApplicationLocale(const std::string&, bool) {
+  virtual void ChangeAppLocale(const std::string&, AppLocaleChangedVia) {
   }
 #endif  // defined(OS_CHROMEOS)