New issue
Advanced search Search tips

Issue 773778 link

Starred by 0 users
Status: Fixed
Owner:
nhiroki@chromium.org
Closed: Nov 2017
Cc:
ikilpatrick@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task

Blocking:
issue 567358



Sign in to add a comment

Worklet: Make sure that addModule() is intercepted by owner document's service worker

Project Member Reported by nhiroki@chromium.org, Oct 11 2017 
This is an issue for tracking the spec discussion [1].

According to the discussion as of [2],

  (1) addModule() is treated as owner document's sub-resource fetch and can be intercepted by owner document's service worker.
  (2) A worklet itself is not controlled by a service worker because it must have a unique origin ( issue 773772 )

Regarding (1), our current implementation should work like that because WorkletGlobalScope uses its owner document's resource fetcher but there are no tests. We should add WPT tests.

[1] https://github.com/whatwg/fetch/pull/527
[2] https://github.com/whatwg/fetch/pull/527#issuecomment-335896175

Comment 1 by nhiroki@chromium.org, Oct 11 2017

Blocking: 567358

Comment 2 by bke...@mozilla.com, Oct 11 2017 

> (2) A worklet itself is not controlled by a service worker because it must have a unique origin ( issue 773772 )

The title of this issue kind of says the opposite of that.

Comment 3 by nhiroki@chromium.org, Oct 11 2017

Summary: Worklet: Make sure that addModule() is intercepted by owner document's service worker (was: Worklet: Make sure that a worklet is controlled by owner document's service worker)
Oh..., thank you for pointing it out. I mean to make sure that addModule() is intercepted by owner document's service worker.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 17 2017 

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

commit e5065d6ee169f0dac94536ae5ae85d2572726c5f
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Tue Oct 17 08:55:46 2017

Worklet: Make WorkletGlobalScope a unique opaque origin

This CL does 2 things:

(1) The Worklet spec defines WorkletGlobalScope must have a unique opaque
origin:

"3. Let origin be a unique opaque origin."
https://drafts.css-houdini.org/worklets/#script-settings-for-worklets

However, our current impl doesn't obey this requirement. PaintWorklet inherits
its owner document's SecurityOrigin. AnimationWorklet and AudioWorklet create a
SecurityOrigin based on their script URL. This CL replaces them with
SecurityOrigin::CreateUnique().

(2) Our current impl checks CORS etc based on ExecutionContext's SecurityOrigin
associated with Modulator. For Worklets, these are WorkletGlobalScope and
WorkletModulatorImpl. However, Worklets need to fetch their scripts as
sub-resources of the owner Document, so the security checks are conducted based
on the owner Document's SecurityOrigin. After changes for (1), SecurityOrigin is
a unique opaque origin and it fails a bunch of tests because of CORS check
failures. To fix this, WorkletModulatorImpl overrides GetSecurityOrigin() to
provide the owner Document's SecurityOrigin for module fetch.

Bug:  773772 ,  773778 
Change-Id: I451999ef09b943c480e907e6536ca8819f446d5b
Reviewed-on: https://chromium-review.googlesource.com/714499
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509328}
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/core/dom/WorkletModulatorImpl.cpp
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/core/dom/WorkletModulatorImpl.h
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoaderTest.cpp
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/core/workers/GlobalScopeCreationParams.cpp
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/core/workers/GlobalScopeCreationParams.h
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.cpp
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.h
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/core/workers/MainThreadWorkletTest.cpp
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/core/workers/ThreadedWorkletGlobalScope.cpp
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/core/workers/ThreadedWorkletGlobalScope.h
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/core/workers/ThreadedWorkletTest.cpp
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/core/workers/WorkerGlobalScope.cpp
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/core/workers/WorkletGlobalScope.cpp
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/core/workers/WorkletGlobalScope.h
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletGlobalScope.cpp
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletGlobalScope.h
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletThread.cpp
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/modules/csspaint/PaintWorkletGlobalScope.cpp
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/modules/csspaint/PaintWorkletGlobalScope.h
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/modules/csspaint/PaintWorkletGlobalScopeProxy.cpp
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScope.cpp
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScope.h
[modify] https://crrev.com/e5065d6ee169f0dac94536ae5ae85d2572726c5f/third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp

Comment 5 by nhiroki@chromium.org, Nov 6 2017

Status: Started (was: Assigned)

Comment 6 by nhiroki@chromium.org, Nov 6 2017 

I came across questions about dynamic import on WorkletGlobalScope and made a comment on the github issue:
https://github.com/whatwg/fetch/pull/527#issuecomment-342038491
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 7 2017 

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

commit b2d291658dd303429b61c707acc34f2bb8b07784
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Tue Nov 07 01:26:10 2017

Worklet: Add WPT tests for ServiceWorker interception on addModule()

WPT Tests for ServiceWorker interception on dynamic import will be added in a
following CL.

Bug:  773778 
Change-Id: I636b39f3e377adb36a664bac9cf2f50516a90ca0
Reviewed-on: https://chromium-review.googlesource.com/754422
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514332}
[add] https://crrev.com/b2d291658dd303429b61c707acc34f2bb8b07784/third_party/WebKit/LayoutTests/external/wpt/worklets/animation-worklet-service-worker-interception.https.html
[add] https://crrev.com/b2d291658dd303429b61c707acc34f2bb8b07784/third_party/WebKit/LayoutTests/external/wpt/worklets/paint-worklet-service-worker-interception.https.html
[add] https://crrev.com/b2d291658dd303429b61c707acc34f2bb8b07784/third_party/WebKit/LayoutTests/external/wpt/worklets/resources/addmodule-window.html
[add] https://crrev.com/b2d291658dd303429b61c707acc34f2bb8b07784/third_party/WebKit/LayoutTests/external/wpt/worklets/resources/service-worker-interception-tests.js
[add] https://crrev.com/b2d291658dd303429b61c707acc34f2bb8b07784/third_party/WebKit/LayoutTests/external/wpt/worklets/resources/service-worker.js

Comment 8 by nhiroki@chromium.org, Nov 8 2017 

Remaining work is to add tests for SW interception on static import. Dynamic import() seems to be disallowed on WorkletGlobalScope ( issue 782538 ).

Comment 9 by nhiroki@chromium.org, Nov 8 2017 

> Remaining work is to add tests for SW interception on static import.

CL is now under review: https://chromium-review.googlesource.com/c/chromium/src/+/758025/4

Comment 11 by nhiroki@chromium.org, Nov 8 2017

Labels: M-64
Status: Fixed (was: Started)
Fixed for addModule() and static import. I'll file a new issue if dynamic import() is allowed on WorkletGlobalScope and we need to test it.
►

Sign in to add a comment