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

chrome/browser/chromeos/locale_change_guard.cc

 // Copyright (c) 2012 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 <algorithm>
+
 #include "ash/shell.h"
 #include "ash/system/tray/system_tray.h"
 #include "ash/system/tray/system_tray_notifier.h"
 #include "base/bind.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/settings/device_settings_service.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/host_desktop.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
 using base::UserMetricsAction;
 using content::WebContents;
 
 namespace chromeos {
 
+namespace {
+
+bool ShouldShowLanguageNotification(const std::string& from_locale,

[Nikita (slow), 2014/07/14 09:37:42]

nit: Language -> Locale

[Nikita (slow), 2014/07/14 09:37:42]

This function might as well have unit_test.

[Alexander Alekseev, 2014/07/14 22:01:59]

Done.

[Alexander Alekseev, 2014/07/14 22:01:59]

Done.

+                                    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_lang != to_lang)
+    return true;
+
+  const char* const ignore_languages[] = {"en", "de", "fr", "it"};

[Nikita (slow), 2014/07/14 09:37:41]

Do you think that pt* (pt/pt-BR/pt-PT) should not be on this list?

[Alexander Alekseev, 2014/07/14 22:01:59]

Actually no. We should show notification for "pt".

+  if (std::find(ignore_languages,

[Nikita (slow), 2014/07/14 09:37:41]

return !std::find(...);

[Alexander Alekseev, 2014/07/14 22:01:59]

Done.

+                ignore_languages + arraysize(ignore_languages),
+                from_lang) != ignore_languages + arraysize(ignore_languages)) {
+    return false;
+  }
+
+  return true;
+}
+
+}  // anonymous namespace
+
 LocaleChangeGuard::LocaleChangeGuard(Profile* profile)
     : profile_(profile),
       reverted_(false),
       session_started_(false),
       main_frame_loaded_(false) {
   DCHECK(profile_);
   registrar_.Add(this, chrome::NOTIFICATION_OWNERSHIP_STATUS_CHANGED,
                  content::NotificationService::AllSources());
 }
 
@@ skipping 100 matching lines @@
     return;
   }
 
   std::string from_locale = prefs->GetString(prefs::kApplicationLocaleBackup);
   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.
+  // Locale change detected

[Nikita (slow), 2014/07/14 09:37:41]

nit: dot at the end.

[Alexander Alekseev, 2014/07/14 22:01:59]

Done.

+  if (!ShouldShowLanguageNotification(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) <<
         "Showing locale change notification in current (not previous) language";
     PrepareChangingLocale(from_locale, to_locale);
   }
 
   ash::Shell::GetInstance()->system_tray_notifier()->NotifyLocaleChanged(
       this, cur_locale, from_locale_, to_locale_);
 }
@@ skipping 39 matching lines @@
     title_text_ = l10n_util::GetStringUTF16(
         IDS_OPTIONS_SETTINGS_SECTION_TITLE_LANGUAGE);
     message_text_ = l10n_util::GetStringFUTF16(
         IDS_LOCALE_CHANGE_MESSAGE, from, to);
     revert_link_text_ = l10n_util::GetStringFUTF16(
         IDS_LOCALE_CHANGE_REVERT_MESSAGE, from);
   }
 }
 
 }  // namespace chromeos