Add client hints support for search prefetch

This refactors client hints to allow main frame prefetch navigations to
add client hints without needing a frame.

Related headers CL: https://chromium-review.googlesource.com/c/chromium/src/+/2552723

Bug: 1142074
Change-Id: I662d4a8529f7546df80d81128d7df8545717fbb8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2551671Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831974}

chrome/browser/prefetch/search_prefetch/base_search_prefetch_request.cc

@@ -5,7 +5,10 @@
 #include "chrome/browser/prefetch/search_prefetch/base_search_prefetch_request.h"
 
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/common/pref_names.h"
+#include "components/prefs/pref_service.h"
 #include "components/variations/net/variations_http_headers.h"
+#include "content/public/browser/client_hints.h"
 #include "content/public/browser/frame_accept_header.h"
 #include "content/public/common/content_constants.h"
 #include "net/base/load_flags.h"
@@ -70,6 +73,15 @@ void BaseSearchPrefetchRequest::StartPrefetchRequest(Profile* profile) {
   resource_request->headers.SetHeader(
       net::HttpRequestHeaders::kAccept,
       content::FrameAcceptHeaderValue(/*allow_sxg_responses=*/true, profile));
+
+  bool js_enabled = profile->GetPrefs() && profile->GetPrefs()->GetBoolean(
+                                               prefs::kWebKitJavascriptEnabled);
+
+  AddClientHintsHeadersToPrefetchNavigation(
+      resource_request->url, &(resource_request->headers), profile,
+      profile->GetClientHintsControllerDelegate(),
+      /*is_ua_override_on=*/false, js_enabled);
+
   // TODO(ryansturm): Find other headers that may need to be set.
   // https://crbug.com/1138648
 

chrome/browser/prefetch/search_prefetch/search_prefetch_service_browsertest.cc

@@ -53,6 +53,7 @@ constexpr char kOmniboxSuggestPrefetchQuery[] = "porgs";
 constexpr char kOmniboxSuggestPrefetchSecondItemQuery[] = "porgsandwich";
 constexpr char kOmniboxSuggestNonPrefetchQuery[] = "puffins";
 constexpr char kLoadInSubframe[] = "/load_in_subframe";
+constexpr char kClientHintsURL[] = "/accept_ch_with_lifetime.html";
 }  // namespace
 
 // A response that hangs after serving the start of the response.
