Commit 6579f5a4 authored by Kenichi Ishibashi's avatar Kenichi Ishibashi Committed by Commit Bot

S13nServiceWorker: Use RenderProcessHost's network factory for shared worker script loading

When S13nServiceWorker is on and NetworkService is off, we should
use content::URLLoaderFactoryImpl as the default network factory
of SharedWorkerScriptLoader because AppCacheRequestHandler wouldn't
work when NetworkService is off. It needs to use non-network
service path when a request is fallback to network to make sure
that an appcache is associated, if any.

SharedWorkerScriptLoader may ask the network factory to create
loaders more than once with the same |request_id| when there are
redirects. However, ResourceDispatcherHostImpl, which is used by
URLLoaderFactoryImpl, doesn't allow using the same |request_id|
multiple times. To work around this restriction we call
ResourceDispatcherHostImpl::CancellRequest() when a redirect happens
while loading a shared worker script.

This CL fixes following test when S13nServiceWorker is enabled:
- external/wpt/html/browsers/offline/appcache/workers/appcache-worker.https.html

Bug: 869302
Change-Id: I5fae74439f82b3314f87a06bf9842db9df75a351
Reviewed-on: https://chromium-review.googlesource.com/1164741
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581818}
parent cc03e427
...@@ -164,20 +164,13 @@ void SharedWorkerHost::Start( ...@@ -164,20 +164,13 @@ void SharedWorkerHost::Start(
// Add the network factory to the bundle to pass to the renderer. The bundle // Add the network factory to the bundle to pass to the renderer. The bundle
// is only provided (along with |script_loader_factory|) if // is only provided (along with |script_loader_factory|) if
// NetworkService/S13nSW is enabled. // NetworkService/S13nServiceWorker is enabled, and default factory isn't
// provided if NetworkService is on but S13nServiceWorker is off.
DCHECK(!script_loader_factory || factory_bundle); DCHECK(!script_loader_factory || factory_bundle);
if (factory_bundle) { if (factory_bundle && !factory_bundle->default_factory_info()) {
DCHECK(base::FeatureList::IsEnabled(network::features::kNetworkService));
network::mojom::URLLoaderFactoryPtrInfo network_factory_info; network::mojom::URLLoaderFactoryPtrInfo network_factory_info;
if (base::FeatureList::IsEnabled(network::features::kNetworkService)) { CreateNetworkFactory(mojo::MakeRequest(&network_factory_info));
// NetworkService is on: Use the network service.
CreateNetworkFactory(mojo::MakeRequest(&network_factory_info));
} else {
// NetworkService is off: RenderProcessHost gives us a non-NetworkService
// network factory.
RenderProcessHost::FromID(process_id_)
->CreateURLLoaderFactory(mojo::MakeRequest(&network_factory_info));
}
DCHECK(!factory_bundle->default_factory_info());
factory_bundle->default_factory_info() = std::move(network_factory_info); factory_bundle->default_factory_info() = std::move(network_factory_info);
// TODO(falken): We might need to set the default factory to AppCache // TODO(falken): We might need to set the default factory to AppCache
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "content/browser/appcache/appcache_request_handler.h" #include "content/browser/appcache/appcache_request_handler.h"
#include "content/browser/loader/navigation_loader_interceptor.h" #include "content/browser/loader/navigation_loader_interceptor.h"
#include "content/browser/loader/resource_dispatcher_host_impl.h"
#include "content/browser/service_worker/service_worker_provider_host.h" #include "content/browser/service_worker/service_worker_provider_host.h"
#include "content/public/browser/resource_context.h" #include "content/public/browser/resource_context.h"
#include "net/url_request/redirect_util.h" #include "net/url_request/redirect_util.h"
...@@ -15,6 +16,7 @@ ...@@ -15,6 +16,7 @@
namespace content { namespace content {
SharedWorkerScriptLoader::SharedWorkerScriptLoader( SharedWorkerScriptLoader::SharedWorkerScriptLoader(
int process_id,
int32_t routing_id, int32_t routing_id,
int32_t request_id, int32_t request_id,
uint32_t options, uint32_t options,
...@@ -25,7 +27,8 @@ SharedWorkerScriptLoader::SharedWorkerScriptLoader( ...@@ -25,7 +27,8 @@ SharedWorkerScriptLoader::SharedWorkerScriptLoader(
ResourceContext* resource_context, ResourceContext* resource_context,
scoped_refptr<network::SharedURLLoaderFactory> default_loader_factory, scoped_refptr<network::SharedURLLoaderFactory> default_loader_factory,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation) const net::MutableNetworkTrafficAnnotationTag& traffic_annotation)
: routing_id_(routing_id), : process_id_(process_id),
routing_id_(routing_id),
request_id_(request_id), request_id_(request_id),
options_(options), options_(options),
resource_request_(resource_request), resource_request_(resource_request),
...@@ -143,6 +146,12 @@ void SharedWorkerScriptLoader::FollowRedirect( ...@@ -143,6 +146,12 @@ void SharedWorkerScriptLoader::FollowRedirect(
interceptor_index_ = 0; interceptor_index_ = 0;
url_loader_client_binding_.Unbind(); url_loader_client_binding_.Unbind();
redirect_info_.reset(); redirect_info_.reset();
// Cancel the request on ResourceDispatcherHost so that we can fall back
// to network again.
DCHECK(ResourceDispatcherHostImpl::Get());
ResourceDispatcherHostImpl::Get()->CancelRequest(process_id_, request_id_);
Start(); Start();
} }
......
...@@ -43,6 +43,7 @@ class SharedWorkerScriptLoader : public network::mojom::URLLoader, ...@@ -43,6 +43,7 @@ class SharedWorkerScriptLoader : public network::mojom::URLLoader,
// non-NetworkService factories used for non-http(s) URLs, e.g., a // non-NetworkService factories used for non-http(s) URLs, e.g., a
// chrome-extension:// URL. // chrome-extension:// URL.
SharedWorkerScriptLoader( SharedWorkerScriptLoader(
int process_id,
int32_t routing_id, int32_t routing_id,
int32_t request_id, int32_t request_id,
uint32_t options, uint32_t options,
...@@ -93,6 +94,7 @@ class SharedWorkerScriptLoader : public network::mojom::URLLoader, ...@@ -93,6 +94,7 @@ class SharedWorkerScriptLoader : public network::mojom::URLLoader,
std::vector<std::unique_ptr<NavigationLoaderInterceptor>> interceptors_; std::vector<std::unique_ptr<NavigationLoaderInterceptor>> interceptors_;
size_t interceptor_index_ = 0; size_t interceptor_index_ = 0;
const int process_id_;
const int32_t routing_id_; const int32_t routing_id_;
const int32_t request_id_; const int32_t request_id_;
const uint32_t options_; const uint32_t options_;
......
...@@ -20,12 +20,14 @@ ...@@ -20,12 +20,14 @@
namespace content { namespace content {
SharedWorkerScriptLoaderFactory::SharedWorkerScriptLoaderFactory( SharedWorkerScriptLoaderFactory::SharedWorkerScriptLoaderFactory(
int process_id,
ServiceWorkerContextWrapper* context, ServiceWorkerContextWrapper* context,
base::WeakPtr<ServiceWorkerProviderHost> service_worker_provider_host, base::WeakPtr<ServiceWorkerProviderHost> service_worker_provider_host,
base::WeakPtr<AppCacheHost> appcache_host, base::WeakPtr<AppCacheHost> appcache_host,
ResourceContext* resource_context, ResourceContext* resource_context,
scoped_refptr<network::SharedURLLoaderFactory> loader_factory) scoped_refptr<network::SharedURLLoaderFactory> loader_factory)
: service_worker_provider_host_(std::move(service_worker_provider_host)), : process_id_(process_id),
service_worker_provider_host_(std::move(service_worker_provider_host)),
appcache_host_(std::move(appcache_host)), appcache_host_(std::move(appcache_host)),
resource_context_(resource_context), resource_context_(resource_context),
loader_factory_(std::move(loader_factory)) { loader_factory_(std::move(loader_factory)) {
...@@ -61,9 +63,9 @@ void SharedWorkerScriptLoaderFactory::CreateLoaderAndStart( ...@@ -61,9 +63,9 @@ void SharedWorkerScriptLoaderFactory::CreateLoaderAndStart(
// Create a SharedWorkerScriptLoader to load the script. // Create a SharedWorkerScriptLoader to load the script.
mojo::MakeStrongBinding( mojo::MakeStrongBinding(
std::make_unique<SharedWorkerScriptLoader>( std::make_unique<SharedWorkerScriptLoader>(
routing_id, request_id, options, resource_request, std::move(client), process_id_, routing_id, request_id, options, resource_request,
service_worker_provider_host_, appcache_host_, resource_context_, std::move(client), service_worker_provider_host_, appcache_host_,
loader_factory_, traffic_annotation), resource_context_, loader_factory_, traffic_annotation),
std::move(request)); std::move(request));
} }
......
...@@ -37,6 +37,7 @@ class SharedWorkerScriptLoaderFactory ...@@ -37,6 +37,7 @@ class SharedWorkerScriptLoaderFactory
// the NetworkService. However, it may internally contain non-NetworkService // the NetworkService. However, it may internally contain non-NetworkService
// factories used for non-http(s) URLs, e.g., a chrome-extension:// URL. // factories used for non-http(s) URLs, e.g., a chrome-extension:// URL.
SharedWorkerScriptLoaderFactory( SharedWorkerScriptLoaderFactory(
int process_id,
ServiceWorkerContextWrapper* context, ServiceWorkerContextWrapper* context,
base::WeakPtr<ServiceWorkerProviderHost> provider_host, base::WeakPtr<ServiceWorkerProviderHost> provider_host,
base::WeakPtr<AppCacheHost> appcache_host, base::WeakPtr<AppCacheHost> appcache_host,
...@@ -56,6 +57,7 @@ class SharedWorkerScriptLoaderFactory ...@@ -56,6 +57,7 @@ class SharedWorkerScriptLoaderFactory
void Clone(network::mojom::URLLoaderFactoryRequest request) override; void Clone(network::mojom::URLLoaderFactoryRequest request) override;
private: private:
const int process_id_;
base::WeakPtr<ServiceWorkerProviderHost> service_worker_provider_host_; base::WeakPtr<ServiceWorkerProviderHost> service_worker_provider_host_;
base::WeakPtr<AppCacheHost> appcache_host_; base::WeakPtr<AppCacheHost> appcache_host_;
ResourceContext* resource_context_ = nullptr; ResourceContext* resource_context_ = nullptr;
......
...@@ -83,6 +83,17 @@ std::unique_ptr<URLLoaderFactoryBundleInfo> CreateFactoryBundle( ...@@ -83,6 +83,17 @@ std::unique_ptr<URLLoaderFactoryBundleInfo> CreateFactoryBundle(
factory_bundle->factories_info().emplace(url::kFileScheme, factory_bundle->factories_info().emplace(url::kFileScheme,
file_factory_ptr.PassInterface()); file_factory_ptr.PassInterface());
} }
// Use RenderProcessHost's network factory as the default factory if
// NetworkService is off. If NetworkService is on the default factory is
// set in CreateScriptLoaderOnIO().
if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) {
network::mojom::URLLoaderFactoryPtr default_factory;
RenderProcessHost::FromID(process_id)
->CreateURLLoaderFactory(mojo::MakeRequest(&default_factory));
factory_bundle->default_factory_info() = default_factory.PassInterface();
}
return factory_bundle; return factory_bundle;
} }
...@@ -116,20 +127,25 @@ void CreateScriptLoaderOnIO( ...@@ -116,20 +127,25 @@ void CreateScriptLoaderOnIO(
url_loader_factory = network::SharedURLLoaderFactory::Create( url_loader_factory = network::SharedURLLoaderFactory::Create(
std::move(blob_url_loader_factory_info)); std::move(blob_url_loader_factory_info));
} else { } else {
// Add the network factory to the bundle. If NetworkService is off the
// default factory was already set in CreateFactoryBundle().
DCHECK(base::FeatureList::IsEnabled(network::features::kNetworkService) ||
factory_bundle_for_browser_info->default_factory_info());
// Create a factory bundle to use. // Create a factory bundle to use.
scoped_refptr<URLLoaderFactoryBundle> factory_bundle = scoped_refptr<URLLoaderFactoryBundle> factory_bundle =
base::MakeRefCounted<URLLoaderFactoryBundle>( base::MakeRefCounted<URLLoaderFactoryBundle>(
std::move(factory_bundle_for_browser_info)); std::move(factory_bundle_for_browser_info));
url_loader_factory = factory_bundle; url_loader_factory = factory_bundle;
// Add the network factory to the bundle. The factory from if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
// CloneNetworkFactory() doesn't support reconnection to the network service // The factory from CloneNetworkFactory() doesn't support reconnection to
// after a crash, but it's OK since it's used for a single shared worker // the network service after a crash, but it's OK since it's used for a
// startup. // single shared worker startup.
network::mojom::URLLoaderFactoryPtr network_factory_ptr; network::mojom::URLLoaderFactoryPtr network_factory_ptr;
loader_factory_getter->CloneNetworkFactory( loader_factory_getter->CloneNetworkFactory(
mojo::MakeRequest(&network_factory_ptr)); mojo::MakeRequest(&network_factory_ptr));
factory_bundle->SetDefaultFactory(std::move(network_factory_ptr)); factory_bundle->SetDefaultFactory(std::move(network_factory_ptr));
}
} }
// It's safe for |appcache_handle_core| to be a raw pointer. The core is owned // It's safe for |appcache_handle_core| to be a raw pointer. The core is owned
...@@ -144,8 +160,9 @@ void CreateScriptLoaderOnIO( ...@@ -144,8 +160,9 @@ void CreateScriptLoaderOnIO(
network::mojom::URLLoaderFactoryAssociatedPtrInfo script_loader_factory; network::mojom::URLLoaderFactoryAssociatedPtrInfo script_loader_factory;
mojo::MakeStrongAssociatedBinding( mojo::MakeStrongAssociatedBinding(
std::make_unique<SharedWorkerScriptLoaderFactory>( std::make_unique<SharedWorkerScriptLoaderFactory>(
context.get(), host->AsWeakPtr(), std::move(appcache_host), process_id, context.get(), host->AsWeakPtr(),
context->resource_context(), std::move(url_loader_factory)), std::move(appcache_host), context->resource_context(),
std::move(url_loader_factory)),
mojo::MakeRequest(&script_loader_factory)); mojo::MakeRequest(&script_loader_factory));
// We continue in StartWorker. // We continue in StartWorker.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment