Commit 2a328a72 authored by Kenichi Ishibashi's avatar Kenichi Ishibashi Committed by Commit Bot

service worker: Assign new resource ID outside ServiceWorkerNewScriptLoader

Resource ID allocation will become async (see the planning doc[1]).
As a preparation, this CL moves resource ID allocation from
ServiceWorkerNewScriptLoader to ServiceWorkerScriptLoaderFactory.
Once resource ID allocation becomes async the factory will create
ServiceWorkerNewScriptLoader asynchronously.

[1] https://docs.google.com/document/d/1j0WP5wsenJISViJzhngSJG3hKcIf39KbfRgCbzgWt5A/edit?usp=sharing

Bug: 1046335
Change-Id: Ia6618085ec4e9413f670e75fe2d074736906a623
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2041677Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739251}
parent 10646707
...@@ -54,10 +54,11 @@ ServiceWorkerNewScriptLoader::CreateAndStart( ...@@ -54,10 +54,11 @@ ServiceWorkerNewScriptLoader::CreateAndStart(
mojo::PendingRemote<network::mojom::URLLoaderClient> client, mojo::PendingRemote<network::mojom::URLLoaderClient> client,
scoped_refptr<ServiceWorkerVersion> version, scoped_refptr<ServiceWorkerVersion> version,
scoped_refptr<network::SharedURLLoaderFactory> loader_factory, scoped_refptr<network::SharedURLLoaderFactory> loader_factory,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation) { const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
int64_t cache_resource_id) {
return base::WrapUnique(new ServiceWorkerNewScriptLoader( return base::WrapUnique(new ServiceWorkerNewScriptLoader(
routing_id, request_id, options, original_request, std::move(client), routing_id, request_id, options, original_request, std::move(client),
version, loader_factory, traffic_annotation)); version, loader_factory, traffic_annotation, cache_resource_id));
} }
// TODO(nhiroki): We're doing multiple things in the ctor. Consider factors out // TODO(nhiroki): We're doing multiple things in the ctor. Consider factors out
...@@ -70,7 +71,8 @@ ServiceWorkerNewScriptLoader::ServiceWorkerNewScriptLoader( ...@@ -70,7 +71,8 @@ ServiceWorkerNewScriptLoader::ServiceWorkerNewScriptLoader(
mojo::PendingRemote<network::mojom::URLLoaderClient> client, mojo::PendingRemote<network::mojom::URLLoaderClient> client,
scoped_refptr<ServiceWorkerVersion> version, scoped_refptr<ServiceWorkerVersion> version,
scoped_refptr<network::SharedURLLoaderFactory> loader_factory, scoped_refptr<network::SharedURLLoaderFactory> loader_factory,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation) const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
int64_t cache_resource_id)
: request_url_(original_request.url), : request_url_(original_request.url),
resource_type_(static_cast<blink::mojom::ResourceType>( resource_type_(static_cast<blink::mojom::ResourceType>(
original_request.resource_type)), original_request.resource_type)),
...@@ -81,14 +83,14 @@ ServiceWorkerNewScriptLoader::ServiceWorkerNewScriptLoader( ...@@ -81,14 +83,14 @@ ServiceWorkerNewScriptLoader::ServiceWorkerNewScriptLoader(
base::SequencedTaskRunnerHandle::Get()), base::SequencedTaskRunnerHandle::Get()),
loader_factory_(std::move(loader_factory)), loader_factory_(std::move(loader_factory)),
client_(std::move(client)) { client_(std::move(client)) {
DCHECK_NE(cache_resource_id,
ServiceWorkerConsts::kInvalidServiceWorkerResourceId);
network::ResourceRequest resource_request(original_request); network::ResourceRequest resource_request(original_request);
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
CheckVersionStatusBeforeLoad(); CheckVersionStatusBeforeLoad();
#endif // DCHECK_IS_ON() #endif // DCHECK_IS_ON()
// TODO(nhiroki): Handle the case where |cache_resource_id| is invalid.
int64_t cache_resource_id = version->context()->storage()->NewResourceId();
// |incumbent_cache_resource_id| is valid if the incumbent service worker // |incumbent_cache_resource_id| is valid if the incumbent service worker
// exists and it's required to do the byte-for-byte check. // exists and it's required to do the byte-for-byte check.
int64_t incumbent_cache_resource_id = int64_t incumbent_cache_resource_id =
......
...@@ -82,7 +82,8 @@ class CONTENT_EXPORT ServiceWorkerNewScriptLoader final ...@@ -82,7 +82,8 @@ class CONTENT_EXPORT ServiceWorkerNewScriptLoader final
mojo::PendingRemote<network::mojom::URLLoaderClient> client, mojo::PendingRemote<network::mojom::URLLoaderClient> client,
scoped_refptr<ServiceWorkerVersion> version, scoped_refptr<ServiceWorkerVersion> version,
scoped_refptr<network::SharedURLLoaderFactory> loader_factory, scoped_refptr<network::SharedURLLoaderFactory> loader_factory,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation); const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
int64_t cache_resource_id);
~ServiceWorkerNewScriptLoader() override; ~ServiceWorkerNewScriptLoader() override;
...@@ -124,7 +125,8 @@ class CONTENT_EXPORT ServiceWorkerNewScriptLoader final ...@@ -124,7 +125,8 @@ class CONTENT_EXPORT ServiceWorkerNewScriptLoader final
mojo::PendingRemote<network::mojom::URLLoaderClient> client, mojo::PendingRemote<network::mojom::URLLoaderClient> client,
scoped_refptr<ServiceWorkerVersion> version, scoped_refptr<ServiceWorkerVersion> version,
scoped_refptr<network::SharedURLLoaderFactory> loader_factory, scoped_refptr<network::SharedURLLoaderFactory> loader_factory,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation); const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
int64_t cache_resource_id);
// Writes the given headers into the service worker script storage. // Writes the given headers into the service worker script storage.
void WriteHeaders(scoped_refptr<HttpResponseInfoIOBuffer> info_buffer); void WriteHeaders(scoped_refptr<HttpResponseInfoIOBuffer> info_buffer);
......
...@@ -220,6 +220,9 @@ class ServiceWorkerNewScriptLoaderTest : public testing::Test { ...@@ -220,6 +220,9 @@ class ServiceWorkerNewScriptLoaderTest : public testing::Test {
int routing_id = 0; int routing_id = 0;
int request_id = 10; int request_id = 10;
uint32_t options = 0; uint32_t options = 0;
// TODO(crbug.com/1046335): NewResourceId() will become async. Add a helper
// function that return a new resource ID synchronously.
int64_t resource_id = context()->storage()->NewResourceId();
network::ResourceRequest request; network::ResourceRequest request;
request.url = url; request.url = url;
...@@ -233,7 +236,8 @@ class ServiceWorkerNewScriptLoaderTest : public testing::Test { ...@@ -233,7 +236,8 @@ class ServiceWorkerNewScriptLoaderTest : public testing::Test {
*out_loader = ServiceWorkerNewScriptLoader::CreateAndStart( *out_loader = ServiceWorkerNewScriptLoader::CreateAndStart(
routing_id, request_id, options, request, (*out_client)->CreateRemote(), routing_id, request_id, options, request, (*out_client)->CreateRemote(),
version_, helper_->url_loader_factory_getter()->GetNetworkFactory(), version_, helper_->url_loader_factory_getter()->GetNetworkFactory(),
net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS)); net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS),
resource_id);
} }
// Returns false if the entry for |url| doesn't exist in the storage. // Returns false if the entry for |url| doesn't exist in the storage.
......
...@@ -141,12 +141,13 @@ void ServiceWorkerScriptLoaderFactory::CreateLoaderAndStart( ...@@ -141,12 +141,13 @@ void ServiceWorkerScriptLoaderFactory::CreateLoaderAndStart(
} }
// Case D.3: // Case D.3:
mojo::MakeSelfOwnedReceiver( // Assign a new resource ID for the script from network.
ServiceWorkerNewScriptLoader::CreateAndStart( int64_t new_resource_id = context_->storage()->NewResourceId();
routing_id, request_id, options, resource_request, std::move(client), // TODO(crbug.com/1046335): Make this async once NewResourceId() becomes
provider_host_->running_hosted_version(), // async.
loader_factory_for_new_scripts_, traffic_annotation), OnResourceIdAssignedForNewScriptLoader(
std::move(receiver)); std::move(receiver), routing_id, request_id, options, resource_request,
std::move(client), traffic_annotation, new_resource_id);
} }
void ServiceWorkerScriptLoaderFactory::Clone( void ServiceWorkerScriptLoaderFactory::Clone(
...@@ -259,4 +260,28 @@ void ServiceWorkerScriptLoaderFactory::OnCopyScriptFinished( ...@@ -259,4 +260,28 @@ void ServiceWorkerScriptLoaderFactory::OnCopyScriptFinished(
resource_request.url), resource_request.url),
std::move(receiver)); std::move(receiver));
} }
void ServiceWorkerScriptLoaderFactory::OnResourceIdAssignedForNewScriptLoader(
mojo::PendingReceiver<network::mojom::URLLoader> receiver,
int32_t routing_id,
int32_t request_id,
uint32_t options,
const network::ResourceRequest& resource_request,
mojo::PendingRemote<network::mojom::URLLoaderClient> client,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
int64_t resource_id) {
if (resource_id == ServiceWorkerConsts::kInvalidServiceWorkerResourceId) {
mojo::Remote<network::mojom::URLLoaderClient>(std::move(client))
->OnComplete(network::URLLoaderCompletionStatus(net::ERR_ABORTED));
return;
}
mojo::MakeSelfOwnedReceiver(
ServiceWorkerNewScriptLoader::CreateAndStart(
routing_id, request_id, options, resource_request, std::move(client),
provider_host_->running_hosted_version(),
loader_factory_for_new_scripts_, traffic_annotation, resource_id),
std::move(receiver));
}
} // namespace content } // namespace content
...@@ -98,6 +98,16 @@ class CONTENT_EXPORT ServiceWorkerScriptLoaderFactory ...@@ -98,6 +98,16 @@ class CONTENT_EXPORT ServiceWorkerScriptLoaderFactory
int64_t new_resource_id, int64_t new_resource_id,
net::Error error); net::Error error);
void OnResourceIdAssignedForNewScriptLoader(
mojo::PendingReceiver<network::mojom::URLLoader> receiver,
int32_t routing_id,
int32_t request_id,
uint32_t options,
const network::ResourceRequest& resource_request,
mojo::PendingRemote<network::mojom::URLLoaderClient> client,
const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
int64_t resource_id);
base::WeakPtr<ServiceWorkerContextCore> context_; base::WeakPtr<ServiceWorkerContextCore> context_;
base::WeakPtr<ServiceWorkerProviderHost> provider_host_; base::WeakPtr<ServiceWorkerProviderHost> provider_host_;
// Can be null if this factory is for an installed service worker. // Can be null if this factory is for an installed service worker.
......
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