@@ -73,6 +74,8 @@ class SearchPrefetchBaseBrowserTest : public InProcessBrowserTest {
     search_server_ = std::make_unique<net::EmbeddedTestServer>(
         net::EmbeddedTestServer::TYPE_HTTPS);
     search_server_->ServeFilesFromSourceDirectory("chrome/test/data");
+    search_server_->ServeFilesFromSourceDirectory(
+        "chrome/test/data/client_hints");
     search_server_->RegisterRequestHandler(
         base::BindRepeating(&SearchPrefetchBaseBrowserTest::HandleSearchRequest,
                             base::Unretained(this)));
@@ -253,6 +256,9 @@ class SearchPrefetchBaseBrowserTest : public InProcessBrowserTest {
     if (request.GetURL().spec().find("favicon") != std::string::npos)
       return nullptr;
 
+    if (request.relative_url == kClientHintsURL)
+      return nullptr;
+
     if (hang_requests_after_start_) {
       return std::make_unique<HangRequestAfterStart>();
     }
@@ -515,6 +521,8 @@ IN_PROC_BROWSER_TEST_P(SearchPrefetchServiceEnabledBrowserTest,
   EXPECT_TRUE(base::Contains(headers["Accept"], "text/html"));
   EXPECT_EQ(1u, search_server_request_count());
   EXPECT_EQ(1u, search_server_prefetch_request_count());
+  // Make sure we don't get client hints headers by default.
+  EXPECT_FALSE(base::Contains(headers, "viewport-width"));
 
   prefetch_status = search_prefetch_service->GetSearchPrefetchStatusForTesting(
       base::ASCIIToUTF16(search_terms));
@@ -551,6 +559,38 @@ IN_PROC_BROWSER_TEST_P(SearchPrefetchServiceEnabledBrowserTest,
   EXPECT_FALSE(prefetch_status.has_value());
 }
 
+IN_PROC_BROWSER_TEST_P(SearchPrefetchServiceEnabledBrowserTest,
+                       BasicClientHintsFunctionality) {
+  // Fetch a response that will set client hints on future requests.
+  GURL client_hints = GetSearchServerQueryURLWithNoQuery(kClientHintsURL);
+  ui_test_utils::NavigateToURL(browser(), client_hints);
+
+  auto* search_prefetch_service =
+      SearchPrefetchServiceFactory::GetForProfile(browser()->profile());
+  EXPECT_NE(nullptr, search_prefetch_service);
+
+  std::string search_terms = "prefetch_content";
+
+  GURL prefetch_url = GetSearchServerQueryURL(search_terms);
+
+  EXPECT_TRUE(search_prefetch_service->MaybePrefetchURL(prefetch_url));
+  auto prefetch_status =
+      search_prefetch_service->GetSearchPrefetchStatusForTesting(
+          base::ASCIIToUTF16(search_terms));
+  ASSERT_TRUE(prefetch_status.has_value());
+  EXPECT_EQ(SearchPrefetchStatus::kInFlight, prefetch_status.value());
+
+  WaitUntilStatusChanges(base::ASCIIToUTF16(search_terms));
+
+  EXPECT_EQ(1u, search_server_requests().size());
+  EXPECT_NE(std::string::npos,
+            search_server_requests()[0].GetURL().spec().find(search_terms));
+  auto headers = search_server_requests()[0].headers;
+
+  // Make sure we can get client hints headers.
+  EXPECT_TRUE(base::Contains(headers, "viewport-width"));
+}
+
 IN_PROC_BROWSER_TEST_P(SearchPrefetchServiceEnabledBrowserTest,
                        502PrefetchFunctionality) {
   auto* search_prefetch_service =

content/browser/client_hints/client_hints.cc

@@ -397,13 +397,13 @@ struct ClientHintsExtendedData {
     // instead. Otherwise, the current frame is an iframe and the main frame URL
     // was committed, so we can safely get it from it. Similarly, an
     // in-navigation main frame doesn't yet have a feature policy.
-    RenderFrameHostImpl* main_frame =
-        frame_tree_node->frame_tree()->GetMainFrame();
-    is_main_frame = frame_tree_node->IsMainFrame();
+    is_main_frame = !frame_tree_node || frame_tree_node->IsMainFrame();
     if (is_main_frame) {
       main_frame_url = url;
       is_1p_origin = true;
     } else {
+      RenderFrameHostImpl* main_frame =
+          frame_tree_node->frame_tree()->GetMainFrame();
       main_frame_url = main_frame->GetLastCommittedURL();
       feature_policy = main_frame->feature_policy();
       is_1p_origin = resource_origin.IsSameOriginWith(
@@ -564,7 +564,7 @@ bool ShouldAddClientHints(const GURL& url,
   // WebView) enable/disable JavaScript on a per-View basis, using the
   // WebPreferences setting.
   return IsValidURLForClientHints(url) && delegate->IsJavaScriptAllowed(url) &&
-         IsJavascriptEnabled(frame_tree_node);
+         (!frame_tree_node || IsJavascriptEnabled(frame_tree_node));
 }
 
 unsigned long RoundRttForTesting(const std::string& host,
@@ -583,6 +583,7 @@ void UpdateNavigationRequestClientUaHeaders(
     bool override_ua,
     FrameTreeNode* frame_tree_node,
     net::HttpRequestHeaders* headers) {
+  DCHECK(frame_tree_node);
   if (!delegate->UserAgentClientHintEnabled() ||
       !ShouldAddClientHints(url, frame_tree_node, delegate)) {
     return;
@@ -593,24 +594,14 @@ void UpdateNavigationRequestClientUaHeaders(
       ClientUaHeaderCallType::kAfterCreated, headers);
 }
 
-void AddNavigationRequestClientHintsHeaders(
-    const GURL& url,
-    net::HttpRequestHeaders* headers,
-    BrowserContext* context,
-    ClientHintsControllerDelegate* delegate,
-    bool is_ua_override_on,
-    FrameTreeNode* frame_tree_node) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  DCHECK_EQ(blink::kWebEffectiveConnectionTypeMappingCount,
-            net::EFFECTIVE_CONNECTION_TYPE_4G + 1u);
-  DCHECK_EQ(blink::kWebEffectiveConnectionTypeMappingCount,
-            static_cast<size_t>(net::EFFECTIVE_CONNECTION_TYPE_LAST));
-  DCHECK(context);
-
-  if (!ShouldAddClientHints(url, frame_tree_node, delegate)) {
-    return;
-  }
+namespace {
 
+void AddRequestClientHintsHeaders(const GURL& url,
+                                  net::HttpRequestHeaders* headers,
+                                  BrowserContext* context,
+                                  ClientHintsControllerDelegate* delegate,
+                                  bool is_ua_override_on,
+                                  FrameTreeNode* frame_tree_node) {
   const ClientHintsExtendedData data(url, frame_tree_node, delegate);
 
   // Add Headers
@@ -662,6 +653,56 @@ void AddNavigationRequestClientHintsHeaders(
   // scheme or a change in the origin.
 }
 
+}  // namespace
+
+void AddPrefetchNavigationRequestClientHintsHeaders(
+    const GURL& url,
+    net::HttpRequestHeaders* headers,
+    BrowserContext* context,
+    ClientHintsControllerDelegate* delegate,
+    bool is_ua_override_on,
+    bool is_javascript_enabled) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_EQ(blink::kWebEffectiveConnectionTypeMappingCount,
+            net::EFFECTIVE_CONNECTION_TYPE_4G + 1u);
+  DCHECK_EQ(blink::kWebEffectiveConnectionTypeMappingCount,
+            static_cast<size_t>(net::EFFECTIVE_CONNECTION_TYPE_LAST));
+  DCHECK(context);
+
+  // Since prefetch navigation doesn't have a related frame tree node,
+  // |is_javascript_enabled| is passed in to get whether a typical frame tree
+  // node would support javascript.
+  if (!is_javascript_enabled || !ShouldAddClientHints(url, nullptr, delegate)) {
+    return;
+  }
+
+  AddRequestClientHintsHeaders(url, headers, context, delegate,
+                               is_ua_override_on, nullptr);
+}
+
+void AddNavigationRequestClientHintsHeaders(
+    const GURL& url,
+    net::HttpRequestHeaders* headers,
+    BrowserContext* context,
+    ClientHintsControllerDelegate* delegate,
+    bool is_ua_override_on,
+    FrameTreeNode* frame_tree_node) {
+  DCHECK(frame_tree_node);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_EQ(blink::kWebEffectiveConnectionTypeMappingCount,
+            net::EFFECTIVE_CONNECTION_TYPE_4G + 1u);
+  DCHECK_EQ(blink::kWebEffectiveConnectionTypeMappingCount,
+            static_cast<size_t>(net::EFFECTIVE_CONNECTION_TYPE_LAST));
+  DCHECK(context);
+
+  if (!ShouldAddClientHints(url, frame_tree_node, delegate)) {
+    return;
+  }
+
+  AddRequestClientHintsHeaders(url, headers, context, delegate,
+                               is_ua_override_on, frame_tree_node);
+}
+
 base::Optional<std::vector<network::mojom::WebClientHintsType>>
 ParseAndPersistAcceptCHForNagivation(
     const GURL& url,

content/browser/client_hints/client_hints.h

@@ -64,6 +64,17 @@ CONTENT_EXPORT void AddNavigationRequestClientHintsHeaders(
     bool is_ua_override_on,
     FrameTreeNode*);
 
+// Adds client hints headers for a prefetch navigation that is not associated
+// with a frame. It must be a main frame navigation. |is_javascript_enabled| is
+// whether JavaScript is enabled in blink or not.
+CONTENT_EXPORT void AddPrefetchNavigationRequestClientHintsHeaders(
+    const GURL& url,
+    net::HttpRequestHeaders* headers,
+    BrowserContext* context,
+    ClientHintsControllerDelegate* delegate,
+    bool is_ua_override_on,
+    bool is_javascript_enabled);
+
 // Parses incoming client hints and persists them as appropriate. Returns
 // hints that were accepted as enabled even if they are not going to be
 // persisted. The distinction is relevant in legacy case where feature policy

content/public/browser/BUILD.gn

@@ -99,6 +99,8 @@ source_set("browser_sources") {
     "child_process_termination_info.h",
     "clear_site_data_utils.h",
     "client_certificate_delegate.h",
+    "client_hints.cc",
+    "client_hints.h",
     "client_hints_controller_delegate.h",
     "color_chooser.h",
     "console_message.h",

content/public/browser/client_hints.cc

+// Copyright 2020 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 "content/public/browser/client_hints.h"
+#include "content/browser/client_hints/client_hints.h"
+
+namespace content {
+
+void AddClientHintsHeadersToPrefetchNavigation(
+    const GURL& url,
+    net::HttpRequestHeaders* headers,
+    BrowserContext* context,
+    ClientHintsControllerDelegate* delegate,
+    bool is_ua_override_on,
+    bool is_javascript_enabled) {
+  AddPrefetchNavigationRequestClientHintsHeaders(url, headers, context,
+                                                 delegate, is_ua_override_on,
+                                                 is_javascript_enabled);
+}
+
+}  // namespace content

content/public/browser/client_hints.h

+// Copyright 2020 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.
+
+#ifndef CONTENT_PUBLIC_BROWSER_CLIENT_HINTS_H_
+#define CONTENT_PUBLIC_BROWSER_CLIENT_HINTS_H_
+
+#include "content/public/browser/client_hints_controller_delegate.h"
+#include "net/http/http_request_headers.h"
+#include "services/network/public/mojom/parsed_headers.mojom-forward.h"
+
+namespace content {
+
+class BrowserContext;
+
+// Adds client hints headers for a prefetch navigation that is not associated
+// with a frame. It must be a main frame navigation. |is_javascript_enabled| is
+// whether JavaScript is enabled in blink or not.
+CONTENT_EXPORT void AddClientHintsHeadersToPrefetchNavigation(
+    const GURL& url,
+    net::HttpRequestHeaders* headers,
+    BrowserContext* context,
+    ClientHintsControllerDelegate* delegate,
+    bool is_ua_override_on,
+    bool is_javascript_enabled);
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_BROWSER_CLIENT_HINTS_H_