Use BCP47 compliant format for locale representation

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

 // Copyright 2015 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.
 
 package org.chromium.chrome.browser.physicalweb;
 
 import android.content.Context;
 import android.graphics.Bitmap;
 import android.os.AsyncTask;
 import android.os.Build;
 import android.text.TextUtils;
 
 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;
 import org.chromium.chrome.GoogleAPIKeys;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeVersionInfo;
 import org.chromium.chrome.browser.physicalweb.PwsClient.FetchIconCallback;
 import org.chromium.chrome.browser.physicalweb.PwsClient.ResolveScanCallback;
 
 import java.net.MalformedURLException;
@@ skipping 109 matching lines @@
                 Log.e(TAG, "Error making request to PWS%s", httpErr);
                 resolveScanCallback.onPwsResults(new ArrayList<PwsResult>());
             }
         };
 
         // Create the request.
         HttpRequest request = null;
         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;
         } catch (JSONException e) {
             Log.e(TAG, "Error creating PWS JSON payload", e);
             return;
         }
         // The callback will be called on the main thread.
         AsyncTask.THREAD_POOL_EXECUTOR.execute(request);
     }
@@ skipping 21 matching lines @@
                 if (responseCode > 0) {
                     httpErr = ", HTTP " + responseCode;
                 }
                 Log.e(TAG, "Error requesting icon%s", httpErr);
             }
         };
 
         // 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;
         }
         // The callback will be called on the main thread.
         AsyncTask.THREAD_POOL_EXECUTOR.execute(request);
     }
 
     /**
      * Recreate the Chrome for Android User-Agent string as closely as possible without calling any
@@ skipping 14 matching lines @@
 
             // Build the User-Agent string.
             // eg: Mozilla/5.0 (Linux; Android 5.1.1; Nexus 4 Build/LMY48T) AppleWebKit/0.0 (KHTML,
             //     like Gecko) Chrome/50.0.2661.89 Mobile Safari/0.0
             sUserAgent = String.format(USER_AGENT_FORMAT, osInfo, product);
         }
         return sUserAgent;
     }
 
     /**
-     * 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);
             sAcceptLanguage = generateAcceptLanguageHeader(acceptLanguages);
             sDefaultLocale = defaultLocale;
         }
         return sAcceptLanguage;
     }
 
     /**
-     * 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.
+        for (String localeString : localeList) {
+            Locale locale = LocaleUtils.forLanguageTag(localeString);
+            String language = locale.getLanguage();
+            String country = locale.getCountry();
+            String languageTag = LocaleUtils.toLanguageTag(locale);
+
+            if (seenLanguageTags.contains(languageTag)) {
                 continue;
             }
-
-            String language = locale.substring(0, 2);
-            String country = locale.substring(3);
-            String languageTag = makeLanguageTag(language, country);
-
-            if (seenLocales.contains(languageTag)) {
-                continue;
+            if (!country.isEmpty()) {
+                seenLanguageTags.add(languageTag);
             }
             uniqueList.add(languageTag);

[Seigo Nonaka, 2016/10/17 04:12:15]

You are creating same locale object twice by calling LocaleUtils.forLanguageTag.
How about keeping Locale object instead of String in uniqueList?
like

HashSet<Locale> seenLocales = new HashSet<>();
ArrayList<Locale> uniqueList = new ArrayList<>();
for (String localeString : localeList) {
  Locale locale = LocaleUtils.forLanguageTag(localeString);
  if (seenLocales.contains(locale)) {
    continue;
  }
  if (!locale.getCountry().isEmpty()) {
    seenLocales.add(locale);
  }
}

...

HashSet<String> seenLanguages = new HashSet<>();
ArrayList<String> outputList = new ArrayList<>();

for (int i = uniqueList.size() - 1; i >= 0; i--) {
  Locale locale = uniqueList.get(i);
  String language = locale.getLanguage();
  String country = locale.getCountry();
  if (!seenLanguages.contains(language)) {
    seenLanguages.add(language);
    outputList.add(language);
  }
  outputList.add(LocaleUtils.toLanguageTag(locale));
}

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

Done.

-            seenLocales.add(languageTag);
         }
 
         // 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.
         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);
     }
 
     /**
      * Given a list of comma-delimited language codes in decreasing order of preference, insert
      * q-values to represent the relative quality/precedence of each language. The logic should
      * match GenerateAcceptLanguageHeader in net/http/http_util.cc.
      * @param languageList A comma-delimited list of language codes containing no whitespace.
@@ skipping 15 matching lines @@
                 langListWithQ.format(",%s;q=0.%d", language, qvalue10);
             }
             // It does not make sense to have 'q=0'.
             if (qvalue10 > kQvalueDecrement10) {
                 qvalue10 -= kQvalueDecrement10;
             }
         }
         return langListWithQ.toString();
     }
 }