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.

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

Support Node / Preload scripts in iframes or workers #12505

Closed
pfrazee opened this issue Apr 2, 2018 · 10 comments
Closed

Support Node / Preload scripts in iframes or workers #12505

pfrazee opened this issue Apr 2, 2018 · 10 comments
Labels
2-0-x enhancement ✨

Comments

@pfrazee
Copy link
Contributor

pfrazee commented Apr 2, 2018

  • Electron version: v2.0.0-beta.6
  • Operating system: MacOS 10.12

Expected behavior

Hey all! Thanks to #10430, the preload script should run in renderers, webviews, iframes, and web workers.

Actual behavior

It only runs in renderers and webviews.

How to reproduce

https://github.com/pfrazee/electron-bug-test-preload

Console will emit true/false for whether the preload ran in the renderer, an iframe, and a worker. At time of writing this demo, only true for renderer.

git clone https://github.com/pfrazee/electron-bug-test-preload
cd electron-bug-test-preload
npm install
npm start
This was referenced Apr 2, 2018
Closed
Closed
@MarshallOfSound
Copy link
Member

Please note that that PR wasn't intended to make preloads run in iframes. This should be considered an enhancement request not a bug 👍

@pfrazee
Copy link
Contributor Author

pfrazee commented Apr 2, 2018

@MarshallOfSound Oh my bad, I misread that.

Well if there's nobody that can hit the enhancement easily, I can try to slot some time to implement it.

@Tim91084
Copy link

This would be a great enhancement. I've also tried using extensions to run code within iframes, and that does not work either. Seems like the extension manifest option "all_frames" is also not supported within electron. So, as far as I can tell, there is no way to accomplish running a script within an iframe similar to how a preload script works for a webview.

@pfrazee, I'd be happy to help out in any way if you needed.

@pfrazee
Copy link
Contributor Author

pfrazee commented Apr 13, 2018 via email

@pfrazee
Copy link
Contributor Author

pfrazee commented Jul 4, 2018 •
edited

I explored what this might require for Web Workers. See the changes made here: master...pfrazee:worker-preload-exploration

The approach I took was to set nodeIntegrationInWorker to true in my test app and then copy over a lot of behaviors from the lib/renderer JS.

Results of this exploration:

  • I was able to successfully include the session preloads.
  • Through some hacks, I was able to get the ipcRenderer API in my preload, but the methods didn't work because the internal implementation didn't find a frame for the Worker. At that point I was out of my depth so I quit. (Note, the ipcRenderer is the only reason I want preloads in iframes and workers: I use it to add Web APIs.)

I believe this approach requires node to be attached to every Web Worker, regardless of whether a preload script is being used or nodeIntegrationInWorker is enabled. I assume that's the same cost behind every other preload script.

Hope this was helpful.

@burtonator
Copy link

Seems like this is still broken as of 3.0.0-beta4

@ErezPasternak ErezPasternak mentioned this issue Aug 21, 2018
Closed
@MarshallOfSound MarshallOfSound changed the title Session setPreloads() not working for iframes or workers Support Node / Preload scripts in iframes or workers Jan 8, 2019
MarshallOfSound added a commit that referenced this issue Jan 17, 2019
@MarshallOfSound
feat: add support for node / preloads in subframes
f5a166d 
This feature has delibrately been built / implemented in such a way
that it has minimum impact on existing apps / code-paths.
Without enabling the new "nodeSupportInSubFrames" option basically none of this
new code will be hit.

The things that I believe need extra scrutiny are:

* Introduction of `event.reply` for IPC events and usage of `event.reply` instead of `event.sender.send()`
* Usage of `node::FreeEnvironment(env)` when the new option is enabled in order to avoid memory leaks.  I have tested this quite a bit and haven't managed to cause a crash but it is still feature flagged behind the "nodeSupportInSubFrames" flag to avoid potential impact.

Closes #10569
Closes #10401
Closes #11868
Closes #12505
Closes #14035
MarshallOfSound added a commit that referenced this issue Jan 22, 2019
@MarshallOfSound
feat: add support for node / preloads in subframes
aadee6c 
This feature has delibrately been built / implemented in such a way
that it has minimum impact on existing apps / code-paths.
Without enabling the new "nodeSupportInSubFrames" option basically none of this
new code will be hit.

