Refactored API method GetLanguageList() into TranslatePrefs.

The method GetLanguageList() exposed by the private API returns
metadata for each available language. It is meant to be used by
all Desktop platforms, but now we need to move it into a location
that can be used by Android and iOS too.
In this change, I have moved the logic to TranslatePrefs.

Note that only the logic that is relevant to all platform has been
moved: the private API for language settings still computes the
info used by ChromeOS and Windows only.

This change does not change any Chrome behaviour.

Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I648b9ed524f1c580f0a813ebf90f06b070d1530b
Reviewed-on: https://chromium-review.googlesource.com/767855
Commit-Queue: Claudio M <claudiomagni@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Michael Martis <martis@chromium.org>
Reviewed-by: Leo Zhang <googleo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516978}

chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc

@@ -13,7 +13,6 @@
 #include <vector>
 
 #include "base/feature_list.h"
-#include "base/i18n/rtl.h"
 #include "base/memory/ptr_util.h"
 #include "base/stl_util.h"
 #include "base/strings/string16.h"
@@ -170,31 +169,9 @@ ExtensionFunction::ResponseAction
 LanguageSettingsPrivateGetLanguageListFunction::Run() {
   // Collect the language codes from the supported accept-languages.
   const std::string app_locale = g_browser_process->GetApplicationLocale();
-  std::vector<std::string> language_codes;
-  l10n_util::GetAcceptLanguagesForLocale(app_locale, &language_codes);
-
-  // Map of display name -> {language code, native display name}.
-  typedef std::pair<std::string, base::string16> LanguagePair;
-  typedef std::map<base::string16, LanguagePair,
-      l10n_util::StringComparator<base::string16>> LanguageMap;
-
-  // Collator used to sort display names in the current locale.
-  UErrorCode error = U_ZERO_ERROR;
-  std::unique_ptr<icu::Collator> collator(
-      icu::Collator::createInstance(icu::Locale(app_locale.c_str()), error));
-  if (U_FAILURE(error))
-    collator.reset();
-  LanguageMap language_map(
-      l10n_util::StringComparator<base::string16>(collator.get()));
-
-  // Build the list of display names and the language map.
-  for (const auto& code : language_codes) {
-    base::string16 display_name = l10n_util::GetDisplayNameForLocale(
-        code, app_locale, false);
-    base::string16 native_display_name = l10n_util::GetDisplayNameForLocale(
-        code, code, false);
-    language_map[display_name] = std::make_pair(code, native_display_name);
-  }
+  std::vector<translate::TranslateLanguageInfo> languages;
+
+  translate::TranslatePrefs::GetLanguageInfoList(app_locale, &languages);
 
   // Get the list of available locales (display languages) and convert to a set.
   const std::vector<std::string>& locales = l10n_util::GetAvailableLocales();
@@ -207,47 +184,25 @@ LanguageSettingsPrivateGetLanguageListFunction::Run() {
   const std::unordered_set<std::string> spellcheck_language_set(
       spellcheck_languages.begin(), spellcheck_languages.end());
 
-  // Get the list of translatable languages and convert to a set.
-  std::vector<std::string> translate_languages;
-  translate::TranslateDownloadManager::GetSupportedLanguages(
-      &translate_languages);
-  const std::unordered_set<std::string> translate_language_set(
-      translate_languages.begin(), translate_languages.end());
-
-  // Build the language list from the language map.
+  // Build the language list.
   std::unique_ptr<base::ListValue> language_list(new base::ListValue);
-  for (const auto& entry : language_map) {
-    const base::string16& display_name = entry.first;
-    const LanguagePair& pair = entry.second;
-
+  for (const auto& entry : languages) {
     language_settings_private::Language language;
-    language.code = pair.first;
 
-    base::string16 adjusted_display_name(display_name);
-    base::i18n::AdjustStringForLocaleDirection(&adjusted_display_name);
-    language.display_name = base::UTF16ToUTF8(adjusted_display_name);
-
-    base::string16 adjusted_native_display_name(pair.second);
-    base::i18n::AdjustStringForLocaleDirection(&adjusted_native_display_name);
-    language.native_display_name =
-        base::UTF16ToUTF8(adjusted_native_display_name);
+    language.code = entry.code;
+    language.display_name = entry.display_name;
+    language.native_display_name = entry.native_display_name;
 
     // Set optional fields only if they differ from the default.
-    if (base::i18n::StringContainsStrongRTLChars(display_name))
-      language.display_name_rtl.reset(new bool(true));
-    if (locale_set.count(pair.first) > 0)
+    if (locale_set.count(entry.code) > 0) {
       language.supports_ui.reset(new bool(true));
-    if (spellcheck_language_set.count(pair.first) > 0)
+    }
+    if (spellcheck_language_set.count(entry.code) > 0) {
       language.supports_spellcheck.reset(new bool(true));
-
-    std::string supports_translate_code = pair.first;
-    if (base::FeatureList::IsEnabled(translate::kImprovedLanguageSettings)) {
-      // Extract the base language: if the base language can be translated, then
-      // even the regional one should be marked as such.
-      translate::ToTranslateLanguageSynonym(&supports_translate_code);
     }
-    if (translate_language_set.count(supports_translate_code) > 0)
+    if (entry.supports_translate) {
       language.supports_translate.reset(new bool(true));
+    }
 
     language_list->Append(language.ToValue());
   }

chrome/common/extensions/api/language_settings_private.idl

@@ -22,9 +22,6 @@ namespace languageSettingsPrivate {
     // The name of the language as it is in its own language.
     DOMString nativeDisplayName;
 
-    // Whether the display name has RTL. Defaults to false.
-    boolean? displayNameRTL;
-
     // Whether the UI can be displayed in this language. Defaults to false.
     boolean? supportsUI;
 

components/translate/core/browser/BUILD.gn

@@ -122,5 +122,6 @@ source_set("unit_tests") {
     "//net:test_support",
     "//testing/gtest",
     "//third_party/metrics_proto",
+    "//ui/base",
   ]
 }

components/translate/core/browser/translate_prefs.cc

@@ -8,10 +8,13 @@
 #include <utility>
 
 #include "base/feature_list.h"
+#include "base/i18n/rtl.h"
 #include "base/memory/ptr_util.h"
+#include "base/strings/string16.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "components/pref_registry/pref_registry_syncable.h"
@@ -21,6 +24,8 @@
 #include "components/translate/core/browser/translate_download_manager.h"
 #include "components/translate/core/browser/translate_pref_names.h"
 #include "components/translate/core/common/translate_util.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/base/l10n/l10n_util_collator.h"
 
 namespace translate {
 
@@ -154,6 +159,11 @@ void DenialTimeUpdate::AddDenialTime(base::Time denial_time) {
     GetDenialTimes()->Remove(0, nullptr);
 }
 
+TranslateLanguageInfo::TranslateLanguageInfo() = default;
+
+TranslateLanguageInfo::TranslateLanguageInfo(
+    const TranslateLanguageInfo& other) = default;
+
 TranslatePrefs::TranslatePrefs(PrefService* user_prefs,
                                const char* accept_languages_pref,
                                const char* preferred_languages_pref)
@@ -337,6 +347,84 @@ void TranslatePrefs::RearrangeLanguage(
   UpdateLanguageList(languages);
 }
 
+// static
+void TranslatePrefs::GetLanguageInfoList(
+    const std::string& app_locale,
+    std::vector<TranslateLanguageInfo>* language_list) {
+  DCHECK(language_list != nullptr);
+
+  if (app_locale.empty()) {
+    return;
+  }
+
+  language_list->clear();
+
+  // Collect the language codes from the supported accept-languages.
+  std::vector<std::string> language_codes;
+  l10n_util::GetAcceptLanguagesForLocale(app_locale, &language_codes);
+
+  // Map of [display name -> {language code, native display name}].
+  typedef std::pair<std::string, base::string16> LanguagePair;
+  typedef std::map<base::string16, LanguagePair,
+                   l10n_util::StringComparator<base::string16>>
+      LanguageMap;
+
+  // Collator used to sort display names in the given locale.
+  UErrorCode error = U_ZERO_ERROR;
+  std::unique_ptr<icu::Collator> collator(
+      icu::Collator::createInstance(icu::Locale(app_locale.c_str()), error));
+  if (U_FAILURE(error)) {
+    collator.reset();
+  }
+  LanguageMap language_map(
+      l10n_util::StringComparator<base::string16>(collator.get()));
+
+  // Build the list of display names and the language map.
+  for (const auto& code : language_codes) {
+    const base::string16 display_name =
+        l10n_util::GetDisplayNameForLocale(code, app_locale, false);
+    const base::string16 native_display_name =
+        l10n_util::GetDisplayNameForLocale(code, code, false);
+    language_map[display_name] = std::make_pair(code, native_display_name);
+  }
+
+  // Get the list of translatable languages and convert to a set.
+  std::vector<std::string> translate_languages;
+  translate::TranslateDownloadManager::GetSupportedLanguages(
+      &translate_languages);
+  const std::set<std::string> translate_language_set(
+      translate_languages.begin(), translate_languages.end());
+
+  // Build the language list from the language map.
+  for (const auto& entry : language_map) {
+    const base::string16& display_name = entry.first;
+    const LanguagePair& pair = entry.second;
+
+    TranslateLanguageInfo language;
+    language.code = pair.first;
+
+    base::string16 adjusted_display_name(display_name);
+    base::i18n::AdjustStringForLocaleDirection(&adjusted_display_name);
+    language.display_name = base::UTF16ToUTF8(adjusted_display_name);
+
+    base::string16 adjusted_native_display_name(pair.second);
+    base::i18n::AdjustStringForLocaleDirection(&adjusted_native_display_name);
+    language.native_display_name =
+        base::UTF16ToUTF8(adjusted_native_display_name);
+
+    std::string supports_translate_code = pair.first;
+    if (base::FeatureList::IsEnabled(translate::kImprovedLanguageSettings)) {
+      // Extract the base language: if the base language can be translated, then
+      // even the regional one should be marked as such.
+      translate::ToTranslateLanguageSynonym(&supports_translate_code);
+    }
+    language.supports_translate =
+        translate_language_set.count(supports_translate_code) > 0;
+
+    language_list->push_back(language);
+  }
+}
+
 void TranslatePrefs::BlockLanguage(const std::string& input_language) {
   DCHECK(!input_language.empty());
 

components/translate/core/browser/translate_prefs.h

@@ -76,6 +76,22 @@ class DenialTimeUpdate {
   base::ListValue* time_list_;  // Weak, owned by the containing prefs service.
 };
 
+// This class holds various info about a language, that are related to Translate
+// Preferences and Language Settings.
+struct TranslateLanguageInfo {
+  TranslateLanguageInfo();
+  TranslateLanguageInfo(const TranslateLanguageInfo&);
+
+  // This ISO code of the language.
+  std::string code;
+  // The display name of the language in the current locale.
+  std::string display_name;
+  // The display name of the language in the language locale.
+  std::string native_display_name;
+  // Whether we support translate for this language.
+  bool supports_translate = false;
+};
+
 // The wrapper of PrefService object for Translate.
 //
 // It is assumed that |prefs_| is alive while this instance is alive.
@@ -149,6 +165,14 @@ class TranslatePrefs {
                          RearrangeSpecifier where,
                          const std::vector<std::string>& enabled_languages);
 
+  // Returns the list of TranslateLanguageInfo for all languages that are
+  // available in the given locale.
+  // The list returned in |languages| is sorted alphabetically based on the
+  // display names in the given locale.
+  static void GetLanguageInfoList(
+      const std::string& app_locale,
+      std::vector<TranslateLanguageInfo>* languages);
+
   bool IsSiteBlacklisted(const std::string& site) const;
   void BlacklistSite(const std::string& site);
   void RemoveSiteFromBlacklist(const std::string& site);

components/translate/core/browser/translate_prefs_unittest.cc

@@ -16,12 +16,16 @@
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "components/translate/core/browser/translate_download_manager.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
-
-using base::test::ScopedFeatureList;
+#include "ui/base/l10n/l10n_util.h"
 
 namespace {
 
+using base::test::ScopedFeatureList;
+using ::testing::ElementsAreArray;
+using ::testing::UnorderedElementsAreArray;
+
 const char kTestLanguage[] = "en";
 
 #if defined(OS_CHROMEOS)
@@ -69,7 +73,7 @@ class TranslatePrefsTest : public testing::Test {
   // Checks that the provided strings are equivalent to the language prefs.
   // Chrome OS uses a different pref, so we need to handle it separately.
   void ExpectLanguagePrefs(const std::string& expected,
-                           const std::string& expected_chromeos) {
+                           const std::string& expected_chromeos) const {
     if (expected.empty()) {
       EXPECT_TRUE(prefs_->GetString(kAcceptLanguagesPref).empty());
     } else {
@@ -86,11 +90,12 @@ class TranslatePrefsTest : public testing::Test {
 
   // Similar to function above: this one expects both ChromeOS and other
   // platforms to have the same value of language prefs.
-  void ExpectLanguagePrefs(const std::string& expected) {
+  void ExpectLanguagePrefs(const std::string& expected) const {
     ExpectLanguagePrefs(expected, expected);
   }
 
-  void ExpectBlockedLanguageListContent(const std::vector<std::string>& list) {
+  void ExpectBlockedLanguageListContent(
+      const std::vector<std::string>& list) const {
     const base::ListValue* const blacklist =
         prefs_->GetList(kTranslateBlockedLanguagesPref);
     const int input_size = list.size();
@@ -102,6 +107,43 @@ class TranslatePrefsTest : public testing::Test {
     }
   }
 
+  // Returns a vector of language codes from the elements of the given
+  // |language_list|.
+  std::vector<std::string> ExtractLanguageCodes(
+      const std::vector<TranslateLanguageInfo>& language_list) const {
+    std::vector<std::string> output;
+    for (const auto& item : language_list) {
+      output.push_back(item.code);
+    }
+    return output;
+  }
+
+  // Returns a vector of display names from the elements of the given
+  // |language_list|.
+  std::vector<std::string> ExtractDisplayNames(
+      const std::vector<TranslateLanguageInfo>& language_list) const {
+    std::vector<std::string> output;
+    for (const auto& item : language_list) {
+      output.push_back(item.display_name);
+    }
+    return output;
+  }
+
+  // Finds and returns the element in |language_list| that has the given code.
+  TranslateLanguageInfo GetLanguageByCode(
+      const std::string& language_code,
+      const std::vector<TranslateLanguageInfo>& language_list) const {
+    // Perform linear search as we don't care much about efficiency in test.
+    // The size of the vector is ~150.
+    TranslateLanguageInfo result;
+    for (const auto& i : language_list) {
+      if (language_code == i.code) {
+        result = i;
+      }
+    }
+    return result;
+  }
+
   std::unique_ptr<sync_preferences::TestingPrefServiceSyncable> prefs_;
   std::unique_ptr<translate::TranslatePrefs> translate_prefs_;
 
@@ -325,6 +367,157 @@ TEST_F(TranslatePrefsTest, UpdateLanguageListFeatureEnabled) {
   ExpectLanguagePrefs("en-US,en,fr,fr-CA");
 }
 
+// Test that GetLanguageInfoList() returns the correct list of languages based
+// on the given locale.
+TEST_F(TranslatePrefsTest, GetLanguageInfoListCorrectLocale) {
+  std::vector<TranslateLanguageInfo> language_list;
+  std::vector<std::string> expected_codes;
+
+  l10n_util::GetAcceptLanguagesForLocale("en-US", &expected_codes);
+  TranslatePrefs::GetLanguageInfoList("en-US", &language_list);
+  std::vector<std::string> codes = ExtractLanguageCodes(language_list);
+  EXPECT_THAT(codes, UnorderedElementsAreArray(expected_codes));
+
+  language_list.clear();
+  expected_codes.clear();
+  codes.clear();
+  l10n_util::GetAcceptLanguagesForLocale("ja", &expected_codes);
+  TranslatePrefs::GetLanguageInfoList("ja", &language_list);
+  codes = ExtractLanguageCodes(language_list);
+  EXPECT_THAT(codes, UnorderedElementsAreArray(expected_codes));
+
+  language_list.clear();
+  expected_codes.clear();
+  codes.clear();
+  l10n_util::GetAcceptLanguagesForLocale("es-AR", &expected_codes);
+  TranslatePrefs::GetLanguageInfoList("es-AR", &language_list);
+  codes = ExtractLanguageCodes(language_list);
+  EXPECT_THAT(codes, UnorderedElementsAreArray(expected_codes));
+}
+
+// Check the output of GetLanguageInfoList().
+TEST_F(TranslatePrefsTest, GetLanguageInfoListOutput) {
+  std::vector<TranslateLanguageInfo> language_list;
+
+  // Empty locale returns empty output.
+  TranslatePrefs::GetLanguageInfoList("", &language_list);
+  EXPECT_TRUE(language_list.empty());
+
+  // Output is sorted.
+  language_list.clear();
+  TranslatePrefs::GetLanguageInfoList("en-US", &language_list);
+  const std::vector<std::string> display_names =
+      ExtractDisplayNames(language_list);
+  std::vector<std::string> sorted(display_names);
+  std::sort(sorted.begin(), sorted.end());
+  EXPECT_THAT(display_names, ElementsAreArray(sorted));
+}
+
+// Check a sample of languages returned by GetLanguageInfoList().
+TEST_F(TranslatePrefsTest, GetLanguageInfoListSampleLanguages) {
+  ScopedFeatureList disable_feature;
+  disable_feature.InitAndDisableFeature(translate::kImprovedLanguageSettings);
+
+  std::vector<TranslateLanguageInfo> language_list;
+  TranslateLanguageInfo language;
+
+  //-----------------------------------
+  // Test with US locale.
+  TranslatePrefs::GetLanguageInfoList("en-US", &language_list);
+
+  language = GetLanguageByCode("en", language_list);
+  EXPECT_EQ("en", language.code);
+  EXPECT_EQ("English", language.display_name);
+  EXPECT_EQ("English", language.native_display_name);
+  EXPECT_TRUE(language.supports_translate);
+
+  language = GetLanguageByCode("en-US", language_list);
+  EXPECT_EQ("en-US", language.code);
+  EXPECT_EQ("English (United States)", language.display_name);
+  EXPECT_EQ("English (United States)", language.native_display_name);
+  EXPECT_FALSE(language.supports_translate);
+
+  language = GetLanguageByCode("it", language_list);
+  EXPECT_EQ("it", language.code);
+  EXPECT_EQ("Italian", language.display_name);
+  EXPECT_EQ("italiano", language.native_display_name);
+  EXPECT_TRUE(language.supports_translate);
+
+  language = GetLanguageByCode("it-IT", language_list);
+  EXPECT_EQ("it-IT", language.code);
+  EXPECT_EQ("Italian (Italy)", language.display_name);
+  EXPECT_EQ("italiano (Italia)", language.native_display_name);
+  EXPECT_FALSE(language.supports_translate);
+
+  language = GetLanguageByCode("ru", language_list);
+  EXPECT_EQ("ru", language.code);
+  EXPECT_EQ("Russian", language.display_name);
+  EXPECT_EQ("русский", language.native_display_name);
+  EXPECT_TRUE(language.supports_translate);
+
+  //-----------------------------------
+  // Test with Italian locale.
+  language_list.clear();
+  TranslatePrefs::GetLanguageInfoList("it", &language_list);
+
+  language = GetLanguageByCode("en-US", language_list);
+  EXPECT_EQ("en-US", language.code);
+  EXPECT_EQ("inglese (Stati Uniti)", language.display_name);
+  EXPECT_EQ("English (United States)", language.native_display_name);
+  EXPECT_FALSE(language.supports_translate);
+
+  language = GetLanguageByCode("it", language_list);
+  EXPECT_EQ("it", language.code);
+  EXPECT_EQ("italiano", language.display_name);
+  EXPECT_EQ("italiano", language.native_display_name);
+  EXPECT_TRUE(language.supports_translate);
+
+  language = GetLanguageByCode("it-IT", language_list);
+  EXPECT_EQ("it-IT", language.code);
+  EXPECT_EQ("italiano (Italia)", language.display_name);
+  EXPECT_EQ("italiano (Italia)", language.native_display_name);
+  EXPECT_FALSE(language.supports_translate);
+
+  language = GetLanguageByCode("fr-FR", language_list);
+  EXPECT_EQ("fr-FR", language.code);
+  EXPECT_EQ("francese (Francia)", language.display_name);
+  EXPECT_EQ("français (France)", language.native_display_name);
+  EXPECT_FALSE(language.supports_translate);
+}
+
+// With feature enabled, GetLanguageInfoList() should set different values for
+// supports_translate.
+// TODO(claudiomagni): Clean up this method once the feature is launched.
+TEST_F(TranslatePrefsTest, GetLanguageInfoListFeatureEnabled) {
+  ScopedFeatureList enable_feature;
+  enable_feature.InitAndEnableFeature(translate::kImprovedLanguageSettings);
+
+  std::vector<TranslateLanguageInfo> language_list;
+  TranslateLanguageInfo language;
+
+  TranslatePrefs::GetLanguageInfoList("en-US", &language_list);
+
+  language = GetLanguageByCode("en", language_list);
+  EXPECT_EQ("en", language.code);
+  EXPECT_TRUE(language.supports_translate);
+
+  language = GetLanguageByCode("en-US", language_list);
+  EXPECT_EQ("en-US", language.code);
+  EXPECT_TRUE(language.supports_translate);
+
+  language = GetLanguageByCode("it", language_list);
+  EXPECT_EQ("it", language.code);
+  EXPECT_TRUE(language.supports_translate);
+
+  language = GetLanguageByCode("it-IT", language_list);
+  EXPECT_EQ("it-IT", language.code);
+  EXPECT_TRUE(language.supports_translate);
+
+  language = GetLanguageByCode("zh-HK", language_list);
+  EXPECT_EQ("zh-HK", language.code);
+  EXPECT_TRUE(language.supports_translate);
+}
+
 TEST_F(TranslatePrefsTest, BlockLanguage) {
   // One language.
   translate_prefs_->BlockLanguage("en-UK");

third_party/closure_compiler/externs/language_settings_private.js

@@ -24,6 +24,7 @@ chrome.languageSettingsPrivate.MoveType = {
   TOP: 'TOP',
   UP: 'UP',
   DOWN: 'DOWN',
+  UNKNOWN: 'UNKNOWN',
 };
 
 /**
@@ -31,7 +32,6 @@ chrome.languageSettingsPrivate.MoveType = {
  *   code: string,
  *   displayName: string,
  *   nativeDisplayName: string,
- *   displayNameRTL: (boolean|undefined),
  *   supportsUI: (boolean|undefined),
  *   supportsSpellcheck: (boolean|undefined),
  *   supportsTranslate: (boolean|undefined)