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

Timestamp:

Nov 19, 2018, 11:09:53 PM (7 years ago)

Author:

[email protected]

Message:

globalFuncImportModule() should return a promise when it clears exceptions.
​https://bugs.webkit.org/show_bug.cgi?id=191792
<rdar://problem/46090763>

Reviewed by Michael Saboff.

JSTests:

• stress/global-import-function-should-return-a-promise-when-clearing-exceptions.js: Added.

Source/JavaScriptCore:

If we're clearing the exceptions in a CatchScope, then it means that we've handled
the exception, and is able to proceed in a normal manner. Hence, we should not
return the empty JSValue in this case: instead, we should return a Promise as
expected by import's API.

The only time when we can't return a promise is when we fail to create a Promise.
In that case, we should be propagating the exception.

Hence, globalFuncImportModule() contains a ThrowScope (for propagating the
exception that arises from failure to create the Promise) wrapping a CatchScope
(for catching any exception that arises from failure to execute the import).

Also fixed similar issues, and some exception check issues in JSModuleLoader and
the jsc shell.

• jsc.cpp:

(GlobalObject::moduleLoaderImportModule):
(GlobalObject::moduleLoaderFetch):

• runtime/JSGlobalObjectFunctions.cpp:

(JSC::globalFuncImportModule):

• runtime/JSModuleLoader.cpp:

(JSC::JSModuleLoader::loadAndEvaluateModule):
(JSC::JSModuleLoader::loadModule):
(JSC::JSModuleLoader::requestImportModule):
(JSC::JSModuleLoader::importModule):
(JSC::JSModuleLoader::resolve):
(JSC::JSModuleLoader::fetch):
(JSC::moduleLoaderParseModule):
(JSC::moduleLoaderResolveSync):

File:

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

trunk/Source/JavaScriptCore/jsc.cpp

@@ -802 +802 @@
 {
     VM& vm = globalObject->vm();
-    auto scope = DECLARE_CATCH_SCOPE(vm);
-
-    auto rejectPromise = [&] (JSValue error) {
-        return JSInternalPromiseDeferred::create(exec, globalObject)->reject(exec, error);
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
+
+    auto* deferred = JSInternalPromiseDeferred::create(exec, globalObject);
+    RETURN_IF_EXCEPTION(throwScope, nullptr);
+
+    auto catchScope = DECLARE_CATCH_SCOPE(vm);
+    auto reject = [&] (JSValue rejectionReason) {
+        catchScope.clearException();
+        auto result = deferred->reject(exec, rejectionReason);
+        catchScope.clearException();
+        return result;
     };
 
     if (sourceOrigin.isNull())
-        return rejectPromise(createError(exec, "Could not resolve the module specifier."_s));
+        return reject(createError(exec, "Could not resolve the module specifier."_s));
 
     auto referrer = sourceOrigin.string();
     auto moduleName = moduleNameValue->value(exec);
-    if (UNLIKELY(scope.exception())) {
-        JSValue exception = scope.exception();
-        scope.clearException();
-        return rejectPromise(exception);
-    }
+    if (UNLIKELY(catchScope.exception()))
+        return reject(catchScope.exception());
 
     auto directoryName = extractDirectoryName(referrer.impl());
     if (!directoryName)
-        return rejectPromise(createError(exec, makeString("Could not resolve the referrer name '", String(referrer.impl()), "'.")));
+        return reject(createError(exec, makeString("Could not resolve the referrer name '", String(referrer.impl()), "'.")));
 
     auto result = JSC::importModule(exec, Identifier::fromString(&vm, resolvePath(directoryName.value(), ModuleName(moduleName))), parameters, jsUndefined());
-    scope.releaseAssertNoException();
+    if (UNLIKELY(catchScope.exception()))
+        return reject(catchScope.exception());
     return result;
 }
@@ -992 +997 @@
 {
     VM& vm = globalObject->vm();
-    auto scope = DECLARE_CATCH_SCOPE(vm);
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
     JSInternalPromiseDeferred* deferred = JSInternalPromiseDeferred::create(exec, globalObject);
+    RETURN_IF_EXCEPTION(throwScope, nullptr);
+
+    auto catchScope = DECLARE_CATCH_SCOPE(vm);
+    auto reject = [&] (JSValue rejectionReason) {
+        catchScope.clearException();
+        auto result = deferred->reject(exec, rejectionReason);
+        catchScope.clearException();
+        return result;
+    };
+
     String moduleKey = key.toWTFString(exec);
-    if (UNLIKELY(scope.exception())) {
-        JSValue exception = scope.exception();
-        scope.clearException();
-        return deferred->reject(exec, exception);
-    }
+    if (UNLIKELY(catchScope.exception()))
+        return reject(catchScope.exception());
 
     // Here, now we consider moduleKey as the fileName.
     Vector<uint8_t> buffer;
-    if (!fetchModuleFromLocalFileSystem(moduleKey, buffer)) {
-        auto result = deferred->reject(exec, createError(exec, makeString("Could not open file '", moduleKey, "'.")));
-        scope.releaseAssertNoException();
-        return result;
-    }
+    if (!fetchModuleFromLocalFileSystem(moduleKey, buffer))
+        return reject(createError(exec, makeString("Could not open file '", moduleKey, "'.")));
 
 #if ENABLE(WEBASSEMBLY)
@@ -1013 +1022 @@
     if (buffer.size() >= 4) {
         if (buffer[0] == '\0' && buffer[1] == 'a' && buffer[2] == 's' && buffer[3] == 'm') {
-            auto result = deferred->resolve(exec, JSSourceCode::create(vm, SourceCode(WebAssemblySourceProvider::create(WTFMove(buffer), SourceOrigin { moduleKey }, moduleKey))));
-            scope.releaseAssertNoException();
+            auto source = SourceCode(WebAssemblySourceProvider::create(WTFMove(buffer), SourceOrigin { moduleKey }, moduleKey));
+            catchScope.releaseAssertNoException();
+            auto sourceCode = JSSourceCode::create(vm, WTFMove(source));
+            catchScope.releaseAssertNoException();
+            auto result = deferred->resolve(exec, sourceCode);
+            catchScope.clearException();
             return result;
         }
@@ -1020 +1033 @@
 #endif
 
-    auto result = deferred->resolve(exec, JSSourceCode::create(vm, makeSource(stringFromUTF(buffer), SourceOrigin { moduleKey }, moduleKey, TextPosition(), SourceProviderSourceType::Module)));
-    scope.releaseAssertNoException();
+    auto sourceCode = JSSourceCode::create(vm, makeSource(stringFromUTF(buffer), SourceOrigin { moduleKey }, moduleKey, TextPosition(), SourceProviderSourceType::Module));
+    catchScope.releaseAssertNoException();
+    auto result = deferred->resolve(exec, sourceCode);
+    catchScope.clearException();
     return result;
 }