[ES6 Module] Reflect Spec PR 2991 to make error handling deterministic #8527
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wpt-pr-bot
approved these changes
Dec 1, 2017
Build PASSEDStarted: 2017-12-19 23:04:08 Failing Jobs
View more information about this build on: |
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-698467
branch
3 times, most recently
from
e7d1146 to
8da0f9e
Compare
Spec update: whatwg/html#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 <hiroshige@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Cr-Commit-Position: refs/heads/master@{#525168}
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-698467
branch
from
8da0f9e to
d3ad756
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Spec update: whatwg/html#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:
And thus this CL updates the following accordingly:
The behavior changes are:
and made deterministic, as intended by the spec update.
with existing instantiation/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 hiroshige@chromium.org
Reviewed-by: Kouhei Ueno kouhei@chromium.org
Cr-Commit-Position: refs/heads/master@{#525168}