Separate "force fulfill" operation from the CommitInstantiated #67
Conversation
This file contains hidden or 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
While the RequestInstantiate fulfills `entry.[[Instantiate]]` promise with the registry entry (as the returned value of the promise), CommitInstantiated step 4 will fulfill it with the module instance proactively. In this patch, we separate the "force fulfill" operation from the CommitInstantiated. Move this operation to the FulFillInstantiate. And here, we will fulfill the `entry.[[Instantiate]]` promise with the registry entry because the hander side requires it.
hubot
pushed a commit
to WebKit/WebKit-http
that referenced
this pull request
Aug 21, 2015
https://bugs.webkit.org/show_bug.cgi?id=147876 Reviewed by Saam Barati. Source/JavaScriptCore: This patch implements ES6 Module Loader part. The implementation is based on the latest draft[1, 2]. The naive implementation poses several problems. This patch attempts to solve the spec issues and proposes the fix[3, 4, 5]. We construct the JSC internal module loader based on the ES6 Promises. The chain of the promises represents the dependency graph of the modules and it automatically enables asynchronous module fetching. To leverage the Promises internally, we use the InternalPromise landed in r188681. The loader has several platform-dependent hooks. The platform can implement these hooks to provide the functionality missing in the module loaders, like "how to fetch the resources". The method table of the JSGlobalObject is extended to accept these hooks from the platform. This patch focus on the loading part. So we don't create the module environment and don't link the modules yet. To test the current module progress easily, we add the `-m` option to the JSC shell. When this option is specified, we load the given script as the module. And to use the module loading inside the JSC shell, we added the simple loader hook for fetching. It fetches the module content from the file system. And to use the ES6 Map in the Loader implementation, we added @get and @set methods to the Map. But it conflicts with the existing `getPrivateName` method. Rename it to `lookUpPrivateName`. [1]: https://whatwg.github.io/loader/ [2]: whatwg/loader@214c7a6 [3]: whatwg/loader#66 [4]: whatwg/loader#67 [5]: whatwg/loader#68 [6]: https://bugs.webkit.org/show_bug.cgi?id=148136 * CMakeLists.txt: * DerivedSources.make: * JavaScriptCore.vcxproj/JavaScriptCore.vcxproj: * JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters: * JavaScriptCore.xcodeproj/project.pbxproj: * builtins/BuiltinNames.h: (JSC::BuiltinNames::lookUpPrivateName): (JSC::BuiltinNames::lookUpPublicName): (JSC::BuiltinNames::getPrivateName): Deleted. (JSC::BuiltinNames::getPublicName): Deleted. * builtins/ModuleLoaderObject.js: Added. (setStateToMax): (newRegistryEntry): (forceFulfillPromise): (fulfillFetch): (fulfillTranslate): (fulfillInstantiate): (instantiation): (requestFetch): (requestTranslate): (requestInstantiate): (requestResolveDependencies.resolveDependenciesPromise.this.requestInstantiate.then.): (requestResolveDependencies.resolveDependenciesPromise.this.requestInstantiate.then): (requestResolveDependencies): (requestInstantiateAll): (provide): * jsc.cpp: (stringFromUTF): (jscSource): (GlobalObject::moduleLoaderFetch): (functionCheckModuleSyntax): (dumpException): (runWithScripts): (printUsageStatement): (CommandLine::parseArguments): (jscmain): (CommandLine::CommandLine): Deleted. * parser/Lexer.cpp: (JSC::Lexer<LChar>::parseIdentifier): (JSC::Lexer<UChar>::parseIdentifier): * parser/ModuleAnalyzer.cpp: (JSC::ModuleAnalyzer::ModuleAnalyzer): (JSC::ModuleAnalyzer::exportVariable): (JSC::ModuleAnalyzer::analyze): * parser/ModuleAnalyzer.h: (JSC::ModuleAnalyzer::moduleRecord): * parser/ModuleRecord.cpp: (JSC::printableName): Deleted. (JSC::ModuleRecord::dump): Deleted. * parser/ModuleRecord.h: (JSC::ModuleRecord::ImportEntry::isNamespace): Deleted. (JSC::ModuleRecord::create): Deleted. (JSC::ModuleRecord::appendRequestedModule): Deleted. (JSC::ModuleRecord::addImportEntry): Deleted. (JSC::ModuleRecord::addExportEntry): Deleted. (JSC::ModuleRecord::addStarExportEntry): Deleted. * parser/Nodes.h: * parser/NodesAnalyzeModule.cpp: (JSC::ImportDeclarationNode::analyzeModule): (JSC::ExportAllDeclarationNode::analyzeModule): (JSC::ExportNamedDeclarationNode::analyzeModule): * runtime/CommonIdentifiers.cpp: (JSC::CommonIdentifiers::lookUpPrivateName): (JSC::CommonIdentifiers::lookUpPublicName): (JSC::CommonIdentifiers::getPrivateName): Deleted. (JSC::CommonIdentifiers::getPublicName): Deleted. * runtime/CommonIdentifiers.h: * runtime/Completion.cpp: (JSC::checkModuleSyntax): (JSC::evaluateModule): * runtime/Completion.h: * runtime/ExceptionHelpers.cpp: (JSC::createUndefinedVariableError): * runtime/Identifier.h: * runtime/JSGlobalObject.cpp: (JSC::JSGlobalObject::init): (JSC::JSGlobalObject::visitChildren): * runtime/JSGlobalObject.h: (JSC::JSGlobalObject::moduleLoader): (JSC::JSGlobalObject::moduleRecordStructure): * runtime/JSModuleRecord.cpp: Renamed from Source/JavaScriptCore/parser/ModuleRecord.cpp. (JSC::JSModuleRecord::destroy): (JSC::JSModuleRecord::finishCreation): (JSC::printableName): (JSC::JSModuleRecord::dump): * runtime/JSModuleRecord.h: Renamed from Source/JavaScriptCore/parser/ModuleRecord.h. (JSC::JSModuleRecord::ImportEntry::isNamespace): (JSC::JSModuleRecord::createStructure): (JSC::JSModuleRecord::create): (JSC::JSModuleRecord::requestedModules): (JSC::JSModuleRecord::JSModuleRecord): (JSC::JSModuleRecord::appendRequestedModule): (JSC::JSModuleRecord::addImportEntry): (JSC::JSModuleRecord::addExportEntry): (JSC::JSModuleRecord::addStarExportEntry): * runtime/MapPrototype.cpp: (JSC::MapPrototype::finishCreation): * runtime/ModuleLoaderObject.cpp: Added. (JSC::ModuleLoaderObject::ModuleLoaderObject): (JSC::ModuleLoaderObject::finishCreation): (JSC::ModuleLoaderObject::getOwnPropertySlot): (JSC::printableModuleKey): (JSC::ModuleLoaderObject::provide): (JSC::ModuleLoaderObject::requestInstantiateAll): (JSC::ModuleLoaderObject::resolve): (JSC::ModuleLoaderObject::fetch): (JSC::ModuleLoaderObject::translate): (JSC::ModuleLoaderObject::instantiate): (JSC::moduleLoaderObjectParseModule): (JSC::moduleLoaderObjectRequestedModules): (JSC::moduleLoaderObjectResolve): (JSC::moduleLoaderObjectFetch): (JSC::moduleLoaderObjectTranslate): (JSC::moduleLoaderObjectInstantiate): * runtime/ModuleLoaderObject.h: Added. (JSC::ModuleLoaderObject::create): (JSC::ModuleLoaderObject::createStructure): * runtime/Options.h: Source/WebCore: Just fill Loader hooks with nullptr. * bindings/js/JSDOMWindowBase.cpp: * bindings/js/JSWorkerGlobalScopeBase.cpp: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@188752 268f45cc-cd09-0410-ab3c-d52691b4dbfc
| @@ -280,7 +281,6 @@ A <dfn>registry entry</dfn> is a record with the following fields: | |||
| 1. Let _instance_ be Instantiation(_loader_, _optionalInstance_, _source_). | |||
| 1. ReturnIfAbrupt(_instance_). | |||
| 1. // TODO: edge case: what if _instance_ is a thenable function? | |||
| 1. Fulfill _entry_.[[Instantiate]] with _instance_. | |||
There was a problem hiding this comment.
CommitInstantiated() is called from:
RequestInstantiate(), in which case the promise in[[Instantiate]]will be resolved implicitely, and there is not need to fulfilled manually.FulfillInstantiate(), in which case the promise should be fulfillled manually with the provided value.
This change seems to be correct.
|
Thanks for your reviews! I'll update the PR to reflect your comments. |
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.
While the RequestInstantiate fulfills
entry.[[Instantiate]]promisewith the registry entry (as the returned value of the promise),
CommitInstantiated step 4 will fulfill it with the module instance proactively.
In this patch, we separate the "force fulfill" operation from the CommitInstantiated.
Move this operation to the FulFillInstantiate. And here, we will fulfill the
entry.[[Instantiate]]promise with the registry entry because the hander side requires it.