Merge pull request #9972 from electron/merge-web-preferences

Inherit webPreferences in windows opened via sandbox or nativeWindowOpen

atom/browser/api/atom_api_window.cc

@@ -10,6 +10,7 @@
 #include "atom/browser/api/atom_api_web_contents.h"
 #include "atom/browser/browser.h"
 #include "atom/browser/native_window.h"
+#include "atom/browser/web_contents_preferences.h"
 #include "atom/common/native_mate_converters/callback.h"
 #include "atom/common/native_mate_converters/file_path_converter.h"
 #include "atom/common/native_mate_converters/gfx_converter.h"
@@ -77,30 +78,43 @@ v8::Local<v8::Value> ToBuffer(v8::Isolate* isolate, void* val, int size) {
 Window::Window(v8::Isolate* isolate, v8::Local<v8::Object> wrapper,
                const mate::Dictionary& options) {
   mate::Handle<class WebContents> web_contents;
-  // If no WebContents was passed to the constructor, create it from options.
-  if (!options.Get("webContents", &web_contents)) {
-    // Use options.webPreferences to create WebContents.
-    mate::Dictionary web_preferences = mate::Dictionary::CreateEmpty(isolate);
-    options.Get(options::kWebPreferences, &web_preferences);
 
-    // Copy the backgroundColor to webContents.
-    v8::Local<v8::Value> value;
-    if (options.Get(options::kBackgroundColor, &value))
-      web_preferences.Set(options::kBackgroundColor, value);
+  // Use options.webPreferences in WebContents.
+  mate::Dictionary web_preferences = mate::Dictionary::CreateEmpty(isolate);
+  options.Get(options::kWebPreferences, &web_preferences);
 
-    v8::Local<v8::Value> transparent;
-    if (options.Get("transparent", &transparent))
-      web_preferences.Set("transparent", transparent);
+  // Copy the backgroundColor to webContents.
+  v8::Local<v8::Value> value;
+  if (options.Get(options::kBackgroundColor, &value))
+    web_preferences.Set(options::kBackgroundColor, value);
+
+  v8::Local<v8::Value> transparent;
+  if (options.Get("transparent", &transparent))
+    web_preferences.Set("transparent", transparent);
 
 #if defined(ENABLE_OSR)
-    // Offscreen windows are always created frameless.
-    bool offscreen;
-    if (web_preferences.Get("offscreen", &offscreen) && offscreen) {
-      auto window_options = const_cast<mate::Dictionary&>(options);
-      window_options.Set(options::kFrame, false);
-    }
+  // Offscreen windows are always created frameless.
+  bool offscreen;
+  if (web_preferences.Get("offscreen", &offscreen) && offscreen) {
+    auto window_options = const_cast<mate::Dictionary&>(options);
+    window_options.Set(options::kFrame, false);
+  }
 #endif
 
+  if (options.Get("webContents", &web_contents)) {
+    // Set webPreferences from options if using an existing webContents.
+    // These preferences will be used when the webContent launches new
+    // render processes.
+    auto* existing_preferences =
+        WebContentsPreferences::FromWebContents(web_contents->web_contents());
+    base::DictionaryValue web_preferences_dict;
+    if (mate::ConvertFromV8(isolate, web_preferences.GetHandle(),
+                        &web_preferences_dict)) {
+      existing_preferences->web_preferences()->Clear();
+      existing_preferences->Merge(web_preferences_dict);
+    }
+
+  } else {
     // Creates the WebContents used by BrowserWindow.
     web_contents = WebContents::Create(isolate, web_preferences);
   }

atom/browser/atom_browser_client.cc

@@ -38,6 +38,7 @@
 #include "content/public/browser/resource_dispatcher_host.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/common/web_preferences.h"
 #include "net/ssl/ssl_cert_request_info.h"
@@ -78,9 +79,10 @@ AtomBrowserClient::~AtomBrowserClient() {
 
 content::WebContents* AtomBrowserClient::GetWebContentsFromProcessID(
     int process_id) {
-  // If the process is a pending process, we should use the old one.
+  // If the process is a pending process, we should use the web contents
+  // for the frame host passed into OverrideSiteInstanceForNavigation.
   if (base::ContainsKey(pending_processes_, process_id))
-    process_id = pending_processes_[process_id];
+    return pending_processes_[process_id];
 
   // Certain render process will be created with no associated render view,
   // for example: ServiceWorker.
@@ -230,19 +232,20 @@ void AtomBrowserClient::OverrideSiteInstanceForNavigation(
       content::BrowserThread::UI, FROM_HERE,
       base::Bind(&Noop, base::RetainedRef(site_instance)));
 
-  // Remember the original renderer process of the pending renderer process.
-  auto current_process = current_instance->GetProcess();
+  // Remember the original web contents for the pending renderer process.
   auto pending_process = (*new_instance)->GetProcess();
-  pending_processes_[pending_process->GetID()] = current_process->GetID();
+  pending_processes_[pending_process->GetID()] =
+      content::WebContents::FromRenderFrameHost(render_frame_host);;
   // Clear the entry in map when process ends.
-  current_process->AddObserver(this);
+  pending_process->AddObserver(this);
 }
 
 void AtomBrowserClient::AppendExtraCommandLineSwitches(
     base::CommandLine* command_line,
     int process_id) {
-  std::string process_type = command_line->GetSwitchValueASCII("type");
-  if (process_type != "renderer")
+  std::string process_type =
+      command_line->GetSwitchValueASCII(::switches::kProcessType);
+  if (process_type != ::switches::kRendererProcess)
     return;
 
   // Copy following switches to child process.
@@ -275,11 +278,9 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches(
   }
 
   content::WebContents* web_contents = GetWebContentsFromProcessID(process_id);
-  if (!web_contents)
-    return;
-
-  WebContentsPreferences::AppendExtraCommandLineSwitches(
-      web_contents, command_line);
+  if (web_contents)
+    WebContentsPreferences::AppendExtraCommandLineSwitches(
+        web_contents, command_line);
 }
 
 void AtomBrowserClient::DidCreatePpapiPlugin(
@@ -419,12 +420,7 @@ void AtomBrowserClient::WebNotificationAllowed(
 void AtomBrowserClient::RenderProcessHostDestroyed(
     content::RenderProcessHost* host) {
   int process_id = host->GetID();
-  for (const auto& entry : pending_processes_) {
-    if (entry.first == process_id || entry.second == process_id) {
-      pending_processes_.erase(entry.first);
-      break;
-    }
-  }
+  pending_processes_.erase(process_id);
   RemoveProcessPreferences(process_id);
 }
 

atom/browser/atom_browser_client.h

@@ -130,8 +130,8 @@ class AtomBrowserClient : public brightray::BrowserClient,
   bool RendererUsesNativeWindowOpen(int process_id);
   bool RendererDisablesPopups(int process_id);
 
-  // pending_render_process => current_render_process.
-  std::map<int, int> pending_processes_;
+  // pending_render_process => web contents.
+  std::map<int, content::WebContents*> pending_processes_;
 
   std::map<int, ProcessPreferences> process_preferences_;
   std::map<int, base::ProcessId> render_process_host_pids_;

lib/browser/guest-window-manager.js

@@ -11,7 +11,9 @@ const frameToGuest = new Map()
 const inheritedWebPreferences = new Map([
   ['contextIsolation', true],
   ['javascript', false],
+  ['nativeWindowOpen', true],
   ['nodeIntegration', false],
+  ['sandbox', true],
   ['webviewTag', false]
 ])
 
@@ -78,19 +80,8 @@ const setupGuest = function (embedder, frameName, guest, options) {
     embedder.send('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_CLOSED_' + guestId)
     embedder.removeListener('render-view-deleted', closedByEmbedder)
   }
-  if (!options.webPreferences.sandbox) {
-    // These events should only be handled when the guest window is opened by a
-    // non-sandboxed renderer for two reasons:
-    //
-    // - `render-view-deleted` is emitted when the popup is closed by the user,
-    //   and that will eventually result in NativeWindow::NotifyWindowClosed
-    //   using a dangling pointer since `destroy()` would have been called by
-    //   `closeByEmbedded`
-    // - No need to emit `ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_CLOSED_` since
-    //   there's no renderer code listening to it.,
-    embedder.once('render-view-deleted', closedByEmbedder)
-    guest.once('closed', closedByUser)
-  }
+  embedder.once('render-view-deleted', closedByEmbedder)
+  guest.once('closed', closedByUser)
   if (frameName) {
     frameToGuest.set(frameName, guest)
     guest.frameName = frameName

lib/renderer/window-setup.js

@@ -127,6 +127,10 @@ module.exports = (ipcRenderer, guestInstanceId, openerId, hiddenPage, usesNative
         return null
       }
     }
+
+    if (openerId != null) {
+      window.opener = getOrCreateProxy(ipcRenderer, openerId)
+    }
   }
 
   window.alert = function (message, title) {
@@ -142,10 +146,6 @@ module.exports = (ipcRenderer, guestInstanceId, openerId, hiddenPage, usesNative
     throw new Error('prompt() is and will not be supported.')
   }
 
-  if (openerId != null) {
-    window.opener = getOrCreateProxy(ipcRenderer, openerId)
-  }
-
   ipcRenderer.on('ELECTRON_GUEST_WINDOW_POSTMESSAGE', function (event, sourceId, message, sourceOrigin) {
     // Manually dispatch event instead of using postMessage because we also need to
     // set event.source.

spec/api-browser-window-spec.js

@@ -1005,6 +1005,7 @@ describe('BrowserWindow module', function () {
             preload: preload
           }
         })
+        ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preload)
         let htmlPath = path.join(fixtures, 'api', 'sandbox.html?window-open')
         const pageUrl = 'file://' + htmlPath
         w.loadURL(pageUrl)
@@ -1035,6 +1036,7 @@ describe('BrowserWindow module', function () {
             preload: preload
           }
         })
+        ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preload)
         let htmlPath = path.join(fixtures, 'api', 'sandbox.html?window-open-external')
         const pageUrl = 'file://' + htmlPath
         let popupWindow
@@ -1066,6 +1068,43 @@ describe('BrowserWindow module', function () {
         })
       })
 
+      it('should inherit the sandbox setting in opened windows', function (done) {
+        w.destroy()
+        w = new BrowserWindow({
+          show: false,
+          webPreferences: {
+            sandbox: true
+          }
+        })
+
+        const preloadPath = path.join(fixtures, 'api', 'new-window-preload.js')
+        ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preloadPath)
+        ipcMain.once('answer', (event, args) => {
+          assert.equal(args.includes('--enable-sandbox'), true)
+          done()
+        })
+        w.loadURL(`file://${path.join(fixtures, 'api', 'new-window.html')}`)
+      })
+
+      it('should open windows with the options configured via new-window event listeners', function (done) {
+        w.destroy()
+        w = new BrowserWindow({
+          show: false,
+          webPreferences: {
+            sandbox: true
+          }
+        })
+
+        const preloadPath = path.join(fixtures, 'api', 'new-window-preload.js')
+        ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preloadPath)
+        ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'foo', 'bar')
+        ipcMain.once('answer', (event, args, webPreferences) => {
+          assert.equal(webPreferences.foo, 'bar')
+          done()
+        })
+        w.loadURL(`file://${path.join(fixtures, 'api', 'new-window.html')}`)
+      })
+
       it('should set ipc event sender correctly', function (done) {
         w.destroy()
         w = new BrowserWindow({
@@ -1326,6 +1365,68 @@ describe('BrowserWindow module', function () {
         })
         w.loadURL('file://' + path.join(fixtures, 'api', 'native-window-open-native-addon.html'))
       })
+
+      it('should inherit the nativeWindowOpen setting in opened windows', function (done) {
+        w.destroy()
+        w = new BrowserWindow({
+          show: false,
+          webPreferences: {
+            nativeWindowOpen: true
+          }
+        })
+
+        const preloadPath = path.join(fixtures, 'api', 'new-window-preload.js')
+        ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preloadPath)
+        ipcMain.once('answer', (event, args) => {
+          assert.equal(args.includes('--native-window-open'), true)
+          done()
+        })
+        w.loadURL(`file://${path.join(fixtures, 'api', 'new-window.html')}`)
+      })
+
+      it('should open windows with the options configured via new-window event listeners', function (done) {
+        w.destroy()
+        w = new BrowserWindow({
+          show: false,
+          webPreferences: {
+            nativeWindowOpen: true
+          }
+        })
+
+        const preloadPath = path.join(fixtures, 'api', 'new-window-preload.js')
+        ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preloadPath)
+        ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'foo', 'bar')
+        ipcMain.once('answer', (event, args, webPreferences) => {
+          assert.equal(webPreferences.foo, 'bar')
+          done()
+        })
+        w.loadURL(`file://${path.join(fixtures, 'api', 'new-window.html')}`)
+      })
+
+      it('retains the original web preferences when window.location is changed to a new origin', async function () {
+        await serveFileFromProtocol('foo', path.join(fixtures, 'api', 'window-open-location-change.html'))
+        await serveFileFromProtocol('bar', path.join(fixtures, 'api', 'window-open-location-final.html'))
+
+        w.destroy()
+        w = new BrowserWindow({
+          show: true,
+          webPreferences: {
+            nodeIntegration: false,
+            nativeWindowOpen: true
+          }
+        })
+
+        return new Promise((resolve, reject) => {
+          ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', path.join(fixtures, 'api', 'window-open-preload.js'))
+          ipcMain.once('answer', (event, args, typeofProcess) => {
+            assert.equal(args.includes('--node-integration=false'), true)
+            assert.equal(args.includes('--native-window-open'), true)
+            assert.equal(typeofProcess, 'undefined')
+            resolve()
+          })
+          w.loadURL(`file://${path.join(fixtures, 'api', 'window-open-location-open.html')}`)
+        })
+      })
     })
   })
 
