Reland "Adding more navigation-like fields to search prefetch requests"

This is a reland of f8a44173

Patchset 1 is the previously landed version.

Original change's description:
> Adding more navigation-like fields to search prefetch requests
>
> This CL adds a few fields to the search prefetch resource request to
> make it more similar to a navigation request. Additionally, this adds
> capability for URLLoader throttles to defer (in this case cancel) or
> change headers on the prefetch request allowing things like variation
> headers to be added more similarly to how navigation requests have them
> added.
>
> Client hints CL:
> https://chromium-review.googlesource.com/c/chromium/src/+/2551671
>
> Bug: 1138648
> Change-Id: Ia868aa624b2f9f573a54432eb0fa62834eacd2f5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2552723
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: Robert Ogden <robertogden@chromium.org>
> Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#833987}

Bug: 1138648
Change-Id: I1a4fefdff4aa85095e2f9f4e567b697aaa1ca0bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2575435Reviewed-by: Robert Ogden <robertogden@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834738}

chrome/browser/prefetch/search_prefetch/base_search_prefetch_request.cc

@@ -4,18 +4,80 @@
 
 #include "chrome/browser/prefetch/search_prefetch/base_search_prefetch_request.h"
 
+#include <vector>
+
+#include "chrome/browser/chrome_content_browser_client.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/browser/render_frame_host.h"
+#include "content/public/browser/url_loader_throttles.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/common/content_constants.h"
 #include "net/base/load_flags.h"
