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

refactor: use helpers for command-line parsing in renderer/init.js #16239

Merged
merged 1 commit into from Jan 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 4 additions & 7 deletions atom/browser/web_contents_preferences.cc
Expand Up @@ -228,19 +228,16 @@ void WebContentsPreferences::AppendCommandLineSwitches(
::switches::kEnableExperimentalWebPlatformFeatures);

// Check if we have node integration specified.
bool enable_node_integration = IsEnabled(options::kNodeIntegration, true);
command_line->AppendSwitchASCII(switches::kNodeIntegration,
enable_node_integration ? "true" : "false");
if (IsEnabled(options::kNodeIntegration))
miniak marked this conversation as resolved.
Show resolved Hide resolved
command_line->AppendSwitch(switches::kNodeIntegration);

// Whether to enable node integration in Worker.
if (IsEnabled(options::kNodeIntegrationInWorker))
command_line->AppendSwitch(switches::kNodeIntegrationInWorker);

// Check if webview tag creation is enabled, default to nodeIntegration value.
// TODO(kevinsawicki): Default to false in 2.0
bool webview_tag = IsEnabled(options::kWebviewTag, enable_node_integration);
command_line->AppendSwitchASCII(switches::kWebviewTag,
webview_tag ? "true" : "false");
if (IsEnabled(options::kWebviewTag))
command_line->AppendSwitch(switches::kWebviewTag);

// If the `sandbox` option was passed to the BrowserWindow's webPreferences,
// pass `--enable-sandbox` to the renderer so it won't have any node.js
Expand Down
34 changes: 34 additions & 0 deletions atom/common/api/atom_api_command_line.cc
@@ -0,0 +1,34 @@
// Copyright (c) 2019 GitHub, Inc.
// Use of this source code is governed by the MIT license that can be
// found in the LICENSE file.

#include "atom/common/native_mate_converters/string16_converter.h"
#include "base/command_line.h"
#include "native_mate/converter.h"
#include "native_mate/dictionary.h"

#include "atom/common/node_includes.h"

namespace {

bool HasSwitch(const std::string& name) {
return base::CommandLine::ForCurrentProcess()->HasSwitch(name.c_str());
}

base::CommandLine::StringType GetSwitchValue(const std::string& name) {
return base::CommandLine::ForCurrentProcess()->GetSwitchValueNative(
name.c_str());
}

void Initialize(v8::Local<v8::Object> exports,
v8::Local<v8::Value> unused,
v8::Local<v8::Context> context,
void* priv) {
mate::Dictionary dict(context->GetIsolate(), exports);
dict.SetMethod("hasSwitch", &HasSwitch);
dict.SetMethod("getSwitchValue", &GetSwitchValue);
}

} // namespace

NODE_BUILTIN_MODULE_CONTEXT_AWARE(atom_common_command_line, Initialize)
1 change: 1 addition & 0 deletions atom/common/node_bindings.cc
Expand Up @@ -58,6 +58,7 @@
V(atom_browser_window) \
V(atom_common_asar) \
V(atom_common_clipboard) \
V(atom_common_command_line) \
V(atom_common_crash_reporter) \
V(atom_common_features) \
V(atom_common_native_image) \
Expand Down
7 changes: 0 additions & 7 deletions atom/renderer/atom_renderer_client.h
Expand Up @@ -35,13 +35,6 @@ class AtomRendererClient : public RendererClientBase {
content::RenderFrame* render_frame) override;

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

