Kouhei Ueno posted comments on this change.
Patch set 1:Commit-Queue +1
PTAL
To view, visit change 541097. To unsubscribe, visit settings.
Kouhei Ueno would like Yutaka Hirano, Hiroshige Hayashizaki, Hiroki Nakagawa and Kinuko Yasuda to review this change.
[ES6 modules] Update module script error state predicate and transition.
This CL updates the following spec concepts:
- Null record is now also treated as an error
- Renamed from module script's erorr.
- Implement "set its [[HostDefined]] field to undefined" step.
- Don't set script's state to errored.
This change is to be introduced in whatwg html spec PR:
https://github.com/whatwg/html/pull/2674
The actual implementation behavior change will be introduced in separate CLs.
Bug: 594639, 727299, https://github.com/whatwg/html/pull/2674
Change-Id: Ie9734344030f99bcdaa3d595f8dbec87d44a7c92
---
M third_party/WebKit/Source/bindings/core/v8/ScriptModuleTest.cpp
M third_party/WebKit/Source/core/dom/ModulatorImpl.cpp
M third_party/WebKit/Source/core/dom/ModuleMapTest.cpp
M third_party/WebKit/Source/core/dom/ModulePendingScript.cpp
M third_party/WebKit/Source/core/dom/ModuleScript.cpp
M third_party/WebKit/Source/core/dom/ModuleScript.h
M third_party/WebKit/Source/core/dom/ScriptModuleResolver.h
M third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.cpp
M third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.h
M third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoaderTest.cpp
M third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp
M third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp
M third_party/WebKit/Source/core/testing/DummyModulator.cpp
13 files changed, 75 insertions(+), 52 deletions(-)
Kouhei Ueno uploaded patch set #3 to this change.
[ES6 modules] Update module script error state predicate and transition.
This CL updates the following spec concepts:
"#concept-module-script-is-errored"
- Null record is now also treated as an error
"#concept-module-script-pre-instantiation-error"
- Renamed from module script's error.
"#concept-module-script-module-record"
- Implement "set its [[HostDefined]] field to undefined" step.
"#concept-module-script-is-errored"
- Don't set script's state to errored.
This change is to be introduced in whatwg html spec PR:
https://github.com/whatwg/html/pull/2674
The actual implementation behavior change will be introduced in separate CLs.
Bug: 594639, 727299, https://github.com/whatwg/html/pull/2674
Change-Id: Ie9734344030f99bcdaa3d595f8dbec87d44a7c92
---
M third_party/WebKit/Source/bindings/core/v8/ScriptModuleTest.cpp
M third_party/WebKit/Source/core/dom/ModulatorImpl.cpp
M third_party/WebKit/Source/core/dom/ModuleMapTest.cpp
M third_party/WebKit/Source/core/dom/ModulePendingScript.cpp
M third_party/WebKit/Source/core/dom/ModuleScript.cpp
M third_party/WebKit/Source/core/dom/ModuleScript.h
M third_party/WebKit/Source/core/dom/ScriptModuleResolver.h
M third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.cpp
M third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.h
M third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoaderTest.cpp
M third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp
M third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp
M third_party/WebKit/Source/core/testing/DummyModulator.cpp
13 files changed, 75 insertions(+), 52 deletions(-)
To view, visit change 541097. To unsubscribe, visit settings.
Kouhei Ueno posted comments on this change.
Patch set 3:Commit-Queue +1
Kouhei Ueno removed a vote from this change.
To view, visit change 541097. To unsubscribe, visit settings.
Hiroki Nakagawa posted comments on this change.
Patch set 3:
LGTM
(4 comments)
File third_party/WebKit/Source/core/dom/ModuleMapTest.cpp:
Patch Set #3, Line 60: void UnregisterModuleScript(ModuleScript*) override {}
Is it ok not to test the call count like RegisterModuleScript?
File third_party/WebKit/Source/core/dom/ModuleScript.h:
Patch Set #3, Line 121: be also be
s/be also be/also be/
File third_party/WebKit/Source/core/dom/ScriptModuleResolver.h:
Patch Set #3, Line 36: Notify
s/Notify/Notifies/
File third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.h:
Patch Set #3, Line 45: UnregisterModuleScript
Can you add a blank line after this line?
To view, visit change 541097. To unsubscribe, visit settings.
Yutaka Hirano posted comments on this change.
Patch set 3:Code-Review +1
(1 comment)
Patch Set #3, Line 16: "#concept-module-script-is-errored"
Is this the same as the first one?
To view, visit change 541097. To unsubscribe, visit settings.
Kinuko Yasuda posted comments on this change.
Patch set 3:
(1 comment)
File third_party/WebKit/Source/core/dom/ModuleScript.h:
Patch Set #3, Line 66: return record_.IsEmpty() || state_ == ModuleInstantiationState::kErrored;
Who could set the state_ to kErrored after this change?
To view, visit change 541097. To unsubscribe, visit settings.
Kouhei Ueno uploaded patch set #4 to this change.
[ES6 modules] Update module script error state predicate and transition.
This CL updates the following spec concepts:
"#concept-module-script-is-errored"
- Null record is now also treated as an error
"#concept-module-script-pre-instantiation-error"
- Renamed from module script's error.
"#concept-module-script-module-record"
- Implement "set its [[HostDefined]] field to undefined" step.
"#concept-module-script-set-pre-instantiation-error"
- Don't set script's state to errored.
This change is to be introduced in whatwg html spec PR:
https://github.com/whatwg/html/pull/2674
The actual implementation behavior change will be introduced in separate CLs.
Bug: 594639, 727299, https://github.com/whatwg/html/pull/2674
Change-Id: Ie9734344030f99bcdaa3d595f8dbec87d44a7c92
---
M third_party/WebKit/Source/bindings/core/v8/ScriptModuleTest.cpp
M third_party/WebKit/Source/core/dom/ModulatorImpl.cpp
M third_party/WebKit/Source/core/dom/ModuleMapTest.cpp
M third_party/WebKit/Source/core/dom/ModulePendingScript.cpp
M third_party/WebKit/Source/core/dom/ModuleScript.cpp
M third_party/WebKit/Source/core/dom/ModuleScript.h
M third_party/WebKit/Source/core/dom/ScriptModuleResolver.h
M third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.cpp
M third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.h
M third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoaderTest.cpp
M third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp
M third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp
M third_party/WebKit/Source/core/testing/DummyModulator.cpp
13 files changed, 75 insertions(+), 52 deletions(-)
To view, visit change 541097. To unsubscribe, visit settings.
Kouhei Ueno posted comments on this change.
Patch set 5:Commit-Queue +1
Kouhei Ueno posted comments on this change.
Patch set 6:
rebased PTAL
(6 comments)
Patch Set #3, Line 16: "#concept-module-script-set-pre-instantiation-error"
Is this the same as the first one?
Done
File third_party/WebKit/Source/core/dom/ModuleMapTest.cpp:
Patch Set #3, Line 60: FAIL() << "UnregisterModuleScript shouldn't be called in ModuleMapTest";
Is it ok not to test the call count like RegisterModuleScript?
Who could set the state_ to kErrored after this change?
No one. After V8 change, state_ will be maintained by V8. I'll re-introduce the state_ check once its in.
Patch Set #3, Line 121: ) const {
s/be also be/also be/
Done
File third_party/WebKit/Source/core/dom/ScriptModuleResolver.h:
Patch Set #3, Line 36: Notifi
s/Notify/Notifies/
Done
File third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.h:
Patch Set #3, Line 45: UnregisterModuleScript
Can you add a blank line after this line?
Done
To view, visit change 541097. To unsubscribe, visit settings.
Kouhei Ueno posted comments on this change.
Patch set 6:Commit-Queue +2
Commit Bot posted comments on this change.
Patch set 6:
CQ is trying da patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"rebased" https://chromium-review.googlesource.com/c/541097/6
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/541097/6
Bot data: {"action": "start", "triggered_at": "2017-06-26T08:08:10.0Z", "cq_cfg_revision": "e12d437dc7f395d72995b548c9dacf21b0b1526e", "revision": "1d1e7e7e620b3aa6461a661fefa6019817dd4d3b"}
Commit Bot merged this change.
[ES6 modules] Update module script error state predicate and transition.
This CL updates the following spec concepts:
"#concept-module-script-is-errored"
- Null record is now also treated as an error
"#concept-module-script-pre-instantiation-error"
- Renamed from module script's error.
"#concept-module-script-module-record"
- Implement "set its [[HostDefined]] field to undefined" step.
"#concept-module-script-set-pre-instantiation-error"
- Don't set script's state to errored.
This change is to be introduced in whatwg html spec PR:
https://github.com/whatwg/html/pull/2674
The actual implementation behavior change will be introduced in separate CLs.
Bug: 594639, 727299, https://github.com/whatwg/html/pull/2674
Change-Id: Ie9734344030f99bcdaa3d595f8dbec87d44a7c92
Reviewed-on: https://chromium-review.googlesource.com/541097
Commit-Queue: Kouhei Ueno <kou...@chromium.org>
Reviewed-by: Yutaka Hirano <yhi...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482213}
---
M third_party/WebKit/Source/bindings/core/v8/ScriptModuleTest.cpp
M third_party/WebKit/Source/core/dom/ModulatorImpl.cpp
M third_party/WebKit/Source/core/dom/ModuleMapTest.cpp
M third_party/WebKit/Source/core/dom/ModulePendingScript.cpp
M third_party/WebKit/Source/core/dom/ModuleScript.cpp
M third_party/WebKit/Source/core/dom/ModuleScript.h
M third_party/WebKit/Source/core/dom/ScriptModuleResolver.h
M third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.cpp
M third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.h
M third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoaderTest.cpp
M third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp
M third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp
M third_party/WebKit/Source/core/testing/DummyModulator.cpp
13 files changed, 73 insertions(+), 49 deletions(-)