Use BCP47 compliant format for locale representation

chrome/browser/android/preferences/pref_service_bridge.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/android/preferences/pref_service_bridge.h"
 
 #include <jni.h>
 #include <stddef.h>
 
 #include <memory>
@@ skipping 45 matching lines @@
 #include "components/password_manager/core/common/password_manager_pref_names.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/core/common/signin_pref_names.h"
 #include "components/strings/grit/components_locale_settings.h"
 #include "components/translate/core/browser/translate_prefs.h"
 #include "components/translate/core/common/translate_pref_names.h"
 #include "components/version_info/version_info.h"
 #include "components/web_resource/web_resource_pref_names.h"
 #include "content/public/browser/browser_thread.h"
 #include "jni/PrefServiceBridge_jni.h"
+#include "third_party/icu/source/common/unicode/uloc.h"
 #include "ui/base/l10n/l10n_util.h"
 
 using base::android::AttachCurrentThread;
 using base::android::CheckException;
 using base::android::ConvertJavaStringToUTF8;
 using base::android::ConvertUTF8ToJavaString;
 using base::android::JavaParamRef;
 using base::android::ScopedJavaLocalRef;
 using base::android::ScopedJavaGlobalRef;
 using content::BrowserThread;
@@ skipping 1064 matching lines @@
 
 // static
 bool PrefServiceBridge::RegisterPrefServiceBridge(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 // static
 // This logic should be kept in sync with prependToAcceptLanguagesIfNecessary in
 // chrome/android/java/src/org/chromium/chrome/browser/
 //     physicalweb/PwsClientImpl.java
-// Input |locales| is a comma separated locale representaion. Each locale
-// representation should be xx_XX style, where xx is a 2-letter
-// ISO 639-1 compliant language code and XX is a 2-letter
-// ISO 3166-1 compliant country code.
+// Input |locales| is a comma separated locale representation that consists of
+// language tags(BCP47 compliant format). Each language tag contains
+// a language code and a country code or a language code only.
 void PrefServiceBridge::PrependToAcceptLanguagesIfNecessary(
     const std::string& locales,
     std::string* accept_languages) {
   std::vector<std::string> locale_list =
       base::SplitString(locales + "," + *accept_languages, ",",
                         base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
 
   std::set<std::string> seen_tags;
   std::vector<std::pair<std::string, std::string>> unique_locale_list;
   for (const std::string& locale_str : locale_list) {
-    // TODO(yirui): Support BCP47 compliant format including 3-letter
-    // country code, '-' separator and missing country case.
-    if (locale_str.size() != 5u ||
-        (locale_str[2] != '_' && locale_str[2] != '-'))
-      continue;  // Skip not well formed locale.
+    char locale_ID[ULOC_FULLNAME_CAPACITY];
+    char language_code[ULOC_LANG_CAPACITY];
+    char country_code[ULOC_COUNTRY_CAPACITY];
+    UErrorCode error = U_ZERO_ERROR;
+    uloc_forLanguageTag(locale_str.c_str(), locale_ID, ULOC_FULLNAME_CAPACITY,
+                        nullptr, &error);
+    uloc_getLanguage(locale_ID, language_code, ULOC_LANG_CAPACITY, &error);
+    uloc_getCountry(locale_ID, country_code, ULOC_COUNTRY_CAPACITY, &error);
+    std::string lang_code(language_code);

[ksk1, 2016/10/17 05:06:04]

Using both lang_code and language_code is confusing.
How about s/language_code/language_code_buffer/ and s/lang_code/language_code/

Same comment for country code.

[Yirui Huang, 2016/10/17 10:40:52]

Done.

+    std::string coun_code(country_code);
 

[ksk1, 2016/10/17 05:06:04]

if (error != U_ZERO_ERROR || lang_code.empty()) {
  continue;
}

[Yirui Huang, 2016/10/17 10:40:52]

U_FAILURE is used for checking.

-    std::string lang_code(locale_str.substr(0, 2));
-    std::string country_code(locale_str.substr(3, 2));
-
-    // Java mostly follows ISO-639-1 and ICU, except for the following three.
+    // Java mostly follows ISO-639-1 and ICU, except for the following four.
     // See documentation on java.util.Locale constructor for more.
     if (lang_code == "iw")
       lang_code = "he";
     else if (lang_code == "ji")
       lang_code = "yi";
     else if (lang_code == "in")
       lang_code = "id";
+    else if (lang_code == "tl")
+      lang_code = "fil";
 
-    std::string language_tag(lang_code + "-" + country_code);
+    std::string language_tag(lang_code + "-" + coun_code);
 
     if (seen_tags.find(language_tag) != seen_tags.end())
       continue;
-    unique_locale_list.push_back(std::make_pair(lang_code, country_code));
-    seen_tags.insert(language_tag);
+    if (coun_code != "")

[ksk1, 2016/10/17 05:06:04]

!coun_code.empty()

[Yirui Huang, 2016/10/17 10:40:52]

same as in Java side, not necessary, so removed check.

+      seen_tags.insert(language_tag);
+    unique_locale_list.push_back(std::make_pair(lang_code, coun_code));
   }
 
   // If language is not in the accept languages list, also add language
   // code. A language code should only be inserted after the last
   // languageTag that contains that language.
   // This will work with the IDS_ACCEPT_LANGUAGE localized strings bundled
   // with Chrome but may fail on arbitrary lists of language tags due to
   // differences in case and whitespace.
   std::set<std::string> seen_languages;
   std::vector<std::string> output_list;
   for (auto it = unique_locale_list.rbegin(); it != unique_locale_list.rend();
        ++it) {
     if (seen_languages.find(it->first) == seen_languages.end()) {
       output_list.push_back(it->first);
       seen_languages.insert(it->first);
     }
-    output_list.push_back(it->first + "-" + it->second);
+    if (!it->second.empty())
+      output_list.push_back(it->first + "-" + it->second);
   }
 
   std::reverse(output_list.begin(), output_list.end());
   *accept_languages = base::JoinString(output_list, ",");
 }
 
 // static
 std::string PrefServiceBridge::GetAndroidPermissionForContentSetting(
     ContentSettingsType content_type) {
   JNIEnv* env = AttachCurrentThread();
   base::android::ScopedJavaLocalRef<jstring> android_permission =
       Java_PrefServiceBridge_getAndroidPermissionForContentSetting(
           env, content_type);
   if (android_permission.is_null())
     return std::string();
 
   return ConvertJavaStringToUTF8(android_permission);
 }
 
 static void SetSupervisedUserId(JNIEnv* env,
                                 const JavaParamRef<jobject>& obj,
                                 const JavaParamRef<jstring>& pref) {
   GetPrefService()->SetString(prefs::kSupervisedUserId,
                               ConvertJavaStringToUTF8(env, pref));
 }