Skip to content
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.

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

[js-api] Review object caching #985

Closed
Ms2ger opened this issue Mar 14, 2019 · 5 comments · Fixed by #1036
Closed

[js-api] Review object caching #985

Ms2ger opened this issue Mar 14, 2019 · 5 comments · Fixed by #1036

Comments

@Ms2ger
Copy link
Collaborator

Ms2ger commented Mar 14, 2019

See the discussion in WebAssembly/threads#118

@Ms2ger
Copy link
Collaborator Author

Ms2ger commented Apr 11, 2019

The suggestion there seems to be that the spec requires that postMessage can produce an existing object in its deserialization. However, the HTML spec doesn't support that:

Set value to a new instance of the interface identified by interfaceName, created in targetRealm.

and the prose about Module agrees with that. (@binji added a test for this in web-platform-tests/wpt#12488.)

In the existing spec, it seems like we only need to remove the following sentence from a note in js-api:

This mapping is used to ensure that, for a given agent, there exists at most one JavaScript object for a particular WebAssembly address.

because it's not true.

The threads proposal will also need to be updated to take that into account.

@binji
Copy link
Member

binji commented Apr 12, 2019

OK, this sounds good to me. Thanks for looking into it.

@littledan
Copy link
Collaborator

The analysis looks right to me; apologies for the confusion. @Ms2ger would you be interested in creating spec PRs and tests for this change?

@binji
Copy link
Member

binji commented Jun 26, 2019

@Ms2ger ping

@Ms2ger
Copy link
Collaborator Author

Ms2ger commented Aug 21, 2019

Wrote some extra tests at web-platform-tests/wpt#18590.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants