Revert "Partial revert of "Use preferred language in extension i18n""

This reverts commit ce0ec6fc.

Reason for revert: While the perf regression may have been caused by the original CL, it was transient. 

Original change's description:
> Partial revert of "Use preferred language in extension i18n"
> 
> This is a logical revert of "Use preferred language in extension i18n"
> (commit 693d01a6). Because of some
> intervening changes causing more complicated merges, it's simpler to
> just undo the side effects of the original change, leaving the changed
> boilerplate intact.
> 
> Reason for revert: Testing perf regression in crbug.com/898191
> 
> If this exonerates the original CL, we will revert this change.
> Otherwise, we will clean up the boilerplate added for the original CL.
> 
> Original change's description:
> > Use preferred language in extension i18n
> >
> > When localizing an extension, prioritize the user's preferred language
> > over the application locale.
> >
> > Normally, these are the same. In some cases, they differ. For example,
> > the user may choose to display Chrome in "en-CA" (the intl.app_locale
> > pref), but because we don't have translations for that, the actual
> > UI locale is "en-GB".
> >
> > This CL makes extensions try to use the user's preferred locale for i18n
> > ("en_CA"), falling back to the UI locale ("en_GB") if no locale
> > directory is found for the preferred locale.
> >
> > This change also updates the "current_locale" manifest key to reflect
> > the user's preferred locale, or the UI locale if intl.app_locale is
> > unset.
> >
> > Note: Like the application locale, the preferred locale is only set at
> > startup (and in certain situations in CrOS). If the user changes their
> > intl.app_locale pref, they have to restart Chrome before extensions are
> > re-localized, just like before.
> >
> > Bug: 874225
> > Change-Id: I1aabe3c3680b77d6522193e764aec15a3d618d2d
> > Reviewed-on: https://chromium-review.googlesource.com/c/1244666
> > Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
> > Reviewed-by: Scott Violet <sky@chromium.org>
> > Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#600619}
> 
> Bug: 898191
> Change-Id: Iaddceb16d61ffc5572a25ed31276443ea5b5baa4
> Reviewed-on: https://chromium-review.googlesource.com/c/1308765
> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
> Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#604819}

TBR=lazyboy@chromium.org,michaelpg@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 898191
Change-Id: Iead107f46039f01b5d5b97b36d1b8fb242f0aee3
Reviewed-on: https://chromium-review.googlesource.com/c/1334194Reviewed-by: Mike Wittman <wittman@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Commit-Queue: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607719}

extensions/common/extension_l10n_util.cc

