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

Export setupContext publically to be used by additional contexts. #3067

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

danlapid
Copy link
Contributor

@danlapid danlapid commented Nov 6, 2024

Currently Worker::Isolate::Impl does the setup of the context. Namely setting up the console and WebAssembly.Module@@HasInstance. This functionality is useful in general to any context we might create even before we have a Worker::Isolate.
This commit moves this functionality to a publicly facing api.

Another part split from #2936

@@ -599,40 +596,12 @@ struct Worker::Isolate::Impl {
KJ_DISALLOW_COPY_AND_MOVE(Lock);

void setupContext(v8::Local<v8::Context> context) {
// Set WebAssembly.Module @@HasInstance
setWebAssemblyModuleHasInstance(*lock, context);

// The V8Inspector implements the `console` object.
KJ_IF_SOME(i, impl.inspector) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only real change is that Set WebAssembly.Module @@HasInstance is now only set after alerting the inspector of a new context.
This shouldn't have any actual effect in practice.

Copy link
Collaborator

@dom96 dom96 left a comment

Choose a reason for hiding this comment

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

Thank you for splitting up these changes!

@danlapid danlapid force-pushed the dlapid/export_worker_setup_context branch from 14c3279 to 77a3124 Compare November 7, 2024 11:36
Currently Worker::Isolate::Impl does the setup of the context.
Namely setting up the console and WebAssembly.Module@@HasInstance.
This functionallity is useful in general to any context we might create
even before we have a Worker::Isolate.
This commit moves this functionallity to a publically facing api.
@danlapid danlapid force-pushed the dlapid/export_worker_setup_context branch from 77a3124 to 22652a5 Compare November 7, 2024 11:43
@danlapid danlapid merged commit a23c799 into main Nov 7, 2024
14 checks passed
@danlapid danlapid deleted the dlapid/export_worker_setup_context branch November 7, 2024 12:19
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