Set User-Agent and Accept-Language request headers for PWS requests

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 20f0a611a287ed7e51c1a3c3d7a6c8507f0b204c..0bfcb4a01da80aa292cfc6b50e4f409bfd3bda14 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
@@ -4,12 +4,16 @@
 
 package org.chromium.chrome.browser.physicalweb;
 
+import android.content.Context;
 import android.graphics.Bitmap;
 import android.os.AsyncTask;
+import android.os.Build;
 
 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;
@@ -21,6 +25,8 @@ import org.json.JSONObject;
 import java.net.MalformedURLException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Formatter;
+import java.util.Locale;
 
 /**
  * This class sends requests to the Physical Web Service.
@@ -30,6 +36,25 @@ class PwsClientImpl implements PwsClient {
     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
+    // Android User-Agent but doesn't need to match perfectly as this value will only be seen by the
+    // Physical Web metadata service and favicon fetcher.
+    // The WebKit version is not accessible from here so it is reported as 0.0.
+    private static final String USER_AGENT_FORMAT =
+            "Mozilla/5.0 (%s) AppleWebKit/0.0 (KHTML, like Gecko) %s Safari/0.0";
+    private static final String OS_INFO_FORMAT = "Linux; Android %s; %s Build/%s";
+    private static final String PRODUCT_FORMAT = "Chrome/%s Mobile";
+
+    private static String sUserAgent;

[nyquist, 2016/05/25 22:02:48]

These are lazily initialized, so could you mention that here?

[mattreynolds, 2016/05/26 00:31:44]

Done.

+    private static String sAcceptLanguage;
+    private static String sDefaultLocale;
+
+    private Context mContext;

[nyquist, 2016/05/25 22:02:48]

Could you add a comment as to the lifetime of this context?

[mattreynolds, 2016/05/26 00:31:44]

Done. (also made it final)

+
+    public PwsClientImpl(Context context) {
+        mContext = context;
+    }
+
     private String getApiKey() {
         if (ChromeVersionInfo.isStableBuild()) {
             return GoogleAPIKeys.GOOGLE_API_KEY;
@@ -117,7 +142,8 @@ class PwsClientImpl implements PwsClient {
         try {
             JSONObject payload = createResolveScanPayload(broadcastUrls);
             String url = ENDPOINT_URL + "?key=" + getApiKey();
-            request = new JsonObjectHttpRequest(url, payload, requestCallback);
+            request = new JsonObjectHttpRequest(url, getUserAgent(), getAcceptLanguage(), payload,
+                    requestCallback);
         } catch (MalformedURLException e) {
             Log.e(TAG, "Error creating PWS HTTP request", e);
             return;
@@ -159,7 +185,8 @@ class PwsClientImpl implements PwsClient {
         // Create the request.
         BitmapHttpRequest request = null;
         try {
-            request = new BitmapHttpRequest(iconUrl, requestCallback);
+            request = new BitmapHttpRequest(iconUrl, getUserAgent(), getAcceptLanguage(),
+                    requestCallback);
         } catch (MalformedURLException e) {
             Log.e(TAG, "Error creating icon request", e);
             return;
@@ -167,4 +194,130 @@ class PwsClientImpl implements PwsClient {
         // 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
+     * native code.
+     * @return A User-Agent string
+     */
+    @VisibleForTesting
+    String getUserAgent() {
+        if (sUserAgent == null) {
+            // Build the OS info string.
+            // eg: Linux; Android 5.1.1; Nexus 4 Build/LMY48T
+            String osInfo = String.format(OS_INFO_FORMAT, Build.VERSION.RELEASE, Build.MODEL,
+                    Build.ID);
+
+            // Build the product string.
+            // eg: Chrome/50.0.2661.89 Mobile
+            String product = String.format(PRODUCT_FORMAT, ChromeVersionInfo.getProductVersion());
+
+            // 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.
+     */
+    @VisibleForTesting
+    String getAcceptLanguage() {
+        String defaultLocale = Locale.getDefault().toString();
+        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.
+        if ("iw".equals(language)) {
+            language = "he";

[nyquist, 2016/05/25 22:02:48]

It's a little bit awkward to reassign the input param. Could you instead have a
local variable that defaults to the input param?

Or, maybe just use a switch?
switch (language) {
  case "iw": return "he" + "-" + region;
  case "ji": return "yi" + "-" + region;
  case "in": return "id" + "-" + region;
  default: return language + "-" + region;
}

[mattreynolds, 2016/05/26 00:31:44]

Done.

+        } else if ("ji".equals(language)) {
+            language = "yi";
+        } else if ("in".equals(language)) {
+            language = "id";
+        }
+
+        return language + "-" + 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

[nyquist, 2016/05/25 22:02:48]

Should you update that code to refer to this code?

[mattreynolds, 2016/05/26 00:31:44]

Done.

+     * @param locale A string representing the default locale.
+     * @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;
+        }
+
+        String language = locale.substring(0, 2);
+        String region = locale.substring(3);
+        String languageTag = makeLanguageTag(language, region);
+
+        if (!acceptLanguages.contains(languageTag)) {

[nyquist, 2016/05/25 22:02:48]

Could you flip this, and just return acceptLanguages here? Then you don't need
the indent, and the code still seems clean to read.

[mattreynolds, 2016/05/26 00:31:44]

Done.

+            Formatter parts = new Formatter();
+            parts.format("%s,", languageTag);
+            // If language is not in the accept languages list, also add language code.
+            if (!acceptLanguages.contains(language + ",") && !acceptLanguages.endsWith(language)) {

[nyquist, 2016/05/25 22:02:48]

Is there ever any chance of this happening:

acceptLanguages = "coolfoo,notsobar"
and language being "foo", and thee this code would trigger for "foo,"?

Or is there a guarantee for the length of all the accept languages?
And if there is, could you mention that here in a comment so new readers won't
have the same naive question as me?

[mattreynolds, 2016/05/26 00:31:44]

It could fail to append the language tag correctly for an arbitrary list of
language tags, but it should never happen here since the strings defined in
Chrome's localized resource files follow a strict format:

https://code.google.com/p/chromium/codesearch#search/&q=id=%5C%22IDS_ACCEPT_L...

+                parts.format("%s,", language);
+            }
+            parts.format("%s", acceptLanguages);
+            return parts.toString();
+        }
+
+        return acceptLanguages;
+    }
+
+    /**
+     * Given a list of comma-delimited language codes, insert q-values to represent the relative
+     * quality/precedence of each language. The logic should match GenerateAcceptLanguageHeader in
+     * net/http/http_util.cc

[nyquist, 2016/05/25 22:02:48]

Do we want a back-reference?

[mattreynolds, 2016/05/26 00:31:44]

Done.

+     * @param languageList A comma-delimited list of language codes containing no whitespace.
+     * @return An Accept-Language header with q-values.
+     */
+    @VisibleForTesting
+    static String generateAcceptLanguageHeader(String languageList) {
+        // We use integers for qvalue and qvalue decrement that are 10 times larger than actual
+        // values to avoid a problem with comparing two floating point numbers.
+        int kQvalueDecrement10 = 2;
+        int qvalue10 = 10;
+        String[] parts = languageList.split(",");
+        Formatter langListWithQ = new Formatter();
+        for (String language : parts) {
+            if (qvalue10 == 10) {
+                // q=1.0 is implicit
+                langListWithQ.format("%s", language);
+            } else {
+                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();
+    }
 }