Skip to content

Commit c1e3c18

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 20817e1 commit c1e3c18

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
}
@@ -1436,7 +1436,6 @@ nsresult ModuleLoaderBase::StartDynamicImport(ModuleLoadRequest* aRequest) {
14361436
if (NS_FAILED(rv)) {
14371437
mLoader->ReportErrorToConsole(aRequest, rv);
14381438
RemoveDynamicImport(aRequest);
1439-
FinishDynamicImportAndReject(aRequest, rv);
14401439
}
14411440
return rv;
14421441
}
@@ -1449,62 +1448,26 @@ void ModuleLoaderBase::FinishDynamicImportAndReject(ModuleLoadRequest* aRequest,
14491448
return;
14501449
}
14511450

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

14851458
if (NS_FAILED(aResult) &&
14861459
aResult != NS_SUCCESS_DOM_SCRIPT_EVALUATION_THREW_UNCATCHABLE) {
1487-
MOZ_ASSERT(!JS_IsExceptionPending(aCx));
1460+
MOZ_ASSERT(!JS_IsExceptionPending(cx));
14881461
nsAutoCString url;
14891462
aRequest->mURI->GetSpec(url);
1490-
JS_ReportErrorNumberASCII(aCx, js::GetErrorMessage, nullptr,
1463+
JS_ReportErrorNumberASCII(cx, js::GetErrorMessage, nullptr,
14911464
JSMSG_DYNAMIC_IMPORT_FAILED, url.get());
1465+
JS::FinishLoadingImportedModuleFailedWithPendingException(cx, promise);
1466+
} else {
1467+
JS::FinishLoadingImportedModuleFailed(cx, UndefinedHandleValue, promise,
1468+
UndefinedHandleValue);
14921469
}
14931470

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

@@ -1665,30 +1628,26 @@ bool ModuleLoaderBase::InstantiateModuleGraph(ModuleLoadRequest* aRequest) {
16651628
}
16661629

16671630
void ModuleLoaderBase::ProcessDynamicImport(ModuleLoadRequest* aRequest) {
1668-
if (!aRequest->mModuleScript) {
1669-
FinishDynamicImportAndReject(aRequest, NS_ERROR_FAILURE);
1631+
AutoJSAPI jsapi;
1632+
if (!jsapi.Init(GetGlobalObject())) {
16701633
return;
16711634
}
16721635

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

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

1689-
if (NS_FAILED(rv)) {
1690-
FinishDynamicImportAndReject(aRequest, rv);
1691-
}
1650+
FinishLoadingImportedModule(cx, aRequest);
16921651
}
16931652

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

‎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)