Skip to content

Conversation

@jriv01
Copy link
Collaborator

@jriv01 jriv01 commented Feb 26, 2025

This change allows usage of Trusted Types compatible wrappers for Wasm Workers when TRUSTED_TYPES is defined.

Similar behavior has already been implemented in libpthread.js, so the changes here are based off of & exhibit the same logic used for Trusted Types & pthread compatibility.

@jriv01 jriv01 marked this pull request as ready for review February 26, 2025 20:16
@jriv01 jriv01 marked this pull request as draft February 26, 2025 20:22
@jriv01 jriv01 marked this pull request as ready for review February 26, 2025 20:47
Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

I guess we need some kind of test for this?

@kripken
Copy link
Member

kripken commented Feb 26, 2025

I guess we need some kind of test for this?

We do have an existing test for TRUSTED_TYPES in test/test_other.py. Can perhaps add a variation to one of the existing WASM_WORKERS tests there, that adds this.

@sbc100 sbc100 enabled auto-merge (squash) March 5, 2025 19:43
@jriv01
Copy link
Collaborator Author

jriv01 commented Mar 5, 2025

I guess we need some kind of test for this?

We do have an existing test for TRUSTED_TYPES in test/test_other.py. Can perhaps add a variation to one of the existing WASM_WORKERS tests there, that adds this.

When you say add a variation that matches the existing TRUSTED_TYPES test, did you mean using @paramaterized?

emscripten/test/test_other.py

Lines 13161 to 13165 in 88ffcb9

@parameterized({
'': [[]],
'trusted': [['-sTRUSTED_TYPES']]
})
def test_pthread_export_es6(self, args):

So something like

   @parameterized({ 
     '': [[]], 
     'trusted': [['-sTRUSTED_TYPES']] 
   }) 
   def test_wasm_worker_hello(self):
     ...

@sbc100
Copy link
Collaborator

sbc100 commented Mar 5, 2025

Yup that looks about right.

auto-merge was automatically disabled March 5, 2025 22:23

Head branch was pushed to by a user without write access

@jriv01
Copy link
Collaborator Author

jriv01 commented Mar 7, 2025

It seems like there's still a CI failing, any idea what may be causing that?

I see it says Test expectations are out-of-date on the PR branch. You can run './tools/maint/rebaseline_tests.py' to create a commit updating the expectations. but I already did that at commit c6305cf

@sbc100
Copy link
Collaborator

sbc100 commented Mar 7, 2025

I think maybe you should try reverting test/other/codesize/test_codesize_files_wasmfs.size. I don't see how this change could have changed the size of that test.

@sbc100 sbc100 enabled auto-merge (squash) March 10, 2025 16:14
@jriv01
Copy link
Collaborator Author

jriv01 commented Mar 10, 2025

There seems to be some new failures despite not changing anything since 43a04dc.

Any idea what might be happening here? I see that f391a0d addresses the test_fs_js_api_wasmfs failure under test-wasm64-4gb so syncing this branch should fix that, but there's two separate failing tests in test-wasm64.

@sbc100 sbc100 merged commit b9f897c into emscripten-core:main Mar 10, 2025
29 checks passed
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.

3 participants