Changeset 223173 in webkit

Timestamp:

Oct 11, 2017, 5:30:37 AM (9 years ago)

Author:

Yusuke Suzuki

Message:

[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:

Location:

trunk/Source

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/builtins/ModuleLoaderPrototype.js (modified) (4 diffs)
• JavaScriptCore/jsc.cpp (modified) (1 diff)
• JavaScriptCore/runtime/Completion.cpp (modified) (2 diffs)
• JavaScriptCore/runtime/JSGlobalObject.cpp (modified) (1 diff)
• JavaScriptCore/runtime/JSGlobalObject.h (modified) (1 diff)
• JavaScriptCore/runtime/JSModuleLoader.cpp (modified) (3 diffs)
• JavaScriptCore/runtime/JSModuleLoader.h (modified) (2 diffs)
• JavaScriptCore/runtime/ModuleLoaderPrototype.cpp (modified) (5 diffs)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/bindings/js/JSDOMWindowBase.cpp (modified) (1 diff)
• WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/builtins/ModuleLoaderPrototype.js

@@ -31 +31 @@
 //    1. Loader.resolve
 //    2. Loader.fetch
-//    3. Loader.instantiate
 
 @globalPrivate
@@ -140 +139 @@
 }
 
-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.
 
@@ -240 +176 @@
 
     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;
@@ -401 +353 @@
 // APIs to control the module loader.
 
-function provide(key, stage, 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.");
+function provideFetch(key, value)
+{
+    "use strict";
+
+    var entry = this.ensureRegistered(key);
+
+    if (entry.state > @ModuleFetch)
+        @throwTypeError("Requested module is already fetched.");
+    this.fulfillFetch(entry, value);
 }
 

trunk/Source/JavaScriptCore/jsc.cpp

@@ -1674 +1674 @@
     &moduleLoaderResolve,
     &moduleLoaderFetch,
-    nullptr, // moduleLoaderInstantiate
     &moduleLoaderCreateImportMetaProperties,
     nullptr, // moduleLoaderEvaluate

trunk/Source/JavaScriptCore/runtime/Completion.cpp

@@ -188 +188 @@
 
     // 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));
 
@@ -228 +228 @@
     // 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));
 

trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp

@@ -255 +255 @@
     nullptr, // moduleLoaderResolve
     nullptr, // moduleLoaderFetch
-    nullptr, // moduleLoaderInstantiate
     nullptr, // moduleLoaderCreateImportMetaProperties
     nullptr, // moduleLoaderEvaluate

trunk/Source/JavaScriptCore/runtime/JSGlobalObject.h

@@ -196 +196 @@
     ModuleLoaderFetchPtr moduleLoaderFetch;
 
-    typedef JSInternalPromise* (*ModuleLoaderInstantiatePtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);
-    ModuleLoaderInstantiatePtr moduleLoaderInstantiate;
-
     typedef JSObject* (*ModuleLoaderCreateImportMetaPropertiesPtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSModuleRecord*, JSValue);
     ModuleLoaderCreateImportMetaPropertiesPtr moduleLoaderCreateImportMetaProperties;

trunk/Source/JavaScriptCore/runtime/JSModuleLoader.cpp

@@ -80 +80 @@
 }
 
-JSValue JSModuleLoader::provide(ExecState* exec, JSValue key, Status status, const SourceCode& sourceCode)
-{
-    VM& vm = exec->vm();
-    auto scope = DECLARE_THROW_SCOPE(vm);
-
-    JSObject* function = jsCast<JSObject*>(get(exec, vm.propertyNames->builtinNames().providePublicName()));
+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().provideFetchPublicName()));
     RETURN_IF_EXCEPTION(scope, { });
     CallData callData;
@@ -94 +94 @@
     MarkedArgumentBuffer arguments;
     arguments.append(key);
-    arguments.append(jsNumber(status));
     arguments.append(JSSourceCode::create(vm, WTFMove(source)));
 
@@ -238 +237 @@
 }
 
-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)
 {

trunk/Source/JavaScriptCore/runtime/JSModuleLoader.h

@@ -64 +64 @@
 
     // 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);
@@ -74 +74 @@
     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);
 

trunk/Source/JavaScriptCore/runtime/ModuleLoaderPrototype.cpp

@@ -54 +54 @@
 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*);
 
@@ -72 +71 @@
     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
@@ -84 +80 @@
     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
@@ -94 +90 @@
     resolve                        moduleLoaderPrototypeResolve                        DontEnum|Function 2
     fetch                          moduleLoaderPrototypeFetch                          DontEnum|Function 2
-    instantiate                    moduleLoaderPrototypeInstantiate                    DontEnum|Function 3
 @end
 */
@@ -204 +199 @@
 }
 
-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)
 {

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp

@@ -73 +73 @@
     &moduleLoaderResolve,
     &moduleLoaderFetch,
-    nullptr, // moduleLoaderInstantiate
     nullptr, // moduleLoaderCreateImportMetaProperties
     &moduleLoaderEvaluate,

trunk/Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp

@@ -60 +60 @@
     nullptr, // moduleLoaderResolve
     nullptr, // moduleLoaderFetch
-    nullptr, // moduleLoaderInstantiate
     nullptr, // moduleLoaderCreateImportMetaProperties
     nullptr, // moduleLoaderEvaluate