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

chrome/browser/chromeos/locale_change_guard.cc

 // Copyright (c) 2011 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.
 
 #include "chrome/browser/chromeos/locale_change_guard.h"
 
 #include "base/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/notifications/system_notification.h"
@@ skipping 48 matching lines @@
   }
   if (reverted_)
     return;
 
   PrefService* prefs = tab_contents_->profile()->GetPrefs();
   if (prefs == NULL)
     return;
 
   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);
   if (browser)
     browser->ExecuteCommand(IDC_EXIT);
 }
 
 void LocaleChangeGuard::CheckLocaleChange(TabContents* tab_contents) {
   // We want notification to be shown no more than once per session.
   if (note_ != NULL)
     return;
   DCHECK(note_ == NULL && tab_contents_ == NULL);
   DCHECK(from_locale_.empty() && to_locale_.empty());
 
   // We check profile Id because:
   // (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();
   if (cur_locale.empty()) {
     NOTREACHED();
     return;
   }
 
   PrefService* prefs = tab_contents->profile()->GetPrefs();
   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.
     return;
   }
 
   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;
   to_locale_ = to_locale;
   tab_contents_ = tab_contents;
   note_.reset(new chromeos::SystemNotification(
       tab_contents->profile(),
       new Delegate(this),
       IDR_DEFAULT_FAVICON,
       l10n_util::GetStringUTF16(
@@ skipping 18 matching lines @@
     return;
   }
 
   // Check whether locale has been reverted or changed.
   // If not: mark current locale as accepted.
   if (reverted_)
     return;
   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"));
   prefs->SetString(prefs::kApplicationLocaleBackup, to_locale_);
   prefs->SetString(prefs::kApplicationLocaleAccepted, to_locale_);
   prefs->ScheduleSavePersistentPrefs();
 }
 
 // static
 void LocaleChangeGuard::Check(TabContents* tab_contents) {
   g_locale_change_guard.Get().CheckLocaleChange(tab_contents);
 }
 
 void LocaleChangeGuard::Delegate::Close(bool by_user) {
   if (by_user)
     master_->AcceptLocaleChange();
 }
 
 std::string LocaleChangeGuard::Delegate::id() const {
   // Arbitrary unique Id.
   return "8c386938-1e3f-11e0-ac7b-18a90520e2e5";
 }
 
 }  // namespace chromeos