Add a unit test to ensure that if RESOURCE_TYPE_PREFETCH is specified in...

Add a unit test to ensure that if RESOURCE_TYPE_PREFETCH is specified in content::ResourceRequest, it's translated into the proper net:: flag on URLRequest.

The test is for the network service case. The old navigation code path has this covered already in RequestDataResourceDispatcherHostBrowserTest.LinkRelPrefetch.

The motivation is to convert a few prefetch browser tests from checking the URLLRequest object (which won't work with network service) to instead look at the ResourceRequest object.

Also de-duplicate BuildLoadFlagsForRequest.

BUG=776589

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I29a66944d17a7e99b2bacbcb0766fa1b1174bd57
Reviewed-on: https://chromium-review.googlesource.com/797926Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520594}

content/browser/loader/resource_dispatcher_host_impl.cc

@@ -71,6 +71,7 @@
 #include "content/browser/streams/stream.h"
 #include "content/browser/streams/stream_context.h"
 #include "content/browser/streams/stream_registry.h"
+#include "content/common/loader_util.h"
 #include "content/common/net/url_request_service_worker_data.h"
 #include "content/common/resource_messages.h"
 #include "content/common/site_isolation_policy.h"
@@ -2625,28 +2626,6 @@ void ResourceDispatcherHostImpl::UnregisterResourceMessageDelegate(
   }
 }
 