@@ -114,8 +114,10 @@ std::string& GetPreferredLocale() {
 
 // Returns the desired locale to use for localization.
 std::string LocaleForLocalization() {
-  // TODO(michaelpg): Check preferred_locale first. That change was reverted to
-  // check if it alleviated: https://crbug.com/898191.
+  std::string preferred_locale =
+      l10n_util::NormalizeLocale(GetPreferredLocale());
+  if (!preferred_locale.empty())
+    return preferred_locale;
   return extension_l10n_util::CurrentLocaleOrDefault();
 }
 
@@ -349,8 +351,17 @@ void GetAllFallbackLocales(const std::string& application_locale,
                            const std::string& default_locale,
                            std::vector<std::string>* all_fallback_locales) {
   DCHECK(all_fallback_locales);
-  // TODO(michaelpg): Check preferred_locale first. That change was reverted to
-  // check if it alleviated: https://crbug.com/898191.
+  // Use the preferred locale if available. Otherwise, fall back to the
+  // application locale or the application locale's parent locales. Thus, a
+  // preferred locale of "en_CA" with an application locale of "en_GB" will
+  // first try to use an en_CA locale folder, followed by en_GB, followed by en.
+  std::string preferred_locale =
+      l10n_util::NormalizeLocale(GetPreferredLocale());
+  if (!preferred_locale.empty() && preferred_locale != default_locale &&
+      preferred_locale != application_locale) {
+    all_fallback_locales->push_back(preferred_locale);
+  }
+
   if (!application_locale.empty() && application_locale != default_locale)
     l10n_util::GetParentLocales(application_locale, all_fallback_locales);
   all_fallback_locales->push_back(default_locale);

extensions/common/extension_l10n_util_unittest.cc

@@ -124,6 +124,7 @@ TEST(ExtensionL10nUtil, GetValidLocalesWithValidLocalesAndMessagesFile) {
 }
 
 TEST(ExtensionL10nUtil, LoadMessageCatalogsValidFallback) {
+  extension_l10n_util::ScopedLocaleForTest scoped_locale("en-US");
   base::FilePath install_dir;
   ASSERT_TRUE(base::PathService::Get(DIR_TEST_DATA, &install_dir));
   install_dir =
@@ -140,6 +141,7 @@ TEST(ExtensionL10nUtil, LoadMessageCatalogsValidFallback) {
 }
 
 TEST(ExtensionL10nUtil, LoadMessageCatalogsMissingFiles) {
+  extension_l10n_util::ScopedLocaleForTest scoped_locale("sr");
   base::ScopedTempDir temp;
   ASSERT_TRUE(temp.CreateUniqueTempDir());
 
@@ -155,6 +157,7 @@ TEST(ExtensionL10nUtil, LoadMessageCatalogsMissingFiles) {
 }
 
 TEST(ExtensionL10nUtil, LoadMessageCatalogsBadJSONFormat) {
+  extension_l10n_util::ScopedLocaleForTest scoped_locale("sr");
   base::ScopedTempDir temp;
   ASSERT_TRUE(temp.CreateUniqueTempDir());
 
@@ -180,6 +183,7 @@ TEST(ExtensionL10nUtil, LoadMessageCatalogsBadJSONFormat) {
 }
 
 TEST(ExtensionL10nUtil, LoadMessageCatalogsDuplicateKeys) {
+  extension_l10n_util::ScopedLocaleForTest scoped_locale("sr");
   base::ScopedTempDir temp;
   ASSERT_TRUE(temp.CreateUniqueTempDir());
 
@@ -599,57 +603,97 @@ TEST(ExtensionL10nUtil, LocalizeManifestWithSearchProviderMsgs) {
   EXPECT_TRUE(error.empty());
 }
 
-// Try with NULL manifest.
+// Tests that we don't relocalize with a null manifest.
 TEST(ExtensionL10nUtil, ShouldRelocalizeManifestWithNullManifest) {
   EXPECT_FALSE(extension_l10n_util::ShouldRelocalizeManifest(NULL));
 }
 
-// Try with default and current locales missing.
+// Tests that we don't relocalize with default and current locales missing.
 TEST(ExtensionL10nUtil, ShouldRelocalizeManifestEmptyManifest) {
   base::DictionaryValue manifest;
   EXPECT_FALSE(extension_l10n_util::ShouldRelocalizeManifest(&manifest));
 }
 
-// Try with missing current_locale.
+// Tests that we relocalize without a current locale.
 TEST(ExtensionL10nUtil, ShouldRelocalizeManifestWithDefaultLocale) {
   base::DictionaryValue manifest;
   manifest.SetString(keys::kDefaultLocale, "en_US");
   EXPECT_TRUE(extension_l10n_util::ShouldRelocalizeManifest(&manifest));
 }
 
-// Try with missing default_locale.
+// Tests that we don't relocalize without a default locale.
 TEST(ExtensionL10nUtil, ShouldRelocalizeManifestWithCurrentLocale) {
+  extension_l10n_util::ScopedLocaleForTest scoped_locale("en-US");
   base::DictionaryValue manifest;
-  manifest.SetString(keys::kCurrentLocale,
-                     extension_l10n_util::CurrentLocaleOrDefault());
+  manifest.SetString(keys::kCurrentLocale, "en_US");
   EXPECT_FALSE(extension_l10n_util::ShouldRelocalizeManifest(&manifest));
 }
 
-// Try with all data present, but with same current_locale as system locale.
+// Tests that we don't relocalize with same current_locale as system locale.
 TEST(ExtensionL10nUtil, ShouldRelocalizeManifestSameCurrentLocale) {
+  extension_l10n_util::ScopedLocaleForTest scoped_locale("en-US");
   base::DictionaryValue manifest;
   manifest.SetString(keys::kDefaultLocale, "en_US");
-  manifest.SetString(keys::kCurrentLocale,
-                     extension_l10n_util::CurrentLocaleOrDefault());
+  manifest.SetString(keys::kCurrentLocale, "en_US");
   EXPECT_FALSE(extension_l10n_util::ShouldRelocalizeManifest(&manifest));
 }
 
-// Try with all data present, but with different current_locale.
+// Tests that we relocalize with a different current_locale.
 TEST(ExtensionL10nUtil, ShouldRelocalizeManifestDifferentCurrentLocale) {
+  extension_l10n_util::ScopedLocaleForTest scoped_locale("en-US");
   base::DictionaryValue manifest;
   manifest.SetString(keys::kDefaultLocale, "en_US");
   manifest.SetString(keys::kCurrentLocale, "sr");
   EXPECT_TRUE(extension_l10n_util::ShouldRelocalizeManifest(&manifest));
 }
 
+// Tests that we don't relocalize with the same current_locale as preferred
+// locale.
+TEST(ExtensionL10nUtil, ShouldRelocalizeManifestSameCurrentLocaleAsPreferred) {
+  extension_l10n_util::ScopedLocaleForTest scoped_locale("en-GB", "en-CA");
+  base::DictionaryValue manifest;
+  manifest.SetString(keys::kDefaultLocale, "en_US");
+  manifest.SetString(keys::kCurrentLocale, "en_CA");
+
+  // Preferred and current locale are both en_CA.
+  EXPECT_FALSE(extension_l10n_util::ShouldRelocalizeManifest(&manifest));
+}
+
+// Tests that we relocalize with a different current_locale from the preferred
+// locale.
+TEST(ExtensionL10nUtil,
+     ShouldRelocalizeManifestDifferentCurrentLocaleThanPreferred) {
+  extension_l10n_util::ScopedLocaleForTest scoped_locale("en-GB", "en-CA");
+  base::DictionaryValue manifest;
+  manifest.SetString(keys::kDefaultLocale, "en_US");
+  manifest.SetString(keys::kCurrentLocale, "en_GB");
+
+  // Requires relocalization as the preferred (en_CA) differs from current
+  // (en_GB).
+  EXPECT_TRUE(extension_l10n_util::ShouldRelocalizeManifest(&manifest));
+}
+
 TEST(ExtensionL10nUtil, GetAllFallbackLocales) {
+  extension_l10n_util::ScopedLocaleForTest scoped_locale("en-US");
   std::vector<std::string> fallback_locales;
   extension_l10n_util::GetAllFallbackLocales("en_US", "all", &fallback_locales);
   ASSERT_EQ(3U, fallback_locales.size());
 
-  CHECK_EQ("en_US", fallback_locales[0]);
-  CHECK_EQ("en", fallback_locales[1]);
-  CHECK_EQ("all", fallback_locales[2]);
+  EXPECT_EQ("en_US", fallback_locales[0]);
+  EXPECT_EQ("en", fallback_locales[1]);
+  EXPECT_EQ("all", fallback_locales[2]);
+}
+
+TEST(ExtensionL10nUtil, GetAllFallbackLocalesWithPreferredLocale) {
+  extension_l10n_util::ScopedLocaleForTest scoped_locale("en-GB", "en-CA");
+  std::vector<std::string> fallback_locales;
+  extension_l10n_util::GetAllFallbackLocales("en_GB", "all", &fallback_locales);
+  ASSERT_EQ(4U, fallback_locales.size());
+
+  EXPECT_EQ("en_CA", fallback_locales[0]);
+  EXPECT_EQ("en_GB", fallback_locales[1]);
+  EXPECT_EQ("en", fallback_locales[2]);
+  EXPECT_EQ("all", fallback_locales[3]);
 }
 
 }  // namespace