[JSC] Perform module specifier validation at parsing time
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Oct 2017 01:55:16 +0000 (01:55 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Oct 2017 01:55:16 +0000 (01:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178256

Reviewed by Darin Adler.

Source/JavaScriptCore:

This patch make module loader's `resolve` operation synchronous. And we validate
module's requested module names when instantiating the module instead of satisfying
module's dependencies. This change is not observable to users. But this is precise
to the spec and this optimizes & simplifies the current module loader a bit by
reducing object allocations.

Previously, we have an object called pair in the module loader. This is pair of
module's name and module's record. And we use it to link one module to dependent
modules. Now, it is replaced with module's registry entry.

We also change our loader functions to take a registry entry instead of a module key.
Previous design is due to the consideration that these APIs may be exposed to users
in whatwg/loader spec. However, this won't happen. This change removes unnecessary
repeatedly hash map lookups.

* builtins/ModuleLoaderPrototype.js:
(globalPrivate.newRegistryEntry):
(requestFetch):
(requestInstantiate):
(requestSatisfy):
(link):
(moduleEvaluation):
(loadModule):
* jsc.cpp:
(GlobalObject::moduleLoaderResolve):
* runtime/AbstractModuleRecord.cpp:
(JSC::AbstractModuleRecord::finishCreation):
(JSC::AbstractModuleRecord::hostResolveImportedModule):
* runtime/JSGlobalObject.h:
* runtime/JSModuleLoader.cpp:
(JSC::JSModuleLoader::resolveSync):
(JSC::JSModuleLoader::resolve):
* runtime/JSModuleLoader.h:
* runtime/ModuleLoaderPrototype.cpp:
(JSC::moduleLoaderPrototypeResolveSync):

Source/WebCore:

No behavior change in the current implementation.

* bindings/js/JSDOMWindowBase.cpp:
(WebCore::JSDOMWindowBase::moduleLoaderResolve):
* bindings/js/JSDOMWindowBase.h:
* bindings/js/ScriptModuleLoader.cpp:
(WebCore::ScriptModuleLoader::resolve):
* bindings/js/ScriptModuleLoader.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223331 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/builtins/ModuleLoaderPrototype.js
Source/JavaScriptCore/jsc.cpp
Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp
Source/JavaScriptCore/runtime/JSGlobalObject.h
Source/JavaScriptCore/runtime/JSModuleLoader.cpp
Source/JavaScriptCore/runtime/JSModuleLoader.h
Source/JavaScriptCore/runtime/ModuleLoaderPrototype.cpp
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSDOMWindowBase.cpp
Source/WebCore/bindings/js/JSDOMWindowBase.h
Source/WebCore/bindings/js/ScriptModuleLoader.cpp
Source/WebCore/bindings/js/ScriptModuleLoader.h

index e40469d..8fc3994 100644 (file)
@@ -1,3 +1,46 @@
+2017-10-15  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Perform module specifier validation at parsing time
+        https://bugs.webkit.org/show_bug.cgi?id=178256
+
+        Reviewed by Darin Adler.
+
+        This patch make module loader's `resolve` operation synchronous. And we validate
+        module's requested module names when instantiating the module instead of satisfying
+        module's dependencies. This change is not observable to users. But this is precise
+        to the spec and this optimizes & simplifies the current module loader a bit by
+        reducing object allocations.
+
+        Previously, we have an object called pair in the module loader. This is pair of
+        module's name and module's record. And we use it to link one module to dependent
+        modules. Now, it is replaced with module's registry entry.
+
+        We also change our loader functions to take a registry entry instead of a module key.
+        Previous design is due to the consideration that these APIs may be exposed to users
+        in whatwg/loader spec. However, this won't happen. This change removes unnecessary
+        repeatedly hash map lookups.
+
+        * builtins/ModuleLoaderPrototype.js:
+        (globalPrivate.newRegistryEntry):
+        (requestFetch):
+        (requestInstantiate):
+        (requestSatisfy):
+        (link):
+        (moduleEvaluation):
+        (loadModule):
+        * jsc.cpp:
+        (GlobalObject::moduleLoaderResolve):
+        * runtime/AbstractModuleRecord.cpp:
+        (JSC::AbstractModuleRecord::finishCreation):
+        (JSC::AbstractModuleRecord::hostResolveImportedModule):
+        * runtime/JSGlobalObject.h:
+        * runtime/JSModuleLoader.cpp:
+        (JSC::JSModuleLoader::resolveSync):
+        (JSC::JSModuleLoader::resolve):
+        * runtime/JSModuleLoader.h:
+        * runtime/ModuleLoaderPrototype.cpp:
+        (JSC::moduleLoaderPrototypeResolveSync):
+
 2017-10-14  Devin Rousso  <webkit@devinrousso.com>
 
         Web Inspector: provide a way to enable/disable event listeners
index ed0ae60..e6ac55b 100644 (file)
@@ -99,6 +99,7 @@ function newRegistryEntry(key)
         module: @undefined, // JSModuleRecord
         linkError: @undefined,
         linkSucceeded: true,
+        evaluated: false,
     };
 }
 
