[JSC] Drop Instantiate hook in ES6 module loader

https://bugs.webkit.org/show_bug.cgi?id=178162

Reviewed by Sam Weinig.

Source/JavaScriptCore:

This patch is a part of patch series for module loader refactoring to adopt
integrity="" parameters and introduce new whatwg module import mechanism.

In this patch, we drop instantiate hook in module loader. This hook is originally
introduced because it is defined in whatwg/loader spec. But this hook is not
used in our implementation, and this hook won't be used since (1) whatwg/loader
spec is abandoned, and (2) this type of hooks should be done in Service Workers.

In addition, this patch applies some cleaning up of our module loader JS code
to simplify things. This change paves the way to more efficient loader implementation
with great flexibility to adopt integrity="" parameters.

* builtins/ModuleLoaderPrototype.js:
(requestInstantiate):
(provideFetch):
provide is changed to provideFetch since we only used this function with Fetch stage parameter.

(fulfillInstantiate): Deleted.
(commitInstantiated): Deleted.
(instantiation): Deleted.
They are merged into requestInstantiate code. This is simpler.

(provide): Deleted.
* jsc.cpp:
* runtime/Completion.cpp:
(JSC::loadAndEvaluateModule):
(JSC::loadModule):
* runtime/JSGlobalObject.cpp:
* runtime/JSGlobalObject.h:
* runtime/JSModuleLoader.cpp:
(JSC::JSModuleLoader::provideFetch):
(JSC::JSModuleLoader::provide): Deleted.
Changed to provideFetch.

(JSC::JSModuleLoader::instantiate): Deleted.
Drop this hook.

* runtime/JSModuleLoader.h:
* runtime/ModuleLoaderPrototype.cpp:
(JSC::moduleLoaderPrototypeInstantiate): Deleted.
Drop this hook.

Source/WebCore:

Drop instantiate hooks.
No behavior change.

* bindings/js/JSDOMWindowBase.cpp:
* bindings/js/JSWorkerGlobalScopeBase.cpp:

Canonical link: https://commits.webkit.org/194413@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223173 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,52 @@
+2017-10-11  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Drop Instantiate hook in ES6 module loader
+        https://bugs.webkit.org/show_bug.cgi?id=178162
+
+        Reviewed by Sam Weinig.
+
+        This patch is a part of patch series for module loader refactoring to adopt
+        integrity="" parameters and introduce new whatwg module import mechanism.
+
+        In this patch, we drop instantiate hook in module loader. This hook is originally
+        introduced because it is defined in whatwg/loader spec. But this hook is not
+        used in our implementation, and this hook won't be used since (1) whatwg/loader
+        spec is abandoned, and (2) this type of hooks should be done in Service Workers.
+
+        In addition, this patch applies some cleaning up of our module loader JS code
+        to simplify things. This change paves the way to more efficient loader implementation
+        with great flexibility to adopt integrity="" parameters.
+
+        * builtins/ModuleLoaderPrototype.js:
+        (requestInstantiate):
+        (provideFetch):
+        provide is changed to provideFetch since we only used this function with Fetch stage parameter.
+
+        (fulfillInstantiate): Deleted.
+        (commitInstantiated): Deleted.
+        (instantiation): Deleted.
+        They are merged into requestInstantiate code. This is simpler.
+
+        (provide): Deleted.
+        * jsc.cpp:
+        * runtime/Completion.cpp:
+        (JSC::loadAndEvaluateModule):
+        (JSC::loadModule):
+        * runtime/JSGlobalObject.cpp:
+        * runtime/JSGlobalObject.h:
+        * runtime/JSModuleLoader.cpp:
+        (JSC::JSModuleLoader::provideFetch):
+        (JSC::JSModuleLoader::provide): Deleted.
+        Changed to provideFetch.
+
+        (JSC::JSModuleLoader::instantiate): Deleted.
+        Drop this hook.
+
+        * runtime/JSModuleLoader.h:
+        * runtime/ModuleLoaderPrototype.cpp:
+        (JSC::moduleLoaderPrototypeInstantiate): Deleted.
+        Drop this hook.
+
 2017-10-10  Saam Barati  <sbarati@apple.com>
 
         Prototype structure transition should be a deferred transition

Source/JavaScriptCore/builtins/ModuleLoaderPrototype.js

@@ -30,7 +30,6 @@
 // Currently, there are 4 hooks.
 //    1. Loader.resolve
 //    2. Loader.fetch
-//    3. Loader.instantiate
 
 @globalPrivate
 function setStateToMax(entry, newState)
@@ -139,69 +138,6 @@ function fulfillFetch(entry, source)
     @setStateToMax(entry, @ModuleInstantiate);
 }
 
-function fulfillInstantiate(entry, optionalInstance, source)
-{
-    // https://whatwg.github.io/loader/#fulfill-instantiate
-
-    "use strict";
-
-    if (!entry.instantiate)
-        entry.instantiate = @newPromiseCapability(@InternalPromise).@promise;
-    this.commitInstantiated(entry, optionalInstance, source);
-
-    // FIXME: The draft fulfills the promise in the CommitInstantiated operation.
-    // But it CommitInstantiated is also used in the requestInstantiate and
-    // we should not "force fulfill" there.
-    // So we separate "force fulfill" operation from the CommitInstantiated operation.
-    // https://github.com/whatwg/loader/pull/67
-    this.forceFulfillPromise(entry.instantiate, entry);
-}
-
-function commitInstantiated(entry, optionalInstance, source)
-{
-    // https://whatwg.github.io/loader/#commit-instantiated
-
-    "use strict";
-
-    var moduleRecord = this.instantiation(optionalInstance, source, entry);
-
-    // FIXME: Described in the draft,
-    //   4. Fulfill entry.[[Instantiate]] with instance.
-    // But, instantiate promise should be fulfilled with the entry.
-    // We remove this statement because instantiate promise will be
-    // fulfilled without this "force fulfill" operation.
-    // https://github.com/whatwg/loader/pull/67
-
-    var dependencies = [];
-    var dependenciesMap = moduleRecord.dependenciesMap;
-    moduleRecord.registryEntry = entry;
-    var requestedModules = this.requestedModules(moduleRecord);
-    for (var i = 0, length = requestedModules.length; i < length; ++i) {
-        var depKey = requestedModules[i];
-        var pair = {
-            key: depKey,
-            value: @undefined
-        };
-        @putByValDirect(dependencies, dependencies.length, pair);
-        dependenciesMap.@set(depKey, pair);
-    }
-    entry.dependencies = dependencies;
-    entry.dependenciesMap = dependenciesMap;
-    entry.module = moduleRecord;
-    @setStateToMax(entry, @ModuleSatisfy);
-}
-
-function instantiation(result, source, entry)
-{
-    // https://whatwg.github.io/loader/#instantiation
-    // FIXME: Current implementation does not support optionalInstance.
-    // https://bugs.webkit.org/show_bug.cgi?id=148171
-
-    "use strict";
-
-    return this.parseModule(entry.key, source);
-}
-
 // Loader.
 
 function requestFetch(key, fetcher)
@@ -239,17 +175,33 @@ function requestInstantiate(key, fetcher)
         return entry.instantiate;
 
     var instantiatePromise = this.requestFetch(key, fetcher).then((source) => {
-        // Hook point.
-        // 3. Loader.instantiate
-        //     https://whatwg.github.io/loader/#browser-instantiate
-        //     Take the key and the fetched source code, and instantiate the module record
-        //     by parsing the module source code.
-        //     It has the chance to provide the optional module instance that is different from
-        //     the ordinary one.
-        return this.instantiate(key, source, fetcher).then((optionalInstance) => {
-            this.commitInstantiated(entry, optionalInstance, source);
-            return entry;
-        });
+        var moduleRecord = this.parseModule(entry.key, source);
+
+        // FIXME: Described in the draft,
+        //   4. Fulfill entry.[[Instantiate]] with instance.
+        // But, instantiate promise should be fulfilled with the entry.
+        // We remove this statement because instantiate promise will be
+        // fulfilled without this "force fulfill" operation.
+        // https://github.com/whatwg/loader/pull/67
+
+        var dependencies = [];
+        var dependenciesMap = moduleRecord.dependenciesMap;
+        moduleRecord.registryEntry = entry;
+        var requestedModules = this.requestedModules(moduleRecord);
+        for (var i = 0, length = requestedModules.length; i < length; ++i) {
+            var depKey = requestedModules[i];
+            var pair = {
+                key: depKey,
+                value: @undefined
+            };
+            @putByValDirect(dependencies, dependencies.length, pair);
+            dependenciesMap.@set(depKey, pair);
+        }
+        entry.dependencies = dependencies;
+        entry.dependenciesMap = dependenciesMap;
+        entry.module = moduleRecord;
+        @setStateToMax(entry, @ModuleSatisfy);
+        return entry;
     });
     entry.instantiate = instantiatePromise;
     return instantiatePromise;