The things that I believe need extra scrutiny are:

* Introduction of `event.reply` for IPC events and usage of `event.reply` instead of `event.sender.send()`
* Usage of `node::FreeEnvironment(env)` when the new option is enabled in order to avoid memory leaks.  I have tested this quite a bit and haven't managed to cause a crash but it is still feature flagged behind the "nodeSupportInSubFrames" flag to avoid potential impact.

Closes #10569
Closes #10401
Closes #11868
Closes #12505
Closes #14035
@pfrazee
Copy link
Contributor Author

pfrazee commented Jan 22, 2019

@MarshallOfSound 🎉 !

MarshallOfSound added a commit that referenced this issue Jan 27, 2019
@MarshallOfSound
feat: preloads and nodeIntegration in iframes (#16425)
f883aa0 
* feat: add support for node / preloads in subframes

This feature has delibrately been built / implemented in such a way
that it has minimum impact on existing apps / code-paths.
Without enabling the new "nodeSupportInSubFrames" option basically none of this
new code will be hit.

The things that I believe need extra scrutiny are:

* Introduction of `event.reply` for IPC events and usage of `event.reply` instead of `event.sender.send()`
* Usage of `node::FreeEnvironment(env)` when the new option is enabled in order to avoid memory leaks.  I have tested this quite a bit and haven't managed to cause a crash but it is still feature flagged behind the "nodeSupportInSubFrames" flag to avoid potential impact.

Closes #10569
Closes #10401
Closes #11868
Closes #12505
Closes #14035

* feat: add support preloads in subframes for sandboxed renderers

* spec: add tests for new nodeSupportInSubFrames option

* spec: fix specs for .reply and ._replyInternal for internal messages

* chore: revert change to use flag instead of environment set size

* chore: clean up subframe impl

* chore: apply suggestions from code review

Co-Authored-By: MarshallOfSound <samuel.r.attard@gmail.com>

* chore: clean up reply usage

* chore: fix TS docs generation

* chore: cleanup after rebase

* chore: rename wrap to add in event fns
MarshallOfSound added a commit that referenced this issue Jan 27, 2019
@MarshallOfSound
feat: preloads and nodeIntegration in iframes (#16425)
0890b0c 
* feat: add support for node / preloads in subframes

This feature has delibrately been built / implemented in such a way
that it has minimum impact on existing apps / code-paths.
Without enabling the new "nodeSupportInSubFrames" option basically none of this
new code will be hit.

The things that I believe need extra scrutiny are:

* Introduction of `event.reply` for IPC events and usage of `event.reply` instead of `event.sender.send()`
* Usage of `node::FreeEnvironment(env)` when the new option is enabled in order to avoid memory leaks.  I have tested this quite a bit and haven't managed to cause a crash but it is still feature flagged behind the "nodeSupportInSubFrames" flag to avoid potential impact.

Closes #10569
Closes #10401
Closes #11868
Closes #12505
Closes #14035

* feat: add support preloads in subframes for sandboxed renderers

* spec: add tests for new nodeSupportInSubFrames option

* spec: fix specs for .reply and ._replyInternal for internal messages

* chore: revert change to use flag instead of environment set size

* chore: clean up subframe impl

* chore: apply suggestions from code review

Co-Authored-By: MarshallOfSound <samuel.r.attard@gmail.com>

* chore: clean up reply usage

* chore: fix TS docs generation

* chore: cleanup after rebase

* chore: rename wrap to add in event fns
@Spongman
Copy link

Spongman commented Aug 4, 2019

is it possible to run preloads when nodeIntegrationInSubFrames is false? nodeIntegrationInSubFrames breaks the code in my iframes as they don't expect the node stuff to be defined, and they break if it is. but i need to run the preloads there regardless. is this possible?

@MarshallOfSound
Copy link
Member

@Spongman nodeIntegrationInSubFrames: true && nodeIntegration: false

@Obi-Dann
Copy link

Obi-Dann commented Oct 9, 2020

Sorry for resurrecting an old issue...
It looks like preload scripts don't run in web workers, it looks like it only the PR only implemented it for iframes #16425, right? I wonder if supporting preloads for web workers can be added as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
2-0-x enhancement ✨
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@burtonator @Spongman @timfish @pfrazee @sofianguy @Tim91084 @MarshallOfSound @Obi-Dann