Set the prefetch load flag without a resource throttle for compatibility with the network service.

This was originally added in 440645 using a resource throttle. Since resource throttles don't work
with the network service (as net runs out of process), instead do this in the same
ContentBrowserClient callback which is used to add extra headers for prerendering requests.

Also convert a few prefetch tests for this to use the new URLLoaderInterceptor so that they pass
with the network service.

Bug: 769401
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I993912ff991dda33c0fb58ae8e38f88fd16ee4ac
Reviewed-on: https://chromium-review.googlesource.com/802996Reviewed-by: Egor Pasko <pasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521034}

chrome/browser/chrome_content_browser_client.cc

@@ -1973,12 +1973,22 @@ bool ChromeContentBrowserClient::IsDataSaverEnabled(
   return prefs && prefs->GetBoolean(prefs::kDataSaverEnabled);
 }
 
-std::unique_ptr<net::HttpRequestHeaders>
-ChromeContentBrowserClient::GetAdditionalNavigationRequestHeaders(
-    content::BrowserContext* context,
-    const GURL& url) const {
-  return client_hints::GetAdditionalNavigationRequestClientHintsHeaders(context,
-                                                                        url);
+void ChromeContentBrowserClient::NavigationRequestStarted(
+    int frame_tree_node_id,
+    const GURL& url,
+    std::unique_ptr<net::HttpRequestHeaders>* extra_headers,
+    int* extra_load_flags) {
+  WebContents* web_contents =
+      WebContents::FromFrameTreeNodeId(frame_tree_node_id);
+  *extra_headers =
+      client_hints::GetAdditionalNavigationRequestClientHintsHeaders(
+          web_contents->GetBrowserContext(), url);
+  prerender::PrerenderContents* prerender_contents =
+      prerender::PrerenderContents::FromWebContents(web_contents);
+  if (prerender_contents) {
+    if (prerender_contents->prerender_mode() == prerender::PREFETCH_ONLY)
+      *extra_load_flags = net::LOAD_PREFETCH;
+  }
 }
 
 bool ChromeContentBrowserClient::AllowAppCache(

chrome/browser/chrome_content_browser_client.h

@@ -160,9 +160,11 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
   std::string GetAcceptLangs(content::BrowserContext* context) override;
   const gfx::ImageSkia* GetDefaultFavicon() override;
   bool IsDataSaverEnabled(content::BrowserContext* context) override;
-  std::unique_ptr<net::HttpRequestHeaders>
-  GetAdditionalNavigationRequestHeaders(content::BrowserContext* context,
-                                        const GURL& url) const override;
+  void NavigationRequestStarted(
+      int frame_tree_node_id,
+      const GURL& url,
+      std::unique_ptr<net::HttpRequestHeaders>* extra_headers,
+      int* extra_load_flags) override;
   bool AllowAppCache(const GURL& manifest_url,
                      const GURL& first_party,
                      content::ResourceContext* context) override;

chrome/browser/prerender/prerender_nostate_prefetch_browsertest.cc

@@ -29,12 +29,14 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/appcache_service.h"
+#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/result_codes.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/test/browser_test_utils.h"
+#include "content/public/test/url_loader_interceptor.h"
 #include "net/base/escape.h"
 #include "net/base/load_flags.h"
 #include "net/dns/mock_host_resolver.h"
@@ -161,6 +163,15 @@ class NoStatePrefetchBrowserTest
                            expected_final_status);
   }
 
+  content::StoragePartition* GetStoragePartition() {
+    return browser()
+        ->tab_strip_model()
+        ->GetActiveWebContents()
+        ->GetMainFrame()
+        ->GetProcess()
+        ->GetStoragePartition();
+  }
+
  private:
   // Schedule a task to retrieve AppCacheInfo from |appcache_service|. This sets
   // |found_manifest| if an appcache exists for |manifest_url|. |callback| will
