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

atom/browser/web_contents_preferences.cc

@@ -100,6 +100,7 @@ WebContentsPreferences::WebContentsPreferences(
   SetDefaultBoolIfUndefined(options::kPlugins, false);
   SetDefaultBoolIfUndefined(options::kExperimentalFeatures, false);
   SetDefaultBoolIfUndefined(options::kNodeIntegration, false);
+  SetDefaultBoolIfUndefined(options::kNodeSupportInSubFrames, false);
   SetDefaultBoolIfUndefined(options::kNodeIntegrationInWorker, false);
   SetDefaultBoolIfUndefined(options::kWebviewTag, false);
   SetDefaultBoolIfUndefined(options::kSandbox, false);
@@ -346,6 +347,9 @@ void WebContentsPreferences::AppendCommandLineSwitches(
     }
   }
 
+  if (IsEnabled(options::kNodeSupportInSubFrames))
+    command_line->AppendSwitch(switches::kNodeSupportInSubFrames);
+
   // We are appending args to a webContents so let's save the current state
   // of our preferences object so that during the lifetime of the WebContents
   // we can fetch the options used to initally configure the WebContents

atom/common/options_switches.cc

@@ -154,6 +154,8 @@ const char kAllowRunningInsecureContent[] = "allowRunningInsecureContent";
 
 const char kOffscreen[] = "offscreen";
 
+const char kNodeSupportInSubFrames[] = "nodeSupportInSubFrames";
+
 }  // namespace options
 
 namespace switches {
@@ -208,6 +210,10 @@ const char kWebviewTag[] = "webview-tag";
 // Command switch passed to renderer process to control nodeIntegration.
 const char kNodeIntegrationInWorker[] = "node-integration-in-worker";
 
+// Command switch passed to renderer process to control whether node
+// environments will be created in sub-frames.
+const char kNodeSupportInSubFrames[] = "node-support-in-subframes";
+
 // Widevine options
 // Path to Widevine CDM binaries.
 const char kWidevineCdmPath[] = "widevine-cdm-path";

atom/common/options_switches.h

@@ -75,6 +75,7 @@ extern const char kSandbox[];
 extern const char kWebSecurity[];
 extern const char kAllowRunningInsecureContent[];
 extern const char kOffscreen[];
+extern const char kNodeSupportInSubFrames[];
 
 }  // namespace options
 
@@ -107,6 +108,7 @@ extern const char kHiddenPage[];
 extern const char kNativeWindowOpen[];
 extern const char kNodeIntegrationInWorker[];
 extern const char kWebviewTag[];
+extern const char kNodeSupportInSubFrames[];
 
 extern const char kWidevineCdmPath[];
 extern const char kWidevineCdmVersion[];

atom/renderer/atom_render_frame_observer.cc

@@ -187,7 +187,7 @@ void AtomRenderFrameObserver::OnBrowserMessage(bool internal,
     return;
 
   blink::WebLocalFrame* frame = render_frame_->GetWebFrame();
-  if (!frame || !render_frame_->IsMainFrame())
+  if (!frame)
     return;
 
   EmitIPCEvent(frame, internal, channel, args, sender_id);

atom/renderer/atom_renderer_client.cc

@@ -79,27 +79,24 @@ 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.
+
+  // 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
   if (!(render_frame->IsMainFrame() &&
         !render_frame->GetWebFrame()->Opener()) &&
-      !IsDevToolsExtension(render_frame))
+      !IsDevToolsExtension(render_frame) &&
+      !base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kNodeSupportInSubFrames)) {
     return;
+  }
 
   injected_frames_.insert(render_frame);
 
-  // Prepare the node bindings.
-  if (!node_integration_initialized_) {
-    node_integration_initialized_ = true;
+  // If this is the first environment we are creating, prepare the node
+  // bindings.
+  if (environments_.size() == 0) {
     node_bindings_->Initialize();
     node_bindings_->PrepareMessageLoop();
   }
@@ -115,6 +112,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 +145,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::kNodeSupportInSubFrames))
+    node::FreeEnvironment(env);
 
   // AtomBindings is tracking node environments.
   atom_bindings_->EnvironmentDestroyed(env);

atom/renderer/atom_renderer_client.h

@@ -53,9 +53,6 @@ class AtomRendererClient : public RendererClientBase {
 
   node::Environment* GetEnvironment(content::RenderFrame* frame) const;
 
-  // Whether the node integration has been initialized.
-  bool node_integration_initialized_ = false;
-
   std::unique_ptr<NodeBindings> node_bindings_;
   std::unique_ptr<AtomBindings> atom_bindings_;
 

docs/api/browser-window.md

@@ -371,6 +371,14 @@ It creates a new `BrowserWindow` with native properties as set by the `options`.
       English and not localized.
     * `navigateOnDragDrop` Boolean (optional) - Whether dragging and dropping a
       file or link onto the page causes a navigation. Default is `false`.
+    * `nodeSupportInSubFrames` Boolean (optional) - Experimental option for
+      enabling NodeJS support in sub-frames such as iframes.  This may have
+      an increased chance of memory leaks.  All your preloads will load for
+      every iframe, you can use `process.isMainFrame` to determine if you are
+      in the main frame or not.  If you have `nodeIntegration=false` and
+      `nodeSupportInSubFrames=true` your preloads will still run in iframes
+      with node enabled but nodeIntegration will be disabled for all iframe
+      content just like your main frame.
 
 When setting minimum or maximum window size with `minWidth`/`maxWidth`/
 `minHeight`/`maxHeight`, it only constrains the users. It won't prevent you from

docs/api/ipc-main.md

@@ -18,7 +18,9 @@ process, see [webContents.send][web-contents-send] for more information.
 * When sending a message, the event name is the `channel`.
 * To reply to a synchronous message, you need to set `event.returnValue`.
 * To send an asynchronous message back to the sender, you can use
-  `event.sender.send(...)`.
+  `event.reply(...)`.  This helper method will automatically handle messages
+  coming from frames that aren't the main frame (E.g. IFrames) whereas
+  `event.sender.send(...)` will always send to the main frame.
 
 An example of sending and handling messages between the render and main
 processes:
@@ -28,7 +30,7 @@ processes:
 const { ipcMain } = require('electron')
 ipcMain.on('asynchronous-message', (event, arg) => {
   console.log(arg) // prints "ping"
-  event.sender.send('asynchronous-reply', 'pong')
+  event.reply('asynchronous-reply', 'pong')
 })
 
 ipcMain.on('synchronous-message', (event, arg) => {
@@ -86,6 +88,10 @@ Removes listeners of the specified `channel`.
 
 The `event` object passed to the `callback` has the following methods:
 
+### `event.frameId`
+
+An `Integer` representing the ID of the renderer frame that sent this message.
+
 ### `event.returnValue`
 
 Set this to the value to be returned in a synchronous message.
@@ -97,3 +103,10 @@ Returns the `webContents` that sent the message, you can call
 [webContents.send][web-contents-send] for more information.
 
 [web-contents-send]: web-contents.md#contentssendchannel-arg1-arg2-
+
+### `event.reply`
+
+A function that will send an IPC message to the renderer frane that sent
+the original message that you are currently handling.  You should use this
+method to "reply" to the sent message in order to guaruntee the reply will go
+to the correct process and frame.

docs/api/process.md

@@ -59,6 +59,11 @@ process.once('loaded', () => {
 A `Boolean`. When app is started by being passed as parameter to the default app, this
 property is `true` in the main process, otherwise it is `undefined`.
 
+### `process.isMainFrame`
+
+A `Boolean`.  Set to `true` when the current renderer context is the "main" renderer
+frame.  If you want the ID of the current frame you should use `webFrame.routingId`.
+
 ### `process.mas`
 
 A `Boolean`. For Mac App Store build, this property is `true`, for other builds it is

docs/api/web-contents.md

@@ -1420,6 +1420,36 @@ app.on('ready', () => {
 </html>
 ```
 
+#### `contents.sendToFrame(frameId, channel[, arg1][, arg2][, ...])`
+
+* `frameId` Integer
+* `channel` String
+* `...args` any[]
+
+Send an asynchronous message to a specific frame in a renderer process via
+`channel`, you can also send arbitrary arguments. Arguments will be serialized
+in JSON internally and hence no functions or prototype chain will be included.
+
+The renderer process can handle the message by listening to `channel` with the
+[`ipcRenderer`](ipc-renderer.md) module.
+
+If you want to get the `frameId` of a given renderer context you should use
+the `webFrame.routingId` value.  E.g.
+
+```js
+// In a renderer process
+console.log('My frameId is:', require('electron').webFrame.routingId)
+```
+
+You can also read `frameId` from all incoming IPC messages in the main process.
+
+```js
+// In the main process
+ipcMain.on('ping', (event) => {
+  console.info('Message came from frameId:', event.frameId)
+})
+```
+
 #### `contents.enableDeviceEmulation(parameters)`
 
 * `parameters` Object

lib/browser/api/web-contents.js

@@ -143,6 +143,18 @@ WebContents.prototype._sendInternalToAll = function (channel, ...args) {
 
   return this._send(internal, sendToAll, channel, args)
 }
+WebContents.prototype.sendToFrame = function (frameId, channel, ...args) {
+  if (typeof channel !== 'string') {
+    throw new Error('Missing required channel argument')
+  } else if (typeof frameId !== 'number') {
+    throw new Error('Missing required frameId argument')
+  }
+
+  const internal = false
+  const sendToAll = false
+
+  return this._sendToFrame(internal, sendToAll, frameId, channel, args)
+}
 WebContents.prototype._sendToFrameInternal = function (frameId, channel, ...args) {
   if (typeof channel !== 'string') {
     throw new Error('Missing required channel argument')
@@ -343,6 +355,16 @@ WebContents.prototype._init = function () {
 
   // Dispatch IPC messages to the ipc module.
   this.on('-ipc-message', function (event, [channel, ...args]) {
+    event.reply = (...args) => {
+      event.sender.sendToFrame(event.frameId, ...args)
+    }
+    Object.defineProperty(event, '_replyInternal', {
+      configurable: false,
+      enumerable: false,
+      value: (...args) => {
+        event.sender._sendToFrameInternal(event.frameId, ...args)
+      }
+    })
     this.emit('ipc-message', event, channel, ...args)
     ipcMain.emit(channel, event, ...args)
   })

lib/browser/chrome-extension.js

@@ -180,7 +180,7 @@ ipcMain.on('CHROME_RUNTIME_SENDMESSAGE', function (event, extensionId, message,
 
   page.webContents._sendInternalToAll(`CHROME_RUNTIME_ONMESSAGE_${extensionId}`, event.sender.id, message, resultID)
   ipcMain.once(`CHROME_RUNTIME_ONMESSAGE_RESULT_${resultID}`, (event, result) => {
-    event.sender._sendInternal(`CHROME_RUNTIME_SENDMESSAGE_RESULT_${originResultID}`, result)
+    event.sender._replyInternal(`CHROME_RUNTIME_SENDMESSAGE_RESULT_${originResultID}`, result)
   })
   resultID++
 })
@@ -196,7 +196,7 @@ ipcMain.on('CHROME_TABS_SEND_MESSAGE', function (event, tabId, extensionId, isBa
 
   contents._sendInternalToAll(`CHROME_RUNTIME_ONMESSAGE_${extensionId}`, senderTabId, message, resultID)
   ipcMain.once(`CHROME_RUNTIME_ONMESSAGE_RESULT_${resultID}`, (event, result) => {
-    event.sender._sendInternal(`CHROME_TABS_SEND_MESSAGE_RESULT_${originResultID}`, result)
+    event.sender._replyInternal(`CHROME_TABS_SEND_MESSAGE_RESULT_${originResultID}`, result)
   })
   resultID++
 })

lib/browser/desktop-capturer.js

@@ -18,7 +18,7 @@ ipcMain.on(electronSources, (event, captureWindow, captureScreen, thumbnailSize,
   event.sender.emit('desktop-capturer-get-sources', customEvent)
 
   if (customEvent.defaultPrevented) {
-    event.sender._sendInternal(capturerResult(id), [])
+    event.sender._replyInternal(capturerResult(id), [])
     return
   }
 
@@ -30,7 +30,7 @@ ipcMain.on(electronSources, (event, captureWindow, captureScreen, thumbnailSize,
       thumbnailSize,
       fetchWindowIcons
     },
-    webContents: event.sender
+    event
   }
   requestsQueue.push(request)
   if (requestsQueue.length === 1) {
@@ -40,14 +40,14 @@ ipcMain.on(electronSources, (event, captureWindow, captureScreen, thumbnailSize,
   // If the WebContents is destroyed before receiving result, just remove the
   // reference from requestsQueue to make the module not send the result to it.
   event.sender.once('destroyed', () => {
-    request.webContents = null
+    request.event = null
   })
 })
 
 desktopCapturer.emit = (event, name, sources, fetchWindowIcons) => {
   // Receiving sources result from main process, now send them back to renderer.
   const handledRequest = requestsQueue.shift()
-  const handledWebContents = handledRequest.webContents
+  const handledWebContents = handledRequest.event ? handledRequest.event.sender : null
   const unhandledRequestsQueue = []
 
   const result = sources.map(source => {
@@ -61,15 +61,15 @@ desktopCapturer.emit = (event, name, sources, fetchWindowIcons) => {
   })
 
   if (handledWebContents) {
-    handledWebContents._sendInternal(capturerResult(handledRequest.id), result)
+    handledRequest.event._replyInternal(capturerResult(handledRequest.id), result)
   }
 
   // Check the queue to see whether there is another identical request & handle
   requestsQueue.forEach(request => {
-    const webContents = request.webContents
+    const event = request.event
     if (deepEqual(handledRequest.options, request.options)) {
-      if (webContents) {
-        webContents._sendInternal(capturerResult(request.id), result)
+      if (event) {
+        event._replyInternal(capturerResult(request.id), result)
       }
     } else {
       unhandledRequestsQueue.push(request)

lib/browser/guest-view-manager.js

@@ -246,7 +246,8 @@ const attachGuest = function (event, embedderFrameId, elementInstanceId, guestIn
     ['nativeWindowOpen', true],
     ['nodeIntegration', false],
     ['enableRemoteModule', false],
-    ['sandbox', true]
+    ['sandbox', true],
+    ['kNodeSupportInSubFrames', false]
   ])
 
   // Inherit certain option values from embedder
@@ -350,7 +351,7 @@ const handleMessage = function (channel, handler) {
 }
 
 handleMessage('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST', function (event, params, requestId) {
-  event.sender._sendInternal(`ELECTRON_RESPONSE_${requestId}`, createGuest(event.sender, params))
+  event.sender._replyInternal(`ELECTRON_RESPONSE_${requestId}`, createGuest(event.sender, params))
 })
 
 handleMessage('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST_SYNC', function (event, params) {
@@ -400,7 +401,7 @@ handleMessage('ELECTRON_GUEST_VIEW_MANAGER_ASYNC_CALL', function (event, request
   }, error => {
     return [errorUtils.serialize(error)]
   }).then(responseArgs => {
-    event.sender._sendInternal(`ELECTRON_GUEST_VIEW_MANAGER_ASYNC_CALL_RESPONSE_${requestId}`, ...responseArgs)
+    event.sender._replyInternal(`ELECTRON_GUEST_VIEW_MANAGER_ASYNC_CALL_RESPONSE_${requestId}`, ...responseArgs)
   })
 })
 

lib/browser/guest-window-manager.js

@@ -16,7 +16,8 @@ const inheritedWebPreferences = new Map([
   ['nodeIntegration', false],
   ['enableRemoteModule', false],
   ['sandbox', true],
-  ['webviewTag', false]
+  ['webviewTag', false],
+  ['kNodeSupportInSubFrames', false]
 ])
 
 // Copy attribute of |parent| to |child| if it is not defined in |child|.