Skip to content

Commit 9eb2289

Browse files
allstarschhallstarschh
committed
Bug 1820594 - Part 25: Call FinishLoadingImportedModule for dynamic import. r=jonco
From the specification, ContinueDynamicImport is done inside the engine. https://tc39.es/ecma262/#sec-ContinueDynamicImport However, our implementation for EvaluateModuleInContext() has bytecode encoding related stuff, so for now I implement the ContinueDynamicImport in the host layer. Also, the updated spec has a slightly different error handling behavior when module.Link() fails. I list them below: Before HTML PR 8253 [whatwg/html#8253] fetch the descendants of and link a module script https://web.archive.org/web/20221130023614/https://html.spec.whatwg.org/#fetch-the-descendants-of-and-link-a-module-script onFetchDescendantsComplete Step 3.2. Perform record.Link(). If this throws an exception, set result's error to rethrow to that exception. HostImportModuleDynamically https://web.archive.org/web/20221130023614/https://html.spec.whatwg.org/#hostimportmoduledynamically(referencingscriptormodule,-modulerequest,-promisecapability) Step 6.3 Otherwise, set promise to the result of running a module script given result and true. run a module script https://web.archive.org/web/20221130023614/https://html.spec.whatwg.org/#run-a-module-script Step 5. If script's error to rethrow is not null, then set evaluationPromise to a promise rejected with script's error to rethrow. -------------------------------------------------------- After ECMA262 PR 2905 [tc39/ecma262#2905] ContinueDynamicImport Step 6.a Let link be Completion(module.Link()). Step 6.b If link is an abrupt completion, then Step 6.b.i. Perform ! Call(promiseCapability.[[Reject]], undefined, « link.[[Value]] »). Step 6.b.ii. Return unused. In short, * The old behavior: It catched the exception thrown during module.Link(), and set the module script's error to rethrow to the exception. Later in Evaluate(), if the module script's error to rethrow is not null, reject the evaluation promise with the error to rethrow. * The new behavior: It simply rejects the evaluation promise with the exception thrown during module.Link(). Differential Revision: https://phabricator.services.mozilla.com/D214577
1 parent a6c79d3 commit 9eb2289

File tree

4 files changed

+44
-93
lines changed

4 files changed

+44
-93
lines changed

‎dom/base/test/jsmodules/chrome.toml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ support-files = [
4747

4848
["test_cyclicImport.html"]
4949

50+
["test_dynamicImport_link_failure.html"]
51+
5052
["test_dynamicImportErrorMessage.html"]
5153

5254
["test_importIntroType.html"]

‎dom/base/test/jsmodules/test_dynamicImport_link_failure.html‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<!DOCTYPE html>
2+
<meta charset=utf-8>
3+
<title>Test link failure in import()</title>
4+
<script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
5+
<script>
6+
SimpleTest.waitForExplicitFinish();
7+
8+
async function testLoaded() {
9+
await import("./import_circular.mjs").then(() => {
10+
ok(false, "Should have thrown a error");
11+
}).catch((error) => {
12+
ok(true, "Error has been thrown");
13+
}).finally(() => {
14+
SimpleTest.finish();
15+
});
16+
}
17+
</script>
18+
<body onload='testLoaded()'></body>

‎js/loader/ModuleLoaderBase.cpp‎

Lines changed: 24 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -325,9 +325,9 @@ bool ModuleLoaderBase::FinishLoadingImportedModule(
325325
aRequest->mStatePrivate.setUndefined();
326326
} else {
327327
JS::Rooted<JSObject*> promise(aCx, aRequest->mDynamicPromise);
328-
JS::Rooted<JS::Value> hostDefined(aCx, JS::PrivateValue(aRequest));
329328
JS::FinishLoadingImportedModule(aCx, referrer, referencingPrivate,
330-
moduleReqObj, promise, module, hostDefined);
329+
moduleReqObj, promise, module,
330+
aRequest->HasScriptLoadContext());
331331
MOZ_ASSERT(!JS_IsExceptionPending(aCx));
332332
aRequest->ClearDynamicImport();
333333
}
@@ -1435,7 +1435,6 @@ nsresult ModuleLoaderBase::StartDynamicImport(ModuleLoadRequest* aRequest) {
14351435
if (NS_FAILED(rv)) {
14361436
mLoader->ReportErrorToConsole(aRequest, rv);
14371437
RemoveDynamicImport(aRequest);
1438-
FinishDynamicImportAndReject(aRequest, rv);
14391438
}
14401439
return rv;
14411440
}
@@ -1448,62 +1447,26 @@ void ModuleLoaderBase::FinishDynamicImportAndReject(ModuleLoadRequest* aRequest,
14481447
return;
14491448
}
14501449

1451-
FinishDynamicImport(jsapi.cx(), aRequest, aResult, nullptr);
1452-
}
1453-
1454-
/* static */
1455-
void ModuleLoaderBase::FinishDynamicImport(
1456-
JSContext* aCx, ModuleLoadRequest* aRequest, nsresult aResult,
1457-
JS::Handle<JSObject*> aEvaluationPromise) {
1458-
LOG(("ScriptLoadRequest (%p): Finish dynamic import %x %d", aRequest,
1459-
unsigned(aResult), JS_IsExceptionPending(aCx)));
1460-
1461-
MOZ_ASSERT_IF(NS_SUCCEEDED(aResult),
1462-
GetCurrentModuleLoader(aCx) == aRequest->mLoader);
1463-
// For failure case, aRequest may have already been unlinked by CC.
1464-
MOZ_ASSERT_IF(
1465-
NS_FAILED(aResult),
1466-
GetCurrentModuleLoader(aCx) == aRequest->mLoader || !aRequest->mLoader);
1467-
1468-
// If aResult is a failed result, we don't have an EvaluationPromise. If it
1469-
// succeeded, evaluationPromise may still be null, but in this case it will
1470-
// be handled by rejecting the dynamic module import promise in the JSAPI.
1471-
MOZ_ASSERT_IF(NS_FAILED(aResult), !aEvaluationPromise);
1472-
1473-
// The request should been removed from mDynamicImportRequests.
1474-
MOZ_ASSERT(!aRequest->mLoader->HasDynamicImport(aRequest));
1475-
1476-
// Complete the dynamic import, report failures indicated by aResult or as a
1477-
// pending exception on the context.
1478-
1479-
if (!aRequest->mDynamicPromise) {
1450+
JSContext* cx = jsapi.cx();
1451+
JS::Rooted<JSObject*> promise(cx, aRequest->mDynamicPromise);
1452+
if (!promise) {
14801453
// Import has already been completed.
14811454
return;
14821455
}
14831456

14841457
if (NS_FAILED(aResult) &&
14851458
aResult != NS_SUCCESS_DOM_SCRIPT_EVALUATION_THREW_UNCATCHABLE) {
1486-
MOZ_ASSERT(!JS_IsExceptionPending(aCx));
1459+
MOZ_ASSERT(!JS_IsExceptionPending(cx));
14871460
nsAutoCString url;
14881461
aRequest->mURI->GetSpec(url);
1489-
JS_ReportErrorNumberASCII(aCx, js::GetErrorMessage, nullptr,
1462+
JS_ReportErrorNumberASCII(cx, js::GetErrorMessage, nullptr,
14901463
JSMSG_DYNAMIC_IMPORT_FAILED, url.get());
1464+
JS::FinishLoadingImportedModuleFailedWithPendingException(cx, promise);
1465+
} else {
1466+
JS::FinishLoadingImportedModuleFailed(cx, UndefinedHandleValue, promise,
1467+
UndefinedHandleValue);
14911468
}
14921469

1493-
JS::Rooted<JS::Value> referencingScript(
1494-
aCx, PrivateFromLoadedScript(aRequest->mDynamicReferencingScript));
1495-
JS::Rooted<JSString*> specifier(aCx, aRequest->mDynamicSpecifier);
1496-
JS::Rooted<JSObject*> promise(aCx, aRequest->mDynamicPromise);
1497-
1498-
JS::Rooted<JSObject*> moduleRequest(
1499-
aCx, JS::CreateModuleRequest(aCx, specifier, aRequest->mModuleType));
1500-
1501-
JS::FinishDynamicModuleImport(aCx, aEvaluationPromise, referencingScript,
1502-
moduleRequest, promise);
1503-
1504-
// FinishDynamicModuleImport clears any pending exception.
1505-
MOZ_ASSERT(!JS_IsExceptionPending(aCx));
1506-
15071470
aRequest->ClearDynamicImport();
15081471
}
15091472

@@ -1664,30 +1627,26 @@ bool ModuleLoaderBase::InstantiateModuleGraph(ModuleLoadRequest* aRequest) {
16641627
}
16651628

16661629
void ModuleLoaderBase::ProcessDynamicImport(ModuleLoadRequest* aRequest) {
1667-
if (!aRequest->mModuleScript) {
1668-
FinishDynamicImportAndReject(aRequest, NS_ERROR_FAILURE);
1630+
AutoJSAPI jsapi;
1631+
if (!jsapi.Init(GetGlobalObject())) {
16691632
return;
16701633
}
16711634

1672-
InstantiateAndEvaluateDynamicImport(aRequest);
1673-
}
1674-
1675-
void ModuleLoaderBase::InstantiateAndEvaluateDynamicImport(
1676-
ModuleLoadRequest* aRequest) {
1677-
MOZ_ASSERT(aRequest->mModuleScript);
1678-
1679-
if (!InstantiateModuleGraph(aRequest)) {
1680-
aRequest->mModuleScript = nullptr;
1635+
JSContext* cx = jsapi.cx();
1636+
JS::Rooted<JSObject*> promise(cx, aRequest->mDynamicPromise);
1637+
if (!aRequest->mModuleScript) {
1638+
FinishDynamicImportAndReject(aRequest, NS_ERROR_FAILURE);
1639+
return;
16811640
}
16821641

1683-
nsresult rv = NS_ERROR_FAILURE;
1684-
if (aRequest->mModuleScript) {
1685-
rv = EvaluateModule(aRequest);
1642+
if (aRequest->mModuleScript->HasParseError()) {
1643+
JS::Rooted<JS::Value> error(cx, aRequest->mModuleScript->ParseError());
1644+
JS::FinishLoadingImportedModuleFailed(cx, UndefinedHandleValue, promise,
1645+
error);
1646+
return;
16861647
}
16871648

1688-
if (NS_FAILED(rv)) {
1689-
FinishDynamicImportAndReject(aRequest, rv);
1690-
}
1649+
FinishLoadingImportedModule(cx, aRequest);
16911650
}
16921651

16931652
nsresult ModuleLoaderBase::EvaluateModule(ModuleLoadRequest* aRequest) {
@@ -1732,11 +1691,6 @@ nsresult ModuleLoaderBase::EvaluateModuleInContext(
17321691
LOG(("ScriptLoadRequest (%p): module has error to rethrow", aRequest));
17331692
JS::Rooted<JS::Value> error(aCx, moduleScript->ErrorToRethrow());
17341693
JS_SetPendingException(aCx, error);
1735-
// For a dynamic import, the promise is rejected. Otherwise an error
1736-
// is either reported by AutoEntryScript.
1737-
if (aRequest->IsDynamicImport()) {
1738-
FinishDynamicImport(aCx, aRequest, NS_OK, nullptr);
1739-
}
17401694
return NS_OK;
17411695
}
17421696

‎js/loader/ModuleLoaderBase.h‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -537,29 +537,6 @@ class ModuleLoaderBase : public nsISupports {
537537
void FinishDynamicImportAndReject(ModuleLoadRequest* aRequest,
538538
nsresult aResult);
539539

540-
/**
541-
* Wrapper for JSAPI FinishDynamicImport function. Takes an optional argument
542-
* `aEvaluationPromise` which, if null, exits early.
543-
*
544-
* This is the Top Level Await version, which works with modules which return
545-
* promises.
546-
*
547-
* @param aCX
548-
* The JSContext for the module.
549-
* @param aRequest
550-
* The module load request for the dynamic module.
551-
* @param aResult
552-
* The result of running ModuleEvaluate -- If this is successful, then
553-
* we can await the associated EvaluationPromise.
554-
* @param aEvaluationPromise
555-
* The evaluation promise returned from evaluating the module. If this
556-
* is null, JS::FinishDynamicImport will reject the dynamic import
557-
* module promise.
558-
*/
559-
static void FinishDynamicImport(JSContext* aCx, ModuleLoadRequest* aRequest,
560-
nsresult aResult,
561-
JS::Handle<JSObject*> aEvaluationPromise);
562-
563540
void RemoveDynamicImport(ModuleLoadRequest* aRequest);
564541

565542
nsresult CreateModuleScript(ModuleLoadRequest* aRequest);

0 commit comments

Comments
 (0)