Use BCP47 compliant format for locale representation

chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PwsClientImpl.java

Index: chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PwsClientImpl.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PwsClientImpl.java b/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PwsClientImpl.java
index c618d54f9f1d96765077dcc65d5b6d3fc252bca9..d3a1d13a15b529593c2935114d76e3a5e9cfcbdb 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PwsClientImpl.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PwsClientImpl.java
@@ -14,6 +14,7 @@ import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
 
+import org.chromium.base.LocaleUtils;
 import org.chromium.base.Log;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
@@ -143,8 +144,8 @@ class PwsClientImpl implements PwsClient {
         try {
             JSONObject payload = createResolveScanPayload(broadcastUrls);
             String url = ENDPOINT_URL + "?key=" + getApiKey();
-            request = new JsonObjectHttpRequest(url, getUserAgent(), getAcceptLanguage(), payload,
-                    requestCallback);
+            request = new JsonObjectHttpRequest(
+                    url, getUserAgent(), updateAcceptLanguage(), payload, requestCallback);
         } catch (MalformedURLException e) {
             Log.e(TAG, "Error creating PWS HTTP request", e);
             return;
@@ -186,8 +187,8 @@ class PwsClientImpl implements PwsClient {
         // Create the request.
         BitmapHttpRequest request = null;
         try {
-            request = new BitmapHttpRequest(iconUrl, getUserAgent(), getAcceptLanguage(),
-                    requestCallback);
+            request = new BitmapHttpRequest(
+                    iconUrl, getUserAgent(), updateAcceptLanguage(), requestCallback);
         } catch (MalformedURLException e) {
             Log.e(TAG, "Error creating icon request", e);
             return;
@@ -222,12 +223,13 @@ class PwsClientImpl implements PwsClient {
     }
 
     /**
-     * Construct the Accept-Language string based on the current locale.
-     * @return An Accept-Language string.
+     * Update an Accept-Language string based on the current default locales and make a string of
+     * an Accept-Language header with q-values.
+     * @return An Accept-Language string made of an Accept-Language header with q-values.
      */
     @VisibleForTesting
-    String getAcceptLanguage() {
-        String defaultLocale = Locale.getDefault().toString();
+    String updateAcceptLanguage() {
+        String defaultLocale = LocaleUtils.getDefaultLocale();
         if (sDefaultLocale == null || !sDefaultLocale.equals(defaultLocale)) {
             String acceptLanguages = mContext.getResources().getString(R.string.accept_languages);
             acceptLanguages = prependToAcceptLanguagesIfNecessary(defaultLocale, acceptLanguages);
@@ -238,62 +240,34 @@ class PwsClientImpl implements PwsClient {
     }
 
     /**
-     * Handle the special cases in converting a language code/region code pair into an ISO-639-1
-     * language tag.
-     * @param language The 2-character language code
-     * @param region The 2-character country code
-     * @return A language tag.
-     */
-    @VisibleForTesting
-    static String makeLanguageTag(String language, String region) {
-        // Java mostly follows ISO-639-1 and ICU, except for the following three.
-        // See documentation on java.util.Locale constructor for more.
-        String isoLanguage;
-        if ("iw".equals(language)) {
-            isoLanguage = "he";
-        } else if ("ji".equals(language)) {
-            isoLanguage = "yi";
-        } else if ("in".equals(language)) {
-            isoLanguage = "id";
-        } else {
-            isoLanguage = language;
-        }
-
-        return isoLanguage + "-" + region;
-    }
-
-    /**
      * Get the language code for the default locale and prepend it to the Accept-Language string if
      * it isn't already present. The logic should match PrependToAcceptLanguagesIfNecessary in
      * chrome/browser/android/preferences/pref_service_bridge.cc
-     * @param locales A string representing a default locale or a list of default locales.
+     * @param locales A string representing a default language tag or a list of default language
+     * tags.
      * @param acceptLanguages The default language list for the language of the user's locale.
      * @return An updated language list.
      */
     @VisibleForTesting
     static String prependToAcceptLanguagesIfNecessary(String locales, String acceptLanguages) {
-        String localeString = locales + "," + acceptLanguages;
-        String[] localeList = localeString.split(",");
+        String localeStrings = locales + "," + acceptLanguages;
+        String[] localeList = localeStrings.split(",");
 
-        HashSet<String> seenLocales = new HashSet<>();
+        HashSet<String> seenLanguageTags = new HashSet<>();
         ArrayList<String> uniqueList = new ArrayList<>();
-        for (String locale : localeList) {
-            // TODO(yirui): Support BCP47 compliant format including 3-letter country code,
-            // '-' separator and missing country case.
-            if (locale.length() != 5 || (locale.charAt(2) != '_' && locale.charAt(2) != '-')) {
-                // Skip checking not well formed locales.
-                continue;
-            }
-
-            String language = locale.substring(0, 2);
-            String country = locale.substring(3);
-            String languageTag = makeLanguageTag(language, country);
+        for (String localeString : localeList) {
+            Locale locale = LocaleUtils.forLanguageTag(localeString);
+            String language = locale.getLanguage();
+            String country = locale.getCountry();

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

if (language.isEmpty()) {
    continue;
}

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

Done.

+            String languageTag = LocaleUtils.toLanguageTag(locale);
 
-            if (seenLocales.contains(languageTag)) {
+            if (seenLanguageTags.contains(languageTag)) {
                 continue;
             }
+            if (!country.isEmpty()) {

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

Why is this if needed?
uniqueList can be inconsistent with seenLanguageTags.
This would never be happen in practice, but this is confusing for future
development.

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

I changed a little bit here, only using uniqueList. 

+                seenLanguageTags.add(languageTag);
+            }
             uniqueList.add(languageTag);
-            seenLocales.add(languageTag);
         }
 
         // If language is not in the accept languages list, also add language code.
@@ -305,13 +279,18 @@ class PwsClientImpl implements PwsClient {
         HashSet<String> seenLanguages = new HashSet<>();
         ArrayList<String> outputList = new ArrayList<>();
         for (int i = uniqueList.size() - 1; i >= 0; i--) {
-            String localeAdd = uniqueList.get(i);
-            String languageAdd = localeAdd.substring(0, 2);
+            String localeStr = uniqueList.get(i);
+            Locale localeAdd = LocaleUtils.forLanguageTag(localeStr);
+            String languageAdd = localeAdd.getLanguage();
+            String countryAdd = localeAdd.getCountry();
+
             if (!seenLanguages.contains(languageAdd)) {
                 seenLanguages.add(languageAdd);
                 outputList.add(languageAdd);
             }
-            outputList.add(localeAdd);
+            if (!countryAdd.isEmpty()) {
+                outputList.add(localeStr);
+            }
         }
         Collections.reverse(outputList);
         return TextUtils.join(",", outputList);