@@ -221,29 +232,38 @@ IN_PROC_BROWSER_TEST_P(NoStatePrefetchBrowserTest, PrefetchSimple) {
 
 // Check that the LOAD_PREFETCH flag is set.
 IN_PROC_BROWSER_TEST_P(NoStatePrefetchBrowserTest, PrefetchLoadFlag) {
-  // TODO(jam): use URLLoaderFactory for subresource.
-  // This will need a separate code path for network service:
-  //   -add way for the
-  //        storage_partition->GetNetworkContext()->CreateURLLoaderFactory
-  //        to return test defined factoory
-  //   -also add a URLLoader unittest to verify that if ResourceRequest has that
-  //        flag it is passed to the net::URLRequest
-  RequestCounter main_counter;
-  RequestCounter script_counter;
-  auto verify_prefetch_only = base::Bind([](net::URLRequest* request) {
-    EXPECT_TRUE(request->load_flags() & net::LOAD_PREFETCH);
-  });
+  GURL prefetch_page = src_server()->GetURL(kPrefetchPage);
+  GURL prefetch_script = src_server()->GetURL(kPrefetchScript);
+
+  bool use_interceptor_for_frame_requests =
+      base::FeatureList::IsEnabled(features::kNetworkService);
+  if (!use_interceptor_for_frame_requests) {
+    // Until http://crbug.com/747130 is fixed, navigation requests won't go
+    // through URLLoader.
+    prerender::test_utils::InterceptRequest(
+        prefetch_page,
+        base::Bind([](net::URLRequest* request) {
+          EXPECT_TRUE(request->load_flags() & net::LOAD_PREFETCH);
+        }));
+  }
 
-  prerender::test_utils::InterceptRequestAndCount(
-      src_server()->GetURL(kPrefetchPage), &main_counter, verify_prefetch_only);
-  prerender::test_utils::InterceptRequestAndCount(
-      src_server()->GetURL(kPrefetchScript), &script_counter,
-      verify_prefetch_only);
+  content::URLLoaderInterceptor interceptor(
+      base::Bind(
+          [](const GURL& prefetch_page, const GURL& prefetch_script,
+             content::URLLoaderInterceptor::RequestParams* params) {
+            if (params->url_request.url == prefetch_page ||
+                params->url_request.url == prefetch_script) {
+              EXPECT_TRUE(params->url_request.load_flags & net::LOAD_PREFETCH);
+            }
+            return false;
+          },
+          prefetch_page, prefetch_script),
+      GetStoragePartition(), use_interceptor_for_frame_requests, true);
 
   std::unique_ptr<TestPrerender> test_prerender =
       PrefetchFromFile(kPrefetchPage, FINAL_STATUS_NOSTATE_PREFETCH_FINISHED);
-  main_counter.WaitForCount(1);
-  script_counter.WaitForCount(1);
+  WaitForRequestCount(prefetch_page, 1);
+  WaitForRequestCount(prefetch_script, 1);
 
   // Verify that the page load did not happen.
   test_prerender->WaitForLoads(0);
@@ -384,18 +404,33 @@ IN_PROC_BROWSER_TEST_P(NoStatePrefetchBrowserTest, Prefetch301LoadFlags) {
       "/server-redirect/?" + net::EscapeQueryParamValue(kPrefetchPage, false);
   GURL redirect_url = src_server()->GetURL(redirect_path);
   GURL page_url = src_server()->GetURL(kPrefetchPage);
-  RequestCounter redirect_counter;
   auto verify_prefetch_only = base::Bind([](net::URLRequest* request) {
     EXPECT_TRUE(request->load_flags() & net::LOAD_PREFETCH);
   });
-  prerender::test_utils::InterceptRequestAndCount(
-      redirect_url, &redirect_counter, verify_prefetch_only);
-  RequestCounter page_counter;
-  prerender::test_utils::InterceptRequestAndCount(page_url, &page_counter,
-                                                  verify_prefetch_only);
+
+  bool use_interceptor = false;
+  if (base::FeatureList::IsEnabled(features::kNetworkService)) {
+    use_interceptor = true;
+  } else {
+    // Until http://crbug.com/747130 is fixed, navigation requests won't go
+    // through URLLoader.
+    prerender::test_utils::InterceptRequest(page_url, verify_prefetch_only);
+  }
+
+  content::URLLoaderInterceptor interceptor(
+      base::Bind(
+          [](const GURL& page_url,
+             content::URLLoaderInterceptor::RequestParams* params) {
+            if (params->url_request.url == page_url)
+              EXPECT_TRUE(params->url_request.load_flags & net::LOAD_PREFETCH);
+            return false;
+          },
+          redirect_url),
+      GetStoragePartition(), use_interceptor, false);
+
   PrefetchFromFile(redirect_path, FINAL_STATUS_NOSTATE_PREFETCH_FINISHED);
-  redirect_counter.WaitForCount(1);
-  page_counter.WaitForCount(1);
+  WaitForRequestCount(redirect_url, 1);
+  WaitForRequestCount(page_url, 1);
 }
 
 // Checks that a subresource 301 redirect is followed.

chrome/browser/prerender/prerender_resource_throttle.cc

@@ -85,7 +85,6 @@ void PrerenderResourceThrottle::OverridePrerenderContentsForTesting(
 
 PrerenderResourceThrottle::PrerenderResourceThrottle(net::URLRequest* request)
     : request_(request),
-      load_flags_(net::LOAD_NORMAL),
       prerender_throttle_info_(new PrerenderThrottleInfo()) {
 // Priorities for prerendering requests are lowered, to avoid competing with
 // other page loads, except on Android where this is less likely to be a
@@ -170,7 +169,6 @@ const char* PrerenderResourceThrottle::GetNameForLogging() const {
 
 void PrerenderResourceThrottle::ResumeHandler() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  request_->SetLoadFlags(request_->load_flags() | load_flags_);
   Resume();
 }
 
@@ -201,10 +199,6 @@ void PrerenderResourceThrottle::WillStartRequestOnUI(
     prerender_throttle_info->Set(prerender_contents->prerender_mode(),
                                  prerender_contents->origin(),
                                  prerender_contents->prerender_manager());
-    BrowserThread::PostTask(
-        BrowserThread::IO, FROM_HERE,
-        base::BindOnce(&PrerenderResourceThrottle::SetPrerenderMode, throttle,
-                       prerender_contents->prerender_mode()));
 
     // Abort any prerenders that spawn requests that use unsupported HTTP
     // methods or schemes.
@@ -329,9 +323,4 @@ PrerenderContents* PrerenderResourceThrottle::PrerenderContentsFromGetter(
   return PrerenderContents::FromWebContents(web_contents_getter.Run());
 }
 
-void PrerenderResourceThrottle::SetPrerenderMode(PrerenderMode mode) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  load_flags_ = (mode == PREFETCH_ONLY) ? net::LOAD_PREFETCH : net::LOAD_NORMAL;
-}
-
 }  // namespace prerender

chrome/browser/prerender/prerender_resource_throttle.h

@@ -89,11 +89,7 @@ class PrerenderResourceThrottle
       const content::ResourceRequestInfo::WebContentsGetter&
           web_contents_getter);
 
-  // Sets the prerender mode. Must be called before |ResumeHandler()|.
-  void SetPrerenderMode(PrerenderMode mode);
-
   net::URLRequest* request_;
-  int load_flags_;  // Load flags to be OR'ed with the existing request flags.
 
   // The throttle changes most request priorities to IDLE during prerendering.
   // The priority is reset back to the original priority when prerendering is

chrome/browser/prerender/prerender_test_utils.cc

@@ -107,16 +107,19 @@ class CountingInterceptor : public net::URLRequestInterceptor {
 class CountingInterceptorWithCallback : public net::URLRequestInterceptor {
  public:
   // Inserts the interceptor object to intercept requests to |url|.  Can be
-  // called on any thread. Assumes that |counter| lives on the UI thread.  The
-  // |callback_io| will be called on IO thread with the net::URLrequest
-  // provided.
+  // called on any thread. Assumes that |counter| (if non-null) lives on the UI
+  // thread.  The |callback_io| will be called on IO thread with the
+  // net::URLrequest provided.
   static void Initialize(const GURL& url,
                          RequestCounter* counter,
                          base::Callback<void(net::URLRequest*)> callback_io) {
+    base::WeakPtr<RequestCounter> weakptr;
+    if (counter)
+      weakptr = counter->AsWeakPtr();
     content::BrowserThread::PostTask(
         content::BrowserThread::IO, FROM_HERE,
         base::BindOnce(&CountingInterceptorWithCallback::CreateAndAddOnIO, url,
-                       counter->AsWeakPtr(), callback_io));
+                       weakptr, callback_io));
   }
 
   // net::URLRequestInterceptor:
@@ -853,6 +856,11 @@ void CreateCountingInterceptorOnIO(
       url, base::MakeUnique<CountingInterceptor>(file, counter));
 }
 
+void InterceptRequest(const GURL& url,
+                      base::Callback<void(net::URLRequest*)> callback_io) {
+  CountingInterceptorWithCallback::Initialize(url, nullptr, callback_io);
+}
+
 void InterceptRequestAndCount(
     const GURL& url,
     RequestCounter* counter,

chrome/browser/prerender/prerender_test_utils.h

@@ -433,6 +433,12 @@ void CreateCountingInterceptorOnIO(
     const base::FilePath& file,
     const base::WeakPtr<RequestCounter>& counter);
 
+// When the |url| hits the net::URLRequestFilter (on the IO thread), executes
+// the |callback_io| providing the request to it. Does not modify the behavior
+// or the request job.
+void InterceptRequest(const GURL& url,
+                      base::Callback<void(net::URLRequest*)> callback_io);
+
 // When the |url| hits the net::URLRequestFilter (on the IO thread), executes
 // the |callback_io| providing the request to it, also pings the |counter| on UI
 // thread. Does not modify the behavior or the request job.

content/browser/frame_host/navigation_request.cc

@@ -135,13 +135,15 @@ bool NeedsHTTPOrigin(net::HttpRequestHeaders* headers,
 
 // TODO(clamy): This should match what's happening in
 // blink::FrameFetchContext::addAdditionalRequestHeaders.
-void AddAdditionalRequestHeaders(net::HttpRequestHeaders* headers,
-                                 const GURL& url,
-                                 FrameMsg_Navigate_Type::Value navigation_type,
-                                 BrowserContext* browser_context,
-                                 const std::string& method,
-                                 const std::string user_agent_override,
-                                 FrameTreeNode* frame_tree_node) {
+void AddAdditionalRequestHeaders(
+    net::HttpRequestHeaders* headers,
+    std::unique_ptr<net::HttpRequestHeaders> embedder_additional_headers,
+    const GURL& url,
+    FrameMsg_Navigate_Type::Value navigation_type,
+    BrowserContext* browser_context,
+    const std::string& method,
+    const std::string user_agent_override,
+    FrameTreeNode* frame_tree_node) {
   if (!url.SchemeIsHTTPOrHTTPS())
     return;
 
@@ -158,9 +160,6 @@ void AddAdditionalRequestHeaders(net::HttpRequestHeaders* headers,
   }
 
   // Attach additional request headers specified by embedder.
-  std::unique_ptr<net::HttpRequestHeaders> embedder_additional_headers =
-      GetContentClient()->browser()->GetAdditionalNavigationRequestHeaders(
-          browser_context, url);
   if (embedder_additional_headers)
     headers->MergeFrom(*(embedder_additional_headers.get()));
 
@@ -394,10 +393,18 @@ NavigationRequest::NavigationRequest(
         frame_tree_node_->navigator()->GetDelegate()->GetUserAgentOverride();
   }
 
+  std::unique_ptr<net::HttpRequestHeaders> embedder_additional_headers;
+  int additional_load_flags = 0;
+  GetContentClient()->browser()->NavigationRequestStarted(
+      frame_tree_node->frame_tree_node_id(), common_params_.url,
+      &embedder_additional_headers, &additional_load_flags);
+  begin_params_->load_flags |= additional_load_flags;
+
   net::HttpRequestHeaders headers;
   headers.AddHeadersFromString(begin_params_->headers);
   AddAdditionalRequestHeaders(
-      &headers, common_params_.url, common_params_.navigation_type,
+      &headers, std::move(embedder_additional_headers), common_params_.url,
+      common_params_.navigation_type,
       frame_tree_node_->navigator()->GetController()->GetBrowserContext(),
       common_params.method, user_agent_override, frame_tree_node);
 

content/public/browser/content_browser_client.cc

@@ -232,13 +232,6 @@ bool ContentBrowserClient::IsDataSaverEnabled(BrowserContext* context) {
   return false;
 }
 
-std::unique_ptr<net::HttpRequestHeaders>
-ContentBrowserClient::GetAdditionalNavigationRequestHeaders(
-    BrowserContext* context,
-    const GURL& url) const {
-  return nullptr;
-}
-
 bool ContentBrowserClient::AllowGetCookie(const GURL& url,
                                           const GURL& first_party,
                                           const net::CookieList& cookie_list,

content/public/browser/content_browser_client.h

@@ -416,10 +416,12 @@ class CONTENT_EXPORT ContentBrowserClient {
   virtual bool IsDataSaverEnabled(BrowserContext* context);
 
   // Allow the embedder to return additional headers that should be sent when
-  // fetching |url|. May return a nullptr.
-  virtual std::unique_ptr<net::HttpRequestHeaders>
-  GetAdditionalNavigationRequestHeaders(BrowserContext* context,
-                                        const GURL& url) const;
+  // fetching |url| as well as add extra load flags.
+  virtual void NavigationRequestStarted(
+      int frame_tree_node_id,
+      const GURL& url,
+      std::unique_ptr<net::HttpRequestHeaders>* extra_headers,
+      int* extra_load_flags) {}
 
   // Allow the embedder to control if the given cookie can be read.
   // This is called on the IO thread.

testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

@@ -901,7 +901,3 @@
 # TODO(jam): switch the tests to hook in via URLLoaderFactory to check load flags.
 -NoStatePrefetchBrowserTest/NoStatePrefetchBrowserTest.IssuesIdlePriorityRequests/0
 -NoStatePrefetchBrowserTest/NoStatePrefetchBrowserTest.IssuesIdlePriorityRequests/1
--NoStatePrefetchBrowserTest/NoStatePrefetchBrowserTest.Prefetch301LoadFlags/0
--NoStatePrefetchBrowserTest/NoStatePrefetchBrowserTest.Prefetch301LoadFlags/1
--NoStatePrefetchBrowserTest/NoStatePrefetchBrowserTest.PrefetchLoadFlag/0
--NoStatePrefetchBrowserTest/NoStatePrefetchBrowserTest.PrefetchLoadFlag/1