Page MenuHomePhabricator

Bug 1820594 - Part 25: Call FinishLoadingImportedModule for dynamic import.
ClosedPublic

Authored by allstarschh on Jun 21 2024, 2:45 PM.
Referenced Files
Unknown Object (File)
Tue, Apr 14, 2:34 PM
Unknown Object (File)
Tue, Apr 14, 3:45 AM
Unknown Object (File)
Tue, Apr 14, 2:39 AM
Unknown Object (File)
Mon, Apr 13, 10:40 PM
Unknown Object (File)
Mon, Apr 13, 10:25 PM
Unknown Object (File)
Mon, Apr 13, 9:19 PM
Unknown Object (File)
Mon, Apr 13, 9:07 PM
Unknown Object (File)
Mon, Apr 13, 12:29 PM

Details

Summary

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 [https://github.com/whatwg/html/pull/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 [https://github.com/tc39/ecma262/pull/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().

Event Timeline

phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".
phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".
phab-bot removed a project: secure-revision.

Code analysis found 3 defects in diff 880410:

  • 3 build errors found by clang-tidy
IMPORTANT: Found 3 defects (error level) that must be fixed before landing.

You can run this analysis locally with:

  • ./mach static-analysis check --outgoing (C/C++)

If you see a problem in this automated review, please report it here.

You can view these defects in the Diff Detail section of Phabricator diff 880410.

3 defects closed compared to the previous diff 880869.


If you see a problem in this automated review, please report it here.

allstarschh retitled this revision from Bug 1820594 - Part 12: Call ModuleLoadRequestedModule for dynamic import. to Bug 1820594 - Part 23: Update error handling for dynamic import..
allstarschh edited the summary of this revision. (Show Details)

For your convenience, here is a patch that fixes all the clang-format defects (use it in your repository with hg import or git apply -p0).

The analysis task source-test-clang-tidy failed, but we could not detect any defect.
Please check this task manually.


If you see a problem in this automated review, please report it here.

allstarschh retitled this revision from Bug 1820594 - Part 23: Update error handling for dynamic import. to Bug 1820594 - Part 26: Call FinishLoadingImportedModule for dynamic import..
allstarschh retitled this revision from Bug 1820594 - Part 26: Call FinishLoadingImportedModule for dynamic import. to Bug 1820594 - Part 26: Call FinishLoadingImportedModule for dynamic import. r?jonco.
allstarschh edited the summary of this revision. (Show Details)
allstarschh retitled this revision from Bug 1820594 - Part 26: Call FinishLoadingImportedModule for dynamic import. r?jonco to Bug 1820594 - Part 26: Call FinishLoadingImportedModule for dynamic import..

Does the commit message need updating as it still talks about ContinueDynamicImport happening in the host layer?

This revision is now accepted and ready to land.May 27 2025, 12:28 PM
allstarschh retitled this revision from Bug 1820594 - Part 26: Call FinishLoadingImportedModule for dynamic import. to Bug 1820594 - Part 25: Call FinishLoadingImportedModule for dynamic import..
This revision is now accepted and ready to land.Jul 22 2025, 1:03 AM