@@ -140,13 +141,12 @@ function fulfillFetch(entry, source)
 
 // Loader.
 
-function requestFetch(key, parameters, fetcher)
+function requestFetch(entry, parameters, fetcher)
 {
     // https://whatwg.github.io/loader/#request-fetch
 
     "use strict";
 
-    var entry = this.ensureRegistered(key);
     if (entry.fetch)
         return entry.fetch;
 
@@ -156,7 +156,7 @@ function requestFetch(key, parameters, fetcher)
     //     Take the key and fetch the resource actually.
     //     For example, JavaScriptCore shell can provide the hook fetching the resource
     //     from the local file system.
-    var fetchPromise = this.fetch(key, parameters, fetcher).then((source) => {
+    var fetchPromise = this.fetch(entry.key, parameters, fetcher).then((source) => {
         @setStateToMax(entry, @ModuleInstantiate);
         return source;
     });
@@ -164,18 +164,18 @@ function requestFetch(key, parameters, fetcher)
     return fetchPromise;
 }
 
-function requestInstantiate(key, parameters, fetcher)
+function requestInstantiate(entry, parameters, fetcher)
 {
     // https://whatwg.github.io/loader/#request-instantiate
 
     "use strict";
 
-    var entry = this.ensureRegistered(key);
     if (entry.instantiate)
         return entry.instantiate;
 
-    var instantiatePromise = this.requestFetch(key, parameters, fetcher).then((source) => {
-        var moduleRecord = this.parseModule(entry.key, source);
+    var instantiatePromise = this.requestFetch(entry, parameters, fetcher).then((source) => {
+        var key = entry.key;
+        var moduleRecord = this.parseModule(key, source);
 
         // FIXME: Described in the draft,
         //   4. Fulfill entry.[[Instantiate]] with instance.
@@ -184,18 +184,15 @@ function requestInstantiate(key, parameters, fetcher)
         // 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);
+        var dependencies = @newArrayWithSize(requestedModules.length);
         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);
+            var depName = requestedModules[i];
+            var depKey = this.resolveSync(depName, key, fetcher);
+            var depEntry = this.ensureRegistered(depKey);
+            @putByValDirect(dependencies, i, depEntry);
+            dependenciesMap.@set(depName, depEntry);
         }
         entry.dependencies = dependencies;
         entry.dependenciesMap = dependenciesMap;
@@ -207,52 +204,37 @@ function requestInstantiate(key, parameters, fetcher)
     return instantiatePromise;
 }
 
-function requestSatisfy(key, parameters, fetcher)
+function requestSatisfy(entry, parameters, fetcher)
 {
     // https://whatwg.github.io/loader/#satisfy-instance
 
     "use strict";
 
-    var entry = this.ensureRegistered(key);
     if (entry.satisfy)
         return entry.satisfy;
 
-    var satisfyPromise = this.requestInstantiate(key, parameters, fetcher).then((entry) => {
-        var depLoads = [];
+    var satisfyPromise = this.requestInstantiate(entry, parameters, fetcher).then((entry) => {
+        var depLoads = @newArrayWithSize(entry.dependencies.length);
         for (var i = 0, length = entry.dependencies.length; i < length; ++i) {
-            let pair = entry.dependencies[i];
-
-            // Hook point.
-            // 1. Loader.resolve.
-            //     https://whatwg.github.io/loader/#browser-resolve
-            //     Take the name and resolve it to the unique identifier for the resource location.
-            //     For example, take the "jquery" and return the URL for the resource.
-            var promise = this.resolve(pair.key, key, fetcher).then((depKey) => {
-                var depEntry = this.ensureRegistered(depKey);
-
-                // Recursive resolving. The dependencies of this entry is being resolved or already resolved.
-                // Stop tracing the circular dependencies.
-                // But to retrieve the instantiated module record correctly,
-                // we need to wait for the instantiation for the dependent module.
-                // For example, reaching here, the module is starting resolving the dependencies.
-                // But the module may or may not reach the instantiation phase in the loader's pipeline.
-                // If we wait for the Satisfy for this module, it construct the circular promise chain and
-                // rejected by the Promises runtime. Since only we need is the instantiated module, instead of waiting
-                // the Satisfy for this module, we just wait Instantiate for this.
-                if (depEntry.satisfy) {
-                    return depEntry.instantiate.then((entry) => {
-                        pair.value = entry.module;
-                        return entry;
-                    });
-                }
-
+            var depEntry = entry.dependencies[i];
+            var promise = @undefined;
+
+            // Recursive resolving. The dependencies of this entry is being resolved or already resolved.
+            // Stop tracing the circular dependencies.
+            // But to retrieve the instantiated module record correctly,
+            // we need to wait for the instantiation for the dependent module.
+            // For example, reaching here, the module is starting resolving the dependencies.
+            // But the module may or may not reach the instantiation phase in the loader's pipeline.
+            // If we wait for the Satisfy for this module, it construct the circular promise chain and
+            // rejected by the Promises runtime. Since only we need is the instantiated module, instead of waiting
+            // the Satisfy for this module, we just wait Instantiate for this.
+            if (depEntry.satisfy)
+                promise = depEntry.instantiate;
+            else {
                 // Currently, module loader do not pass any information for non-top-level module fetching.
-                return this.requestSatisfy(depKey, @undefined, fetcher).then((entry) => {
-                    pair.value = entry.module;
-                    return entry;
-                });
-            });
-            @putByValDirect(depLoads, depLoads.length, promise);
+                promise = this.requestSatisfy(depEntry, @undefined, fetcher);
+            }
+            @putByValDirect(depLoads, i, promise);
         }
 
         return @InternalPromise.internalAll(depLoads).then((modules) => {
@@ -284,10 +266,8 @@ function link(entry, fetcher)
         // we can call moduleDeclarationInstantiation with the correct order
         // without constructing the dependency graph by calling dependencyGraph.
         var dependencies = entry.dependencies;
-        for (var i = 0, length = dependencies.length; i < length; ++i) {
-            var pair = dependencies[i];
-            this.link(pair.value.registryEntry, fetcher);
-        }
+        for (var i = 0, length = dependencies.length; i < length; ++i)
+            this.link(dependencies[i], fetcher);
 
         this.moduleDeclarationInstantiation(entry.module, entry.key, fetcher);
     } catch (error) {
@@ -299,26 +279,22 @@ function link(entry, fetcher)
 
 // Module semantics.
 
-function moduleEvaluation(moduleRecord, fetcher)
+function moduleEvaluation(entry, fetcher)
 {
     // http://www.ecma-international.org/ecma-262/6.0/#sec-moduleevaluation
 
     "use strict";
 
-    if (moduleRecord.evaluated)
+    if (entry.evaluated)
         return;
-    moduleRecord.evaluated = true;
-
-    var entry = moduleRecord.registryEntry;
+    entry.evaluated = true;
 
     // The contents of the [[RequestedModules]] is cloned into entry.dependencies.
     var dependencies = entry.dependencies;
-    for (var i = 0, length = dependencies.length; i < length; ++i) {
-        var pair = dependencies[i];
-        var requiredModuleRecord = pair.value;
-        this.moduleEvaluation(requiredModuleRecord, fetcher);
-    }
-    this.evaluate(entry.key, moduleRecord, fetcher);
+    for (var i = 0, length = dependencies.length; i < length; ++i)
+        this.moduleEvaluation(dependencies[i], fetcher);
+
+    this.evaluate(entry.key, entry.module, fetcher);
 }
 
 // APIs to control the module loader.
@@ -343,7 +319,7 @@ function loadModule(moduleName, parameters, fetcher)
     // Take the name and resolve it to the unique identifier for the resource location.
     // For example, take the "jquery" and return the URL for the resource.
     return this.resolve(moduleName, @undefined, fetcher).then((key) => {
-        return this.requestSatisfy(key, parameters, fetcher);
+        return this.requestSatisfy(this.ensureRegistered(key), parameters, fetcher);
     }).then((entry) => {
         return entry.key;
     });
@@ -358,7 +334,7 @@ function linkAndEvaluateModule(key, fetcher)
         @throwTypeError("Requested module is not instantiated yet.");
 
     this.link(entry, fetcher);
-    return this.moduleEvaluation(entry.module, fetcher);
+    return this.moduleEvaluation(entry, fetcher);
 }
 
 function loadAndEvaluateModule(moduleName, parameters, fetcher)
@@ -374,7 +350,7 @@ function requestImportModule(key, parameters, fetcher)
 {
     "use strict";
 
-    return this.requestSatisfy(key, parameters, fetcher).then((entry) => {
+    return this.requestSatisfy(this.ensureRegistered(key), parameters, fetcher).then((entry) => {
         this.linkAndEvaluateModule(entry.key, fetcher);
         return this.getModuleNamespaceObject(entry.module);
     });
index b56e776..d3ee28d 100644 (file)
@@ -1658,7 +1658,7 @@ protected:
     }
 
     static JSInternalPromise* moduleLoaderImportModule(JSGlobalObject*, ExecState*, JSModuleLoader*, JSString*, JSValue, const SourceOrigin&);
-    static JSInternalPromise* moduleLoaderResolve(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);
+    static Identifier moduleLoaderResolve(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);
     static JSInternalPromise* moduleLoaderFetch(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);
     static JSObject* moduleLoaderCreateImportMetaProperties(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSModuleRecord*, JSValue);
 };
@@ -1840,57 +1840,46 @@ JSInternalPromise* GlobalObject::moduleLoaderImportModule(JSGlobalObject* global
     return result;
 }
 
-JSInternalPromise* GlobalObject::moduleLoaderResolve(JSGlobalObject* globalObject, ExecState* exec, JSModuleLoader*, JSValue keyValue, JSValue referrerValue, JSValue)
+Identifier GlobalObject::moduleLoaderResolve(JSGlobalObject* globalObject, ExecState* exec, JSModuleLoader*, JSValue keyValue, JSValue referrerValue, JSValue)
 {
     VM& vm = globalObject->vm();
-    auto scope = DECLARE_CATCH_SCOPE(vm);
+    auto scope = DECLARE_THROW_SCOPE(vm);
 
-    JSInternalPromiseDeferred* deferred = JSInternalPromiseDeferred::create(exec, globalObject);
     scope.releaseAssertNoException();
     const Identifier key = keyValue.toPropertyKey(exec);
-    if (UNLIKELY(scope.exception())) {
-        JSValue exception = scope.exception();
-        scope.clearException();
-        return deferred->reject(exec, exception);
-    }
+    RETURN_IF_EXCEPTION(scope, { });
+
+    if (key.isSymbol())
+        return key;
 
-    if (key.isSymbol()) {
-        auto result = deferred->resolve(exec, keyValue);
-        scope.releaseAssertNoException();
-        return result;
-    }
     if (referrerValue.isUndefined()) {
         auto directoryName = currentWorkingDirectory();
-        if (!directoryName)
-            return deferred->reject(exec, createError(exec, ASCIILiteral("Could not resolve the current working directory.")));
-        auto result = deferred->resolve(exec, jsString(exec, resolvePath(directoryName.value(), ModuleName(key.impl()))));
-        scope.releaseAssertNoException();
-        return result;
+        if (!directoryName) {
+            throwException(exec, scope, createError(exec, ASCIILiteral("Could not resolve the current working directory.")));
+            return { };
+        }
+        return Identifier::fromString(&vm, resolvePath(directoryName.value(), ModuleName(key.impl())));
     }
 
     const Identifier referrer = referrerValue.toPropertyKey(exec);
-    if (UNLIKELY(scope.exception())) {
-        JSValue exception = scope.exception();
-        scope.clearException();
-        return deferred->reject(exec, exception);
-    }
+    RETURN_IF_EXCEPTION(scope, { });
 
     if (referrer.isSymbol()) {
         auto directoryName = currentWorkingDirectory();
-        if (!directoryName)
-            return deferred->reject(exec, createError(exec, ASCIILiteral("Could not resolve the current working directory.")));
-        auto result = deferred->resolve(exec, jsString(exec, resolvePath(directoryName.value(), ModuleName(key.impl()))));
-        scope.releaseAssertNoException();
-        return result;
+        if (!directoryName) {
+            throwException(exec, scope, createError(exec, ASCIILiteral("Could not resolve the current working directory.")));
+            return { };
+        }
+        return Identifier::fromString(&vm, resolvePath(directoryName.value(), ModuleName(key.impl())));
     }
 
     // If the referrer exists, we assume that the referrer is the correct absolute path.
     auto directoryName = extractDirectoryName(referrer.impl());
-    if (!directoryName)
-        return deferred->reject(exec, createError(exec, makeString("Could not resolve the referrer name '", String(referrer.impl()), "'.")));
-    auto result = deferred->resolve(exec, jsString(exec, resolvePath(directoryName.value(), ModuleName(key.impl()))));
-    scope.releaseAssertNoException();
-    return result;
+    if (!directoryName) {
+        throwException(exec, scope, createError(exec, makeString("Could not resolve the referrer name '", String(referrer.impl()), "'.")));
+        return { };
+    }
+    return Identifier::fromString(&vm, resolvePath(directoryName.value(), ModuleName(key.impl())));
 }
 
 static void convertShebangToJSComment(Vector<char>& buffer)
index b03ace2..c5e33dc 100644 (file)
@@ -54,8 +54,6 @@ void AbstractModuleRecord::finishCreation(ExecState* exec, VM& vm)
 {
     Base::finishCreation(vm);
     ASSERT(inherits(vm, info()));
-    putDirect(vm, Identifier::fromString(&vm, ASCIILiteral("registryEntry")), jsUndefined());
-    putDirect(vm, Identifier::fromString(&vm, ASCIILiteral("evaluated")), jsBoolean(false));
 
     auto scope = DECLARE_THROW_SCOPE(vm);
     JSMap* map = JSMap::create(exec, vm, globalObject()->mapStructure());
@@ -149,10 +147,10 @@ AbstractModuleRecord* AbstractModuleRecord::hostResolveImportedModule(ExecState*
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
     JSValue moduleNameValue = identifierToJSValue(exec, moduleName);
-    JSValue pair = m_dependenciesMap->JSMap::get(exec, moduleNameValue);
+    JSValue entry = m_dependenciesMap->JSMap::get(exec, moduleNameValue);
     RETURN_IF_EXCEPTION(scope, nullptr);
     scope.release();
-    return jsCast<AbstractModuleRecord*>(pair.get(exec, Identifier::fromString(exec, "value")));
+    return jsCast<AbstractModuleRecord*>(entry.get(exec, Identifier::fromString(exec, "module")));
 }
 
 auto AbstractModuleRecord::resolveImport(ExecState* exec, const Identifier& localName) -> Resolution
index 4a250b9..78d6112 100644 (file)
@@ -189,7 +189,7 @@ struct GlobalObjectMethodTable {
     typedef JSInternalPromise* (*ModuleLoaderImportModulePtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSString*, JSValue, const SourceOrigin&);
     ModuleLoaderImportModulePtr moduleLoaderImportModule;
 
-    typedef JSInternalPromise* (*ModuleLoaderResolvePtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);
+    typedef Identifier (*ModuleLoaderResolvePtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);
     ModuleLoaderResolvePtr moduleLoaderResolve;
 
     typedef JSInternalPromise* (*ModuleLoaderFetchPtr)(JSGlobalObject*, ExecState*, JSModuleLoader*, JSValue, JSValue, JSValue);
index 50f5148..984162a 100644 (file)
@@ -202,7 +202,7 @@ JSInternalPromise* JSModuleLoader::importModule(ExecState* exec, JSString* modul
     return deferred->promise();
 }
 
-JSInternalPromise* JSModuleLoader::resolve(ExecState* exec, JSValue name, JSValue referrer, JSValue scriptFetcher)
+Identifier JSModuleLoader::resolveSync(ExecState* exec, JSValue name, JSValue referrer, JSValue scriptFetcher)
 {
     if (Options::dumpModuleLoadingState())
         dataLog("Loader [resolve] ", printableModuleKey(exec, name), "\n");
@@ -210,9 +210,25 @@ JSInternalPromise* JSModuleLoader::resolve(ExecState* exec, JSValue name, JSValu
     JSGlobalObject* globalObject = exec->lexicalGlobalObject();
     if (globalObject->globalObjectMethodTable()->moduleLoaderResolve)
         return globalObject->globalObjectMethodTable()->moduleLoaderResolve(globalObject, exec, this, name, referrer, scriptFetcher);
-    JSInternalPromiseDeferred* deferred = JSInternalPromiseDeferred::create(exec, globalObject);
-    deferred->resolve(exec, name);
-    return deferred->promise();
+    return name.toPropertyKey(exec);
+}
+
+JSInternalPromise* JSModuleLoader::resolve(ExecState* exec, JSValue name, JSValue referrer, JSValue scriptFetcher)
+{
+    VM& vm = exec->vm();
+    auto scope = DECLARE_CATCH_SCOPE(vm);
+
+    JSInternalPromiseDeferred* deferred = JSInternalPromiseDeferred::create(exec, exec->lexicalGlobalObject());
+    scope.releaseAssertNoException();
+    const Identifier moduleKey = resolveSync(exec, name, referrer, scriptFetcher);
+    if (UNLIKELY(scope.exception())) {
+        JSValue exception = scope.exception();
+        scope.clearException();
+        return deferred->reject(exec, exception);
+    }
+    auto result = deferred->resolve(exec, identifierToJSValue(vm, moduleKey));
+    scope.releaseAssertNoException();
+    return result;
 }
 
 JSInternalPromise* JSModuleLoader::fetch(ExecState* exec, JSValue key, JSValue parameters, JSValue scriptFetcher)
index 89a4736..5404199 100644 (file)
@@ -72,6 +72,7 @@ public:
     // Platform dependent hooked APIs.
     JSInternalPromise* importModule(ExecState*, JSString* moduleName, JSValue parameters, const SourceOrigin& referrer);
     JSInternalPromise* resolve(ExecState*, JSValue name, JSValue referrer, JSValue scriptFetcher);
+    Identifier resolveSync(ExecState*, JSValue name, JSValue referrer, JSValue scriptFetcher);
     JSInternalPromise* fetch(ExecState*, JSValue key, JSValue parameters, JSValue scriptFetcher);
     JSObject* createImportMetaProperties(ExecState*, JSValue key, JSModuleRecord*, JSValue scriptFetcher);
 
index e836380..3d4f44a 100644 (file)
@@ -27,6 +27,7 @@
 #include "ModuleLoaderPrototype.h"
 
 #include "BuiltinNames.h"
+#include "CatchScope.h"
 #include "CodeProfiling.h"
 #include "Error.h"
 #include "Exception.h"
@@ -52,6 +53,7 @@ static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeRequestedModules(ExecSt
 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 moduleLoaderPrototypeResolveSync(ExecState*);
 static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeFetch(ExecState*);
 static EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeGetModuleNamespaceObject(ExecState*);
 
@@ -86,6 +88,7 @@ const ClassInfo ModuleLoaderPrototype::s_info = { "ModuleLoader", &Base::s_info,
     parseModule                    moduleLoaderPrototypeParseModule                    DontEnum|Function 2
     requestedModules               moduleLoaderPrototypeRequestedModules               DontEnum|Function 1
     resolve                        moduleLoaderPrototypeResolve                        DontEnum|Function 2
+    resolveSync                    moduleLoaderPrototypeResolveSync                    DontEnum|Function 2
     fetch                          moduleLoaderPrototypeFetch                          DontEnum|Function 3
 @end
 */
@@ -182,6 +185,19 @@ EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeResolve(ExecState* exec)
     return JSValue::encode(loader->resolve(exec, exec->argument(0), exec->argument(1), exec->argument(2)));
 }
 
+EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeResolveSync(ExecState* exec)
+{
+    VM& vm = exec->vm();
+    auto scope = DECLARE_CATCH_SCOPE(vm);
+
+    JSModuleLoader* loader = jsDynamicCast<JSModuleLoader*>(vm, exec->thisValue());
+    if (!loader)
+        return JSValue::encode(jsUndefined());
+    auto result = loader->resolveSync(exec, exec->argument(0), exec->argument(1), exec->argument(2));
+    RETURN_IF_EXCEPTION(scope, encodedJSValue());
+    return JSValue::encode(identifierToJSValue(vm, result));
+}
+
 EncodedJSValue JSC_HOST_CALL moduleLoaderPrototypeFetch(ExecState* exec)
 {
     VM& vm = exec->vm();
index fabd670..2dd8b49 100644 (file)
@@ -1,3 +1,19 @@
+2017-10-15  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Perform module specifier validation at parsing time
+        https://bugs.webkit.org/show_bug.cgi?id=178256
+
+        Reviewed by Darin Adler.
+
+        No behavior change in the current implementation.
+
+        * bindings/js/JSDOMWindowBase.cpp:
+        (WebCore::JSDOMWindowBase::moduleLoaderResolve):
+        * bindings/js/JSDOMWindowBase.h:
+        * bindings/js/ScriptModuleLoader.cpp:
+        (WebCore::ScriptModuleLoader::resolve):
+        * bindings/js/ScriptModuleLoader.h:
+
 2017-10-15  Chris Dumez  <cdumez@apple.com>
 
         DOMTokenList shouldn't add empty attributes
index ad28e8d..f8da87b 100644 (file)
@@ -364,13 +364,12 @@ void JSDOMWindowBase::fireFrameClearedWatchpointsForWindow(DOMWindow* window)
     }
 }
 
-JSC::JSInternalPromise* JSDOMWindowBase::moduleLoaderResolve(JSC::JSGlobalObject* globalObject, JSC::ExecState* exec, JSC::JSModuleLoader* moduleLoader, JSC::JSValue moduleName, JSC::JSValue importerModuleKey, JSC::JSValue scriptFetcher)
+JSC::Identifier JSDOMWindowBase::moduleLoaderResolve(JSC::JSGlobalObject* globalObject, JSC::ExecState* exec, JSC::JSModuleLoader* moduleLoader, JSC::JSValue moduleName, JSC::JSValue importerModuleKey, JSC::JSValue scriptFetcher)
 {
     JSDOMWindowBase* thisObject = JSC::jsCast<JSDOMWindowBase*>(globalObject);
     if (RefPtr<Document> document = thisObject->wrapped().document())
         return document->moduleLoader()->resolve(globalObject, exec, moduleLoader, moduleName, importerModuleKey, scriptFetcher);
-    JSC::JSInternalPromiseDeferred* deferred = JSC::JSInternalPromiseDeferred::create(exec, globalObject);
-    return deferred->reject(exec, jsUndefined());
+    return { };
 }
 
 JSC::JSInternalPromise* JSDOMWindowBase::moduleLoaderFetch(JSC::JSGlobalObject* globalObject, JSC::ExecState* exec, JSC::JSModuleLoader* moduleLoader, JSC::JSValue moduleKey, JSC::JSValue parameters, JSC::JSValue scriptFetcher)
index 2150c6a..2985e4a 100644 (file)
@@ -75,7 +75,7 @@ protected:
     JSC::WatchpointSet m_windowCloseWatchpoints;
 
 private:
-    static JSC::JSInternalPromise* moduleLoaderResolve(JSC::JSGlobalObject*, JSC::ExecState*, JSC::JSModuleLoader*, JSC::JSValue, JSC::JSValue, JSC::JSValue);
+    static JSC::Identifier moduleLoaderResolve(JSC::JSGlobalObject*, JSC::ExecState*, JSC::JSModuleLoader*, JSC::JSValue, JSC::JSValue, JSC::JSValue);
     static JSC::JSInternalPromise* moduleLoaderFetch(JSC::JSGlobalObject*, JSC::ExecState*, JSC::JSModuleLoader*, JSC::JSValue, JSC::JSValue, JSC::JSValue);
     static JSC::JSValue moduleLoaderEvaluate(JSC::JSGlobalObject*, JSC::ExecState*, JSC::JSModuleLoader*, JSC::JSValue, JSC::JSValue, JSC::JSValue);
     static JSC::JSInternalPromise* moduleLoaderImportModule(JSC::JSGlobalObject*, JSC::ExecState*, JSC::JSModuleLoader*, JSC::JSString*, JSC::JSValue, const JSC::SourceOrigin&);
index 1c62f42..95eab30 100644 (file)
@@ -85,26 +85,25 @@ static Expected<URL, ASCIILiteral> resolveModuleSpecifier(Document& document, co
     return result;
 }
 
-JSC::JSInternalPromise* ScriptModuleLoader::resolve(JSC::JSGlobalObject* jsGlobalObject, JSC::ExecState* exec, JSC::JSModuleLoader*, JSC::JSValue moduleNameValue, JSC::JSValue importerModuleKey, JSC::JSValue)
+JSC::Identifier ScriptModuleLoader::resolve(JSC::JSGlobalObject*, JSC::ExecState* exec, JSC::JSModuleLoader*, JSC::JSValue moduleNameValue, JSC::JSValue importerModuleKey, JSC::JSValue)
 {
-    auto& globalObject = *JSC::jsCast<JSDOMGlobalObject*>(jsGlobalObject);
-    auto& jsPromise = *JSC::JSInternalPromiseDeferred::create(exec, &globalObject);
-    auto promise = DeferredPromise::create(globalObject, jsPromise);
+    JSC::VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
 
     // We use a Symbol as a special purpose; It means this module is an inline module.
     // So there is no correct URL to retrieve the module source code. If the module name
     // value is a Symbol, it is used directly as a module key.
-    if (moduleNameValue.isSymbol()) {
-        promise->resolve<IDLAny>(JSC::Symbol::create(exec->vm(), asSymbol(moduleNameValue)->privateName().uid()));
-        return jsPromise.promise();
-    }
+    if (moduleNameValue.isSymbol())
+        return JSC::Identifier::fromUid(asSymbol(moduleNameValue)->privateName());
 
     if (!moduleNameValue.isString()) {
-        promise->reject(TypeError, ASCIILiteral("Importer module key is not a Symbol or a String."));
-        return jsPromise.promise();
+        JSC::throwTypeError(exec, scope, ASCIILiteral("Importer module key is not a Symbol or a String."));
+        return { };
     }
 
     String specifier = asString(moduleNameValue)->value(exec);
+    RETURN_IF_EXCEPTION(scope, { });
+
     URL baseURL;
     if (isRootModule(importerModuleKey))
         baseURL = m_document.baseURL();
@@ -120,12 +119,11 @@ JSC::JSInternalPromise* ScriptModuleLoader::resolve(JSC::JSGlobalObject* jsGloba
 
     auto result = resolveModuleSpecifier(m_document, specifier, baseURL);
     if (!result) {
-        promise->reject(TypeError, result.error());
-        return jsPromise.promise();
+        JSC::throwTypeError(exec, scope, result.error());
+        return { };
     }
 
-    promise->resolve<IDLDOMString>(result->string());
-    return jsPromise.promise();
+    return JSC::Identifier::fromString(&vm, result->string());
 }
 
 static void rejectToPropagateNetworkError(DeferredPromise& deferred, ModuleFetchFailureKind failureKind, ASCIILiteral message)
index 79fdf47..cdb656e 100644 (file)
@@ -55,7 +55,7 @@ public:
 
     Document& document() { return m_document; }
 
-    JSC::JSInternalPromise* resolve(JSC::JSGlobalObject*, JSC::ExecState*, JSC::JSModuleLoader*, JSC::JSValue moduleName, JSC::JSValue importerModuleKey, JSC::JSValue scriptFetcher);
+    JSC::Identifier resolve(JSC::JSGlobalObject*, JSC::ExecState*, JSC::JSModuleLoader*, JSC::JSValue moduleName, JSC::JSValue importerModuleKey, JSC::JSValue scriptFetcher);
     JSC::JSInternalPromise* fetch(JSC::JSGlobalObject*, JSC::ExecState*, JSC::JSModuleLoader*, JSC::JSValue moduleKey, JSC::JSValue parameters, JSC::JSValue scriptFetcher);
     JSC::JSValue evaluate(JSC::JSGlobalObject*, JSC::ExecState*, JSC::JSModuleLoader*, JSC::JSValue moduleKey, JSC::JSValue moduleRecord, JSC::JSValue scriptFetcher);
     JSC::JSInternalPromise* importModule(JSC::JSGlobalObject*, JSC::ExecState*, JSC::JSModuleLoader*, JSC::JSString*, JSC::JSValue parameters, const JSC::SourceOrigin&);