+#include "net/cookies/site_for_cookies.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "services/network/public/cpp/resource_request.h"
+#include "services/network/public/mojom/fetch_api.mojom.h"
+#include "third_party/blink/public/common/loader/url_loader_throttle.h"
+#include "third_party/blink/public/mojom/loader/resource_load_info.mojom-shared.h"
+#include "url/origin.h"
+
+namespace {
+
+// A custom URLLoaderThrottle delegate that is very sensitive. Anything that
+// would delay or cancel the request is treated the same, which would prevent
+// the prefetch request.
+class CheckForCancelledOrPausedDelegate
+    : public blink::URLLoaderThrottle::Delegate {
+ public:
+  CheckForCancelledOrPausedDelegate() = default;
+  ~CheckForCancelledOrPausedDelegate() override = default;
+
+  CheckForCancelledOrPausedDelegate(const CheckForCancelledOrPausedDelegate&) =
+      delete;
+  CheckForCancelledOrPausedDelegate& operator=(
+      const CheckForCancelledOrPausedDelegate&) = delete;
+
+  // URLLoaderThrottle::Delegate:
+  void CancelWithError(int error_code,
+                       base::StringPiece custom_reason) override {
+    cancelled_or_paused_ = true;
+  }
+
+  void Resume() override {}
+
+  void PauseReadingBodyFromNet() override { cancelled_or_paused_ = true; }
+
+  void RestartWithFlags(int additional_load_flags) override {
+    cancelled_or_paused_ = true;
+  }
+
+  void RestartWithURLResetAndFlags(int additional_load_flags) override {
+    cancelled_or_paused_ = true;
+  }
+
+  void RestartWithURLResetAndFlagsNow(int additional_load_flags) override {
+    cancelled_or_paused_ = true;
+  }
+
+  void RestartWithModifiedHeadersNow(
+      const net::HttpRequestHeaders& modified_headers) override {
+    cancelled_or_paused_ = true;
+  }
+
+  bool cancelled_or_paused() const { return cancelled_or_paused_; }
+
+ private:
+  bool cancelled_or_paused_ = false;
+};
+
+}  // namespace
 
 BaseSearchPrefetchRequest::BaseSearchPrefetchRequest(
     const GURL& prefetch_url,
@@ -25,7 +87,7 @@ BaseSearchPrefetchRequest::BaseSearchPrefetchRequest(
 
 BaseSearchPrefetchRequest::~BaseSearchPrefetchRequest() = default;
 
-void BaseSearchPrefetchRequest::StartPrefetchRequest(Profile* profile) {
+bool BaseSearchPrefetchRequest::StartPrefetchRequest(Profile* profile) {
   net::NetworkTrafficAnnotationTag network_traffic_annotation =
       net::DefineNetworkTrafficAnnotation("search_prefetch_service", R"(
         semantics {
@@ -61,6 +123,8 @@ void BaseSearchPrefetchRequest::StartPrefetchRequest(Profile* profile) {
           }
         })");
 
+  url::Origin prefetch_origin = url::Origin::Create(prefetch_url_);
+
   auto resource_request = std::make_unique<network::ResourceRequest>();
   resource_request->load_flags |= net::LOAD_PREFETCH;
   resource_request->url = prefetch_url_;
@@ -70,8 +134,26 @@ void BaseSearchPrefetchRequest::StartPrefetchRequest(Profile* profile) {
   resource_request->report_raw_headers = true;
   resource_request->credentials_mode =
       network::mojom::CredentialsMode::kInclude;
-  variations::AppendVariationsHeaderUnknownSignedIn(
-      prefetch_url_, variations::InIncognito::kNo, resource_request.get());
+  resource_request->method = "GET";
+  resource_request->mode = network::mojom::RequestMode::kNavigate;
+  resource_request->site_for_cookies =
+      net::SiteForCookies::FromUrl(prefetch_url_);
+  resource_request->destination = network::mojom::RequestDestination::kDocument;
+  resource_request->resource_type =
+      static_cast<int>(blink::mojom::ResourceType::kMainFrame);
+  resource_request->trusted_params = network::ResourceRequest::TrustedParams();
+  // We don't handle redirects, so |kOther| makes sense here.
+  resource_request->trusted_params->isolation_info = net::IsolationInfo::Create(
+      net::IsolationInfo::RequestType::kOther, prefetch_origin, prefetch_origin,
+      resource_request->site_for_cookies);
+  resource_request->referrer_policy = net::ReferrerPolicy::NO_REFERRER;
+
+  // Tack an 'Upgrade-Insecure-Requests' header to outgoing navigational
+  // requests, as described in
+  // https://w3c.github.io/webappsec/specs/upgrade/#feature-detect
+  resource_request->headers.SetHeader("Upgrade-Insecure-Requests", "1");
+  resource_request->headers.SetHeader(net::HttpRequestHeaders::kUserAgent,
+                                      GetUserAgent());
   resource_request->headers.SetHeader(content::kCorsExemptPurposeHeaderName,
                                       "prefetch");
   resource_request->headers.SetHeader(
@@ -86,13 +168,37 @@ void BaseSearchPrefetchRequest::StartPrefetchRequest(Profile* 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
+  // Before sending out the request, allow throttles to modify the request (not
+  // the URL). The rest of the URL Loader throttle calls are captured in the
+  // navigation stack. Headers can be added by throttles at this point, which we
+  // want to capture.
+  auto wc_getter =
+      base::BindRepeating([]() -> content::WebContents* { return nullptr; });
+  std::vector<std::unique_ptr<blink::URLLoaderThrottle>> throttles =
+      content::CreateContentBrowserURLLoaderThrottles(
+          *resource_request, profile, std::move(wc_getter),
+          /*navigation_ui_data=*/nullptr,
+          content::RenderFrameHost::kNoFrameTreeNodeId);
+
+  bool should_defer = false;
+  for (auto& throttle : throttles) {
+    CheckForCancelledOrPausedDelegate cancel_or_pause_delegate;
+    throttle->set_delegate(&cancel_or_pause_delegate);
+    throttle->WillStartRequest(resource_request.get(), &should_defer);
+    // Make sure throttles are deleted before |cancel_or_pause_delegate| in case
+    // they call into the delegate in the destructor.
+    throttle.reset();
+    if (should_defer || resource_request->url != prefetch_url_ ||
+        cancel_or_pause_delegate.cancelled_or_paused()) {
+      return false;
+    }
+  }
 
   current_status_ = SearchPrefetchStatus::kInFlight;
 
   StartPrefetchRequestInternal(profile, std::move(resource_request),
                                network_traffic_annotation);
+  return true;
 }
 
 void BaseSearchPrefetchRequest::CancelPrefetch() {

chrome/browser/prefetch/search_prefetch/base_search_prefetch_request.h

@@ -49,8 +49,10 @@ class BaseSearchPrefetchRequest {
       delete;
 
   // Starts the network request to prefetch |prefetch_url_|. Sets various fields
-  // on a resource request and calls |StartPrefetchRequestInternal()|.
-  void StartPrefetchRequest(Profile* profile);
+  // on a resource request and calls |StartPrefetchRequestInternal()|. Returns
+  // |false| if the request is not started (i.e., it would be deferred by
+  // throttles).
+  bool StartPrefetchRequest(Profile* profile);
 
   // Marks a prefetch as canceled and stops any ongoing fetch.
   void CancelPrefetch();

chrome/browser/prefetch/search_prefetch/search_prefetch_service.cc

@@ -127,9 +127,11 @@ bool SearchPrefetchService::MaybePrefetchURL(const GURL& url) {
   }
 
   DCHECK(prefetch_request);
+  if (!prefetch_request->StartPrefetchRequest(profile_)) {
+    return false;
+  }
 
   prefetches_.emplace(search_terms, std::move(prefetch_request));
-  prefetches_[search_terms]->StartPrefetchRequest(profile_);
   prefetch_expiry_timers_.emplace(search_terms,
                                   std::make_unique<base::OneShotTimer>());
   prefetch_expiry_timers_[search_terms]->Start(

chrome/browser/prefetch/search_prefetch/search_prefetch_service_browsertest.cc

@@ -59,6 +59,8 @@ constexpr char kOmniboxSuggestPrefetchSecondItemQuery[] = "porgsandwich";
 constexpr char kOmniboxSuggestNonPrefetchQuery[] = "puffins";
 constexpr char kLoadInSubframe[] = "/load_in_subframe";
 constexpr char kClientHintsURL[] = "/accept_ch_with_lifetime.html";
+constexpr char kThrottleHeader[] = "porgs-header";
+constexpr char kThrottleHeaderValue[] = "porgs-header-value";
 }  // namespace
 
 // A response that hangs after serving the start of the response.
@@ -73,6 +75,99 @@ class HangRequestAfterStart : public net::test_server::BasicHttpResponse {
   }
 };
 
+// A delegate to cancel prefetch requests by setting |defer| to true.
+class DeferringThrottle : public blink::URLLoaderThrottle {
+ public:
+  DeferringThrottle() = default;
+  ~DeferringThrottle() override = default;
+
+  void WillStartRequest(network::ResourceRequest* request,
+                        bool* defer) override {
+    *defer = true;
+  }
+};
+
+class ThrottleAllContentBrowserClient : public ChromeContentBrowserClient {
+ public:
+  ThrottleAllContentBrowserClient() = default;
+  ~ThrottleAllContentBrowserClient() override = default;
+
+  // ContentBrowserClient overrides:
+  std::vector<std::unique_ptr<blink::URLLoaderThrottle>>
+  CreateURLLoaderThrottles(
+      const network::ResourceRequest& request,
+      content::BrowserContext* browser_context,
+      const base::RepeatingCallback<content::WebContents*()>& wc_getter,
+      content::NavigationUIData* navigation_ui_data,
+      int frame_tree_node_id) override {
+    std::vector<std::unique_ptr<blink::URLLoaderThrottle>> throttles;
+    throttles.push_back(std::make_unique<DeferringThrottle>());
+    return throttles;
+  }
+};
+
+// A delegate to cancel prefetch requests by calling cancel on |delegate_|.
+class CancellingThrottle : public blink::URLLoaderThrottle {
+ public:
+  CancellingThrottle() = default;
+  ~CancellingThrottle() override = default;
+
+  void WillStartRequest(network::ResourceRequest* request,
+                        bool* defer) override {
+    delegate_->CancelWithError(net::ERR_ABORTED);
+  }
+};
+
+class CancelAllContentBrowserClient : public ChromeContentBrowserClient {
+ public:
+  CancelAllContentBrowserClient() = default;
+  ~CancelAllContentBrowserClient() override = default;
+
+  // ContentBrowserClient overrides:
+  std::vector<std::unique_ptr<blink::URLLoaderThrottle>>
+  CreateURLLoaderThrottles(
+      const network::ResourceRequest& request,
+      content::BrowserContext* browser_context,
+      const base::RepeatingCallback<content::WebContents*()>& wc_getter,
+      content::NavigationUIData* navigation_ui_data,
+      int frame_tree_node_id) override {
+    std::vector<std::unique_ptr<blink::URLLoaderThrottle>> throttles;
+    throttles.push_back(std::make_unique<CancellingThrottle>());
+    return throttles;
+  }
+};
+
+// A delegate to add a custom header to prefetches.
+class AddHeaderModifyingThrottle : public blink::URLLoaderThrottle {
+ public:
+  AddHeaderModifyingThrottle() = default;
+  ~AddHeaderModifyingThrottle() override = default;
+
+  void WillStartRequest(network::ResourceRequest* request,
+                        bool* defer) override {
+    request->headers.SetHeader(kThrottleHeader, kThrottleHeaderValue);
+  }
+};
+
+class AddHeaderContentBrowserClient : public ChromeContentBrowserClient {
+ public:
+  AddHeaderContentBrowserClient() = default;
+  ~AddHeaderContentBrowserClient() override = default;
+
+  // ContentBrowserClient overrides:
+  std::vector<std::unique_ptr<blink::URLLoaderThrottle>>
+  CreateURLLoaderThrottles(
+      const network::ResourceRequest& request,
+      content::BrowserContext* browser_context,
+      const base::RepeatingCallback<content::WebContents*()>& wc_getter,
+      content::NavigationUIData* navigation_ui_data,
+      int frame_tree_node_id) override {
+    std::vector<std::unique_ptr<blink::URLLoaderThrottle>> throttles;
+    throttles.push_back(std::make_unique<AddHeaderModifyingThrottle>());
+    return throttles;
+  }
+};
+
 class SearchPrefetchBaseBrowserTest : public InProcessBrowserTest {
  public:
   SearchPrefetchBaseBrowserTest() {
@@ -538,6 +633,9 @@ IN_PROC_BROWSER_TEST_P(SearchPrefetchServiceEnabledBrowserTest,
   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"));
+  EXPECT_TRUE(base::Contains(headers, "User-Agent"));
+  ASSERT_TRUE(base::Contains(headers, "Upgrade-Insecure-Requests"));
+  EXPECT_TRUE(base::Contains(headers["Upgrade-Insecure-Requests"], "1"));
 
   prefetch_status = search_prefetch_service->GetSearchPrefetchStatusForTesting(
       base::ASCIIToUTF16(search_terms));
@@ -545,6 +643,79 @@ IN_PROC_BROWSER_TEST_P(SearchPrefetchServiceEnabledBrowserTest,
   EXPECT_EQ(SearchPrefetchStatus::kCanBeServed, prefetch_status.value());
 }
 
+IN_PROC_BROWSER_TEST_P(SearchPrefetchServiceEnabledBrowserTest,
+                       PrefetchThrottled) {
+  ThrottleAllContentBrowserClient browser_client;
+  auto* old_client = content::SetBrowserClientForTesting(&browser_client);
+  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_FALSE(search_prefetch_service->MaybePrefetchURL(prefetch_url));
+  auto prefetch_status =
+      search_prefetch_service->GetSearchPrefetchStatusForTesting(
+          base::ASCIIToUTF16(search_terms));
+  EXPECT_FALSE(prefetch_status.has_value());
+  content::SetBrowserClientForTesting(old_client);
+}
+
+IN_PROC_BROWSER_TEST_P(SearchPrefetchServiceEnabledBrowserTest,
+                       PrefetchCancelledByThrottle) {
+  CancelAllContentBrowserClient browser_client;
+  auto* old_client = content::SetBrowserClientForTesting(&browser_client);
+  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_FALSE(search_prefetch_service->MaybePrefetchURL(prefetch_url));
+  auto prefetch_status =
+      search_prefetch_service->GetSearchPrefetchStatusForTesting(
+          base::ASCIIToUTF16(search_terms));
+  EXPECT_FALSE(prefetch_status.has_value());
+  content::SetBrowserClientForTesting(old_client);
+}
+
+IN_PROC_BROWSER_TEST_P(SearchPrefetchServiceEnabledBrowserTest,
+                       PrefetchThrottleAddsHeader) {
+  AddHeaderContentBrowserClient browser_client;
+  auto* old_client = content::SetBrowserClientForTesting(&browser_client);
+  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));
+
+  auto headers = search_server_requests()[0].headers;
+  EXPECT_EQ(1u, search_server_requests().size());
+  ASSERT_TRUE(base::Contains(headers, kThrottleHeader));
+  EXPECT_TRUE(base::Contains(headers[kThrottleHeader], kThrottleHeaderValue));
+
+  prefetch_status = search_prefetch_service->GetSearchPrefetchStatusForTesting(
+      base::ASCIIToUTF16(search_terms));
+  ASSERT_TRUE(prefetch_status.has_value());
+  EXPECT_EQ(SearchPrefetchStatus::kCanBeServed, prefetch_status.value());
+  content::SetBrowserClientForTesting(old_client);
+}
+
 class HeaderObserverContentBrowserClient : public ChromeContentBrowserClient {
  public:
   HeaderObserverContentBrowserClient() = default;