Issue 763597 in chromium: Update Blink-side module script implementation according to the spec PRs 2971/2991

[hirosh… via monorail]

Updates:
Cc: modul...@chromium.org hirosh...@chromium.org

Comment #12 on issue 763597 by hirosh...@chromium.org: Update Blink-side module script implementation according to the spec PRs 2971/2991
https://bugs.chromium.org/p/chromium/issues/detail?id=763597#c12

Issue 750024 has been merged into this issue.

--
You received this message because:
1. You were specifically CC'd on the issue

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

Reply to this email to add a comment.

[bugdro… via monorail]

Comment #13 on issue 763597 by bugd...@chromium.org: Update Blink-side module script implementation according to the spec PRs 2971/2991
https://bugs.chromium.org/p/chromium/issues/detail?id=763597#c13

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

commit 960460d86dcd0d20f9c382ec82924ce1a2e7940f
Author: Hiroshige Hayashizaki <hiro...@chromium.org>
Date: Tue Dec 05 17:13:11 2017

Fire load event for link modulepreload with non-fetch errors

According to the spec comment, load events (instead of error events)
should be fired for modules with non-fetch errors such as parse errors.

Bug: 740886, 763597
Change-Id: I6074b36631e1299b90d52bc01e42c5ddeb957b31
Reviewed-on: https://chromium-review.googlesource.com/802251
Reviewed-by: Kouhei Ueno <kou...@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksak...@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiro...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521726}
[modify] https://crrev.com/960460d86dcd0d20f9c382ec82924ce1a2e7940f/third_party/WebKit/LayoutTests/external/wpt/preload/modulepreload.html
[modify] https://crrev.com/960460d86dcd0d20f9c382ec82924ce1a2e7940f/third_party/WebKit/Source/core/loader/LinkLoader.cpp

[bugdro… via monorail]

Comment #14 on issue 763597 by bugd...@chromium.org: Update Blink-side module script implementation according to the spec PRs 2971/2991
https://bugs.chromium.org/p/chromium/issues/detail?id=763597#c14

The following revision refers to this bug:

https://chromium.googlesource.com/chromium/src.git/+/6be1eccf7c32b6c0e830798991e7457f83e8442b

commit 6be1eccf7c32b6c0e830798991e7457f83e8442b
Author: Hiroshige Hayashizaki <hiro...@chromium.org>
Date: Tue Dec 19 22:46:25 2017

[ES6 Module] Reflect Spec PR 2991 to make error handling deterministic

Spec update: https://github.com/whatwg/html/pull/2991
V8-side change: https://chromium-review.googlesource.com/763369

Data members for errors are restructured:

Removed module script's "is errored" #concept-module-script-is-errored
=> Removed ModuleScript::IsErrored().

Removed module script's "error" #concept-module-script-error
=> Removed Modulator::GetError(), ModuleScript::CreateError(), and
ModuleScript::CreateErrorInternal().

Removed "set the pre-instantiation error"
#concept-module-script-set-pre-instantiation-error
=> Removed ModuleScript::SetErrorAndClearRecord().

Introduced script's "error to rethrow" #concept-script-error-to-rethrow
=> Added ModuleScript::error_to_rethrow_ and its accessors.
Also renamed ModuleScript::preinstantiation_error_ to
parse_error_, and added its accessors,

HTML/Blink no longer checks modules' status:

Removed references to [[Status]] and [[ErrorCompletion]] field.
=> Removed Modulator::GetRecordStatus(), ModuleScript::RecordStatus(),
and calls to ScriptModule::ErrorCompletion().

Removed module script's "has instantiated"
#concept-module-script-has-instantiated
=> Removed ModuleScript::HasInstantiated().

A subsequent CL will do further cleanup:
https://chromium-review.googlesource.com/802465

Error handling algorithms in the HTML spec were updated:
- #creating-a-module-script
- #run-a-module-script
- [FFPE] #finding-the-first-parse-error
- [IMSGF] #internal-module-script-graph-fetching-procedure
- [FD] #fetch-the-descendants-of-a-module-script
- [FDaI] #fetch-the-descendants-of-and-instantiate-a-module-script
- #hostresolveimportedmodule(referencingscriptormodule,-specifier)

And thus this CL updates the following accordingly:
- ModuleScript::Create()
- ModulatorImplBase::ExecuteModule()
- ModuleTreeLinker.cpp
- ScriptModuleResolverImpl::Resolve()

The behavior changes are:
- User-facing: the error reported (to window.onerror etc.) is changed
and made deterministic, as intended by the spec update.
- V8-facing: this CL
- invokes module instantiation of a module graph
with existing instantiation/evaluation errors.
- invokes evaluation of a module graph with existing evaluation errors.
These cases already occur, but this CL does these intentionally.