@@ -400,30 +352,15 @@ function moduleEvaluation(moduleRecord, fetcher)
 
 // APIs to control the module loader.
 
-function provide(key, stage, value)
+function provideFetch(key, value)
 {
     "use strict";
 
     var entry = this.ensureRegistered(key);
 
-    if (stage === @ModuleFetch) {
-        if (entry.state > @ModuleFetch)
-            @throwTypeError("Requested module is already fetched.");
-        this.fulfillFetch(entry, value);
-        return;
-    }
-
-    if (stage === @ModuleInstantiate) {
-        if (entry.state > @ModuleInstantiate)
-            @throwTypeError("Requested module is already instantiated.");
-        this.fulfillFetch(entry, @undefined);
-        entry.fetch.then((source) => {
-            this.fulfillInstantiate(entry, value, source);
-        });
-        return;
-    }
-
-    @throwTypeError("Requested module is already ready to be executed.");
+    if (entry.state > @ModuleFetch)
+        @throwTypeError("Requested module is already fetched.");
+    this.fulfillFetch(entry, value);
 }
 
 function loadAndEvaluateModule(moduleName, referrer, fetcher)

Source/JavaScriptCore/jsc.cpp

@@ -1673,7 +1673,6 @@ const GlobalObjectMethodTable GlobalObject::s_globalObjectMethodTable = {
     &moduleLoaderImportModule,
     &moduleLoaderResolve,
     &moduleLoaderFetch,
-    nullptr, // moduleLoaderInstantiate
     &moduleLoaderCreateImportMetaProperties,
     nullptr, // moduleLoaderEvaluate
     nullptr, // promiseRejectionTracker

Source/JavaScriptCore/runtime/Completion.cpp

@@ -187,7 +187,7 @@ JSInternalPromise* loadAndEvaluateModule(ExecState* exec, const SourceCode& sour
     JSGlobalObject* globalObject = exec->vmEntryGlobalObject();
 
     // Insert the given source code to the ModuleLoader registry as the fetched registry entry.
-    globalObject->moduleLoader()->provide(exec, key, JSModuleLoader::Status::Fetch, source);
+    globalObject->moduleLoader()->provideFetch(exec, key, source);
     RETURN_IF_EXCEPTION(scope, rejectPromise(exec, globalObject));
 
     return loadAndEvaluateModule(lock, exec, globalObject, key, jsUndefined(), scriptFetcher);
@@ -227,7 +227,7 @@ JSInternalPromise* loadModule(ExecState* exec, const SourceCode& source, JSValue
 
     // Insert the given source code to the ModuleLoader registry as the fetched registry entry.
     // FIXME: Introduce JSSourceCode object to wrap around this source.
-    globalObject->moduleLoader()->provide(exec, key, JSModuleLoader::Status::Fetch, source);
+    globalObject->moduleLoader()->provideFetch(exec, key, source);
     RETURN_IF_EXCEPTION(scope, rejectPromise(exec, globalObject));
 
     return loadModule(lock, exec, globalObject, key, jsUndefined(), scriptFetcher);

Source/JavaScriptCore/runtime/JSGlobalObject.cpp

@@ -254,7 +254,6 @@ const GlobalObjectMethodTable JSGlobalObject::s_globalObjectMethodTable = {
     nullptr, // moduleLoaderImportModule
     nullptr, // moduleLoaderResolve
     nullptr, // moduleLoaderFetch
-    nullptr, // moduleLoaderInstantiate
     nullptr, // moduleLoaderCreateImportMetaProperties
     nullptr, // moduleLoaderEvaluate
     nullptr, // promiseRejectionTracker

Source/JavaScriptCore/runtime/JSGlobalObject.h

@@ -195,9 +195,6 @@ struct GlobalObjectMethodTable {
     typedef JSInternalPromise* (*ModuleLoaderFetchPtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue);
     ModuleLoaderFetchPtr moduleLoaderFetch;
 
-    typedef JSInternalPromise* (*ModuleLoaderInstantiatePtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);
-    ModuleLoaderInstantiatePtr moduleLoaderInstantiate;
-
     typedef JSObject* (*ModuleLoaderCreateImportMetaPropertiesPtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSModuleRecord*, JSValue);
     ModuleLoaderCreateImportMetaPropertiesPtr moduleLoaderCreateImportMetaProperties;
 

Source/JavaScriptCore/runtime/JSModuleLoader.cpp

@@ -79,12 +79,12 @@ static String printableModuleKey(ExecState* exec, JSValue key)
     return vm.propertyNames->emptyIdentifier.impl();
 }
 
-JSValue JSModuleLoader::provide(ExecState* exec, JSValue key, Status status, const SourceCode& sourceCode)
+JSValue JSModuleLoader::provideFetch(ExecState* exec, JSValue key, const SourceCode& sourceCode)
 {
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    JSObject* function = jsCast<JSObject*>(get(exec, vm.propertyNames->builtinNames().providePublicName()));
+    JSObject* function = jsCast<JSObject*>(get(exec, vm.propertyNames->builtinNames().provideFetchPublicName()));
     RETURN_IF_EXCEPTION(scope, { });
     CallData callData;
     CallType callType = JSC::getCallData(function, callData);
@@ -93,7 +93,6 @@ JSValue JSModuleLoader::provide(ExecState* exec, JSValue key, Status status, con
     SourceCode source { sourceCode };
     MarkedArgumentBuffer arguments;
     arguments.append(key);
-    arguments.append(jsNumber(status));
     arguments.append(JSSourceCode::create(vm, WTFMove(source)));
 
     scope.release();
@@ -237,19 +236,6 @@ JSInternalPromise* JSModuleLoader::fetch(ExecState* exec, JSValue key, JSValue s
     return deferred->promise();
 }
 
-JSInternalPromise* JSModuleLoader::instantiate(ExecState* exec, JSValue key, JSValue source, JSValue scriptFetcher)
-{
-    if (Options::dumpModuleLoadingState())
-        dataLog("Loader [instantiate] ", printableModuleKey(exec, key), "\n");
-
-    JSGlobalObject* globalObject = exec->lexicalGlobalObject();
-    if (globalObject->globalObjectMethodTable()->moduleLoaderInstantiate)
-        return globalObject->globalObjectMethodTable()->moduleLoaderInstantiate(globalObject, exec, this, key, source, scriptFetcher);
-    JSInternalPromiseDeferred* deferred = JSInternalPromiseDeferred::create(exec, globalObject);
-    deferred->resolve(exec, jsUndefined());
-    return deferred->promise();
-}
-
 JSObject* JSModuleLoader::createImportMetaProperties(ExecState* exec, JSValue key, JSModuleRecord* moduleRecord, JSValue scriptFetcher)
 {
     JSGlobalObject* globalObject = exec->lexicalGlobalObject();

Source/JavaScriptCore/runtime/JSModuleLoader.h

@@ -63,7 +63,7 @@ class JSModuleLoader : public JSNonFinalObject {
     }
 
     // APIs to control the module loader.
-    JSValue provide(ExecState*, JSValue key, Status, const SourceCode&);
+    JSValue provideFetch(ExecState*, JSValue key, const SourceCode&);
     JSInternalPromise* loadAndEvaluateModule(ExecState*, JSValue moduleName, JSValue referrer, JSValue scriptFetcher);
     JSInternalPromise* loadModule(ExecState*, JSValue moduleName, JSValue referrer, JSValue scriptFetcher);
     JSValue linkAndEvaluateModule(ExecState*, JSValue moduleKey, JSValue scriptFetcher);
@@ -73,7 +73,6 @@ class JSModuleLoader : public JSNonFinalObject {
     JSInternalPromise* importModule(ExecState*, JSString* moduleName, const SourceOrigin& referrer);
     JSInternalPromise* resolve(ExecState*, JSValue name, JSValue referrer, JSValue scriptFetcher);
     JSInternalPromise* fetch(ExecState*, JSValue key, JSValue scriptFetcher);
-    JSInternalPromise* instantiate(ExecState*, JSValue key, JSValue source, JSValue scriptFetcher);
     JSObject* createImportMetaProperties(ExecState*, JSValue key, JSModuleRecord*, JSValue scriptFetcher);
 
     // Additional platform dependent hooked APIs.

Source/JavaScriptCore/runtime/ModuleLoaderPrototype.cpp

@@ -53,7 +53,6 @@ static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeEvaluate(ExecState*);
 static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeModuleDeclarationInstantiation(ExecState*);
 static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeResolve(ExecState*);
 static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeFetch(ExecState*);
-static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeInstantiate(ExecState*);
 static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeGetModuleNamespaceObject(ExecState*);
 
 }
@@ -71,9 +70,6 @@ const ClassInfo ModuleLoaderPrototype::s_info = { "ModuleLoader", &Base::s_info,
     ensureRegistered               JSBuiltin                                           DontEnum|Function 1
     forceFulfillPromise            JSBuiltin                                           DontEnum|Function 2
     fulfillFetch                   JSBuiltin                                           DontEnum|Function 2
-    fulfillInstantiate             JSBuiltin                                           DontEnum|Function 2
-    commitInstantiated             JSBuiltin                                           DontEnum|Function 3
-    instantiation                  JSBuiltin                                           DontEnum|Function 3
     requestFetch                   JSBuiltin                                           DontEnum|Function 2
     requestInstantiate             JSBuiltin                                           DontEnum|Function 2
     requestSatisfy                 JSBuiltin                                           DontEnum|Function 2
@@ -83,7 +79,7 @@ const ClassInfo ModuleLoaderPrototype::s_info = { "ModuleLoader", &Base::s_info,
     moduleDeclarationInstantiation moduleLoaderPrototypeModuleDeclarationInstantiation DontEnum|Function 3
     moduleEvaluation               JSBuiltin                                           DontEnum|Function 2
     evaluate                       moduleLoaderPrototypeEvaluate                       DontEnum|Function 3
-    provide                        JSBuiltin                                           DontEnum|Function 3
+    provideFetch                   JSBuiltin                                           DontEnum|Function 2
     loadAndEvaluateModule          JSBuiltin                                           DontEnum|Function 3
     loadModule                     JSBuiltin                                           DontEnum|Function 3
     linkAndEvaluateModule          JSBuiltin                                           DontEnum|Function 2
@@ -93,7 +89,6 @@ const ClassInfo ModuleLoaderPrototype::s_info = { "ModuleLoader", &Base::s_info,
     requestedModules               moduleLoaderPrototypeRequestedModules               DontEnum|Function 1
     resolve                        moduleLoaderPrototypeResolve                        DontEnum|Function 2
     fetch                          moduleLoaderPrototypeFetch                          DontEnum|Function 2
-    instantiate                    moduleLoaderPrototypeInstantiate                    DontEnum|Function 3
 @end
 */
 
@@ -203,21 +198,6 @@ EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeFetch(ExecState* exec)
     return JSValue::encode(loader->fetch(exec, exec->argument(0), exec->argument(1)));
 }
 
-EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeInstantiate(ExecState* exec)
-{
-    VM& vm = exec->vm();
-    // Hook point, Loader.instantiate
-    // https://whatwg.github.io/loader/#browser-instantiate
-    // Take the key and the fetched source code, and instantiate the module record
-    // by parsing the module source code.
-    // It has the chance to provide the optional module instance that is different from
-    // the ordinary one.
-    JSModuleLoader* loader = jsDynamicCast<JSModuleLoader*>(vm, exec->thisValue());
-    if (!loader)
-        return JSValue::encode(jsUndefined());
-    return JSValue::encode(loader->instantiate(exec, exec->argument(0), exec->argument(1), exec->argument(2)));
-}
-
 EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeGetModuleNamespaceObject(ExecState* exec)
 {
     VM& vm = exec->vm();

Source/WebCore/ChangeLog

@@ -1,3 +1,16 @@
+2017-10-11  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Drop Instantiate hook in ES6 module loader
+        https://bugs.webkit.org/show_bug.cgi?id=178162
+
+        Reviewed by Sam Weinig.
+
+        Drop instantiate hooks.
+        No behavior change.
+
+        * bindings/js/JSDOMWindowBase.cpp:
+        * bindings/js/JSWorkerGlobalScopeBase.cpp:
+
 2017-10-11  Alicia Boya García  <aboya@igalia.com>
 
         [MSE][GStreamer] Add dump of append pipeline

Source/WebCore/bindings/js/JSDOMWindowBase.cpp

@@ -72,7 +72,6 @@ const GlobalObjectMethodTable JSDOMWindowBase::s_globalObjectMethodTable = {
     &moduleLoaderImportModule,
     &moduleLoaderResolve,
     &moduleLoaderFetch,
-    nullptr, // moduleLoaderInstantiate
     nullptr, // moduleLoaderCreateImportMetaProperties
     &moduleLoaderEvaluate,
     &promiseRejectionTracker,