Fix the generation of Accept-Language headers.

We add the base language to the Accept-Language headers, if a corresponding
language+region code is present.
This change wants to address a long-standing bug that occasionally makes
websites return resources in a non-preferred language.

See design doc: https://docs.google.com/document/d/10eGUww_2Ufv-YyGwnmr9ke_89Q6By_94v02FM_NTU24

Bug: 737232
Change-Id: I67bea7cb368c744330ec11773fe4a6a69c0e3959
Reviewed-on: https://chromium-review.googlesource.com/625496
Commit-Queue: Claudio M <[email protected]>
Reviewed-by: Dimitri Glazkov <[email protected]>
Cr-Commit-Position: refs/heads/master@{#499136}

chrome/browser/about_flags.cc

@@ -3200,6 +3200,11 @@ const FeatureEntry kFeatureEntries[] = {
      flag_descriptions::kCaptureThumbnailOnLoadFinishedDescription, kOsDesktop,
      FEATURE_VALUE_TYPE(features::kCaptureThumbnailOnLoadFinished)},
 
+    {"use-new-accept-language-header",
+     flag_descriptions::kUseNewAcceptLanguageHeaderName,
+     flag_descriptions::kUseNewAcceptLanguageHeaderDescription, kOsAll,
+     FEATURE_VALUE_TYPE(features::kUseNewAcceptLanguageHeader)},
+
 #if defined(OS_WIN)
     {"enable-d3d-vsync", flag_descriptions::kEnableD3DVsync,
      flag_descriptions::kEnableD3DVsyncDescription, kOsWin,

chrome/browser/flag_descriptions.cc

@@ -942,6 +942,12 @@ const char kOverlayScrollbarsFlashWhenMouseEnterDescription[] =
     "Flash Overlay Scrollbars When Mouse Enter a scrollable area. You must also"
     " enable Overlay Scrollbars.";
 
+const char kUseNewAcceptLanguageHeaderName[] = "Use new Accept-Language header";
+const char kUseNewAcceptLanguageHeaderDescription[] =
+    "Adds the base language code after other corresponding language+region "
+    "codes. This ensures that users receive content in their preferred "
+    "language.";
+
 const char kOverscrollHistoryNavigationName[] = "Overscroll history navigation";
 const char kOverscrollHistoryNavigationDescription[] =
     "Experimental history navigation in response to horizontal overscroll.";

chrome/browser/flag_descriptions.h

@@ -583,6 +583,9 @@ extern const char kOverlayScrollbarsFlashAfterAnyScrollUpdateDescription[];
 extern const char kOverlayScrollbarsFlashWhenMouseEnterName[];
 extern const char kOverlayScrollbarsFlashWhenMouseEnterDescription[];
 
+extern const char kUseNewAcceptLanguageHeaderName[];
+extern const char kUseNewAcceptLanguageHeaderDescription[];
+
 extern const char kOverscrollHistoryNavigationName[];
 extern const char kOverscrollHistoryNavigationDescription[];
 extern const char kOverscrollHistoryNavigationSimpleUi[];

chrome/browser/net/chrome_http_user_agent_settings.cc

@@ -4,18 +4,71 @@
 
 #include "chrome/browser/net/chrome_http_user_agent_settings.h"
 
+#include "base/feature_list.h"
+#include "base/strings/string_piece.h"
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "chrome/common/chrome_content_client.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/http/http_util.h"
 
+namespace {
+
+// Helper class that builds the list of languages for the Accept-Language
+// headers.
+// The output is a comma-separated list of languages as string.
+// Duplicates are removed.
+class AcceptLanguageBuilder {
+ public:
+  // Adds a language to the string.
+  // Duplicates are ignored.
+  void AddLanguageCode(const std::string& language) {
+    if (seen_.find(language) == seen_.end()) {
+      if (str_.empty()) {
+        base::StringAppendF(&str_, "%s", language.c_str());
+      } else {
+        base::StringAppendF(&str_, ",%s", language.c_str());
+      }
+      seen_.insert(language);
+    }
+  }
+
+  // Returns the string constructed up to this point.
+  std::string GetString() const { return str_; }
+
+ private:
+  // The string that contains the list of languages, comma-separated.
+  std::string str_;
+  // Set the remove duplicates.
+  std::unordered_set<std::string> seen_;
+};
+
+// Extract the base language code from a language code.
+// If there is no '-' in the code, the original code is returned.
+std::string GetBaseLanguageCode(const std::string& language_code) {
+  const std::vector<std::string> tokens = base::SplitString(
+      language_code, "-", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
+  return tokens.empty() ? "" : tokens[0];
+}
+
+}  // namespace
+
 ChromeHttpUserAgentSettings::ChromeHttpUserAgentSettings(PrefService* prefs) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   pref_accept_language_.Init(prefs::kAcceptLanguages, prefs);
   last_pref_accept_language_ = *pref_accept_language_;
+
+  const std::string accept_languages_str =
+      base::FeatureList::IsEnabled(features::kUseNewAcceptLanguageHeader)
+          ? ExpandLanguageList(last_pref_accept_language_)
+          : last_pref_accept_language_;
   last_http_accept_language_ =
-      net::HttpUtil::GenerateAcceptLanguageHeader(last_pref_accept_language_);
+      net::HttpUtil::GenerateAcceptLanguageHeader(accept_languages_str);
+
   pref_accept_language_.MoveToThread(
       content::BrowserThread::GetTaskRunnerForThread(
           content::BrowserThread::IO));
@@ -25,6 +78,35 @@ ChromeHttpUserAgentSettings::~ChromeHttpUserAgentSettings() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 }
 
+std::string ChromeHttpUserAgentSettings::ExpandLanguageList(
+    const std::string& language_prefs) {
+  const std::vector<std::string> languages = base::SplitString(
+      language_prefs, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
+
+  if (languages.empty())
+    return "";
+
+  AcceptLanguageBuilder builder;
+
+  const int size = languages.size();
+  for (int i = 0; i < size; ++i) {
+    const std::string& language = languages[i];
+    builder.AddLanguageCode(language);
+
+    // Extract the base language
+    const std::string& base_language = GetBaseLanguageCode(language);
+
+    // Look ahead and add the base language if the next language is not part
+    // of the same family.
+    const int j = i + 1;
+    if (j >= size || GetBaseLanguageCode(languages[j]) != base_language) {
+      builder.AddLanguageCode(base_language);
+    }
+  }
+
+  return builder.GetString();
+}
+
 void ChromeHttpUserAgentSettings::CleanupOnUIThread() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   pref_accept_language_.Destroy();
@@ -33,11 +115,17 @@ void ChromeHttpUserAgentSettings::CleanupOnUIThread() {
 std::string ChromeHttpUserAgentSettings::GetAcceptLanguage() const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   std::string new_pref_accept_language = *pref_accept_language_;
+
   if (new_pref_accept_language != last_pref_accept_language_) {
+    const std::string accept_languages_str =
+        base::FeatureList::IsEnabled(features::kUseNewAcceptLanguageHeader)
+            ? ExpandLanguageList(new_pref_accept_language)
+            : new_pref_accept_language;
     last_http_accept_language_ =
-        net::HttpUtil::GenerateAcceptLanguageHeader(new_pref_accept_language);
+        net::HttpUtil::GenerateAcceptLanguageHeader(accept_languages_str);
     last_pref_accept_language_ = new_pref_accept_language;
   }
+
   return last_http_accept_language_;
 }
 

chrome/browser/net/chrome_http_user_agent_settings.h

@@ -23,6 +23,9 @@ class ChromeHttpUserAgentSettings : public net::HttpUserAgentSettings {
   // Must be called on the IO thread.
   ~ChromeHttpUserAgentSettings() override;
 
+  // Adds the base language if a corresponding language+region code is present.
+  static std::string ExpandLanguageList(const std::string& language_prefs);
+
   void CleanupOnUIThread();
 
   // net::HttpUserAgentSettings implementation

chrome/browser/net/chrome_http_user_agent_settings_unittest.cc

@@ -0,0 +1,39 @@
+// Copyright (c) 2017 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.
+
+#include "chrome/browser/net/chrome_http_user_agent_settings.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+// Test the expansion of the Language List.
+TEST(ChromeHttpUserAgentSettings, ExpandLanguageList) {
+  std::string output = ChromeHttpUserAgentSettings::ExpandLanguageList("");
+  EXPECT_EQ("", output);
+
+  output = ChromeHttpUserAgentSettings::ExpandLanguageList("en-US");
+  EXPECT_EQ("en-US,en", output);
+
+  output = ChromeHttpUserAgentSettings::ExpandLanguageList("fr");
+  EXPECT_EQ("fr", output);
+
+  // The base language is added after all regional codes...
+  output = ChromeHttpUserAgentSettings::ExpandLanguageList("en-US,en-CA");
+  EXPECT_EQ("en-US,en-CA,en", output);
+
+  // ... but before other language families.
+  output = ChromeHttpUserAgentSettings::ExpandLanguageList("en-US,en-CA,fr");
+  EXPECT_EQ("en-US,en-CA,en,fr", output);
+
+  output =
+      ChromeHttpUserAgentSettings::ExpandLanguageList("en-US,en-CA,fr,en-AU");
+  EXPECT_EQ("en-US,en-CA,en,fr,en-AU", output);
+
+  output = ChromeHttpUserAgentSettings::ExpandLanguageList("en-US,en-CA,fr-CA");
+  EXPECT_EQ("en-US,en-CA,en,fr-CA,fr", output);
+
+  // Add a base language even if it's already in the list.
+  output = ChromeHttpUserAgentSettings::ExpandLanguageList(
+      "en-US,fr-CA,it,fr,es-AR,it-IT");
+  EXPECT_EQ("en-US,en,fr-CA,fr,it,es-AR,es,it-IT", output);
+}

chrome/common/chrome_features.cc

@@ -326,6 +326,12 @@ const base::Feature kOneGoogleBarOnLocalNtp{"OneGoogleBarOnLocalNtp",
                                             base::FEATURE_DISABLED_BY_DEFAULT};
 #endif
 
+// Adds the base language code to the Language-Accept headers if at least one
+// corresponding language+region code is present in the user preferences.
+// For example: "en-US, fr-FR" --> "en-US, en, fr-FR, fr".
+const base::Feature kUseNewAcceptLanguageHeader{
+    "UseNewAcceptLanguageHeader", base::FEATURE_DISABLED_BY_DEFAULT};
+
 // Enables Permissions Blacklisting via Safe Browsing.
 const base::Feature kPermissionsBlacklist{
     "PermissionsBlacklist", base::FEATURE_DISABLED_BY_DEFAULT};

chrome/common/chrome_features.h

@@ -169,6 +169,8 @@ extern const base::Feature kOfflinePageDownloadSuggestionsFeature;
 extern const base::Feature kOneGoogleBarOnLocalNtp;
 #endif
 
+extern const base::Feature kUseNewAcceptLanguageHeader;
+
 extern const base::Feature kPermissionsBlacklist;
 
 #if defined(OS_WIN)

chrome/test/BUILD.gn

@@ -3204,6 +3204,7 @@ test("unit_tests") {
     "../browser/metrics/thread_watcher_android_unittest.cc",
     "../browser/metrics/thread_watcher_unittest.cc",
     "../browser/mod_pagespeed/mod_pagespeed_metrics_unittest.cc",
+    "../browser/net/chrome_http_user_agent_settings_unittest.cc",
     "../browser/net/chrome_network_delegate_unittest.cc",
     "../browser/net/dns_probe_runner_unittest.cc",
     "../browser/net/dns_probe_service_unittest.cc",

net/http/http_util.cc

@@ -738,20 +738,16 @@ std::string HttpUtil::ConvertHeadersBackToHTTPResponse(const std::string& str) {
   return disassembled_headers;
 }
 
-// TODO(jungshik): 1. If the list is 'fr-CA,fr-FR,en,de', we have to add
-// 'fr' after 'fr-CA' with the same q-value as 'fr-CA' because
-// web servers, in general, do not fall back to 'fr' and may end up picking
-// 'en' which has a lower preference than 'fr-CA' and 'fr-FR'.
-// 2. This function assumes that the input is a comma separated list
-// without any whitespace. As long as it comes from the preference and
+// TODO(jungshik): This function assumes that the input is a comma separated
+// list without any whitespace. As long as it comes from the preference and
 // a user does not manually edit the preference file, it's the case. Still,
 // we may have to make it more robust.
 std::string HttpUtil::GenerateAcceptLanguageHeader(
     const std::string& raw_language_list) {
   // 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.
-  const unsigned int kQvalueDecrement10 = 2;
+  const unsigned int kQvalueDecrement10 = 1;
   unsigned int qvalue10 = 10;
   base::StringTokenizer t(raw_language_list, ",");
   std::string lang_list_with_q;

net/http/http_util_unittest.cc

@@ -709,11 +709,19 @@ TEST(HttpUtilTest, SpecForRequestForUrlWithFtpScheme) {
 }
 
 TEST(HttpUtilTest, GenerateAcceptLanguageHeader) {
-  EXPECT_EQ(std::string("en-US,fr;q=0.8,de;q=0.6"),
-            HttpUtil::GenerateAcceptLanguageHeader("en-US,fr,de"));
-  EXPECT_EQ(std::string("en-US,fr;q=0.8,de;q=0.6,ko;q=0.4,zh-CN;q=0.2,"
-                        "ja;q=0.2"),
-            HttpUtil::GenerateAcceptLanguageHeader("en-US,fr,de,ko,zh-CN,ja"));
+  std::string header = HttpUtil::GenerateAcceptLanguageHeader("");
+  EXPECT_TRUE(header.empty());
+
+  header = HttpUtil::GenerateAcceptLanguageHeader("es");
+  EXPECT_EQ(std::string("es"), header);
+
+  header = HttpUtil::GenerateAcceptLanguageHeader("en-US,fr,de");
+  EXPECT_EQ(std::string("en-US,fr;q=0.9,de;q=0.8"), header);
+
+  header = HttpUtil::GenerateAcceptLanguageHeader("en-US,fr,de,ko,zh-CN,ja");
+  EXPECT_EQ(
+      std::string("en-US,fr;q=0.9,de;q=0.8,ko;q=0.7,zh-CN;q=0.6,ja;q=0.5"),
+      header);
 }
 
 // HttpResponseHeadersTest.GetMimeType also tests ParseContentType.

tools/metrics/histograms/enums.xml

@@ -23160,6 +23160,7 @@ uploading your change for review.  These are checked by presubmit scripts.
   <int value="-1617183455" label="OfflineRecentPages:disabled"/>
   <int value="-1616855537" label="enable-manual-password-generation:disabled"/>
   <int value="-1614912400" label="enable-link-disambiguation-popup"/>
+  <int value="-1613583483" label="UseNewAcceptLanguageHeader:enabled"/>
   <int value="-1611305202" label="KeepPrefetchedContentSuggestions:disabled"/>
   <int value="-1607691647" label="MojoVideoEncodeAccelerator:disabled"/>
   <int value="-1605567628" label="disable-overlay-scrollbar"/>
@@ -23560,6 +23561,7 @@ uploading your change for review.  These are checked by presubmit scripts.
   <int value="-351127770" label="enable-offline-pages-as-bookmarks"/>
   <int value="-349437334" label="UseDdljsonApi:disabled"/>
   <int value="-349057743" label="extensions-on-chrome-urls"/>
+  <int value="-346413328" label="UseNewAcceptLanguageHeader:disabled"/>
   <int value="-345838366" label="enable-hosted-apps-in-windows"/>
   <int value="-345324571" label="enable-quirks-client"/>
   <int value="-344343842" label="disable-experimental-app-list"/>