New issue
Advanced search Search tips

Issue 788593 link

Starred by 3 users
Status: Fixed
Owner:
nhiroki@chromium.org
Closed: Dec 2017
Cc:
kouhei@chromium.org
nhiroki@chromium.org
gsat...@chromium.org
adamk@chromium.org
neis@chromium.org
hirosh...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug

Blocked on:
issue 680046


Sign in to add a comment

Reject dynamic import() on workers with informative message

Project Member Reported by dstockwell@google.com, Nov 27 2017 
Chrome Version       : 64.0.3269.3

What steps will reproduce the problem?
1. `new Worker('worker.js');`
2. script.js: `import('./anything.js');`

What is the expected result?
'./anything.js' is imported

What happens instead of that?
Uncaught (in promise) undefined worker.js:1

Comment 1 by adamk@chromium.org, Nov 27 2017

Cc: nhiroki@chromium.org kouhei@chromium.org adamk@chromium.org gsat...@chromium.org
Status: Available (was: Unconfirmed)
nhiroki, is this something you have background on, given your work on modules in worklets? Any ideas about what's missing for support for modules in workers?

Comment 2 by nhiroki@chromium.org, Nov 27 2017

Blockedon: 680046
Components: -Blink>JavaScript>Language Blink>HTML>Script
Owner: nhiroki@chromium.org
Status: Assigned (was: Available)
Modules in workers are not implemented yet (issue 680046). I'm now writing a design doc and going to implement it in 2018Q1-Q2.

Comment 3 by dstockwell@google.com, Nov 29 2017 

Could we provide a more useful error, or make import() not valid in workers in the meantime?

Comment 4 by gsat...@chromium.org, Nov 29 2017 

I can definitely see that the current behavior can be confusing; rejecting the promise with a more useful error seems like a good idea. nhiroki@, can you please look into this?

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

Labels: -Pri-3 Pri-2
Status: Started (was: Assigned)
Sure. I'll work on it.
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 1 2017 

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

commit e7ea91203b8c29c7327355f8289107026fb63418
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Fri Dec 01 08:01:11 2017

Worker: Reject dynamic import() on WorkerGlobalScope with informative message

This CL makes dynamic import() on (Dedicated/Shared/Service)WorkerGlobalScope
reject the promise with an informative message for developers.

Notes:
- ES6 Modules for workers are now being implemented, and dynamic import() will
  be available on WorkerGlobaScope once the implementation is completed.
- The promise was rejected even before this CL but its error message was not
  useful.

Bug: 680046,  788593 
Change-Id: I270ed9d1d5a9dedaff5dc777ccf78ed99cf887f2
Reviewed-on: https://chromium-review.googlesource.com/799536
Reviewed-by: Hayato Ito <hayato@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520882}
[add] https://crrev.com/e7ea91203b8c29c7327355f8289107026fb63418/third_party/WebKit/LayoutTests/fast/workers/resources/shared-worker-dynamic-import.js
[add] https://crrev.com/e7ea91203b8c29c7327355f8289107026fb63418/third_party/WebKit/LayoutTests/fast/workers/resources/worker-dynamic-import.js
[add] https://crrev.com/e7ea91203b8c29c7327355f8289107026fb63418/third_party/WebKit/LayoutTests/fast/workers/shared-worker-dynamic-import.html
[add] https://crrev.com/e7ea91203b8c29c7327355f8289107026fb63418/third_party/WebKit/LayoutTests/fast/workers/worker-dynamic-import.html
[add] https://crrev.com/e7ea91203b8c29c7327355f8289107026fb63418/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium/dynamic-import.html
[add] https://crrev.com/e7ea91203b8c29c7327355f8289107026fb63418/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium/resources/dynamic-import-worker.js
[modify] https://crrev.com/e7ea91203b8c29c7327355f8289107026fb63418/third_party/WebKit/Source/core/dom/BUILD.gn
[modify] https://crrev.com/e7ea91203b8c29c7327355f8289107026fb63418/third_party/WebKit/Source/core/dom/Modulator.cpp
[modify] https://crrev.com/e7ea91203b8c29c7327355f8289107026fb63418/third_party/WebKit/Source/core/dom/ModulatorImplBase.h
[add] https://crrev.com/e7ea91203b8c29c7327355f8289107026fb63418/third_party/WebKit/Source/core/dom/WorkerModulatorImpl.cpp
[add] https://crrev.com/e7ea91203b8c29c7327355f8289107026fb63418/third_party/WebKit/Source/core/dom/WorkerModulatorImpl.h
[modify] https://crrev.com/e7ea91203b8c29c7327355f8289107026fb63418/third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.cpp
[modify] https://crrev.com/e7ea91203b8c29c7327355f8289107026fb63418/third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.h
[modify] https://crrev.com/e7ea91203b8c29c7327355f8289107026fb63418/third_party/WebKit/Source/core/workers/WorkletGlobalScope.cpp
[modify] https://crrev.com/e7ea91203b8c29c7327355f8289107026fb63418/third_party/WebKit/Source/core/workers/WorkletGlobalScope.h

