Do not use local override for language settings: always sync.

chrome/browser/chromeos/locale_change_guard.cc

Index: chrome/browser/chromeos/locale_change_guard.cc
diff --git a/chrome/browser/chromeos/locale_change_guard.cc b/chrome/browser/chromeos/locale_change_guard.cc
index fe9cf9598f57fdb455a97cba7580f9c1d656f2b9..d3b5dc6b07348cb362f765d30a8d5418d690d500 100644
--- a/chrome/browser/chromeos/locale_change_guard.cc
+++ b/chrome/browser/chromeos/locale_change_guard.cc
@@ -66,10 +66,7 @@ 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()->ChangeApplicationLocale(from_locale_);

[Nikita (slow), 2011/01/25 14:35:31]

In this case should we clear LocaleBackup setting?
Why it's needed?

[Denis Lagno, 2011/01/25 15:46:43]

ChangeApplicationLocale will set both kApplicationLocaleBackup and
kApplicationLocale to current locale.

value of kApplicationLocaleBackup steps into significance only in 2 cases:

(1) Backup is non-empty and kApplicationLocale is empty (during initial
bootstrap of kApplicationLocale on first logins).

(2) both Backup and kApplicationLocale are non-empty and differ (in that case
notification may be shown).

 
   Browser* browser = Browser::GetBrowserForController(
       &tab_contents_->controller(), NULL);
@@ -88,10 +85,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 +99,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 +107,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)

[Nikita (slow), 2011/01/25 14:35:31]

Could you describe case which we're optimizing here?

[Denis Lagno, 2011/01/25 15:46:43]

it is not optimization, it is double check.
This code is here to make it clear (and ensure) that we do not show notification
if notification (with the same language) was already accepted.

It is double check because login_utils.cc contains code:

  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);


so, if current language was accepted then Backup == kApplicationLocale, so
previous check will return.


(optimization is the first check:
  if (cur_profile_id == profile_id_)
    return;  // We have already checked this profile, exiting fast.

+    return;  // Already accepted.
 
   // Locale change detected, showing notification.
   from_locale_ = from_locale;
@@ -160,10 +150,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"));