Skip to content

script: use run_a_classic_script algorithm for worker scripts #39905

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 8 commits into from
Oct 21, 2025
Merged

script: use run_a_classic_script algorithm for worker scripts #39905

merged 8 commits into from
Oct 21, 2025
+92 −82

Conversation

@Gae24
Copy link
Contributor

@Gae24 Gae24 commented Oct 15, 2025 •
edited
Loading

Workers now call run_a_classic_worker_script, also scripts gets unminified.

Testing: no new tests passes are expected

@Gae24
Copy link
Contributor Author

Gae24 commented Oct 15, 2025

I wasn't able to do a wpt run inside my fork, it would get stuck inside smoketest and then timeout.

@Gae24 Gae24 added the T-linux-wpt Do a try run of the WPT label Oct 15, 2025
@github-actions github-actions bot removed the T-linux-wpt Do a try run of the WPT label Oct 15, 2025
@github-actions
Copy link

github-actions bot commented Oct 15, 2025

🔨 Triggering try run (#18543155941) for Linux (WPT)

@github-actions
Copy link

github-actions bot commented Oct 15, 2025

⚠️ Try run (#18543155941) failed.

Gae24 added 3 commits October 16, 2025 09:30
@Gae24
script: call run_a_classic_script inside workerglobalscope instead of…
9549830 
… execute_script

use encoding_rs to utf_8 decode fetch response

Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
@Gae24
fix lint
156ca4d 
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
@Gae24
unminify script
44772d1 
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
@Gae24 Gae24 added the T-linux-wpt Do a try run of the WPT label Oct 16, 2025
@github-actions github-actions bot removed the T-linux-wpt Do a try run of the WPT label Oct 16, 2025
@github-actions
Copy link

github-actions bot commented Oct 16, 2025

🔨 Triggering try run (#18553864380) for Linux (WPT)

mrobinson
mrobinson reviewed Oct 16, 2025
View reviewed changes
Gae24 added 3 commits October 16, 2025 16:57
@Gae24
cleanup can_run_script, pass introduction_type inside worker
8a9cc1d 
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
@Gae24
edit execute_script to use run_a_classic_script
a643499 
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
@Gae24
pass script thread's unminify_js value when initializing a workerglob…
7e97a6f 
…alscope

Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
Gae24
Gae24 commented Oct 17, 2025
View reviewed changes
@Gae24
fix clippy warning
b68d89f 
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
@Gae24 Gae24 marked this pull request as ready for review October 17, 2025 11:26
@Gae24 Gae24 requested a review from gterzian as a code owner October 17, 2025 11:26
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 17, 2025
jdm
jdm requested changes Oct 17, 2025
View reviewed changes
Copy link
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

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

This looks like a great improvement! We just need a small fix to get the unminifying working correctly, I think.

@servo-highfive servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 17, 2025
@Gae24
pass unminify_js flag through WorkerGlobalScopeInit
46e07c6 
Signed-off-by: Gae24 <96017547+Gae24@users.noreply.github.com>
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Oct 19, 2025
@Gae24
Copy link
Contributor Author

Gae24 commented Oct 21, 2025

Now the first worker will get the unminify_js flag by checking if Window's GlobalScope has a unminified_js_dir, nested workers will recursively do the same check.
Tested with workers/semantics/multiple-workers/003.html

jdm
jdm approved these changes Oct 21, 2025
View reviewed changes
Copy link
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

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

Lovely cleanup!

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Oct 21, 2025
@jdm jdm added this pull request to the merge queue Oct 21, 2025
@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 Oct 21, 2025
Merged via the queue into servo:main with commit 8e68903 Oct 21, 2025
33 checks passed
@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 Oct 21, 2025
@Gae24 Gae24 deleted the create-script branch October 21, 2025 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mrobinson mrobinson mrobinson left review comments

@jdm jdm jdm approved these changes

@gterzian gterzian Awaiting requested review from gterzian gterzian is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Gae24 @mrobinson @jdm @servo-highfive