ChromeOS: should not show "Language changed" notification for certain languages.

BUG=317718
TEST=manually tested

Review URL: https://codereview.chromium.org/382973002

Cr-Commit-Position: refs/heads/master@{#289985}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289985 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/locale_change_guard.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/chromeos/locale_change_guard.h"
 
+#include <algorithm>
+
 #include "ash/shell.h"
 #include "ash/system/tray/system_tray.h"
 #include "ash/system/tray/system_tray_notifier.h"
@@ -33,6 +35,17 @@ using content::WebContents;
 
 namespace chromeos {
 
+namespace {
+
+// This is the list of languages that do not require user notification when
+// locale is switched automatically between regions within the same language.
+//
+// New language in kAcceptLanguageList should be added either here or to
+// to the exception list in unit test.
+const char* const kSkipShowNotificationLanguages[4] = {"en", "de", "fr", "it"};
+
+}  // anonymous namespace
+
 LocaleChangeGuard::LocaleChangeGuard(Profile* profile)
     : profile_(profile),
       reverted_(false),
@@ -153,7 +166,11 @@ void LocaleChangeGuard::Check() {
   if (prefs->GetString(prefs::kApplicationLocaleAccepted) == to_locale)
     return;  // Already accepted.
 
-  // Locale change detected, showing notification.
+  // Locale change detected.
+  if (!ShouldShowLocaleChangeNotification(from_locale, to_locale))
+    return;
+
+  // Showing notification.
   if (from_locale_ != from_locale || to_locale_ != to_locale) {
     // Falling back to showing message in current locale.
     LOG(ERROR) <<
@@ -212,4 +229,35 @@ void LocaleChangeGuard::PrepareChangingLocale(
   }
 }
 
+// static
+bool LocaleChangeGuard::ShouldShowLocaleChangeNotification(
+    const std::string& from_locale,
+    const std::string& to_locale) {
+  const std::string from_lang = l10n_util::GetLanguage(from_locale);
+  const std::string to_lang = l10n_util::GetLanguage(to_locale);
+
+  if (from_locale == to_locale)
+    return false;
+
+  if (from_lang != to_lang)
+    return true;
+
+  const char* const* begin = kSkipShowNotificationLanguages;
+  const char* const* end = kSkipShowNotificationLanguages +
+                           arraysize(kSkipShowNotificationLanguages);
+
+  return std::find(begin, end, from_lang) == end;
+}
+
+// static
+const char* const*
+LocaleChangeGuard::GetSkipShowNotificationLanguagesForTesting() {
+  return kSkipShowNotificationLanguages;
+}
+
+// static
+size_t LocaleChangeGuard::GetSkipShowNotificationLanguagesSizeForTesting() {
+  return arraysize(kSkipShowNotificationLanguages);
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/locale_change_guard.h

@@ -9,6 +9,7 @@
 
 #include "ash/system/locale/locale_observer.h"
 #include "base/compiler_specific.h"
+#include "base/gtest_prod_util.h"
 #include "base/lazy_instance.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -47,6 +48,11 @@ class LocaleChangeGuard : public content::NotificationObserver,
   void OnLogin();
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(LocaleChangeGuardTest,
+                           ShowNotificationLocaleChanged);
+  FRIEND_TEST_ALL_PREFIXES(LocaleChangeGuardTest,
+                           ShowNotificationLocaleChangedList);
+
   void RevertLocaleChangeCallback(const base::ListValue* list);
   void Check();
 
@@ -55,6 +61,13 @@ class LocaleChangeGuard : public content::NotificationObserver,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
+  // Returns true if we should notify user about automatic locale change.
+  static bool ShouldShowLocaleChangeNotification(const std::string& from_locale,
+                                                 const std::string& to_locale);
+
+  static const char* const* GetSkipShowNotificationLanguagesForTesting();
+  static size_t GetSkipShowNotificationLanguagesSizeForTesting();
+
   std::string from_locale_;
   std::string to_locale_;
   Profile* profile_;

chrome/browser/chromeos/locale_change_guard_unittest.cc

+// Copyright 2014 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 <string.h>
+
+#include "base/macros.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/l10n/l10n_util.h"
+
+namespace {
+
+// These languages require user notification when locale is automatically
+// switched between different regions within the same language.
+const char* const kShowNotificationLanguages[] = {
+    "af",   // Afrikaans
+    "am",   // Amharic
+    "ar",   // Arabic
+    "az",   // Azerbaijani
+    "be",   // Belarusian
+    "bg",   // Bulgarian
+    "bh",   // Bihari
+    "bn",   // Bengali
+    "br",   // Breton
+    "bs",   // Bosnian
+    "ca",   // Catalan
+    "co",   // Corsican
+    "cs",   // Czech
+    "cy",   // Welsh
+    "da",   // Danish
+    "el",   // Greek
+    "eo",   // Esperanto
+    "es",   // Spanish
+    "et",   // Estonian
+    "eu",   // Basque
+    "fa",   // Persian
+    "fi",   // Finnish
+    "fil",  // Filipino
+    "fo",   // Faroese
+    "fy",   // Frisian
+    "ga",   // Irish
+    "gd",   // Scots Gaelic
+    "gl",   // Galician
+    "gn",   // Guarani
+    "gu",   // Gujarati
+    "ha",   // Hausa
+    "haw",  // Hawaiian
+    "he",   // Hebrew
+    "hi",   // Hindi
+    "hr",   // Croatian
+    "hu",   // Hungarian
+    "hy",   // Armenian
+    "ia",   // Interlingua
+    "id",   // Indonesian
+    "is",   // Icelandic
+    "ja",   // Japanese
+    "jw",   // Javanese
+    "ka",   // Georgian
+    "kk",   // Kazakh
+    "km",   // Cambodian
+    "kn",   // Kannada
+    "ko",   // Korean
+    "ku",   // Kurdish
+    "ky",   // Kyrgyz
+    "la",   // Latin
+    "ln",   // Lingala
+    "lo",   // Laothian
+    "lt",   // Lithuanian
+    "lv",   // Latvian
+    "mk",   // Macedonian
+    "ml",   // Malayalam
+    "mn",   // Mongolian
+    "mo",   // Moldavian
+    "mr",   // Marathi
+    "ms",   // Malay
+    "mt",   // Maltese
+    "nb",   // Norwegian (Bokmal)
+    "ne",   // Nepali
+    "nl",   // Dutch
+    "nn",   // Norwegian (Nynorsk)
+    "no",   // Norwegian
+    "oc",   // Occitan
+    "om",   // Oromo
+    "or",   // Oriya
+    "pa",   // Punjabi
+    "pl",   // Polish
+    "ps",   // Pashto
+    "pt",   // Portuguese
+    "qu",   // Quechua
+    "rm",   // Romansh
+    "ro",   // Romanian
+    "ru",   // Russian
+    "sd",   // Sindhi
+    "sh",   // Serbo-Croatian
+    "si",   // Sinhalese
+    "sk",   // Slovak
+    "sl",   // Slovenian
+    "sn",   // Shona
+    "so",   // Somali
+    "sq",   // Albanian
+    "sr",   // Serbian
+    "st",   // Sesotho
+    "su",   // Sundanese
+    "sv",   // Swedish
+    "sw",   // Swahili
+    "ta",   // Tamil
+    "te",   // Telugu
+    "tg",   // Tajik
+    "th",   // Thai
+    "ti",   // Tigrinya
+    "tk",   // Turkmen
+    "to",   // Tonga
+    "tr",   // Turkish
+    "tt",   // Tatar
+    "tw",   // Twi
+    "ug",   // Uighur
+    "uk",   // Ukrainian
+    "ur",   // Urdu
+    "uz",   // Uzbek
+    "vi",   // Vietnamese
+    "xh",   // Xhosa
+    "yi",   // Yiddish
+    "yo",   // Yoruba
+    "zh",   // Chinese
+    "zu",   // Zulu
+};
+
+}  // anonymous namespace
+
+namespace chromeos {
+
+TEST(LocaleChangeGuardTest, ShowNotificationLocaleChanged) {
+  // "en" is used as "global default" in many places.
+  EXPECT_TRUE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("en", "it"));
+  EXPECT_TRUE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("it", "en"));
+
+  // Between two latin locales.
+  EXPECT_TRUE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("fr", "it"));
+  EXPECT_TRUE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("it", "fr"));
+
+  // en <-> non-latin locale
+  EXPECT_TRUE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("en", "zh"));
+  EXPECT_TRUE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("zh", "en"));
+
+  // latin <-> non-latin locale
+  EXPECT_TRUE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("fr", "zh"));
+  EXPECT_TRUE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("zh", "fr"));
+
+  // same language
+  EXPECT_FALSE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("en", "en"));
+  EXPECT_FALSE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("fr", "fr"));
+  EXPECT_FALSE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("zh", "zh"));
+  EXPECT_FALSE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("en", "en-US"));
+  EXPECT_FALSE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("en-GB", "en-US"));
+
+  // Different regions within the same language
+  EXPECT_FALSE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("en", "en-au"));
+  EXPECT_FALSE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("en-AU", "en"));
+  EXPECT_FALSE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("en-AU", "en-GB"));
+
+  EXPECT_TRUE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("zh", "zh-CN"));
+  EXPECT_TRUE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("zh-CN", "zh-TW"));
+  EXPECT_TRUE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("es", "es-419"));
+  EXPECT_TRUE(
+      LocaleChangeGuard::ShouldShowLocaleChangeNotification("es", "es-ES"));
+}
+
+TEST(LocaleChangeGuardTest, ShowNotificationLocaleChangedList) {
+  for (size_t i = 0; i < l10n_util::GetAcceptLanguageListSizeForTesting();
+       ++i) {
+    const char* const locale = l10n_util::GetAcceptLanguageListForTesting()[i];
+    const char* const dash = strchr(locale, '-');
+    const std::string language =
+        (dash ? std::string(locale, dash - locale) : std::string(locale));
+
+    const char* const* allowed_begin = kShowNotificationLanguages;
+    const char* const* allowed_end =
+        kShowNotificationLanguages + arraysize(kShowNotificationLanguages);
+    const bool notification_allowed =
+        (std::find(allowed_begin, allowed_end, language) != allowed_end);
+
+    const char* const* skipped_begin =
+        LocaleChangeGuard::GetSkipShowNotificationLanguagesForTesting();
+    const char* const* skipped_end =
+        skipped_begin +
+        LocaleChangeGuard::GetSkipShowNotificationLanguagesSizeForTesting();
+    const bool notification_skipped =
+        (std::find(skipped_begin, skipped_end, language) != skipped_end);
+
+    EXPECT_TRUE(notification_allowed ^ notification_skipped)
+        << "Language '" << language << "' (from locale '" << locale
+        << "') must be in exactly one list: either "
+           "kSkipShowNotificationLanguages (found=" << notification_skipped
+        << ") or kShowNotificationLanguages (found=" << notification_allowed
+        << ").";
+  }
+}
+
+}  // namespace chromeos

chrome/browser/ui/webui/chromeos/login/l10n_util.cc

@@ -126,10 +126,10 @@ scoped_ptr<base::ListValue> GetLanguageList(
        it != language_index.end(); ++it) {
     const std::string& language_id = it->first;
 
-    const size_t dash_pos = language_id.find_first_of('-');
+    const std::string lang = l10n_util::GetLanguage(language_id);
 
     // Ignore non-specific codes.
-    if (dash_pos == std::string::npos || dash_pos == 0)
+    if (lang.empty() || lang == language_id)
       continue;
 
     if (std::find(base_language_codes.begin(),

chrome/chrome_tests_unit.gypi

@@ -737,6 +737,7 @@
         'browser/chromeos/input_method/input_method_util_unittest.cc',
         'browser/chromeos/kiosk_mode/kiosk_mode_idle_logout_unittest.cc',
         'browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc',
+        'browser/chromeos/locale_change_guard_unittest.cc',
         'browser/chromeos/login/auth/online_attempt_unittest.cc',
         'browser/chromeos/login/auth/parallel_authenticator_unittest.cc',
         'browser/chromeos/login/existing_user_controller_auto_login_unittest.cc',

ui/base/l10n/l10n_util.cc

@@ -311,6 +311,11 @@ std::string GetCanonicalLocale(const std::string& locale) {
   return base::i18n::GetCanonicalLocale(locale.c_str());
 }
 
+std::string GetLanguage(const std::string& locale) {
+  const std::string::size_type hyphen_pos = locale.find('-');
+  return std::string(locale, 0, hyphen_pos);
+}
+
 bool CheckAndResolveLocale(const std::string& locale,
                            std::string* resolved_locale) {
 #if defined(OS_MACOSX)
@@ -334,10 +339,9 @@ bool CheckAndResolveLocale(const std::string& locale,
   // does not support but available on Windows. We fall
   // back to en-US in GetApplicationLocale so that it's a not critical,
   // but we can do better.
-  std::string::size_type hyphen_pos = locale.find('-');
-  std::string lang(locale, 0, hyphen_pos);
-  if (hyphen_pos != std::string::npos && hyphen_pos > 0) {
-    std::string region(locale, hyphen_pos + 1);
+  const std::string lang(GetLanguage(locale));
+  if (lang.size() < locale.size()) {
+    std::string region(locale, lang.size() + 1);
     std::string tmp_locale(lang);
     // Map es-RR other than es-ES to es-419 (Chrome's Latin American
     // Spanish locale).
@@ -877,4 +881,12 @@ int GetLocalizedContentsWidthInPixels(int pixel_resource_id) {
   return width;
 }
 
+const char* const* GetAcceptLanguageListForTesting() {
+  return kAcceptLanguageList;
+}
+
+size_t GetAcceptLanguageListSizeForTesting() {
+  return arraysize(kAcceptLanguageList);
+}
+
 }  // namespace l10n_util

ui/base/l10n/l10n_util.h

@@ -24,6 +24,9 @@ namespace l10n_util {
 // std::string as an argument.
 UI_BASE_EXPORT std::string GetCanonicalLocale(const std::string& locale);
 
+// Takes normalized locale as |locale|. Returns language part (before '-').
+UI_BASE_EXPORT std::string GetLanguage(const std::string& locale);
+
 // This method translates a generic locale name to one of the locally defined
 // ones. This method returns true if it succeeds.
 UI_BASE_EXPORT bool CheckAndResolveLocale(const std::string& locale,
@@ -185,6 +188,10 @@ UI_BASE_EXPORT void GetAcceptLanguagesForLocale(
 // designer given constraints which might dependent on the language used.
 UI_BASE_EXPORT int GetLocalizedContentsWidthInPixels(int pixel_resource_id);
 
+const char* const* GetAcceptLanguageListForTesting();
+
+size_t GetAcceptLanguageListSizeForTesting();
+
 }  // namespace l10n_util
 
 #endif  // UI_BASE_L10N_L10N_UTIL_H_