Bug: 763597
Change-Id: Ia02bb484290c5c07ab27e9c966db19a6e8d2596f
Reviewed-on: https://chromium-review.googlesource.com/698467
Commit-Queue: Hiroshige Hayashizaki <hiro...@chromium.org>
Reviewed-by: Kouhei Ueno <kou...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525168}
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/choice-of-error-1.html
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/choice-of-error-1a.js
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/choice-of-error-1b.js
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/choice-of-error-2.html
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/choice-of-error-2a.js
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/choice-of-error-2b.js
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/choice-of-error-3.html
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/choice-of-error-3a.js
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/choice-of-error-3b.js
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/dynamic-imports-script-error.html
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/error-type-1.html
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/error-type-1.js
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/error-type-2.html
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/error-type-2.js
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/error-type-3.html
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/error-type-3.js
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/instantiation-error-1.html
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/instantiation-error-2.html
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/instantiation-error-3.html
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/instantiation-error-4.html
[delete] https://crrev.com/5cce023721315702e4e5d475859ef5951d391731/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/instantiation-error-5-expected.txt
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/instantiation-error-5.html
[add] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/throw2.js
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/bindings/core/v8/ScriptModuleTest.cpp
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/dom/DynamicModuleResolver.cpp
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/dom/DynamicModuleResolverTest.cpp
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/dom/Modulator.h
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/dom/ModulatorImplBase.cpp
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/dom/ModulatorImplBase.h
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/dom/ModulePendingScript.cpp
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/dom/ModuleScript.cpp
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/dom/ModuleScript.h
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.cpp
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/dom/ScriptModuleResolverImplTest.cpp
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoaderTest.cpp
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.h
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/testing/DummyModulator.cpp
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/testing/DummyModulator.h
[modify] https://crrev.com/6be1eccf7c32b6c0e830798991e7457f83e8442b/third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.cpp

[bugdro… via monorail]

Comment #15 on issue 763597 by bugd...@chromium.org: Update Blink-side module script implementation according to the spec PRs 2971/2991
https://bugs.chromium.org/p/chromium/issues/detail?id=763597#c15

The following revision refers to this bug:

https://chromium.googlesource.com/chromium/src.git/+/d0118cdf372f3cbc9eccfb024e3eb0d0287a3945

commit d0118cdf372f3cbc9eccfb024e3eb0d0287a3945
Author: Hiroshige Hayashizaki <hiro...@chromium.org>
Date: Wed Dec 20 02:39:23 2017

Remove ScriptModule::Status()/ErrorCompletion()

They should be no longer used in Blink, as the spec update
https://github.com/whatwg/html/pull/2991
removes the references them from the HTML spec.

Bug: 763597
Change-Id: I515b12a36a8e6f97b72ef6695a05da2f89ecfa8e
Reviewed-on: https://chromium-review.googlesource.com/802465
Commit-Queue: Hiroshige Hayashizaki <hiro...@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhi...@chromium.org>
Reviewed-by: Kouhei Ueno <kou...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525235}
[modify] https://crrev.com/d0118cdf372f3cbc9eccfb024e3eb0d0287a3945/third_party/WebKit/Source/bindings/core/v8/ScriptModule.cpp
[modify] https://crrev.com/d0118cdf372f3cbc9eccfb024e3eb0d0287a3945/third_party/WebKit/Source/bindings/core/v8/ScriptModule.h
[modify] https://crrev.com/d0118cdf372f3cbc9eccfb024e3eb0d0287a3945/third_party/WebKit/Source/bindings/core/v8/ScriptModuleTest.cpp
[modify] https://crrev.com/d0118cdf372f3cbc9eccfb024e3eb0d0287a3945/third_party/WebKit/Source/core/dom/ScriptModuleResolverImplTest.cpp

[bugdro… via monorail]

Comment #16 on issue 763597 by bugd...@chromium.org: Update Blink-side module script implementation according to the spec PRs 2971/2991
https://bugs.chromium.org/p/chromium/issues/detail?id=763597#c16

The following revision refers to this bug:

https://chromium.googlesource.com/chromium/src.git/+/493a404fb9a975817a8c8962225380219e82ae40

commit 493a404fb9a975817a8c8962225380219e82ae40
Author: Hiroshige Hayashizaki <hiro...@chromium.org>
Date: Tue Jan 02 05:17:34 2018

[ES6 Modules] Fix test expectations for Issue 763597

As both of Blink-side and v8-side changes have landed:
- Blink: https://chromium-review.googlesource.com/698467
- V8: https://chromium-review.googlesource.com/763369
(Auto-roll: https://chromium-review.googlesource.com/845221)

This CL re-enables the tests that were disabled for
the transitioning.

Bug: 763597, v8:1569
Change-Id: I27a407bac3ab8f60100ded0b7f842f9bf648620d
Reviewed-on: https://chromium-review.googlesource.com/843011
Reviewed-by: Kouhei Ueno <kou...@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiro...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526440}
[modify] https://crrev.com/493a404fb9a975817a8c8962225380219e82ae40/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/493a404fb9a975817a8c8962225380219e82ae40/third_party/WebKit/Source/bindings/core/v8/ScriptModuleTest.cpp

[hirosh… via monorail]

Updates:
Status: Fixed

Comment #17 on issue 763597 by hirosh...@chromium.org: Update Blink-side module script implementation according to the spec PRs 2971/2991
https://bugs.chromium.org/p/chromium/issues/detail?id=763597#c17

(No comment was entered for this change.)