Changeset 238391 in webkit for trunk/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.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/runtime/JSGlobalObjectFunctions.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp

@@ -784 +784 @@
 {
     VM& vm = exec->vm();
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
+
+    auto* globalObject = exec->lexicalGlobalObject();
+
+    auto* promise = JSPromiseDeferred::create(exec, globalObject);
+    RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
+
     auto catchScope = DECLARE_CATCH_SCOPE(vm);
-
-    auto* globalObject = exec->lexicalGlobalObject();
-
-    auto* promise = JSPromiseDeferred::create(exec, globalObject);
-    CLEAR_AND_RETURN_IF_EXCEPTION(catchScope, encodedJSValue());
+    auto reject = [&] (JSValue rejectionReason) {
+        catchScope.clearException();
+        promise->reject(exec, rejectionReason);
+        catchScope.clearException();
+        return JSValue::encode(promise->promise());
+    };
 
     auto sourceOrigin = exec->callerSourceOrigin();
     RELEASE_ASSERT(exec->argumentCount() == 1);
     auto* specifier = exec->uncheckedArgument(0).toString(exec);
-    if (Exception* exception = catchScope.exception()) {
-        catchScope.clearException();
-        promise->reject(exec, exception->value());
-        CLEAR_AND_RETURN_IF_EXCEPTION(catchScope, encodedJSValue());
-        return JSValue::encode(promise->promise());
-    }
+    if (Exception* exception = catchScope.exception())
+        return reject(exception->value());
 
     // We always specify parameters as undefined. Once dynamic import() starts accepting fetching parameters,
@@ -805 +809 @@
     JSValue parameters = jsUndefined();
     auto* internalPromise = globalObject->moduleLoader()->importModule(exec, specifier, parameters, sourceOrigin);
-    if (Exception* exception = catchScope.exception()) {
-        catchScope.clearException();
-        promise->reject(exec, exception->value());
-        CLEAR_AND_RETURN_IF_EXCEPTION(catchScope, encodedJSValue());
-        return JSValue::encode(promise->promise());
-    }
+    if (Exception* exception = catchScope.exception())
+        return reject(exception->value());
     promise->resolve(exec, internalPromise);
-    CLEAR_AND_RETURN_IF_EXCEPTION(catchScope, encodedJSValue());
-
+
+    catchScope.clearException();
     return JSValue::encode(promise->promise());
 }