-int ResourceDispatcherHostImpl::BuildLoadFlagsForRequest(
-    const ResourceRequest& request_data,
-    bool is_sync_load) {
-  int load_flags = request_data.load_flags;
-
-  // Although EV status is irrelevant to sub-frames and sub-resources, we have
-  // to perform EV certificate verification on all resources because an HTTP
-  // keep-alive connection created to load a sub-frame or a sub-resource could
-  // be reused to load a main frame.
-  load_flags |= net::LOAD_VERIFY_EV_CERT;
-  if (request_data.resource_type == RESOURCE_TYPE_MAIN_FRAME) {
-    load_flags |= net::LOAD_MAIN_FRAME_DEPRECATED;
-  } else if (request_data.resource_type == RESOURCE_TYPE_PREFETCH) {
-    load_flags |= net::LOAD_PREFETCH;
-  }
-
-  if (is_sync_load)
-    load_flags |= net::LOAD_IGNORE_LIMITS;
-
-  return load_flags;
-}
-
 bool ResourceDispatcherHostImpl::ShouldServiceRequest(
     int child_id,
     const ResourceRequest& request_data,

content/browser/loader/resource_dispatcher_host_impl.h

@@ -695,9 +695,6 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
   void UnregisterResourceMessageDelegate(const GlobalRequestID& id,
                                          ResourceMessageDelegate* delegate);
 
-  int BuildLoadFlagsForRequest(const ResourceRequest& request_data,
-                               bool is_sync_load);
-
   // Consults the RendererSecurity policy to determine whether the
   // ResourceDispatcherHostImpl should service this request.  A request might
   // be disallowed if the renderer is not authorized to retrieve the request

content/browser/loader/resource_loader_unittest.cc

@@ -728,6 +728,19 @@ TEST_F(ClientCertResourceLoaderTest, StoreAsyncCancel) {
   base::RunLoop().RunUntilIdle();
 }
 
+// Tests that a RESOURCE_TYPE_PREFETCH request sets the LOAD_PREFETCH flag.
+TEST_F(ResourceLoaderTest, PrefetchFlag) {
+  std::unique_ptr<net::URLRequest> request(
+      resource_context_.GetRequestContext()->CreateRequest(
+          test_async_url(), net::DEFAULT_PRIORITY, nullptr /* delegate */,
+          TRAFFIC_ANNOTATION_FOR_TESTS));
+  SetUpResourceLoader(std::move(request), RESOURCE_TYPE_PREFETCH, true);
+
+  loader_->StartRequest();
+  raw_ptr_resource_handler_->WaitUntilResponseComplete();
+  EXPECT_EQ(test_data(), raw_ptr_resource_handler_->body());
+}
+
 // Test the case the ResourceHandler defers nothing.
 TEST_F(ResourceLoaderTest, SyncResourceHandler) {
   loader_->StartRequest();

content/common/loader_util.cc

@@ -8,7 +8,9 @@
 
 #include "base/strings/stringprintf.h"
 #include "content/public/common/resource_devtools_info.h"
+#include "content/public/common/resource_request.h"
 #include "content/public/common/resource_response.h"
+#include "net/base/load_flags.h"
 #include "net/base/mime_sniffer.h"
 #include "net/http/http_raw_request_headers.h"
 #include "net/http/http_util.h"
@@ -135,4 +137,25 @@ void AttachAcceptHeader(ResourceType type, net::URLRequest* request) {
   request->SetExtraRequestHeaderByName(kAcceptHeader, accept_value, false);
 }
 
+int BuildLoadFlagsForRequest(const ResourceRequest& request,
+                             bool is_sync_load) {
+  int load_flags = request.load_flags;
+
+  // Although EV status is irrelevant to sub-frames and sub-resources, we have
+  // to perform EV certificate verification on all resources because an HTTP
+  // keep-alive connection created to load a sub-frame or a sub-resource could
+  // be reused to load a main frame.
+  load_flags |= net::LOAD_VERIFY_EV_CERT;
+  if (request.resource_type == RESOURCE_TYPE_MAIN_FRAME) {
+    load_flags |= net::LOAD_MAIN_FRAME_DEPRECATED;
+  } else if (request.resource_type == RESOURCE_TYPE_PREFETCH) {
+    load_flags |= net::LOAD_PREFETCH;
+  }
+
+  if (is_sync_load)
+    load_flags |= net::LOAD_IGNORE_LIMITS;
+
+  return load_flags;
+}
+
 }  // namespace content

content/common/loader_util.h

@@ -16,6 +16,7 @@ class URLRequest;
 
 namespace content {
 struct ResourceDevToolsInfo;
+struct ResourceRequest;
 struct ResourceResponse;
 
 // Helper utilities shared between network service and ResourceDispatcherHost
@@ -34,6 +35,9 @@ scoped_refptr<ResourceDevToolsInfo> BuildDevToolsInfo(
 
 void AttachAcceptHeader(ResourceType type, net::URLRequest* request);
 
+int BuildLoadFlagsForRequest(const ResourceRequest& request_data,
+                             bool is_sync_load);
+
 }  // namespace content
 
 #endif  // CONTENT_COMMON_LOADER_UTIL_H_

content/network/url_loader.cc

@@ -22,7 +22,6 @@
 #include "content/public/common/url_loader_factory.mojom.h"
 #include "mojo/public/cpp/system/simple_watcher.h"
 #include "net/base/elements_upload_data_stream.h"
-#include "net/base/load_flags.h"
 #include "net/base/mime_sniffer.h"
 #include "net/base/upload_bytes_element_reader.h"
 #include "net/base/upload_file_element_reader.h"
@@ -35,28 +34,6 @@ namespace content {
 namespace {
 constexpr size_t kDefaultAllocationSize = 512 * 1024;
 
-// TODO: this duplicates ResourceDispatcherHostImpl::BuildLoadFlagsForRequest.
-int BuildLoadFlagsForRequest(const ResourceRequest& request,
-                             bool is_sync_load) {
-  int load_flags = request.load_flags;
-
-  // Although EV status is irrelevant to sub-frames and sub-resources, we have
-  // to perform EV certificate verification on all resources because an HTTP
-  // keep-alive connection created to load a sub-frame or a sub-resource could
-  // be reused to load a main frame.
-  load_flags |= net::LOAD_VERIFY_EV_CERT;
-  if (request.resource_type == RESOURCE_TYPE_MAIN_FRAME) {
-    load_flags |= net::LOAD_MAIN_FRAME_DEPRECATED;
-  } else if (request.resource_type == RESOURCE_TYPE_PREFETCH) {
-    load_flags |= net::LOAD_PREFETCH;
-  }
-
-  if (is_sync_load)
-    load_flags |= net::LOAD_IGNORE_LIMITS;
-
-  return load_flags;
-}
-
 // TODO: this duplicates some of PopulateResourceResponse in
 // content/browser/loader/resource_loader.cc
 void PopulateResourceResponse(net::URLRequest* request,

content/network/url_loader_unittest.cc

@@ -32,6 +32,7 @@
 #include "mojo/public/c/system/data_pipe.h"
 #include "mojo/public/cpp/system/wait.h"
 #include "net/base/io_buffer.h"
+#include "net/base/load_flags.h"
 #include "net/base/mime_sniffer.h"
 #include "net/base/net_errors.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
@@ -180,6 +181,28 @@ class SimpleDataPipeGetter : public network::mojom::DataPipeGetter {
   DISALLOW_COPY_AND_ASSIGN(SimpleDataPipeGetter);
 };
 
+class RequestInterceptor : public net::URLRequestInterceptor {
+ public:
+  using InterceptCallback = base::Callback<void(net::URLRequest*)>;
+
+  explicit RequestInterceptor(const InterceptCallback& callback)
+      : callback_(callback) {}
+  ~RequestInterceptor() override {}
+
+  // URLRequestInterceptor implementation:
+  net::URLRequestJob* MaybeInterceptRequest(
+      net::URLRequest* request,
+      net::NetworkDelegate* network_delegate) const override {
+    callback_.Run(request);
+    return nullptr;
+  }
+
+ private:
+  InterceptCallback callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(RequestInterceptor);
+};
+
 }  // namespace
 
 class URLLoaderTest : public testing::Test {
@@ -315,6 +338,15 @@ class URLLoaderTest : public testing::Test {
     EXPECT_EQ(actual_body, expected_body);
   }
 
+  // Adds an interceptor that can examine the URLRequest object.
+  void AddRequestObserver(
+      const GURL& url,
+      const RequestInterceptor::InterceptCallback& callback) {
+    net::URLRequestFilter::GetInstance()->AddUrlInterceptor(
+        url, std::unique_ptr<net::URLRequestInterceptor>(
+                 new RequestInterceptor(callback)));
+  }
+
   net::EmbeddedTestServer* test_server() { return &test_server_; }
   NetworkContext* context() { return context_.get(); }
   TestURLLoaderClient* client() { return &client_; }
@@ -985,6 +1017,21 @@ TEST_F(URLLoaderTest, DoNotOverrideAcceptHeader) {
   EXPECT_EQ(it->second, "custom/*");
 }
 
+// Tests that a RESOURCE_TYPE_PREFETCH request sets the LOAD_PREFETCH flag.
+TEST_F(URLLoaderTest, SetPrefetchFlag) {
+  set_resource_type(RESOURCE_TYPE_PREFETCH);
+  GURL url = test_server()->GetURL("/simple_page.html");
+  int load_flags = 0;
+  AddRequestObserver(url, base::Bind(
+                              [](int* load_flags, net::URLRequest* request) {
+                                *load_flags = request->load_flags();
+                              },
+                              &load_flags));
+  EXPECT_EQ(net::OK, Load(url));
+
+  EXPECT_TRUE(load_flags & net::LOAD_PREFETCH);
+}
+
 TEST_F(URLLoaderTest, UploadBytes) {
   const std::string kRequestBody = "Request Body";