Allow extensions to intercept service worker navigation preload requests.
Heavily based on an original CL by Charles Vazac <[email protected]>
at https://chromium-review.googlesource.com/c/chromium/src/+/1396938
Service worker navigation preload[1] is a web platform feature that
involves sending a request to network at the same time as starting up
the service worker for a navigation. This network request was not
visible to extensions.
This CL:
- Modifies the network service path for service worker navigation
preload requests so that the embedder is aware of them.
- In web_request_permissions.cc, if a request is not considered a browser
side navigation, it becomes opaque to extensions. This CL makes it so
that service worker navigation preload requests[1] are exempted from
that.
This CL deletes the unit test for navigation preload in
ServiceWorkerNavigationLoaderTest. It was too much hassle getting that
to work with a TestWebContents because the infrastructure for enabling
NetworkService in content_unittests is not set up yet so
unittest_test_suite.cc typically disables NetworkService (issue 901092).
We have sufficient navigation preload test coverage in browser tests and
web tests.
[1] https://w3c.github.io/ServiceWorker/#navigationpreloadmanager
Bug: 914062
Change-Id: I4c91521e55c1103a1a67d973e595094b78357c34
Reviewed-on: https://chromium-review.googlesource.com/c/1491168
Reviewed-by: Kinuko Yasuda <[email protected]>
Reviewed-by: Varun Khaneja <[email protected]>
Reviewed-by: Karan Bhatia <[email protected]>
Commit-Queue: Matt Falkenhagen <[email protected]>
Cr-Commit-Position: refs/heads/master@{#636598}
diff --git a/chrome/browser/extensions/api/web_request/web_request_apitest.cc b/chrome/browser/extensions/api/web_request/web_request_apitest.cc
index 14f8a64..f0d7fd6 100644
--- a/chrome/browser/extensions/api/web_request/web_request_apitest.cc
+++ b/chrome/browser/extensions/api/web_request/web_request_apitest.cc
@@ -288,6 +288,29 @@
const char* expected_content_regular_window,
const char* exptected_content_incognito_window);
+ void InstallRequestHeaderModifyingExtension() {
+ TestExtensionDir test_dir;
+ test_dir.WriteManifest(R"({
+ "name": "Web Request Header Modifying Extension",
+ "manifest_version": 2,
+ "version": "0.1",
+ "background": { "scripts": ["background.js"] },
+ "permissions": ["<all_urls>", "webRequest", "webRequestBlocking"]
+ })");
+ test_dir.WriteFile(FILE_PATH_LITERAL("background.js"), R"(
+ chrome.webRequest.onBeforeSendHeaders.addListener(function(details) {
+ details.requestHeaders.push({name: 'foo', value: 'bar'});
+ return {requestHeaders: details.requestHeaders};
+ }, {urls: ['*://*/echoheader*']}, ['blocking', 'requestHeaders']);
+
+ chrome.test.sendMessage('ready');
+ )");
+
+ ExtensionTestMessageListener listener("ready", false);
+ ASSERT_TRUE(LoadExtension(test_dir.UnpackedPath()));
+ EXPECT_TRUE(listener.WaitUntilSatisfied());
+ }
+
// Ensures requests made by the |worker_script_name| service worker can be
// intercepted by extensions.
void RunServiceWorkerFetchTest(const std::string& worker_script_name);
@@ -320,6 +343,18 @@
test_dirs_.push_back(std::move(dir));
}
+ void RegisterServiceWorker(const std::string& worker_path,
+ const base::Optional<std::string>& scope) {
+ GURL url = embedded_test_server()->GetURL(
+ "/service_worker/create_service_worker.html");
+ EXPECT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
+ std::string script = content::JsReplace("register($1, $2);", worker_path,
+ scope ? *scope : std::string());
+ EXPECT_EQ(
+ "DONE",
+ EvalJs(browser()->tab_strip_model()->GetActiveWebContents(), script));
+ }
+
private:
std::vector<std::unique_ptr<TestExtensionDir>> test_dirs_;
};
@@ -752,39 +787,21 @@
embedded_test_server()->ServeFilesFromSourceDirectory("content/test/data");
ASSERT_TRUE(embedded_test_server()->Start());
- TestExtensionDir test_dir;
- test_dir.WriteManifest(R"({
- "name": "Web Request Service Worker Test",
- "manifest_version": 2,
- "version": "0.1",
- "background": { "scripts": ["background.js"] },
- "permissions": ["<all_urls>", "webRequest", "webRequestBlocking"]
- })");
- test_dir.WriteFile(FILE_PATH_LITERAL("background.js"), R"(
- chrome.webRequest.onBeforeSendHeaders.addListener(function(details) {
- details.requestHeaders.push({name: 'foo', value: 'bar'});
- return {requestHeaders: details.requestHeaders};
- }, {urls: ['*://*/echoheader*']}, ['blocking', 'requestHeaders']);
+ // Install the test extension.
+ InstallRequestHeaderModifyingExtension();
- chrome.test.sendMessage('ready');
- )");
-
- ExtensionTestMessageListener listener("ready", false);
- ASSERT_TRUE(LoadExtension(test_dir.UnpackedPath()));
- EXPECT_TRUE(listener.WaitUntilSatisfied());
-
- // Register the |worker_script_name| as a service worker.
- EXPECT_TRUE(ui_test_utils::NavigateToURL(
- browser(), embedded_test_server()->GetURL(
- "/service_worker/create_service_worker.html")));
- EXPECT_EQ("DONE", EvalJs(browser()->tab_strip_model()->GetActiveWebContents(),
- "register('" + worker_script_name + "');"));
-
+ // Register a service worker and navigate to a page it controls.
+ RegisterServiceWorker(worker_script_name, base::nullopt);
EXPECT_TRUE(ui_test_utils::NavigateToURL(
browser(),
embedded_test_server()->GetURL("/service_worker/fetch_from_page.html")));
- // Ensure the extension was able to intercept the service worker request and
- // modify the request headers.
+
+ // Make a fetch from the controlled page. Depending on the worker script, the
+ // fetch might go to the service worker and be re-issued, or might fallback to
+ // network, or skip the worker, etc. In any case, this function expects a
+ // network request to happen, and that the extension modify the headers of the
+ // request before it goes to network. Verify that it was able to inject a
+ // header of "foo=bar".
EXPECT_EQ("bar", EvalJs(browser()->tab_strip_model()->GetActiveWebContents(),
"fetch_from_page('/echoheader?foo');"));
}
@@ -2527,6 +2544,43 @@
RunServiceWorkerFetchTest("empty.js");
}
+// Ensure that extensions can intercept service worker navigation preload
+// requests.
+IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
+ ServiceWorkerNavigationPreload) {
+ ASSERT_TRUE(embedded_test_server()->Start());
+
+ // Install the test extension.
+ InstallRequestHeaderModifyingExtension();
+
+ // Register a service worker that uses navigation preload.
+ RegisterServiceWorker("/service_worker/navigation_preload_worker.js",
+ "/echoheader");
+
+ // Navigate to "/echoheader". The browser will detect that the service worker
+ // above is registered with this scope and has navigation preload enabled.
+ // So it will send the navigation preload request to network while at the same
+ // time starting up the service worker. The service worker will get the
+ // response for the navigation preload request, and respond with it to create
+ // the page.
+ GURL url = embedded_test_server()->GetURL(
+ "/echoheader?foo&service-worker-navigation-preload");
+ ui_test_utils::NavigateToURL(browser(), url);
+
+ // Since the request was to "/echoheader", the response describes the request
+ // headers.
+ //
+ // The extension is expected to add a "foo: bar" header to the request
+ // before it goes to network. Verify that it did.
+ //
+ // The browser adds a "service-worker-navigation-preload: true" header for
+ // navigation preload requests, so also sanity check that header to prove
+ // that this test is really testing the navigation preload request.
+ EXPECT_EQ("bar\ntrue",
+ EvalJs(browser()->tab_strip_model()->GetActiveWebContents(),
+ "document.body.textContent;"));
+}
+
// Ensure we don't strip off initiator incorrectly in web request events when
// both the normal and incognito contexts are active. Regression test for
// crbug.com/934398.
diff --git a/chrome/browser/extensions/extension_apitest.cc b/chrome/browser/extensions/extension_apitest.cc
index 2111eaa..fe4b44e 100644
--- a/chrome/browser/extensions/extension_apitest.cc
+++ b/chrome/browser/extensions/extension_apitest.cc
@@ -40,6 +40,7 @@
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/embedded_test_server/http_request.h"
#include "net/test/embedded_test_server/http_response.h"
+#include "net/test/embedded_test_server/request_handler_util.h"
#include "net/test/spawned_test_server/spawned_test_server.h"
namespace extensions {
@@ -76,19 +77,22 @@
base::CompareCase::SENSITIVE))
return nullptr;
- size_t query_string_pos = request.relative_url.find('?');
- std::string header_name =
- request.relative_url.substr(query_string_pos + 1);
+ std::string content;
+ net::test_server::RequestQuery headers =
+ net::test_server::ParseQuery(request.GetURL());
+ for (const auto& header : headers) {
+ std::string header_name = header.first;
+ std::string header_value;
+ if (request.headers.find(header_name) != request.headers.end())
+ header_value = request.headers.at(header_name);
+ if (!content.empty())
+ content += "\n";
+ content += header_value;
+ }
- std::string header_value;
- auto it = request.headers.find(header_name);
- if (it != request.headers.end())
- header_value = it->second;
-
- std::unique_ptr<net::test_server::BasicHttpResponse> http_response(
- new net::test_server::BasicHttpResponse);
+ auto http_response = std::make_unique<net::test_server::BasicHttpResponse>();
http_response->set_code(net::HTTP_OK);
- http_response->set_content(header_value);
+ http_response->set_content(content);
return std::move(http_response);
}
@@ -146,6 +150,7 @@
} // namespace
+// TODO(karandeepb): See if this custom handling can be removed.
ExtensionApiTest::ExtensionApiTest() {
embedded_test_server()->RegisterRequestHandler(
base::Bind(&HandleServerRedirectRequest));
diff --git a/components/safe_browsing/android/remote_database_manager.cc b/components/safe_browsing/android/remote_database_manager.cc
index efd76d2..23addbe 100644
--- a/components/safe_browsing/android/remote_database_manager.cc
+++ b/components/safe_browsing/android/remote_database_manager.cc
@@ -119,7 +119,7 @@
if (ints_str.empty()) {
// By default, we check all types except a few.
static_assert(content::RESOURCE_TYPE_LAST_TYPE ==
- content::RESOURCE_TYPE_PLUGIN_RESOURCE + 1,
+ content::RESOURCE_TYPE_NAVIGATION_PRELOAD + 1,
"Decide if new resource type should be skipped on mobile.");
for (int t_int = 0; t_int < content::RESOURCE_TYPE_LAST_TYPE; t_int++) {
content::ResourceType t = static_cast<content::ResourceType>(t_int);
diff --git a/content/browser/loader/navigation_url_loader_impl.cc b/content/browser/loader/navigation_url_loader_impl.cc
index edc10efb..30e8b6d6 100644
--- a/content/browser/loader/navigation_url_loader_impl.cc
+++ b/content/browser/loader/navigation_url_loader_impl.cc
@@ -5,6 +5,7 @@
#include "content/browser/loader/navigation_url_loader_impl.h"
#include <memory>
+#include <utility>
#include "base/bind.h"
#include "base/bind_helpers.h"
@@ -136,13 +137,6 @@
signed_exchange_utils::IsSignedExchangeHandlingEnabled();
}
-// Request ID for browser initiated requests. We start at -2 on the same lines
-// as ResourceDispatcherHostImpl.
-int g_next_request_id = -2;
-GlobalRequestID MakeGlobalRequestID() {
- return GlobalRequestID(-1, g_next_request_id--);
-}
-
size_t GetCertificateChainsSizeInKB(const net::SSLInfo& ssl_info) {
base::Pickle cert_pickle;
ssl_info.cert->Persist(&cert_pickle);
@@ -381,29 +375,6 @@
mojo::BindingSet<network::mojom::URLLoaderFactory> bindings_;
};
-// Creates a URLLoaderFactory that uses |header_client|. This should have the
-// same settings as the factory from the URLLoaderFactoryGetter.
-std::unique_ptr<network::SharedURLLoaderFactoryInfo>
-CreateNetworkFactoryInfoWithHeaderClient(
- network::mojom::TrustedURLLoaderHeaderClientPtrInfo header_client,
- StoragePartitionImpl* partition) {
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
- network::mojom::URLLoaderFactoryPtrInfo factory_info;
- network::mojom::URLLoaderFactoryParamsPtr params =
- network::mojom::URLLoaderFactoryParams::New();
- params->header_client = std::move(header_client);
- params->process_id = network::mojom::kBrowserProcessId;
- params->is_corb_enabled = false;
- params->disable_web_security =
- base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kDisableWebSecurity);
-
- partition->GetNetworkContext()->CreateURLLoaderFactory(
- mojo::MakeRequest(&factory_info), std::move(params));
- return std::make_unique<network::WrapperSharedURLLoaderFactoryInfo>(
- std::move(factory_info));
-}
-
} // namespace
// Kept around during the lifetime of the navigation request, and is
@@ -1741,8 +1712,12 @@
partition->url_loader_factory_getter()->GetNetworkFactoryInfo();
if (header_client) {
needs_loader_factory_interceptor = true;
- network_factory_info = CreateNetworkFactoryInfoWithHeaderClient(
- std::move(header_client), partition);
+ network::mojom::URLLoaderFactoryPtrInfo factory_info;
+ CreateURLLoaderFactoryWithHeaderClient(
+ std::move(header_client), mojo::MakeRequest(&factory_info), partition);
+ network_factory_info =
+ std::make_unique<network::WrapperSharedURLLoaderFactoryInfo>(
+ std::move(factory_info));
}
DCHECK(!request_controller_);
@@ -1838,6 +1813,7 @@
delegate_->OnRequestFailed(status);
}
+// static
void NavigationURLLoaderImpl::SetBeginNavigationInterceptorForTesting(
const BeginNavigationInterceptor& interceptor) {
DCHECK(!BrowserThread::IsThreadInitialized(BrowserThread::IO) ||
@@ -1846,6 +1822,7 @@
g_interceptor.Get() = interceptor;
}
+// static
void NavigationURLLoaderImpl::SetURLLoaderFactoryInterceptorForTesting(
const URLLoaderFactoryInterceptor& interceptor) {
DCHECK(!BrowserThread::IsThreadInitialized(BrowserThread::IO) ||
@@ -1854,6 +1831,33 @@
g_loader_factory_interceptor.Get() = interceptor;
}
+// static
+void NavigationURLLoaderImpl::CreateURLLoaderFactoryWithHeaderClient(
+ network::mojom::TrustedURLLoaderHeaderClientPtrInfo header_client,
+ network::mojom::URLLoaderFactoryRequest factory_request,
+ StoragePartitionImpl* partition) {
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
+ network::mojom::URLLoaderFactoryParamsPtr params =
+ network::mojom::URLLoaderFactoryParams::New();
+ params->header_client = std::move(header_client);
+ params->process_id = network::mojom::kBrowserProcessId;
+ params->is_corb_enabled = false;
+ params->disable_web_security =
+ base::CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kDisableWebSecurity);
+
+ partition->GetNetworkContext()->CreateURLLoaderFactory(
+ std::move(factory_request), std::move(params));
+}
+
+// static
+GlobalRequestID NavigationURLLoaderImpl::MakeGlobalRequestID() {
+ DCHECK_CURRENTLY_ON(BrowserThread::IO);
+ // Start at -2 on the same lines as ResourceDispatcherHostImpl.
+ static int s_next_request_id = -2;
+ return GlobalRequestID(-1, s_next_request_id--);
+}
+
void NavigationURLLoaderImpl::OnRequestStarted(base::TimeTicks timestamp) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
delegate_->OnRequestStarted(timestamp);
diff --git a/content/browser/loader/navigation_url_loader_impl.h b/content/browser/loader/navigation_url_loader_impl.h
index 2afea45..b993353 100644
--- a/content/browser/loader/navigation_url_loader_impl.h
+++ b/content/browser/loader/navigation_url_loader_impl.h
@@ -27,6 +27,7 @@
class NavigationLoaderInterceptor;
class ResourceContext;
class StoragePartition;
+class StoragePartitionImpl;
struct GlobalRequestID;
class CONTENT_EXPORT NavigationURLLoaderImpl : public NavigationURLLoader {
@@ -90,6 +91,18 @@
static void SetURLLoaderFactoryInterceptorForTesting(
const URLLoaderFactoryInterceptor& interceptor);
+ // Creates a URLLoaderFactory for a navigation. The factory uses
+ // |header_client|. This should have the same settings as the factory from the
+ // URLLoaderFactoryGetter. Called on the UI thread.
+ static void CreateURLLoaderFactoryWithHeaderClient(
+ network::mojom::TrustedURLLoaderHeaderClientPtrInfo header_client,
+ network::mojom::URLLoaderFactoryRequest factory_request,
+ StoragePartitionImpl* partition);
+
+ // Returns a Request ID for browser-initiated navigation requests. Called on
+ // the IO thread.
+ static GlobalRequestID MakeGlobalRequestID();
+
private:
class URLLoaderRequestController;
void OnRequestStarted(base::TimeTicks timestamp);
diff --git a/content/browser/service_worker/service_worker_fetch_dispatcher.cc b/content/browser/service_worker/service_worker_fetch_dispatcher.cc
index 99401257..e6785f1 100644
--- a/content/browser/service_worker/service_worker_fetch_dispatcher.cc
+++ b/content/browser/service_worker/service_worker_fetch_dispatcher.cc
@@ -18,18 +18,25 @@
#include "content/browser/child_process_security_policy_impl.h"
#include "content/browser/devtools/service_worker_devtools_agent_host.h"
#include "content/browser/devtools/service_worker_devtools_manager.h"
+#include "content/browser/loader/navigation_url_loader_impl.h"
#include "content/browser/loader/resource_dispatcher_host_impl.h"
#include "content/browser/loader/resource_request_info_impl.h"
#include "content/browser/loader/resource_requester_info.h"
#include "content/browser/loader/url_loader_factory_impl.h"
#include "content/browser/service_worker/embedded_worker_status.h"
#include "content/browser/service_worker/service_worker_context_core.h"
+#include "content/browser/service_worker/service_worker_context_wrapper.h"
#include "content/browser/service_worker/service_worker_metrics.h"
#include "content/browser/service_worker/service_worker_version.h"
+#include "content/browser/storage_partition_impl.h"
#include "content/browser/url_loader_factory_getter.h"
#include "content/common/service_worker/service_worker_types.h"
+#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/common/content_client.h"
#include "content/public/common/content_features.h"
#include "content/public/common/navigation_policy.h"
#include "mojo/public/cpp/bindings/strong_binding.h"
@@ -40,6 +47,8 @@
#include "net/log/net_log_event_type.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
#include "net/url_request/url_request.h"
+#include "services/network/public/cpp/features.h"
+#include "services/network/public/cpp/wrapper_shared_url_loader_factory.h"
#include "services/network/throttling/throttling_controller.h"
#include "third_party/blink/public/common/service_worker/service_worker_status_code.h"
#include "third_party/blink/public/common/service_worker/service_worker_utils.h"
@@ -319,6 +328,48 @@
}
}
+// Creates the network URLLoaderFactory for the navigation preload request.
+void CreateNetworkFactoryForNavigationPreloadOnUI(
+ const ServiceWorkerFetchDispatcher::WebContentsGetter& web_contents_getter,
+ scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
+ network::mojom::URLLoaderFactoryRequest request) {
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
+ DCHECK(base::FeatureList::IsEnabled(network::features::kNetworkService));
+
+ WebContents* web_contents = web_contents_getter.Run();
+ StoragePartitionImpl* partition = context_wrapper->storage_partition();
+ if (!web_contents || !partition) {
+ // The navigation was cancelled or we are in shutdown. Just drop the
+ // request. Otherwise, we might go to network without consulting the
+ // embedder first, which would break guarantees.
+ return;
+ }
+
+ // Follow what NavigationURLLoaderImpl does for the initiator passed to
+ // WillCreateURLLoaderFactory():
+ // Navigation requests are not associated with any particular
+ // |network::ResourceRequest::request_initiator| origin - using an opaque
+ // origin instead.
+ url::Origin initiator = url::Origin();
+
+ // We ignore the value of |bypass_redirect_checks_unused| since a redirects is
+ // just relayed to the service worker where preloadResponse is resolved as
+ // redirect.
+ bool bypass_redirect_checks_unused;
+
+ // Consult the embedder.
+ network::mojom::TrustedURLLoaderHeaderClientPtrInfo header_client;
+ GetContentClient()->browser()->WillCreateURLLoaderFactory(
+ web_contents->GetBrowserContext(), web_contents->GetMainFrame(),
+ web_contents->GetMainFrame()->GetProcess()->GetID(),
+ true /* is_navigation */, false /* is_download */, initiator, &request,
+ &header_client, &bypass_redirect_checks_unused);
+
+ // Make the network factory.
+ NavigationURLLoaderImpl::CreateURLLoaderFactoryWithHeaderClient(
+ std::move(header_client), std::move(request), partition);
+}
+
} // namespace
// ResponseCallback is owned by the callback that is passed to
@@ -405,11 +456,18 @@
class ServiceWorkerFetchDispatcher::URLLoaderAssets
: public base::RefCounted<ServiceWorkerFetchDispatcher::URLLoaderAssets> {
public:
+ // Non-NetworkService.
URLLoaderAssets(
std::unique_ptr<network::mojom::URLLoaderFactory> url_loader_factory,
std::unique_ptr<DelegatingURLLoaderClient> url_loader_client)
: url_loader_factory_(std::move(url_loader_factory)),
url_loader_client_(std::move(url_loader_client)) {}
+ // NetworkService.
+ URLLoaderAssets(
+ scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory,
+ std::unique_ptr<DelegatingURLLoaderClient> url_loader_client)
+ : shared_url_loader_factory_(std::move(shared_url_loader_factory)),
+ url_loader_client_(std::move(url_loader_client)) {}
void MaybeReportToDevTools(std::pair<int, int> worker_id,
int fetch_event_id) {
@@ -420,7 +478,13 @@
friend class base::RefCounted<URLLoaderAssets>;
virtual ~URLLoaderAssets() {}
+ // Non-NetworkService:
std::unique_ptr<network::mojom::URLLoaderFactory> url_loader_factory_;
+
+ // NetworkService:
+ scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;
+
+ // Both:
std::unique_ptr<DelegatingURLLoaderClient> url_loader_client_;
DISALLOW_COPY_AND_ASSIGN(URLLoaderAssets);
@@ -650,9 +714,7 @@
Referrer::ReferrerPolicyForUrlRequest(original_info->GetReferrerPolicy());
request.is_prerendering = original_info->IsPrerendering();
request.load_flags = original_request->load_flags();
- // Set to SUB_RESOURCE because we shouldn't trigger NavigationResourceThrottle
- // for the service worker navigation preload request.
- request.resource_type = RESOURCE_TYPE_SUB_RESOURCE;
+ request.resource_type = RESOURCE_TYPE_NAVIGATION_PRELOAD;
request.priority = original_request->priority();
request.skip_service_worker = true;
request.do_not_prompt_for_login = true;
@@ -704,6 +766,8 @@
bool ServiceWorkerFetchDispatcher::MaybeStartNavigationPreloadWithURLLoader(
const network::ResourceRequest& original_request,
URLLoaderFactoryGetter* url_loader_factory_getter,
+ scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
+ const WebContentsGetter& web_contents_getter,
base::OnceClosure on_response) {
if (resource_type_ != RESOURCE_TYPE_MAIN_FRAME &&
resource_type_ != RESOURCE_TYPE_SUB_FRAME) {
@@ -716,9 +780,7 @@
return false;
network::ResourceRequest resource_request(original_request);
- // Set to SUB_RESOURCE because we shouldn't trigger NavigationResourceThrottle
- // for the service worker navigation preload request.
- resource_request.resource_type = RESOURCE_TYPE_SUB_RESOURCE;
+ resource_request.resource_type = RESOURCE_TYPE_NAVIGATION_PRELOAD;
resource_request.skip_service_worker = true;
resource_request.do_not_prompt_for_login = true;
@@ -730,6 +792,28 @@
"Service-Worker-Navigation-Preload",
version_->navigation_preload_state().header);
+ // Create the network factory.
+ scoped_refptr<network::SharedURLLoaderFactory> factory;
+ if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+ // In the network service case, create the factory on the UI thread.
+ network::mojom::URLLoaderFactoryPtr network_factory;
+ auto factory_request = mojo::MakeRequest(&network_factory);
+
+ base::PostTaskWithTraits(
+ FROM_HERE, {BrowserThread::UI},
+ base::BindOnce(&CreateNetworkFactoryForNavigationPreloadOnUI,
+ web_contents_getter, std::move(context_wrapper),
+ mojo::MakeRequest(&network_factory)));
+ factory = base::MakeRefCounted<network::WrapperSharedURLLoaderFactory>(
+ std::move(network_factory));
+ } else {
+ // In the non-network-service case, use |url_loader_factory_getter|. Unlike
+ // the network service case, we don't need to go to the UI thread to tell
+ // the embedder about the factory since the request will go to
+ // ResourceDispatcherHost which talks to the embedder then.
+ factory = url_loader_factory_getter->GetNetworkFactory();
+ }
+
preload_handle_ = blink::mojom::FetchEventPreloadHandle::New();
// Create the DelegatingURLLoaderClient, which becomes the
@@ -741,26 +825,28 @@
std::move(inner_url_loader_client), std::move(on_response),
resource_request);
- // Start the network request for the URL using the network loader.
- // TODO(falken): What to do about routing_id, request_id?
+ // Use NavigationURLLoaderImpl to get a unique request id across
+ // browser-initiated navigations and navigation preloads.
+ int request_id = NavigationURLLoaderImpl::MakeGlobalRequestID().request_id;
+
+ // Start the network request for the URL using the network factory.
+ // TODO(falken): What to do about routing_id.
network::mojom::URLLoaderClientPtr url_loader_client_to_pass;
url_loader_client->Bind(&url_loader_client_to_pass);
network::mojom::URLLoaderPtr url_loader;
- url_loader_factory_getter->GetNetworkFactory()->CreateLoaderAndStart(
- mojo::MakeRequest(&url_loader), -1 /* routing_id? */,
- -1 /* request_id? */, network::mojom::kURLLoadOptionNone,
- resource_request, std::move(url_loader_client_to_pass),
+
+ factory->CreateLoaderAndStart(
+ mojo::MakeRequest(&url_loader), -1 /* routing_id? */, request_id,
+ network::mojom::kURLLoadOptionNone, resource_request,
+ std::move(url_loader_client_to_pass),
net::MutableNetworkTrafficAnnotationTag(
kNavigationPreloadTrafficAnnotation));
preload_handle_->url_loader = url_loader.PassInterface();
DCHECK(!url_loader_assets_);
- // Unlike the non-S13N code path, we don't own the URLLoaderFactory being used
- // (it's the generic network factory), so we don't need to pass it to
- // URLLoaderAssets to keep it alive.
url_loader_assets_ = base::MakeRefCounted<URLLoaderAssets>(
- nullptr /* url_loader_factory */, std::move(url_loader_client));
+ std::move(factory), std::move(url_loader_client));
return true;
}
diff --git a/content/browser/service_worker/service_worker_fetch_dispatcher.h b/content/browser/service_worker/service_worker_fetch_dispatcher.h
index 63e1fdb..9c169b10 100644
--- a/content/browser/service_worker/service_worker_fetch_dispatcher.h
+++ b/content/browser/service_worker/service_worker_fetch_dispatcher.h
@@ -6,6 +6,7 @@
#define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_FETCH_DISPATCHER_H_
#include <memory>
+#include <string>
#include "base/callback.h"
#include "base/macros.h"
@@ -52,6 +53,7 @@
blink::mojom::ServiceWorkerStreamHandlePtr,
blink::mojom::ServiceWorkerFetchEventTimingPtr,
scoped_refptr<ServiceWorkerVersion>)>;
+ using WebContentsGetter = base::RepeatingCallback<WebContents*()>;
ServiceWorkerFetchDispatcher(blink::mojom::FetchAPIRequestPtr request,
ResourceType resource_type,
@@ -72,6 +74,8 @@
bool MaybeStartNavigationPreloadWithURLLoader(
const network::ResourceRequest& original_request,
URLLoaderFactoryGetter* url_loader_factory_getter,
+ scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
+ const WebContentsGetter& web_contents_getter,
base::OnceClosure on_response);
// Dispatches a fetch event to the |version| given in ctor, and fires
diff --git a/content/browser/service_worker/service_worker_navigation_loader.cc b/content/browser/service_worker/service_worker_navigation_loader.cc
index 60e00c11..ca894b5 100644
--- a/content/browser/service_worker/service_worker_navigation_loader.cc
+++ b/content/browser/service_worker/service_worker_navigation_loader.cc
@@ -5,6 +5,7 @@
#include "content/browser/service_worker/service_worker_navigation_loader.h"
#include <sstream>
+#include <string>
#include <utility>
#include "base/bind.h"
@@ -14,6 +15,8 @@
#include "base/optional.h"
#include "base/strings/strcat.h"
#include "base/trace_event/trace_event.h"
+#include "content/browser/service_worker/service_worker_context_core.h"
+#include "content/browser/service_worker/service_worker_context_wrapper.h"
#include "content/browser/service_worker/service_worker_provider_host.h"
#include "content/browser/service_worker/service_worker_version.h"
#include "content/browser/url_loader_factory_getter.h"
@@ -227,6 +230,14 @@
return;
}
+ base::WeakPtr<ServiceWorkerContextCore> core = active_worker->context();
+ if (!core) {
+ CommitCompleted(net::ERR_ABORTED, "No service worker context");
+ return;
+ }
+ scoped_refptr<ServiceWorkerContextWrapper> context = core->wrapper();
+ DCHECK(context);
+
// Dispatch the fetch event.
fetch_dispatcher_ = std::make_unique<ServiceWorkerFetchDispatcher>(
blink::mojom::FetchAPIRequest::From(resource_request_),
@@ -242,6 +253,7 @@
did_navigation_preload_ =
fetch_dispatcher_->MaybeStartNavigationPreloadWithURLLoader(
resource_request_, url_loader_factory_getter_.get(),
+ std::move(context), provider_host_->web_contents_getter(),
base::DoNothing(/* TODO(crbug/762357): metrics? */));
// Record worker start time here as |fetch_dispatcher_| will start a service
diff --git a/content/browser/service_worker/service_worker_navigation_loader_unittest.cc b/content/browser/service_worker/service_worker_navigation_loader_unittest.cc
index 425b53e..9dffb5e6 100644
--- a/content/browser/service_worker/service_worker_navigation_loader_unittest.cc
+++ b/content/browser/service_worker/service_worker_navigation_loader_unittest.cc
@@ -83,103 +83,6 @@
return response;
}
-// NavigationPreloadLoaderClient mocks the renderer-side URLLoaderClient for the
-// navigation preload network request performed by the browser. In production
-// code, this is ServiceWorkerContextClient::NavigationPreloadRequest,
-// which it forwards the response to FetchEvent#preloadResponse. Here, it
-// simulates passing the response to FetchEvent#respondWith.
-//
-// The navigation preload test is quite involved. The flow of data is:
-// 1. ServiceWorkerNavigationLoader asks ServiceWorkerFetchDispatcher to start
-// navigation preload.
-// 2. ServiceWorkerFetchDispatcher starts the network request which is mocked
-// by EmbeddedWorkerTestHelper's default network loader factory. The
-// response is sent to
-// ServiceWorkerFetchDispatcher::DelegatingURLLoaderClient.
-// 3. DelegatingURLLoaderClient sends the response to the |preload_handle|
-// that was passed to Helper::OnFetchEvent().
-// 4. Helper::OnFetchEvent() creates NavigationPreloadLoaderClient, which
-// receives the response.
-// 5. NavigationPreloadLoaderClient calls OnFetchEvent()'s callbacks
-// with the response.
-// 6. Like all FetchEvent responses, the response is sent to
-// ServiceWorkerNavigationLoader::DidDispatchFetchEvent, and the
-// RequestHandler is returned.
-class NavigationPreloadLoaderClient final
- : public network::mojom::URLLoaderClient {
- public:
- NavigationPreloadLoaderClient(
- blink::mojom::FetchEventPreloadHandlePtr preload_handle,
- blink::mojom::ServiceWorkerFetchResponseCallbackPtr response_callback,
- blink::mojom::ServiceWorker::DispatchFetchEventCallback finish_callback)
- : url_loader_(std::move(preload_handle->url_loader)),
- binding_(this, std::move(preload_handle->url_loader_client_request)),
- response_callback_(std::move(response_callback)),
- finish_callback_(std::move(finish_callback)) {
- binding_.set_connection_error_handler(
- base::BindOnce(&NavigationPreloadLoaderClient::OnConnectionError,
- base::Unretained(this)));
- }
- ~NavigationPreloadLoaderClient() override = default;
-
- // network::mojom::URLLoaderClient implementation
- void OnReceiveResponse(
- const network::ResourceResponseHead& response_head) override {
- response_head_ = response_head;
- }
- void OnStartLoadingResponseBody(
- mojo::ScopedDataPipeConsumerHandle body) override {
- body_ = std::move(body);
- // We could call OnResponseStream() here, but for simplicity, don't do
- // anything until OnComplete().
- }
- void OnComplete(const network::URLLoaderCompletionStatus& status) override {
- blink::mojom::ServiceWorkerStreamCallbackPtr stream_callback;
- auto stream_handle = blink::mojom::ServiceWorkerStreamHandle::New();
- stream_handle->callback_request = mojo::MakeRequest(&stream_callback);
- stream_handle->stream = std::move(body_);
-
- // Simulate passing the navigation preload response to
- // FetchEvent#respondWith.
- auto response = blink::mojom::FetchAPIResponse::New();
- response->url_list =
- std::vector<GURL>(response_head_.url_list_via_service_worker);
- response->status_code = response_head_.headers->response_code();
- response->status_text = response_head_.headers->GetStatusText();
- response->response_type = response_head_.response_type;
- response_callback_->OnResponseStream(
- std::move(response), std::move(stream_handle),
- blink::mojom::ServiceWorkerFetchEventTiming::New());
- std::move(finish_callback_)
- .Run(blink::mojom::ServiceWorkerEventStatus::COMPLETED);
- stream_callback->OnCompleted();
- delete this;
- }
- void OnReceiveRedirect(
- const net::RedirectInfo& redirect_info,
- const network::ResourceResponseHead& response_head) override {}
- void OnUploadProgress(int64_t current_position,
- int64_t total_size,
- OnUploadProgressCallback ack_callback) override {}
- void OnReceiveCachedMetadata(const std::vector<uint8_t>& data) override {}
- void OnTransferSizeUpdated(int32_t transfer_size_diff) override {}
-
- void OnConnectionError() { delete this; }
-
- private:
- network::mojom::URLLoaderPtr url_loader_;
- mojo::Binding<network::mojom::URLLoaderClient> binding_;
-
- network::ResourceResponseHead response_head_;
- mojo::ScopedDataPipeConsumerHandle body_;
-
- // Callbacks that complete Helper::OnFetchEvent().
- blink::mojom::ServiceWorkerFetchResponseCallbackPtr response_callback_;
- blink::mojom::ServiceWorker::DispatchFetchEventCallback finish_callback_;
-
- DISALLOW_COPY_AND_ASSIGN(NavigationPreloadLoaderClient);
-};
-
// Simulates a service worker handling fetch events. The response can be
// customized via RespondWith* functions.
class FetchEventServiceWorker : public FakeServiceWorker {
@@ -216,13 +119,6 @@
// Tells this worker to respond to fetch events with an error response.
void RespondWithError() { response_mode_ = ResponseMode::kErrorResponse; }
- // Tells this worker to respond to fetch events with
- // FetchEvent#preloadResponse. See NavigationPreloadLoaderClient's
- // documentation for details.
- void RespondWithNavigationPreloadResponse() {
- response_mode_ = ResponseMode::kNavigationPreloadResponse;
- }
-
// Tells this worker to respond to fetch events with the redirect response.
void RespondWithRedirectResponse(const GURL& new_url) {
response_mode_ = ResponseMode::kRedirect;
@@ -326,12 +222,6 @@
std::move(finish_callback)
.Run(blink::mojom::ServiceWorkerEventStatus::REJECTED);
break;
- case ResponseMode::kNavigationPreloadResponse:
- // Deletes itself when done.
- new NavigationPreloadLoaderClient(std::move(params->preload_handle),
- std::move(response_callback),
- std::move(finish_callback));
- break;
case ResponseMode::kFailFetchEventDispatch:
// Simulate failure by stopping the worker before the event finishes.
// This causes ServiceWorkerVersion::StartRequest() to call its error
@@ -380,7 +270,6 @@
kStream,
kFallbackResponse,
kErrorResponse,
- kNavigationPreloadResponse,
kFailFetchEventDispatch,
kDeferredResponse,
kEarlyResponse,
@@ -1063,32 +952,6 @@
0);
}
-// Test responding to the fetch event with the navigation preload response.
-TEST_F(ServiceWorkerNavigationLoaderTest, NavigationPreload) {
- registration_->EnableNavigationPreload(true);
- service_worker_->RespondWithNavigationPreloadResponse();
-
- // Perform the request
- LoaderResult result = StartRequest(CreateRequest());
- ASSERT_EQ(LoaderResult::kHandledRequest, result);
- client_.RunUntilComplete();
-
- EXPECT_EQ(net::OK, client_.completion_status().error_code);
- const network::ResourceResponseHead& info = client_.response_head();
- EXPECT_EQ(200, info.headers->response_code());
-
- std::unique_ptr<network::ResourceResponseHead> expected_info =
- CreateResponseInfoFromServiceWorker();
- expected_info->did_service_worker_navigation_preload = true;
- ExpectResponseInfo(info, *expected_info);
-
- std::string response;
- EXPECT_TRUE(client_.response_body().is_valid());
- EXPECT_TRUE(
- mojo::BlockingCopyToString(client_.response_body_release(), &response));
- EXPECT_EQ("this body came from the network", response);
-}
-
// Test responding to the fetch event with a redirect response.
TEST_F(ServiceWorkerNavigationLoaderTest, Redirect) {
base::HistogramTester histogram_tester;
diff --git a/content/public/common/resource_type.h b/content/public/common/resource_type.h
index b908921..8c87f6f 100644
--- a/content/public/common/resource_type.h
+++ b/content/public/common/resource_type.h
@@ -30,7 +30,9 @@
RESOURCE_TYPE_SERVICE_WORKER = 15, // the main resource of a service worker.
RESOURCE_TYPE_CSP_REPORT = 16, // a report of Content Security Policy
// violations.
- RESOURCE_TYPE_PLUGIN_RESOURCE = 17, // a resource that a plugin requested.
+ RESOURCE_TYPE_PLUGIN_RESOURCE = 17, // a resource that a plugin requested.
+ RESOURCE_TYPE_NAVIGATION_PRELOAD =
+ 18, // a service worker navigation preload request.
RESOURCE_TYPE_LAST_TYPE
};
diff --git a/content/public/common/resource_type.mojom b/content/public/common/resource_type.mojom
index 98211a5..35295cd 100644
--- a/content/public/common/resource_type.mojom
+++ b/content/public/common/resource_type.mojom
@@ -26,5 +26,7 @@
RESOURCE_TYPE_CSP_REPORT = 16, // a report of Content Security Policy
// violations.
RESOURCE_TYPE_PLUGIN_RESOURCE = 17, // a resource that a plugin requested.
+ RESOURCE_TYPE_NAVIGATION_PRELOAD =
+ 18, // a service worker navigation preload request.
RESOURCE_TYPE_LAST_TYPE
};
diff --git a/content/public/common/resource_type_struct_traits.cc b/content/public/common/resource_type_struct_traits.cc
index 5bc012f..414852d 100644
--- a/content/public/common/resource_type_struct_traits.cc
+++ b/content/public/common/resource_type_struct_traits.cc
@@ -47,6 +47,8 @@
return content::mojom::ResourceType::RESOURCE_TYPE_CSP_REPORT;
case content::RESOURCE_TYPE_PLUGIN_RESOURCE:
return content::mojom::ResourceType::RESOURCE_TYPE_PLUGIN_RESOURCE;
+ case content::RESOURCE_TYPE_NAVIGATION_PRELOAD:
+ return content::mojom::ResourceType::RESOURCE_TYPE_NAVIGATION_PRELOAD;
case content::RESOURCE_TYPE_LAST_TYPE:
return content::mojom::ResourceType::RESOURCE_TYPE_LAST_TYPE;
}
@@ -114,6 +116,9 @@
case content::mojom::ResourceType::RESOURCE_TYPE_PLUGIN_RESOURCE:
*output = content::RESOURCE_TYPE_PLUGIN_RESOURCE;
return true;
+ case content::mojom::ResourceType::RESOURCE_TYPE_NAVIGATION_PRELOAD:
+ *output = content::RESOURCE_TYPE_NAVIGATION_PRELOAD;
+ return true;
case content::mojom::ResourceType::RESOURCE_TYPE_LAST_TYPE:
*output = content::RESOURCE_TYPE_LAST_TYPE;
return true;
diff --git a/extensions/browser/api/declarative_net_request/ruleset_manager.cc b/extensions/browser/api/declarative_net_request/ruleset_manager.cc
index 4b607e9..f54f621 100644
--- a/extensions/browser/api/declarative_net_request/ruleset_manager.cc
+++ b/extensions/browser/api/declarative_net_request/ruleset_manager.cc
@@ -54,6 +54,7 @@
case content::RESOURCE_TYPE_LAST_TYPE:
case content::RESOURCE_TYPE_PREFETCH:
case content::RESOURCE_TYPE_SUB_RESOURCE:
+ case content::RESOURCE_TYPE_NAVIGATION_PRELOAD:
return flat_rule::ElementType_OTHER;
case content::RESOURCE_TYPE_MAIN_FRAME:
return flat_rule::ElementType_MAIN_FRAME;
diff --git a/extensions/browser/api/web_request/web_request_info.cc b/extensions/browser/api/web_request/web_request_info.cc
index a51cbc3..d3ec4efa 100644
--- a/extensions/browser/api/web_request/web_request_info.cc
+++ b/extensions/browser/api/web_request/web_request_info.cc
@@ -276,6 +276,7 @@
render_process_id = url_loader->GetProcessId();
frame_id = url_loader->GetRenderFrameId();
}
+ type = static_cast<content::ResourceType>(url_loader->GetResourceType());
} else {
// There may be basic process and frame info associated with the request
// even when |info| is null. Attempt to grab it as a last ditch effort. If
diff --git a/extensions/browser/api/web_request/web_request_permissions.cc b/extensions/browser/api/web_request/web_request_permissions.cc
index 9d6b0dda..ae4b7e2 100644
--- a/extensions/browser/api/web_request/web_request_permissions.cc
+++ b/extensions/browser/api/web_request/web_request_permissions.cc
@@ -285,11 +285,14 @@
if (is_request_from_browser) {
// Hide all non-navigation requests made by the browser. crbug.com/884932.
- if (!request.is_browser_side_navigation)
+ if (!request.is_browser_side_navigation &&
+ request.type != content::RESOURCE_TYPE_NAVIGATION_PRELOAD) {
return true;
+ }
DCHECK(request.type == content::RESOURCE_TYPE_MAIN_FRAME ||
- request.type == content::RESOURCE_TYPE_SUB_FRAME);
+ request.type == content::RESOURCE_TYPE_SUB_FRAME ||
+ request.type == content::RESOURCE_TYPE_NAVIGATION_PRELOAD);
// Hide sub-frame requests to clientsX.google.com.
// TODO(crbug.com/890006): Determine if the code here can be cleaned up
diff --git a/extensions/browser/api/web_request/web_request_resource_type.cc b/extensions/browser/api/web_request/web_request_resource_type.cc
index 0c05a2d..b0da58b 100644
--- a/extensions/browser/api/web_request/web_request_resource_type.cc
+++ b/extensions/browser/api/web_request/web_request_resource_type.cc
@@ -78,6 +78,8 @@
return WebRequestResourceType::CSP_REPORT;
case content::RESOURCE_TYPE_PLUGIN_RESOURCE:
return WebRequestResourceType::OBJECT;
+ case content::RESOURCE_TYPE_NAVIGATION_PRELOAD:
+ return WebRequestResourceType::OTHER;
case content::RESOURCE_TYPE_LAST_TYPE:
return WebRequestResourceType::OTHER;
}
diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc
index 7d2455b6..a867be6 100644
--- a/services/network/url_loader.cc
+++ b/services/network/url_loader.cc
@@ -1052,6 +1052,10 @@
report_raw_headers_ = want_raw_headers_ && allow;
}
+uint32_t URLLoader::GetResourceType() const {
+ return resource_type_;
+}
+
// static
URLLoader* URLLoader::ForRequest(const net::URLRequest& request) {
auto* pointer =
diff --git a/services/network/url_loader.h b/services/network/url_loader.h
index 8ab338a..796893b 100644
--- a/services/network/url_loader.h
+++ b/services/network/url_loader.h
@@ -114,6 +114,7 @@
uint32_t GetRenderFrameId() const;
uint32_t GetProcessId() const;
+ uint32_t GetResourceType() const;
const net::HttpRequestHeaders& custom_proxy_pre_cache_headers() const {
return custom_proxy_pre_cache_headers_;
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index e388a87..b9fe67d 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -8159,7 +8159,7 @@
<enum name="ContentResourceType">
<obsolete>
- Superseded by ContentResourceType in December 2015 when SUB_RESOURCE was
+ Superseded by ContentResourceType2 in December 2015 when SUB_RESOURCE was
split into RESOURCE_TYPE_SUB_RESOURCE and RESOURCE_TYPE_PLUGIN_RESOURCE, and
PING was split into RESOURCE_TYPE_PING and RESOURCE_TYPE_CSP_REPORT.
</obsolete>
@@ -8200,6 +8200,7 @@
<int value="15" label="RESOURCE_TYPE_SERVICE_WORKER"/>
<int value="16" label="RESOURCE_TYPE_CSP_REPORT"/>
<int value="17" label="RESOURCE_TYPE_PLUGIN_RESOURCE"/>
+ <int value="18" label="RESOURCE_TYPE_NAVIGATION_PRELOAD"/>
</enum>
<enum name="ContentSetting">