Bug 1345957 - Tighten LocaleService::NegotiateLanguages input handling. r=jfkthame

MozReview-Commit-ID: 9ZxHI0RpYpi

--HG--
extra : rebase_source : 1db7f4b41f62acfd8e654fa2bdf9bfabb7350f0a

intl/locale/LocaleService.cpp

@@ -432,6 +432,7 @@ LocaleService::FilterMatches(const nsTArray<nsCString>& aRequested,
 
   for (auto& requested : aRequested) {
     if (requested.IsEmpty()) {
+      MOZ_ASSERT(!requested.IsEmpty(), "Locale string cannot be empty.");
       continue;
     }
 
@@ -504,32 +505,41 @@ LocaleService::FilterMatches(const nsTArray<nsCString>& aRequested,
   }
 }
 
-bool
+void
 LocaleService::NegotiateLanguages(const nsTArray<nsCString>& aRequested,
                                   const nsTArray<nsCString>& aAvailable,
                                   const nsACString& aDefaultLocale,
                                   LangNegStrategy aStrategy,
                                   nsTArray<nsCString>& aRetVal)
 {
-  // If the strategy is Lookup, we require the defaultLocale to be set.
+  MOZ_ASSERT(aDefaultLocale.IsEmpty() || Locale(aDefaultLocale).IsValid(),
+    "If specified, default locale must be a valid BCP47 language tag.");
+
   if (aStrategy == LangNegStrategy::Lookup && aDefaultLocale.IsEmpty()) {
-    return false;
+    NS_WARNING("Default locale should be specified when using lookup strategy.");
   }
 
   FilterMatches(aRequested, aAvailable, aStrategy, aRetVal);
 
   if (aStrategy == LangNegStrategy::Lookup) {
+    // If the strategy is Lookup and Filtering returned no matches, use
+    // the default locale.
     if (aRetVal.Length() == 0) {
-      // If the strategy is Lookup and Filtering returned no matches, use
-      // the default locale.
-      aRetVal.AppendElement(aDefaultLocale);
+      // If the default locale is empty, we already issued a warning, so
+      // now we will just pick up the LocaleService's defaultLocale.
+      if (aDefaultLocale.IsEmpty()) {
+        nsAutoCString defaultLocale;
+        GetDefaultLocale(defaultLocale);
+        aRetVal.AppendElement(defaultLocale);
+      } else {
+        aRetVal.AppendElement(aDefaultLocale);
+      }
     }
   } else if (!aDefaultLocale.IsEmpty() && !aRetVal.Contains(aDefaultLocale)) {
     // If it's not a Lookup strategy, add the default locale only if it's
     // set and it's not in the results already.
     aRetVal.AppendElement(aDefaultLocale);
   }
-  return true;
 }
 
 bool
@@ -835,12 +845,8 @@ LocaleService::NegotiateLanguages(const char** aRequested,
   LangNegStrategy strategy = ToLangNegStrategy(aStrategy);
 
   AutoTArray<nsCString, 100> supportedLocales;
-  bool result = NegotiateLanguages(requestedLocales, availableLocales,
-                                   defaultLocale, strategy, supportedLocales);
-
-  if (!result) {
-    return NS_ERROR_INVALID_ARG;
-  }
+  NegotiateLanguages(requestedLocales, availableLocales,
+                     defaultLocale, strategy, supportedLocales);
 
   *aRetVal =
     static_cast<char**>(moz_xmalloc(sizeof(char*) * supportedLocales.Length()));

intl/locale/LocaleService.h

@@ -243,7 +243,7 @@ public:
    *
    * (See mozILocaleService.idl for a JS-callable version of this.)
    */
-  bool NegotiateLanguages(const nsTArray<nsCString>& aRequested,
+  void NegotiateLanguages(const nsTArray<nsCString>& aRequested,
                           const nsTArray<nsCString>& aAvailable,
                           const nsACString& aDefaultLocale,
                           LangNegStrategy aLangNegStrategy,

intl/locale/MozLocale.cpp

@@ -18,6 +18,8 @@ using namespace mozilla::intl;
  */
 Locale::Locale(const nsACString& aLocale)
 {
+  MOZ_ASSERT(!aLocale.IsEmpty(), "Locale string cannot be empty");
+
   int32_t position = 0;
 
   if (!IsASCII(aLocale)) {

intl/locale/tests/gtest/TestLocaleServiceNegotiate.cpp

@@ -30,3 +30,24 @@ TEST(Intl_Locale_LocaleService, Negotiate) {
   ASSERT_TRUE(supportedLocales[0].EqualsLiteral("sr-Cyrl"));
   ASSERT_TRUE(supportedLocales[1].EqualsLiteral("en-US"));
 }
+
+TEST(Intl_Locale_LocaleService, UseLSDefaultLocale) {
+  nsTArray<nsCString> requestedLocales;
+  nsTArray<nsCString> availableLocales;
+  nsTArray<nsCString> supportedLocales;
+  nsAutoCString defaultLocale("");
+  LocaleService::LangNegStrategy strategy =
+    LocaleService::LangNegStrategy::Lookup;
+
+  requestedLocales.AppendElement(NS_LITERAL_CSTRING("sr"));
+
+  availableLocales.AppendElement(NS_LITERAL_CSTRING("de"));
+
+  LocaleService::GetInstance()->NegotiateLanguages(
+      requestedLocales, availableLocales, defaultLocale, strategy, supportedLocales);
+
+  nsAutoCString lsDefaultLocale;
+  LocaleService::GetInstance()->GetDefaultLocale(lsDefaultLocale);
+  ASSERT_TRUE(supportedLocales.Length() == 1);
+  ASSERT_TRUE(supportedLocales[0].Equals(lsDefaultLocale));
+}