New issue
Advanced search Search tips

Issue 726576 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 567358
issue 738772



Sign in to add a comment

Worklet: Implement "module responses map" concept

Project Member Reported by nhiroki@chromium.org, May 26 2017

Issue description

Cc: ikilpatrick@chromium.org kouhei@chromium.org
Blocking: 738772
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 12 2017

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

commit e56293066201e2f371cbf9e91eb79292d4a32748
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Wed Jul 12 04:23:10 2017

Worklet: Implement the "module responses map" concept

This CL implements the "module responses map" concept defined in the Worklet
spec. This map caches module scripts to ensure that WorkletGlobalScopes created
at different times contain the same set of script source text and have the same
behaviour.

The map is not used yet in this CL. Following CLs will wire it up with
ModuleScriptLoader via WorkletModuleResponsesMapProxy (see the design doc for
details).

Worklet spec: https://drafts.css-houdini.org/worklets/#module-responses-map
Design doc: https://docs.google.com/document/d/1cgLcrua7H_7x_o5GlzYrAi2qt-TqTzgtOeixFAugR6g/edit?usp=sharing

Bug:  726576 
Change-Id: Ib2af974e4b48eaf23ed2a38c74da5f2a503568d9
Reviewed-on: https://chromium-review.googlesource.com/549601
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485859}
[modify] https://crrev.com/e56293066201e2f371cbf9e91eb79292d4a32748/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/e56293066201e2f371cbf9e91eb79292d4a32748/third_party/WebKit/Source/core/loader/BUILD.gn
[add] https://crrev.com/e56293066201e2f371cbf9e91eb79292d4a32748/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptCreationParams.h
[modify] https://crrev.com/e56293066201e2f371cbf9e91eb79292d4a32748/third_party/WebKit/Source/core/workers/BUILD.gn
[add] https://crrev.com/e56293066201e2f371cbf9e91eb79292d4a32748/third_party/WebKit/Source/core/workers/WorkletModuleResponsesMap.cpp
[add] https://crrev.com/e56293066201e2f371cbf9e91eb79292d4a32748/third_party/WebKit/Source/core/workers/WorkletModuleResponsesMap.h
[add] https://crrev.com/e56293066201e2f371cbf9e91eb79292d4a32748/third_party/WebKit/Source/core/workers/WorkletModuleResponsesMapTest.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 24 2017

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

commit ecfe9a967a991bc86142f78803515da4ca766a5b
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Mon Jul 24 06:06:30 2017

Worklet: Fix TaskRunnerHelper::Get() for MainThreadWorkletGlobalScope

Before this CL, TaskRunnerHelper::Get() with MainThreadWorkletGlobalScope hits
an assertion failure (NOTREACHED) in MainThreadWorkletGlobalScope::GetThread().
This is because MainThreadWorkletGlobalScope lives on the main thread and a
WorkerThread instance doesn't exist for that.

After this CL, TaskRunnerHelper::Get() with MainThreadWorkletGlobalScope
retrieves a task runner from an associated frame.

Bug:  726576 
Change-Id: Iabf0ee81b6998f2173615cadbed53729a85b9a93
Reviewed-on: https://chromium-review.googlesource.com/582055
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488922}
[modify] https://crrev.com/ecfe9a967a991bc86142f78803515da4ca766a5b/third_party/WebKit/Source/core/dom/TaskRunnerHelper.cpp
[modify] https://crrev.com/ecfe9a967a991bc86142f78803515da4ca766a5b/third_party/WebKit/Source/core/workers/DedicatedWorkerTest.cpp
[modify] https://crrev.com/ecfe9a967a991bc86142f78803515da4ca766a5b/third_party/WebKit/Source/core/workers/MainThreadWorkletTest.cpp
[modify] https://crrev.com/ecfe9a967a991bc86142f78803515da4ca766a5b/third_party/WebKit/Source/core/workers/ThreadedWorkletTest.cpp

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 25 2017

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

commit e31e407d3469cb1d6f6371d9704ecdf2df5c9471
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Tue Jul 25 03:05:56 2017

Worklet: Introduce WorkletModuleResponsesMapProxy

