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 org.json.JSONArray;
-import org.json.JSONException;
-import org.json.JSONObject;
-
 import java.net.MalformedURLException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Formatter;
-import java.util.Locale;
+import java.util.HashSet;
 
 /**
  * This class sends requests to the Physical Web Service.
  */
 class PwsClientImpl implements PwsClient {
     private static final String TAG = "PhysicalWeb";
     private static final String ENDPOINT_URL =
             "https://physicalweb.googleapis.com/v1alpha1/urls:resolve";
 
     // Format strings for creating the User-Agent string. It should somewhat resemble the Chrome for
@@ skipping 176 matching lines @@
             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.
      */
     @VisibleForTesting
     String getAcceptLanguage() {

[ksk1, 2016/10/11 09:58:08]

Can we rename this to getAcceptLanguages or something?
Also, please update the comment and variable names properly.

[Yirui Huang, 2016/10/11 12:17:32]

Done.

-        String defaultLocale = Locale.getDefault().toString();
+        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;
     }
 
     /**
@@ skipping 18 matching lines @@
             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 locale A string representing the default locale.
+     * @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 locale, String acceptLanguages)
-    {
-        if (locale.length() != 5 || locale.charAt(2) != '_') {
-            return acceptLanguages;
+    static String prependToAcceptLanguagesIfNecessary(String locales, String acceptLanguages) {

[ksk1, 2016/10/11 09:58:08]

I suspect that this CL contains your previous CL. Maybe, need to rebase?

[Yirui Huang, 2016/10/11 12:17:32]

Done.

+        String localeString = locales + "," + acceptLanguages;
+        String[] localeList = localeString.split(",");
+
+        HashSet<String> seenLocales = new HashSet<>();
+        ArrayList<String> uniqueList = new ArrayList<>();
+        for (String locale : localeList) {
+            String language;
+            String country;
+            if (locale.length() == 2 || locale.length() == 3) {
+                language = locale;
+                country = "";
+            } else if (locale.charAt(2) == '-') {
+                language = locale.substring(0, 2);
+                country = locale.substring(3);
+            } else if (locale.charAt(3) == '-') {
+                language = locale.substring(0, 3);
+                country = locale.substring(4);
+            } else {
+                continue;
+            }
+
+            String languageTag = makeLanguageTag(language, country);
+
+            if (seenLocales.contains(languageTag)) {
+                continue;
+            }
+            if (country != "") {
+                seenLocales.add(languageTag);
+            }
+            uniqueList.add(languageTag);
         }
 
-        String language = locale.substring(0, 2);
-        String region = locale.substring(3);
-        String languageTag = makeLanguageTag(language, region);
+        // 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);
+            int lastChar = localeAdd.length() - 1;
+            String languageAdd;
+            if (localeAdd.charAt(lastChar) == '-') {
+                languageAdd = localeAdd.substring(0, lastChar);
+            } else if (localeAdd.charAt(2) == '-') {
+                languageAdd = localeAdd.substring(0, 2);
+            } else { // length of the language code is 3.
+                languageAdd = localeAdd.substring(0, 3);
+            }
 
-        if (acceptLanguages.contains(languageTag)) {
-            return acceptLanguages;
+            if (!seenLanguages.contains(languageAdd)) {
+                seenLanguages.add(languageAdd);
+                outputList.add(languageAdd);
+            }
+            if (lastChar != languageAdd.length()) {
+                outputList.add(localeAdd);
+            }
         }
-
-        Formatter parts = new Formatter();
-        parts.format("%s,", languageTag);
-        // If language is not in the accept languages list, also add language code.
-        // This will work with the IDS_ACCEPT_LANGUAGES localized strings bundled with Chrome but
-        // may fail on arbitrary lists of language tags due to differences in case and whitespace.
-        if (!acceptLanguages.contains(language + ",") && !acceptLanguages.endsWith(language)) {
-            parts.format("%s,", language);
-        }
-        parts.format("%s", acceptLanguages);
-        return parts.toString();
+        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.
      * @return An Accept-Language header with q-values.
      */
     @VisibleForTesting
@@ skipping 12 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();
     }
 }