Skip to content

script: Clean up prepare the script element related code and remove ModuleOwner#44179

Merged
Gae24 merged 8 commits into
servo:mainfrom
Gae24:drop-module-owner
Apr 18, 2026
Merged

script: Clean up prepare the script element related code and remove ModuleOwner#44179
Gae24 merged 8 commits into
servo:mainfrom
Gae24:drop-module-owner

Conversation

@Gae24

@Gae24 Gae24 commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Drop the ModuleOwner logic in favour of closures passed down by the script fetching initiator.
When processing inline module scripts a task in now queued on the networking task source. Since Rc<ModuleTree> is not Send, a result field is now introduced to HTMLScriptElement, which is initialized before queueing the task.

This slightly improves inline-async-inserted-execorder.html, which now fails at the fourth assertion instead of stopping at the second one (the inline module script with no dependencies still resolves after the one that has a parse error).

Testing: Covered by existing tests.

@Gae24 Gae24 requested a review from gterzian as a code owner April 13, 2026 18:22
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 13, 2026
@Gae24 Gae24 force-pushed the drop-module-owner branch from 9984c47 to 4c85989 Compare April 14, 2026 08:00

@jdm jdm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! I'm glad to see this moving closer to the way the specification is written.

Comment thread components/script/script_module.rs Outdated
Comment thread components/script/module_loading.rs Outdated
@servo-highfive servo-highfive added S-needs-rebase There are merge conflict errors. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 17, 2026
Gae24 added 7 commits April 17, 2026 22:28
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
@Gae24 Gae24 force-pushed the drop-module-owner branch from 4c85989 to 889355c Compare April 17, 2026 20:38
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-rebase There are merge conflict errors. labels Apr 17, 2026
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
@jdm jdm added this pull request to the merge queue Apr 18, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 18, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 18, 2026
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Apr 18, 2026

@CYBAI CYBAI left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the great clean up! 😍 Using on_complete callback with globalscope looks much easier to understand now 🤯

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Apr 18, 2026
@Gae24 Gae24 added this pull request to the merge queue Apr 18, 2026
@servo-highfive servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Apr 18, 2026
Merged via the queue into servo:main with commit 2b21c52 Apr 18, 2026
36 checks passed
@Gae24 Gae24 deleted the drop-module-owner branch April 18, 2026 16:31
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants