New issue
Advanced search Search tips

Issue 842566 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 680046



Sign in to add a comment

Specify an appropriate request context for module script loading for dedicated workers

Project Member Reported by nhiroki@chromium.org, May 14 2018

Issue description

Currently module scripts are always loaded as WebURLRequest::kRequestContextScript[1]. See ScriptResource::Fetch() called from DocumentModuleScriptFetcher::Fetch():
https://chromium.googlesource.com/chromium/src/+/7af3bc79c29cbd9b3360154959856f451a7b25f4/third_party/blink/renderer/core/loader/resource/script_resource.cc#149

However, for a top-level module script of workers, it should be loaded as kRequestContext(Shared/Service)Worker:
https://fetch.spec.whatwg.org/#concept-request-destination

For descendant module scripts, I think WebURLRequest::kRequestContextScript is correct. I sent a PR to the Fetch spec for clarifying it:
https://github.com/whatwg/fetch/pull/724
 
This could also be useful for overriding a referrer for the top-level module script (see 842553)
(see  issue 842553 )
> For descendant module scripts, I think WebURLRequest::kRequestContextScript is correct. I sent a PR to the Fetch spec for clarifying it:
> https://github.com/whatwg/fetch/pull/724

In the PR, annevk pointed out "script" is specified in the spec of import():
https://html.spec.whatwg.org/#hostimportmoduledynamically(referencingscriptormodule,-specifier,-promisecapability)
> However, for a top-level module script of workers, it should be loaded as kRequestContext(Shared/Service)Worker:
> https://fetch.spec.whatwg.org/#concept-request-destination

"12. Let destination be "sharedworker" if is shared is true, and "worker" otherwise."
https://html.spec.whatwg.org/multipage/workers.html#worker-processing-model
I read though the spec more, and it looks like static import() on workers is also handled as worker's request context.

Summary:
- Top-level worker script and static import => worker's context
- Dynamic import => script's context

Our implementation correctly works for the case of dynamic import, but not for the case of top-level worker script and static import.
Owner: nhiroki@chromium.org
Status: Started (was: Available)
kouhei@, hiroshige@: Is there any way to know whether the current module load is initiated from dynamic import around ModuleScriptLoader?
OK, I understand how to do this. The spec defines that the "request context" is passed through module components as "destination". I'm now making a patch:
https://chromium-review.googlesource.com/c/chromium/src/+/1058734
Blocking: 680046
Project Member

Comment 10 by bugdroid1@chromium.org, May 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/72fbe934d9fd3d62dc5892d9b00768a72eece63f

commit 72fbe934d9fd3d62dc5892d9b00768a72eece63f
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Thu May 17 11:32:39 2018

ES Modules: Specify "destination" for module loading

This CL passes the "destination"[1] parameter through module loaders. This enables
workers/worklets to specify a unique destination defined in each spec.

Before this CL, any module scripts are loaded with the "script" destination.
After this CL, top-level and descendant module scripts for dedicated workers are
loaded with the "worker" destination, top-level and descendant module scripts
for documents and dynamic import() are still loaded with the "script"
destination though. Module scripts for worklets also must be loaded with their
own destination defined in each worklet spec (e.g., "paintworklet"), but such
destinations haven't been added yet and the "script" destination is still used.
Shared workers and service workers haven't supported module loading yet.
Subsequent CLs will address them.

Regarding detailed changes, this CL adds the "destination" parameters to
components for module loading, stops ScriptResource overriding the
"destination", and uses the given "destination" instead. Note that, in Chrome,
WebURLRequest::RequestContext enum[2] represents the "destination".

[1] https://fetch.spec.whatwg.org/#concept-request-destination
[2] https://chromium.googlesource.com/chromium/src/+/a204023d46bc02634f1c6f164cae99d327d53b86/third_party/blink/public/platform/web_url_request.h#73

Bug:  842566 
Change-Id: Ib777fcc28de62e1c348ef5c79f97079e5399abaf
Reviewed-on: https://chromium-review.googlesource.com/1058734
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559493}
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/loader/document_loader.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/loader/link_loader.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/loader/modulescript/module_script_fetch_request.h
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/loader/modulescript/module_script_loader.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/loader/modulescript/module_tree_linker.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/loader/modulescript/module_tree_linker.h
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/loader/modulescript/module_tree_linker_registry.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/loader/modulescript/module_tree_linker_registry.h
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/loader/modulescript/module_tree_linker_test.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/loader/resource/script_resource.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/script/dynamic_module_resolver.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/script/dynamic_module_resolver_test.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/script/modulator.h
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/script/modulator_impl_base.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/script/modulator_impl_base.h
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/testing/dummy_modulator.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/testing/dummy_modulator.h
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/workers/dedicated_worker_global_scope.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/workers/dedicated_worker_global_scope.h
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/workers/shared_worker_global_scope.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/workers/shared_worker_global_scope.h
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/workers/worker_global_scope.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/workers/worker_global_scope.h
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/workers/worker_or_worklet_global_scope.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/workers/worker_or_worklet_global_scope.h
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/workers/worker_thread_test_helper.h
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/workers/worklet_global_scope.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/core/workers/worklet_module_responses_map_test.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/modules/serviceworkers/service_worker_global_scope.cc
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/modules/serviceworkers/service_worker_global_scope.h
[modify] https://crrev.com/72fbe934d9fd3d62dc5892d9b00768a72eece63f/third_party/blink/renderer/platform/loader/fetch/script_fetch_options.cc

Labels: M-68
Status: Fixed (was: Started)
Summary: Specify an appropriate request context for module script loading for dedicated workers (was: Specify an appropriate request context for module script loading for workers)
I'll file separate issues for shared workers, service workers, and worklets.

Sign in to add a comment