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

fix: wasm code generation in the renderer #25777

Merged
merged 1 commit into from
Oct 8, 2020
Merged

Conversation

codebytere
Copy link
Member

Description of Change

Closes #25127.

This issue arose from the fact that when contextIsolation is enabled, Node.js does not have control over aspects of the context being used in the renderer process. It therefore goes to use its own callback handlers by default, but has not set embedder data for the context in question, and so when it pulls the data piece representing whether Wasm codegen should be enabled from the index it thinks such data will be at, this data piece is actually (don't ask) String.prototype.link represented as a v8::Function. We handle this by asking for the current Environment, and if none exists, we know that we're in the renderer with contextIsolation enabled and should fall back to Blink's handler.

cc @MarshallOfSound @zcbenz

Checklist

Release Notes

Notes: Fixed an issue where Wasm code generation erroneously showed as being disallowed by embedder when contextIsolation was enabled.

@codebytere codebytere requested a review from a team as a code owner October 5, 2020 20:54
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Oct 5, 2020
shell/common/node_bindings.cc Outdated Show resolved Hide resolved
shell/common/node_bindings.cc Outdated Show resolved Hide resolved
@codebytere codebytere force-pushed the wasm-cb-handler branch 2 times, most recently from 0c727b4 to cf8b7b7 Compare October 5, 2020 21:03
@codebytere codebytere force-pushed the wasm-cb-handler branch 6 times, most recently from 9f26ea0 to 2d17344 Compare October 6, 2020 16:56
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Oct 6, 2020
@codebytere codebytere merged commit 042d25e into master Oct 8, 2020
@release-clerk
Copy link

release-clerk bot commented Oct 8, 2020

Release Notes Persisted

Fixed an issue where Wasm code generation erroneously showed as being disallowed by embedder when contextIsolation was enabled.

@trop
Copy link
Contributor

trop bot commented Oct 8, 2020

I was unable to backport this PR to "10-x-y" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Contributor

trop bot commented Oct 8, 2020

I have automatically backported this PR to "11-x-y", please check out #25829

@trop trop bot removed the target/11-x-y label Oct 8, 2020
@trop
Copy link
Contributor

trop bot commented Oct 20, 2020

@codebytere has manually backported this PR to "10-x-y", please check out #26063

ericmandel added a commit to ericmandel/js9 that referenced this pull request Oct 29, 2020
 -- re-enable contextIsolation for version after that
 -- and make a versions array for future checks on versions
 -- references:
 -- electron/electron#26063
 -- electron/electron#25777
pull bot pushed a commit to FairyWorld/tool_chromium that referenced this pull request Oct 25, 2021
This CL exposes WasmCodeGenerationCheckCallbackInMainThread.
This is done so that Electron can access this callback for customization
purposes, as we need to use different handlers for different processes
depending on whether or not we currently have access to Node.js.

See electron/electron#25777 for a more thorough
explanation.

Change-Id: Ib697973e88028ebf1922e4afd6bf069fc6580584
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2527943
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/main@{#934533}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
This CL exposes WasmCodeGenerationCheckCallbackInMainThread.
This is done so that Electron can access this callback for customization
purposes, as we need to use different handlers for different processes
depending on whether or not we currently have access to Node.js.

See electron/electron#25777 for a more thorough
explanation.

Change-Id: Ib697973e88028ebf1922e4afd6bf069fc6580584
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2527943
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/main@{#934533}
NOKEYCHECK=True
GitOrigin-RevId: 23a9b2d0ff5786bcf5d3ebd90b2b68380ed27158
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.

v10.0.0: cannot compile wasm when contextIsolation is enabled
2 participants