// content::ContentRendererClient:
void RenderThreadStarted() override;
void RenderFrameCreated(content::RenderFrame*) override;
Expand Down
1 change: 1 addition & 0 deletions filenames.gni
Expand Up @@ -524,6 +524,7 @@ filenames = {
"atom/common/api/atom_api_clipboard.cc",
"atom/common/api/atom_api_clipboard.h",
"atom/common/api/atom_api_clipboard_mac.mm",
"atom/common/api/atom_api_command_line.cc",
"atom/common/api/atom_api_crash_reporter.cc",
"atom/common/api/atom_api_key_weak_map.h",
"atom/common/api/atom_api_native_image.cc",
Expand Down
53 changes: 18 additions & 35 deletions lib/renderer/init.js
Expand Up @@ -31,41 +31,24 @@ const ipcRenderer = require('@electron/internal/renderer/ipc-renderer-internal')
require('@electron/internal/renderer/web-frame-init')()

// Process command line arguments.
let nodeIntegration = false
let webviewTag = false
let contextIsolation = false
let preloadScript = null
let preloadScripts = []
let isBackgroundPage = false
let appPath = null
let guestInstanceId = null
let openerId = null
for (const arg of process.argv) {
if (arg.indexOf('--guest-instance-id=') === 0) {
// This is a guest web view.
guestInstanceId = parseInt(arg.substr(arg.indexOf('=') + 1))
} else if (arg.indexOf('--opener-id=') === 0) {
// This is a guest BrowserWindow.
openerId = parseInt(arg.substr(arg.indexOf('=') + 1))
} else if (arg.indexOf('--node-integration=') === 0) {
nodeIntegration = arg.substr(arg.indexOf('=') + 1) === 'true'
} else if (arg.indexOf('--preload=') === 0) {
preloadScript = arg.substr(arg.indexOf('=') + 1)
} else if (arg === '--background-page') {
isBackgroundPage = true
} else if (arg.indexOf('--app-path=') === 0) {
appPath = arg.substr(arg.indexOf('=') + 1)
} else if (arg.indexOf('--webview-tag=') === 0) {
webviewTag = arg.substr(arg.indexOf('=') + 1) === 'true'
} else if (arg === '--context-isolation') {
contextIsolation = true
} else if (arg.indexOf('--preload-scripts') === 0) {
preloadScripts = arg.substr(arg.indexOf('=') + 1).split(path.delimiter)
}
const { hasSwitch, getSwitchValue } = process.atomBinding('command_line')

const parseOption = function (name, defaultValue, converter = value => value) {
return hasSwitch(name) ? converter(getSwitchValue(name)) : defaultValue
}

const hiddenPage = process.argv.includes('--hidden-page')
const usesNativeWindowOpen = process.argv.includes('--native-window-open')
const contextIsolation = hasSwitch('context-isolation')
let nodeIntegration = hasSwitch('node-integration')
const webviewTag = hasSwitch('webview-tag')
const isHiddenPage = hasSwitch('hidden-page')
const isBackgroundPage = hasSwitch('background-page')
const usesNativeWindowOpen = hasSwitch('native-window-open')

const preloadScript = parseOption('preload', null)
const preloadScripts = parseOption('preload-scripts', [], value => value.split(path.delimiter))
const appPath = parseOption('app-path', null)
const guestInstanceId = parseOption('guest-instance-id', null, value => parseInt(value))
const openerId = parseOption('opener-id', null, value => parseInt(value))

// The webContents preload script is loaded after the session preload scripts.
if (preloadScript) {
Expand All @@ -74,7 +57,7 @@ if (preloadScript) {

// Pass the arguments to isolatedWorld.
if (contextIsolation) {
const isolatedWorldArgs = { ipcRenderer, guestInstanceId, hiddenPage, openerId, usesNativeWindowOpen }
const isolatedWorldArgs = { ipcRenderer, guestInstanceId, isHiddenPage, openerId, usesNativeWindowOpen }
v8Util.setHiddenValue(global, 'isolated-world-args', isolatedWorldArgs)
}

Expand All @@ -91,7 +74,7 @@ if (window.location.protocol === 'chrome-devtools:') {
nodeIntegration = false
} else {
// Override default web functions.
require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, hiddenPage, usesNativeWindowOpen)
require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen)

// Inject content scripts.
require('@electron/internal/renderer/content-scripts-injector')
Expand Down
Empty file removed lib/renderer/override.js
Empty file.
6 changes: 4 additions & 2 deletions lib/sandboxed_renderer/init.js
Expand Up @@ -105,6 +105,8 @@ function preloadRequire (module) {
throw new Error('module not found')
}

const { hasSwitch } = process.atomBinding('command_line')

switch (window.location.protocol) {
case 'chrome-devtools:': {
// Override some inspector APIs.
Expand All @@ -113,7 +115,7 @@ switch (window.location.protocol) {
}
case 'chrome-extension:': {
// Inject the chrome.* APIs that chrome extensions require
const isBackgroundPage = preloadProcess.argv.includes('--background-page')
const isBackgroundPage = hasSwitch('background-page')
require('@electron/internal/renderer/chrome-api').injectTo(window.location.hostname, isBackgroundPage, window)
break
}
Expand All @@ -123,7 +125,7 @@ if (binding.guestInstanceId) {
process.guestInstanceId = parseInt(binding.guestInstanceId)
}

if (!process.guestInstanceId && preloadProcess.argv.includes('--webview-tag=true')) {
if (!process.guestInstanceId && hasSwitch('webview-tag')) {
// don't allow recursive `<webview>`
require('@electron/internal/renderer/web-view/web-view').setupWebView(window)
}
Expand Down
2 changes: 1 addition & 1 deletion spec/api-browser-window-spec.js
Expand Up @@ -1971,7 +1971,7 @@ describe('BrowserWindow module', () => {
const p = emittedOnce(ipcMain, 'answer')
w.loadFile(path.join(fixtures, 'api', 'window-open-location-open.html'))
const [, args, typeofProcess] = await p
expect(args).to.include('--node-integration=false')
expect(args).not.to.include('--node-integration')
expect(args).to.include('--native-window-open')
expect(typeofProcess).to.eql('undefined')
})
Expand Down