@@ -2706,3 +2807,20 @@ const isScaleFactorRounding = () => {
   // Return true if scale factor is odd number above 2
   return scaleFactor > 2 && scaleFactor % 2 === 1
 }
+
+function serveFileFromProtocol (protocolName, filePath) {
+  return new Promise((resolve, reject) => {
+    protocol.registerBufferProtocol(protocolName, (request, callback) => {
+      callback({
+        mimeType: 'text/html',
+        data: fs.readFileSync(filePath)
+      })
+    }, (error) => {
+      if (error != null) {
+        reject(error)
+      } else {
+        resolve()
+      }
+    })
+  })
+}

spec/fixtures/api/new-window-preload.js

@@ -0,0 +1,4 @@
+const {ipcRenderer, remote} = require('electron')
+
+ipcRenderer.send('answer', process.argv, remote.getCurrentWindow().webContents.getWebPreferences())
+window.close()

spec/fixtures/api/new-window.html

@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title></title>
+  </head>
+  <body>
+    <script type="text/javascript">
+      setTimeout(function () {
+        window.open('http://localhost')
+      })
+    </script>
+  </body>
+</html>

spec/fixtures/api/window-open-location-change.html

@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title></title>
+  </head>
+  <body>
+    foo
+    <script type="text/javascript">
+      setTimeout(function () {
+        window.location = 'bar://page'
+      })
+    </script>
+  </body>
+</html>

spec/fixtures/api/window-open-location-final.html

@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title></title>
+  </head>
+  <body>
+    bar
+  </body>
+</html>

spec/fixtures/api/window-open-location-open.html

@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title></title>
+  </head>
+  <body>
+    <script>
+      window.open('foo://page')
+    </script>
+  </body>
+</html>

spec/fixtures/api/window-open-preload.js

@@ -0,0 +1,8 @@
+const {ipcRenderer} = require('electron')
+
+setImmediate(function () {
+  if (window.location.toString() === 'bar://page') {
+    ipcRenderer.send('answer', process.argv, typeof global.process)
+    window.close()
+  }
+})

spec/static/main.js

@@ -264,6 +264,12 @@ ipcMain.on('prevent-next-new-window', (event, id) => {
   webContents.fromId(id).once('new-window', event => event.preventDefault())
 })
 
+ipcMain.on('set-web-preferences-on-next-new-window', (event, id, key, value) => {
+  webContents.fromId(id).once('new-window', (event, url, frameName, disposition, options) => {
+    options.webPreferences[key] = value
+  })
+})
+
 ipcMain.on('prevent-next-will-attach-webview', (event) => {
   event.sender.once('will-attach-webview', event => event.preventDefault())
 })