This CL introduces WorkletModuleResponsesMapProxy that mediates
WorkletModuleResponsesMap on the main thread and
WorkletModuleResponsesMap::Client implementation on the worklet context thread.

The proxy and map are not used yet in this CL. Following CLs will implement
module script loader and fetcher for worklets that use them, and add unittests.

Worklet spec: https://drafts.css-houdini.org/worklets/#module-responses-map
Design doc: https://docs.google.com/document/d/1cgLcrua7H_7x_o5GlzYrAi2qt-TqTzgtOeixFAugR6g/edit?usp=sharing

TBR=ikilpatrick@chromium.org

Bug:  726576 
Change-Id: Iece7ecb002aea6ac50b2825a8acdd8347a2c3543
Reviewed-on: https://chromium-review.googlesource.com/577331
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@{#489201}
[modify] https://crrev.com/e31e407d3469cb1d6f6371d9704ecdf2df5c9471/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptCreationParams.h
[modify] https://crrev.com/e31e407d3469cb1d6f6371d9704ecdf2df5c9471/third_party/WebKit/Source/core/workers/BUILD.gn
[modify] https://crrev.com/e31e407d3469cb1d6f6371d9704ecdf2df5c9471/third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.cpp
[modify] https://crrev.com/e31e407d3469cb1d6f6371d9704ecdf2df5c9471/third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.h
[modify] https://crrev.com/e31e407d3469cb1d6f6371d9704ecdf2df5c9471/third_party/WebKit/Source/core/workers/ThreadedWorkletMessagingProxy.cpp
[modify] https://crrev.com/e31e407d3469cb1d6f6371d9704ecdf2df5c9471/third_party/WebKit/Source/core/workers/ThreadedWorkletMessagingProxy.h
[modify] https://crrev.com/e31e407d3469cb1d6f6371d9704ecdf2df5c9471/third_party/WebKit/Source/core/workers/Worklet.cpp
[modify] https://crrev.com/e31e407d3469cb1d6f6371d9704ecdf2df5c9471/third_party/WebKit/Source/core/workers/Worklet.h
[modify] https://crrev.com/e31e407d3469cb1d6f6371d9704ecdf2df5c9471/third_party/WebKit/Source/core/workers/WorkletGlobalScopeProxy.h
[add] https://crrev.com/e31e407d3469cb1d6f6371d9704ecdf2df5c9471/third_party/WebKit/Source/core/workers/WorkletModuleResponsesMapProxy.cpp
[add] https://crrev.com/e31e407d3469cb1d6f6371d9704ecdf2df5c9471/third_party/WebKit/Source/core/workers/WorkletModuleResponsesMapProxy.h
[modify] https://crrev.com/e31e407d3469cb1d6f6371d9704ecdf2df5c9471/third_party/WebKit/Source/modules/csspaint/PaintWorkletGlobalScopeProxy.cpp
[modify] https://crrev.com/e31e407d3469cb1d6f6371d9704ecdf2df5c9471/third_party/WebKit/Source/modules/csspaint/PaintWorkletGlobalScopeProxy.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 28 2017

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

commit fde3987799fa1d4901a3663b9f78568acd96a46e
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Fri Jul 28 03:41:12 2017

Worklet: Make WorkletModuleResponsesMap reject invalid URLs

Bug:  726576 
Change-Id: I43396f1b657eac689d5f753ca3916ae9007eb8bc
Reviewed-on: https://chromium-review.googlesource.com/587697
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490245}
[modify] https://crrev.com/fde3987799fa1d4901a3663b9f78568acd96a46e/third_party/WebKit/Source/core/workers/WorkletModuleResponsesMap.cpp
[modify] https://crrev.com/fde3987799fa1d4901a3663b9f78568acd96a46e/third_party/WebKit/Source/core/workers/WorkletModuleResponsesMapTest.cpp

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 28 2017

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

commit acec16de38e53fe3d63c8db66a34565e1828188f
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Fri Jul 28 13:05:00 2017

Worklet: Implement the custom module fetch algorithm for worklets

Worklet basically depends on ES6 modules loading, but it has a bit different
fetch sequence[1] to ensure all worklet global scopes see the same module
scripts.