Comment 8 by nhiroki@chromium.org, Dec 4 2017

Labels: M-65
commit e7ea91203b8c29c7327355f8289107026fb63418 was:
  initially in 65.0.3283.0

Comment 9 by nhiroki@chromium.org, Dec 4 2017

Labels: Merge-Request-64
Status: Fixed (was: Started)
Summary: Reject dynamic import() on workers with informative message (was: dynamic import() from worker always rejects)

Comment 10 by nhiroki@chromium.org, Dec 4 2017 

^^^ I'd like to merge the change to M64. This change should be enabled as soon as possible to avoid making developers confused.
Project Member

Comment 11 by sheriffbot@chromium.org, Dec 5 2017

Labels: -Merge-Request-64 Hotlist-Merge-Approved Merge-Approved-64
Your change meets the bar and is auto-approved for M64. Please go ahead and merge the CL to branch 3282 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 6 2017

Labels: -merge-approved-64 merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/64bba80353f39bcb14adabfb75c60cca56b2758d

commit 64bba80353f39bcb14adabfb75c60cca56b2758d
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Wed Dec 06 01:36:47 2017

[Merge to M64] Worker: Reject dynamic import() on WorkerGlobalScope with informative message

This CL makes dynamic import() on (Dedicated/Shared/Service)WorkerGlobalScope
reject the promise with an informative message for developers.

Notes:
- ES6 Modules for workers are now being implemented, and dynamic import() will
  be available on WorkerGlobaScope once the implementation is completed.
- The promise was rejected even before this CL but its error message was not
  useful.

Bug: 680046,  788593 
Change-Id: I270ed9d1d5a9dedaff5dc777ccf78ed99cf887f2
Reviewed-on: https://chromium-review.googlesource.com/799536
Reviewed-by: Hayato Ito <hayato@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#520882}(cherry picked from commit e7ea91203b8c29c7327355f8289107026fb63418)
Reviewed-on: https://chromium-review.googlesource.com/809990
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#48}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[add] https://crrev.com/64bba80353f39bcb14adabfb75c60cca56b2758d/third_party/WebKit/LayoutTests/fast/workers/resources/shared-worker-dynamic-import.js
[add] https://crrev.com/64bba80353f39bcb14adabfb75c60cca56b2758d/third_party/WebKit/LayoutTests/fast/workers/resources/worker-dynamic-import.js
[add] https://crrev.com/64bba80353f39bcb14adabfb75c60cca56b2758d/third_party/WebKit/LayoutTests/fast/workers/shared-worker-dynamic-import.html
[add] https://crrev.com/64bba80353f39bcb14adabfb75c60cca56b2758d/third_party/WebKit/LayoutTests/fast/workers/worker-dynamic-import.html
[add] https://crrev.com/64bba80353f39bcb14adabfb75c60cca56b2758d/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium/dynamic-import.html
[add] https://crrev.com/64bba80353f39bcb14adabfb75c60cca56b2758d/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium/resources/dynamic-import-worker.js
[modify] https://crrev.com/64bba80353f39bcb14adabfb75c60cca56b2758d/third_party/WebKit/Source/core/dom/BUILD.gn
[modify] https://crrev.com/64bba80353f39bcb14adabfb75c60cca56b2758d/third_party/WebKit/Source/core/dom/Modulator.cpp
[modify] https://crrev.com/64bba80353f39bcb14adabfb75c60cca56b2758d/third_party/WebKit/Source/core/dom/ModulatorImplBase.h
[add] https://crrev.com/64bba80353f39bcb14adabfb75c60cca56b2758d/third_party/WebKit/Source/core/dom/WorkerModulatorImpl.cpp
[add] https://crrev.com/64bba80353f39bcb14adabfb75c60cca56b2758d/third_party/WebKit/Source/core/dom/WorkerModulatorImpl.h
[modify] https://crrev.com/64bba80353f39bcb14adabfb75c60cca56b2758d/third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.cpp
[modify] https://crrev.com/64bba80353f39bcb14adabfb75c60cca56b2758d/third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.h
[modify] https://crrev.com/64bba80353f39bcb14adabfb75c60cca56b2758d/third_party/WebKit/Source/core/workers/WorkletGlobalScope.cpp
[modify] https://crrev.com/64bba80353f39bcb14adabfb75c60cca56b2758d/third_party/WebKit/Source/core/workers/WorkletGlobalScope.h
►

Sign in to add a comment