fix: load window-setup in sandboxed renderer (#21416)

filenames.auto.gni

@@ -165,6 +165,7 @@ auto_filenames = {
     "lib/renderer/web-view/web-view-element.ts",
     "lib/renderer/web-view/web-view-impl.ts",
     "lib/renderer/web-view/web-view-init.ts",
+    "lib/renderer/window-setup.ts",
     "lib/sandboxed_renderer/api/exports/electron.ts",
     "lib/sandboxed_renderer/api/module-list.ts",
     "lib/sandboxed_renderer/init.js",

lib/browser/guest-window-manager.js

@@ -75,8 +75,10 @@ const mergeBrowserWindowOptions = function (embedder, options) {
     }
   }
 
-  // Sets correct openerId here to give correct options to 'new-window' event handler
-  options.webPreferences.openerId = embedder.id
+  if (!webPreferences.nativeWindowOpen) {
+    // Sets correct openerId here to give correct options to 'new-window' event handler
+    options.webPreferences.openerId = embedder.id
+  }
 
   return options
 }

lib/browser/rpc-server.js

@@ -112,11 +112,15 @@ ipcMainUtils.handleSync('ELECTRON_BROWSER_SANDBOX_LOAD', async function (event)
     contentScripts = getContentScripts()
   }
 
+  const webPreferences = event.sender.getLastWebPreferences() || {}
+
   return {
     contentScripts,
     preloadScripts: await Promise.all(preloadPaths.map(path => getPreloadScript(path))),
     isRemoteModuleEnabled: isRemoteModuleEnabled(event.sender),
     isWebViewTagEnabled: guestViewManager.isWebViewTagEnabled(event.sender),
+    guestInstanceId: webPreferences.guestInstanceId,
+    openerId: webPreferences.openerId,
     process: {
       arch: process.arch,
       platform: process.platform,

lib/renderer/window-setup.ts

@@ -177,7 +177,7 @@ class BrowserWindowProxy {
 export const windowSetup = (
   guestInstanceId: number, openerId: number, isHiddenPage: boolean, usesNativeWindowOpen: boolean
 ) => {
-  if (guestInstanceId == null) {
+  if (!process.sandboxed && guestInstanceId == null) {
     // Override default window.close.
     window.close = function () {
       ipcRendererInternal.sendSync('ELECTRON_BROWSER_WINDOW_CLOSE')
@@ -197,53 +197,57 @@ export const windowSetup = (
         return null
       }
     }
+  }
 
-    if (openerId != null) {
-      window.opener = getOrCreateProxy(openerId)
-    }
+  if (openerId != null) {
+    window.opener = getOrCreateProxy(openerId)
   }
 
   // But we do not support prompt().
   window.prompt = function () {
     throw new Error('prompt() is and will not be supported.')
   }
 
-  ipcRendererInternal.on('ELECTRON_GUEST_WINDOW_POSTMESSAGE', function (
-    _event, sourceId: number, message: any, sourceOrigin: string
-  ) {
-    // Manually dispatch event instead of using postMessage because we also need to
-    // set event.source.
-    //
-    // Why any? We can't construct a MessageEvent and we can't
-    // use `as MessageEvent` because you're not supposed to override
-    // data, origin, and source
-    const event: any = document.createEvent('Event')
-    event.initEvent('message', false, false)
-
-    event.data = message
-    event.origin = sourceOrigin
-    event.source = getOrCreateProxy(sourceId)
-
-    window.dispatchEvent(event as MessageEvent)
-  })
-
-  window.history.back = function () {
-    ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_BACK')
+  if (!usesNativeWindowOpen || openerId != null) {
+    ipcRendererInternal.on('ELECTRON_GUEST_WINDOW_POSTMESSAGE', function (
+      _event, sourceId: number, message: any, sourceOrigin: string
+    ) {
+      // Manually dispatch event instead of using postMessage because we also need to
+      // set event.source.
+      //
+      // Why any? We can't construct a MessageEvent and we can't
+      // use `as MessageEvent` because you're not supposed to override
+      // data, origin, and source
+      const event: any = document.createEvent('Event')
+      event.initEvent('message', false, false)
+
+      event.data = message
+      event.origin = sourceOrigin
+      event.source = getOrCreateProxy(sourceId)
+
+      window.dispatchEvent(event as MessageEvent)
+    })
   }
 
-  window.history.forward = function () {
-    ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_FORWARD')
-  }
+  if (!process.sandboxed) {
+    window.history.back = function () {
+      ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_BACK')
+    }
 
-  window.history.go = function (offset: number) {
-    ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_TO_OFFSET', +offset)
-  }
+    window.history.forward = function () {
+      ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_FORWARD')
+    }
 
-  Object.defineProperty(window.history, 'length', {
-    get: function () {
-      return ipcRendererInternal.sendSync('ELECTRON_NAVIGATION_CONTROLLER_LENGTH')
+    window.history.go = function (offset: number) {
+      ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_TO_OFFSET', +offset)
     }
-  })
+
+    Object.defineProperty(window.history, 'length', {
+      get: function () {
+        return ipcRendererInternal.sendSync('ELECTRON_NAVIGATION_CONTROLLER_LENGTH')
+      }
+    })
+  }
 
   if (guestInstanceId != null) {
     // Webview `document.visibilityState` tracks window visibility (and ignores

lib/sandboxed_renderer/init.js

@@ -30,7 +30,13 @@ const { ipcRendererInternal } = require('@electron/internal/renderer/ipc-rendere
 const ipcRendererUtils = require('@electron/internal/renderer/ipc-renderer-internal-utils')
 
 const {
-  contentScripts, preloadScripts, isRemoteModuleEnabled, isWebViewTagEnabled, process: processProps
+  contentScripts,
+  preloadScripts,
+  isRemoteModuleEnabled,
+  isWebViewTagEnabled,
+  guestInstanceId,
+  openerId,
+  process: processProps
 } = ipcRendererUtils.invokeSync('ELECTRON_BROWSER_SANDBOX_LOAD')
 
 process.isRemoteModuleEnabled = isRemoteModuleEnabled
@@ -109,6 +115,11 @@ function preloadRequire (module) {
 const { hasSwitch } = process.electronBinding('command_line')
 
 const contextIsolation = hasSwitch('context-isolation')
+const isHiddenPage = hasSwitch('hidden-page')
+const usesNativeWindowOpen = true
+
+// The arguments to be passed to isolated world.
+const isolatedWorldArgs = { ipcRendererInternal, guestInstanceId, isHiddenPage, openerId, usesNativeWindowOpen }
 
 switch (window.location.protocol) {
   case 'devtools:': {
@@ -127,21 +138,28 @@ switch (window.location.protocol) {
     break
   }
   default: {
+    // Override default web functions.
+    const { windowSetup } = require('@electron/internal/renderer/window-setup')
+    windowSetup(guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen)
+
     // Inject content scripts.
     if (!process.electronBinding('features').isExtensionsEnabled()) {
       require('@electron/internal/renderer/content-scripts-injector')(contentScripts)
     }
   }
 }
 
-const guestInstanceId = binding.guestInstanceId && parseInt(binding.guestInstanceId)
-
 // Load webview tag implementation.
 if (process.isMainFrame) {
   const { webViewInit } = require('@electron/internal/renderer/web-view/web-view-init')
   webViewInit(contextIsolation, isWebViewTagEnabled, guestInstanceId)
 }
 
+// Pass the arguments to isolatedWorld.
+if (contextIsolation) {
+  v8Util.setHiddenValue(global, 'isolated-world-args', isolatedWorldArgs)
+}
+
 // Wrap the script into a function executed in global scope. It won't have
 // access to the current scope, so we'll expose a few objects as arguments:
 //

shell/renderer/atom_sandboxed_renderer_client.cc

@@ -146,12 +146,6 @@ void AtomSandboxedRendererClient::InitializeBindings(
   process.SetReadOnly("sandboxed", true);
   process.SetReadOnly("type", "renderer");
   process.SetReadOnly("isMainFrame", is_main_frame);
-
-  // Pass in CLI flags needed to setup the renderer
-  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(switches::kGuestInstanceID))
-    b.Set(options::kGuestInstanceID,
-          command_line->GetSwitchValueASCII(switches::kGuestInstanceID));
 }
 
 void AtomSandboxedRendererClient::RenderFrameCreated(

spec-main/chromium-spec.ts

@@ -768,23 +768,28 @@ describe('chromium features', () => {
     afterEach(closeAllWindows)
 
     describe('when opened from main window', () => {
-      for (const {parent, child, nodeIntegration, nativeWindowOpen, openerAccessible} of table) {
-        const description = `when parent=${s(parent)} opens child=${s(child)} with nodeIntegration=${nodeIntegration} nativeWindowOpen=${nativeWindowOpen}, child should ${openerAccessible ? '' : 'not '}be able to access opener`
-        it(description, async () => {
-          const w = new BrowserWindow({show: false, webPreferences: { nodeIntegration: true, nativeWindowOpen }})
-          await w.loadURL(parent)
-          const childOpenerLocation = await w.webContents.executeJavaScript(`new Promise(resolve => {
-            window.addEventListener('message', function f(e) {
-              resolve(e.data)
+      for (const { parent, child, nodeIntegration, nativeWindowOpen, openerAccessible } of table) {
+        for (const sandboxPopup of [false, true]) {
+          const description = `when parent=${s(parent)} opens child=${s(child)} with nodeIntegration=${nodeIntegration} nativeWindowOpen=${nativeWindowOpen} sandboxPopup=${sandboxPopup}, child should ${openerAccessible ? '' : 'not '}be able to access opener`
+          it(description, async () => {
+            const w = new BrowserWindow({show: false, webPreferences: { nodeIntegration: true, nativeWindowOpen }})
+            w.webContents.once('new-window', (e, url, frameName, disposition, options) => {
+              options!.webPreferences!.sandbox = sandboxPopup
             })
-            window.open(${JSON.stringify(child)}, "", "show=no,nodeIntegration=${nodeIntegration ? "yes" : "no"}")
-          })`)
-          if (openerAccessible) {
-            expect(childOpenerLocation).to.be.a('string')
-          } else {
-            expect(childOpenerLocation).to.be.null()
-          }
-        })
+            await w.loadURL(parent)
+            const childOpenerLocation = await w.webContents.executeJavaScript(`new Promise(resolve => {
+              window.addEventListener('message', function f(e) {
+                resolve(e.data)
+              })
+              window.open(${JSON.stringify(child)}, "", "show=no,nodeIntegration=${nodeIntegration ? "yes" : "no"}")
+            })`)
+            if (openerAccessible) {
+              expect(childOpenerLocation).to.be.a('string')
+            } else {
+              expect(childOpenerLocation).to.be.null()
+            }
+          })
+        }
       }
     })