Extend with a language code in http accept languages

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 86cb7c43a271e42fa198afd2d116d7c5d678c0b0..cdc5f90e3ec9d8a6e6519319f4b4193be1cd901e 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
@@ -27,7 +27,6 @@ import org.chromium.chrome.browser.physicalweb.PwsClient.ResolveScanCallback;
 import java.net.MalformedURLException;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Formatter;
 import java.util.HashSet;
 import java.util.Locale;
@@ -252,38 +251,17 @@ class PwsClientImpl implements PwsClient {
         String localeStrings = locales + "," + acceptLanguages;
         String[] localeList = localeStrings.split(",");
 
-        ArrayList<Locale> uniqueList = new ArrayList<>();
+        HashSet<Locale> seenLocales = new HashSet<>();
+        ArrayList<String> uniqueList = new ArrayList<>();
         for (String localeString : localeList) {
             Locale locale = LocaleUtils.forLanguageTag(localeString);
-            if (uniqueList.contains(locale) || locale.getLanguage().isEmpty()) {
+            if (seenLocales.contains(locale) || locale.getLanguage().isEmpty()) {
                 continue;
             }
-            uniqueList.add(locale);
+            seenLocales.add(locale);
+            uniqueList.add(LocaleUtils.toLanguageTag(locale));

[Ryan Sleevi, 2016/12/15 23:44:07]

I'm having trouble convincing myself this is right, but fair warning: I'm not
terribly good at Java :)

Previously, uniqueList was truly unique because old-258 made sure no dupes (of
locale) are introduced.

Your new code searches seenLocales, but adds based on .toLanguageTag. Can two
locales result in the same language tag (I thought they could)? is LocaleUtil's
not reflexive?

That is, line 262 (when combined with 257) could be rewritten as:
uniqueList.add(LocaleUtils.toLanguageTag(localeUtils.forLanguageTag(localeString)))


If it was reflexive, couldn't this just be written as
uniqueList.add(localeString) ? Why or why not?

[Yirui Huang, 2016/12/16 08:00:08]

The reason why LocaleUtils is involved is because that Java keeps some
deprecated languages while Chromium uses others, a language mapping is applied. 

[jungshik at Google, 2016/12/16 08:01:45]

It depends on an input string (input locale_string). Round-tripping via
forLanguageTag followed by toLanguageTag effectively canonicalize the locale
identifier. ( see, for instance,
https://cs.chromium.org/chromium/src/v8/src/runtime/runtime-i18n.cc?rcl=0&l=69
). 

In this particular case, the source of locale identifiers is not random user
input but Chrome itself or Android (locale list).  So,  I don't see  a need to
canonicalize (it should be already canonicalized). 

         }
-
-        // 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--) {
-            Locale localeAdd = uniqueList.get(i);
-            String languageAdd = localeAdd.getLanguage();
-            String countryAdd = localeAdd.getCountry();
-
-            if (!seenLanguages.contains(languageAdd)) {
-                seenLanguages.add(languageAdd);
-                outputList.add(languageAdd);
-            }
-            if (!countryAdd.isEmpty()) {
-                outputList.add(LocaleUtils.toLanguageTag(localeAdd));
-            }
-        }
-        Collections.reverse(outputList);
-        return TextUtils.join(",", outputList);
+        return TextUtils.join(",", uniqueList);
     }
 
     /**