Skip to content

Commit

Permalink
Remove node integration from renderer
Browse files Browse the repository at this point in the history
  • Loading branch information
@tarruda
tarruda committed Aug 2, 2016
1 parent eb73303 commit e0dd107
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 119 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ node_modules/
*.pyc
debug.log
npm-debug.log
vendor/
14 changes: 2 additions & 12 deletions atom/browser/atom_browser_client.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,18 +265,8 @@ bool AtomBrowserClient::CanCreateWindow(
bool* no_javascript_access) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);

if (delegate_) {
content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
base::Bind(&api::App::OnCreateWindow,
base::Unretained(static_cast<api::App*>(delegate_)),
target_url,
frame_name,
disposition,
render_process_id,
opener_render_frame_id));
}

return false;
*no_javascript_access = false;
return true;
}

brightray::BrowserMainParts* AtomBrowserClient::OverrideCreateBrowserMainParts(
Expand Down
93 changes: 3 additions & 90 deletions atom/renderer/atom_renderer_client.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,12 @@
#include <vector>

#include "atom/common/api/api_messages.h"
#include "atom/common/api/atom_bindings.h"
#include "atom/common/api/event_emitter_caller.h"
#include "atom/common/color_util.h"
#include "atom/common/native_mate_converters/value_converter.h"
#include "atom/common/node_bindings.h"
#include "atom/common/node_includes.h"
#include "atom/common/options_switches.h"
#include "atom/renderer/atom_render_view_observer.h"
#include "atom/renderer/guest_view_container.h"
#include "atom/renderer/node_array_buffer_bridge.h"
#include "atom/renderer/preferences_manager.h"
#include "base/command_line.h"
#include "chrome/renderer/media/chrome_key_systems.h"
Expand Down Expand Up @@ -70,21 +66,6 @@ class AtomRenderFrameObserver : public content::RenderFrameObserver {
renderer_client_->DidClearWindowObject(render_frame_);
}

void DidCreateScriptContext(v8::Handle<v8::Context> context,
int extension_group,
int world_id) override {
if (world_id_ != -1 && world_id_ != world_id)
return;
world_id_ = world_id;
renderer_client_->DidCreateScriptContext(context, render_frame_);
}
void WillReleaseScriptContext(v8::Local<v8::Context> context,
int world_id) override {
if (world_id_ != world_id)
return;
renderer_client_->WillReleaseScriptContext(context, render_frame_);
}

private:
content::RenderFrame* render_frame_;
int world_id_;
Expand Down Expand Up @@ -117,9 +98,7 @@ bool IsDevToolsExtension(content::RenderFrame* render_frame) {

} // namespace

AtomRendererClient::AtomRendererClient()
: node_bindings_(NodeBindings::Create(false)),
atom_bindings_(new AtomBindings) {
AtomRendererClient::AtomRendererClient() {
// Parse --standard-schemes=scheme1,scheme2
base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
std::string custom_schemes = command_line->GetSwitchValueASCII(
Expand All @@ -139,8 +118,6 @@ void AtomRendererClient::RenderThreadStarted() {
blink::WebCustomElement::addEmbedderCustomElementName("webview");
blink::WebCustomElement::addEmbedderCustomElementName("browserplugin");

OverrideNodeArrayBuffer();

preferences_manager_.reset(new PreferencesManager);

#if defined(OS_WIN)
Expand Down Expand Up @@ -203,26 +180,6 @@ void AtomRendererClient::DidClearWindowObject(
render_frame->GetWebFrame()->executeScript(blink::WebScriptSource("void 0"));
}

void AtomRendererClient::RunScriptsAtDocumentStart(
content::RenderFrame* render_frame) {
// Inform the document start pharse.
node::Environment* env = node_bindings_->uv_env();
if (env) {
v8::HandleScope handle_scope(env->isolate());
mate::EmitEvent(env->isolate(), env->process_object(), "document-start");
}
}

void AtomRendererClient::RunScriptsAtDocumentEnd(
content::RenderFrame* render_frame) {
// Inform the document end pharse.
node::Environment* env = node_bindings_->uv_env();
if (env) {
v8::HandleScope handle_scope(env->isolate());
mate::EmitEvent(env->isolate(), env->process_object(), "document-end");
}
}

blink::WebSpeechSynthesizer* AtomRendererClient::OverrideSpeechSynthesizer(
blink::WebSpeechSynthesizerClient* client) {
return new TtsDispatcher(client);
Expand All @@ -242,52 +199,8 @@ bool AtomRendererClient::OverrideCreatePlugin(
return true;
}

void AtomRendererClient::DidCreateScriptContext(
v8::Handle<v8::Context> context, content::RenderFrame* render_frame) {
// Only allow node integration for the main frame, unless it is a devtools
// extension page.
if (!render_frame->IsMainFrame() && !IsDevToolsExtension(render_frame))
return;

// Whether the node binding has been initialized.
bool first_time = node_bindings_->uv_env() == nullptr;

// Prepare the node bindings.
if (first_time) {
node_bindings_->Initialize();
node_bindings_->PrepareMessageLoop();
}

// Setup node environment for each window.
node::Environment* env = node_bindings_->CreateEnvironment(context);

// Add atom-shell extended APIs.
atom_bindings_->BindTo(env->isolate(), env->process_object());
AddRenderBindings(env->isolate(), env->process_object(),
preferences_manager_.get());

// Load everything.
node_bindings_->LoadEnvironment(env);

if (first_time) {
// Make uv loop being wrapped by window context.
node_bindings_->set_uv_env(env);

// Give the node loop a run to make sure everything is ready.
node_bindings_->RunMessageLoop();
}
}

void AtomRendererClient::WillReleaseScriptContext(
v8::Handle<v8::Context> context, content::RenderFrame* render_frame) {
// Only allow node integration for the main frame, unless it is a devtools
// extension page.
if (!render_frame->IsMainFrame() && !IsDevToolsExtension(render_frame))
return;

node::Environment* env = node::Environment::GetCurrent(context);
if (env)
mate::EmitEvent(env->isolate(), env->process_object(), "exit");
bool AtomRendererClient::AllowPopup() {
return true;
}

bool AtomRendererClient::ShouldFork(blink::WebLocalFrame* frame,
Expand Down
17 changes: 1 addition & 16 deletions atom/renderer/atom_renderer_client.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,39 +14,26 @@ namespace atom {

class AtomBindings;
class PreferencesManager;
class NodeBindings;

class AtomRendererClient : public content::ContentRendererClient {
public:
AtomRendererClient();
virtual ~AtomRendererClient();

void DidClearWindowObject(content::RenderFrame* render_frame);
void DidCreateScriptContext(
v8::Handle<v8::Context> context, content::RenderFrame* render_frame);
void WillReleaseScriptContext(
v8::Handle<v8::Context> context, content::RenderFrame* render_frame);

private:
enum NodeIntegration {
ALL,
EXCEPT_IFRAME,
MANUAL_ENABLE_IFRAME,
DISABLE,
};

// content::ContentRendererClient:
void RenderThreadStarted() override;
void RenderFrameCreated(content::RenderFrame*) override;
void RenderViewCreated(content::RenderView*) override;
void RunScriptsAtDocumentStart(content::RenderFrame* render_frame) override;
void RunScriptsAtDocumentEnd(content::RenderFrame* render_frame) override;
blink::WebSpeechSynthesizer* OverrideSpeechSynthesizer(
blink::WebSpeechSynthesizerClient* client) override;
bool OverrideCreatePlugin(content::RenderFrame* render_frame,
blink::WebLocalFrame* frame,
const blink::WebPluginParams& params,
blink::WebPlugin** plugin) override;
bool AllowPopup() override;
bool ShouldFork(blink::WebLocalFrame* frame,
const GURL& url,
const std::string& http_method,
Expand All @@ -61,8 +48,6 @@ class AtomRendererClient : public content::ContentRendererClient {
std::vector<std::unique_ptr<::media::KeySystemProperties>>* key_systems)
override;

std::unique_ptr<NodeBindings> node_bindings_;
std::unique_ptr<AtomBindings> atom_bindings_;
std::unique_ptr<PreferencesManager> preferences_manager_;

DISALLOW_COPY_AND_ASSIGN(AtomRendererClient);
Expand Down
2 changes: 1 addition & 1 deletion lib/renderer/override.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ if (process.guestInstanceId == null) {
}

// Make the browser window or guest view emit "new-window" event.
window.open = function (url, frameName, features) {
function windowOpenInNewRenderer(url, frameName, features) {
var feature, guestId, i, j, len, len1, name, options, ref1, ref2, value
if (frameName == null) {
frameName = ''
Expand Down

1 comment on commit e0dd107

@tarruda
Copy link
Owner Author

@tarruda tarruda commented on e0dd107 Aug 2, 2016

Choose a reason for hiding this comment

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

@zcbenz Hi, just an electron user here.

For a certain app I'm porting from CEF, I need to have the native window.open API restored, so I'm trying to modify electron to allow this use case(I don't expect this to be accepted in electron, and I'm ok with maintaining a fork).

I'm aware of the arguments you presented against this, but for this specific app I don't need node.js API in the renderer process, so I made this commit to disable node.js integration in the renderer process and also restore the native window.open functionality from webkit.

With this commit, window.open is restored to the native implementation(at least it returns the right window object), but unfortunately the window seems to be hidden. Apart from the fact that the window is not visible, it seems to be working property, eg: I can call methods(such as close()) and read properties from it (such as "closed") and even evaluate javascript in its context.

I'm hoping this is something simple to fix, but I have little experience in chrome's content module. If this is simple, could you give me some hints on how to fix it?

Thanks anyway!

Please sign in to comment.