First, the module fetcher queries the module responses map[2] about whether a
module script in question has already been cached. If the module script is in
the map, the fetcher retrieves it and instantiates modules from that. If the
module script isn't in the map, the fetcher fetches the module script via the
default module fetch path and then caches it in the map.

To implement this custom fetch, this CL separates module fetch logic in
ModuleScriptLoader into ModuleScriptFetcher. ModuleScriptFetcher provides
virtual functions and WorkletModuleScriptFetcher overrides them to query
WorkletModuleResponsesMap.

Random notes:
- This changes FetchParameters from STACK_ALLOCATED / WTF_MAKE_NONCOPYABLE to
  DISALLOW_NEW because FetchParameters needs to be kept as a member of
  WorkletModuleScriptFetcher during accessing WorkletModuleResponsesMap.
- Accessing WorkletModuleResponsesMap involves thread hops. This is achieved by
  WorkletModuleResponsesMapProxy introduced by previous CLs.
  WorkletModuleScriptFetcher inherits WorkletModuleResponsesMap::Client and
  queries the map via the proxy.
- This dedups console errors in worklet-import-blocked-expected.txt. Before this
  change, module fetch is issued twice because 2 PaintWorkletGlobalScopes are
  created by nature and the CSP check is also conducted twice. After this
  change, module fetch is issued only one time for the module responses map.
- [Tests] ModuleScriptFetcher is covered by existing tests in
  ModuleScriptLoaderTest.
- [Tests] WorkletModuleScriptFetcher is covered by existing worklet layout tests
  (http/tests/worklet/) and newly added tests in ModuleScriptLoaderTest.

Design doc: https://docs.google.com/document/d/1cgLcrua7H_7x_o5GlzYrAi2qt-TqTzgtOeixFAugR6g/edit?usp=sharing

[1] https://drafts.css-houdini.org/worklets/#fetch-a-worklet-script
[2] https://drafts.css-houdini.org/worklets/#module-responses-map

Bug:  726576 
Change-Id: I87be1a7d9054cb6d0324dafb24c9a47fc9181476
Reviewed-on: https://chromium-review.googlesource.com/583968
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490373}
[modify] https://crrev.com/acec16de38e53fe3d63c8db66a34565e1828188f/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/worklet-import-blocked-expected.txt
[modify] https://crrev.com/acec16de38e53fe3d63c8db66a34565e1828188f/third_party/WebKit/Source/core/loader/BUILD.gn
[add] https://crrev.com/acec16de38e53fe3d63c8db66a34565e1828188f/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptFetcher.cpp
[add] https://crrev.com/acec16de38e53fe3d63c8db66a34565e1828188f/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptFetcher.h
[modify] https://crrev.com/acec16de38e53fe3d63c8db66a34565e1828188f/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoader.cpp
[modify] https://crrev.com/acec16de38e53fe3d63c8db66a34565e1828188f/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoader.h
[modify] https://crrev.com/acec16de38e53fe3d63c8db66a34565e1828188f/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoaderTest.cpp
[add] https://crrev.com/acec16de38e53fe3d63c8db66a34565e1828188f/third_party/WebKit/Source/core/loader/modulescript/WorkletModuleScriptFetcher.cpp
[add] https://crrev.com/acec16de38e53fe3d63c8db66a34565e1828188f/third_party/WebKit/Source/core/loader/modulescript/WorkletModuleScriptFetcher.h
[modify] https://crrev.com/acec16de38e53fe3d63c8db66a34565e1828188f/third_party/WebKit/Source/core/workers/WorkletGlobalScope.cpp
[modify] https://crrev.com/acec16de38e53fe3d63c8db66a34565e1828188f/third_party/WebKit/Source/core/workers/WorkletGlobalScope.h
[modify] https://crrev.com/acec16de38e53fe3d63c8db66a34565e1828188f/third_party/WebKit/Source/core/workers/WorkletModuleResponsesMap.h
[modify] https://crrev.com/acec16de38e53fe3d63c8db66a34565e1828188f/third_party/WebKit/Source/platform/loader/fetch/FetchParameters.h

Cc: xidac...@chromium.org
Labels: -M-61 M-62
Status: Fixed (was: Started)
Fixed. Now global scopes associated with the same document see the same module scripts.

+xidachen@ fyi

Sign in to add a comment