mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions 1

feat: preloads and nodeIntegration in iframes (#16425)

Browse source 
* 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
This commit is contained in:
Samuel Attard 2019-01-22 11:24:46 -08:00 • committed by GitHub
parent 92b9525cfd
commit 58a6fe13d6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
26 changed files with 332 additions and 49 deletions
Download patch file Download diff file Expand all files Collapse all files

42
atom/renderer/atom_renderer_client.cc
View file

@ -79,25 +79,27 @@ void AtomRendererClient::DidCreateScriptContext(
content::RenderFrame* render_frame) {
RendererClientBase::DidCreateScriptContext(context, render_frame);
// Only allow node integration for the main frame of the top window, unless it
// is a devtools extension page. Allowing child frames or child windows to
// have node integration would result in memory leak, since we don't destroy
// node environment when script context is destroyed.
//
// DevTools extensions do not follow this rule because our implementation
// requires node integration in iframes to work. And usually DevTools
// extensions do not dynamically add/remove iframes.
//
// TODO(zcbenz): Do not create Node environment if node integration is not
// enabled.
if (!(render_frame->IsMainFrame() &&
!render_frame->GetWebFrame()->Opener()) &&
!IsDevToolsExtension(render_frame))
// Do not load node if we're aren't a main frame or a devtools extension
// unless node support has been explicitly enabled for sub frames
bool is_main_frame =
render_frame->IsMainFrame() && !render_frame->GetWebFrame()->Opener();
bool is_devtools = IsDevToolsExtension(render_frame);
bool allow_node_in_subframes =
base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kNodeIntegrationInSubFrames);
bool should_load_node =
is_main_frame || is_devtools || allow_node_in_subframes;
if (!should_load_node) {
return;
}
injected_frames_.insert(render_frame);
// Prepare the node bindings.
// If this is the first environment we are creating, prepare the node
// bindings.
if (!node_integration_initialized_) {
node_integration_initialized_ = true;
node_bindings_->Initialize();
@ -115,6 +117,8 @@ void AtomRendererClient::DidCreateScriptContext(
// Add Electron extended APIs.
atom_bindings_->BindTo(env->isolate(), env->process_object());
AddRenderBindings(env->isolate(), env->process_object());
mate::Dictionary process_dict(env->isolate(), env->process_object());
process_dict.SetReadOnly("isMainFrame", render_frame->IsMainFrame());
// Load everything.
node_bindings_->LoadEnvironment(env);
@ -146,11 +150,13 @@ void AtomRendererClient::WillReleaseScriptContext(
if (env == node_bindings_->uv_env())
node_bindings_->set_uv_env(nullptr);
// Destroy the node environment.
// This is disabled because pending async tasks may still use the environment
// and would cause crashes later. Node does not seem to clear all async tasks
// when the environment is destroyed.
// node::FreeEnvironment(env);
// Destroy the node environment. We only do this if node support has been
// enabled for sub-frames to avoid a change-of-behavior / introduce crashes
// for existing users.
// TODO(MarshallOfSOund): Free the environment regardless of this switch
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kNodeIntegrationInSubFrames))
node::FreeEnvironment(env);
// AtomBindings is tracking node environments.
atom_bindings_->EnvironmentDestroyed(env);