Changeset 223331 in webkit for trunk/Source/JavaScriptCore/jsc.cpp

Timestamp:

Oct 15, 2017, 6:55:16 PM (8 years ago)

Author:

Yusuke Suzuki

Message:

[JSC] Perform module specifier validation at parsing time
​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:

File:

• trunk/Source/JavaScriptCore/jsc.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/jsc.cpp

@@ -1659 +1659 @@
 
     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);
@@ -1841 +1841 @@
 }
 
-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);
-
-    JSInternalPromiseDeferred* deferred = JSInternalPromiseDeferred::create(exec, globalObject);
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
     scope.releaseAssertNoException();
     const Identifier key = keyValue.toPropertyKey(exec);
-    if (UNLIKELY(scope.exception())) {
-        JSValue exception = scope.exception();
-        scope.clearException();
-        return deferred->reject(exec, exception);
-    }
-
-    if (key.isSymbol()) {
-        auto result = deferred->resolve(exec, keyValue);
-        scope.releaseAssertNoException();
-        return result;
-    }
+    RETURN_IF_EXCEPTION(scope, { });
+
+    if (key.isSymbol())
+        return key;
+
     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())));
 }