Merge pull request #9389 from electron/fix-remote-exception-sandbox

Keep track of remote object reference count.

lib/browser/rpc-server.js

@@ -394,6 +394,11 @@ ipcMain.on('ELECTRON_BROWSER_DEREFERENCE', function (event, id) {
   objectsRegistry.remove(event.sender.getId(), id)
 })
 
+ipcMain.on('ELECTRON_BROWSER_CONTEXT_RELEASE', (e) => {
+  objectsRegistry.clear(e.sender.getId())
+  e.returnValue = null
+})
+
 ipcMain.on('ELECTRON_BROWSER_GUEST_WEB_CONTENTS', function (event, guestInstanceId) {
   try {
     let guestViewManager = require('./guest-view-manager')

lib/renderer/api/remote.js

@@ -304,6 +304,10 @@ ipcRenderer.on('ELECTRON_RENDERER_RELEASE_CALLBACK', function (event, id) {
   callbacksRegistry.remove(id)
 })
 
+process.on('exit', () => {
+  ipcRenderer.sendSync('ELECTRON_BROWSER_CONTEXT_RELEASE')
+})
+
 // Get remote module.
 exports.require = function (module) {
   return metaToValue(ipcRenderer.sendSync('ELECTRON_BROWSER_REQUIRE', module))

spec/api-browser-window-spec.js

@@ -1161,7 +1161,6 @@ describe('BrowserWindow module', function () {
           }
         })
         w.loadURL('file://' + path.join(fixtures, 'api', 'sandbox.html?allocate-memory'))
-        w.webContents.openDevTools({mode: 'detach'})
         ipcMain.once('answer', function (event, {bytesBeforeOpen, bytesAfterOpen, bytesAfterClose}) {
           const memoryIncreaseByOpen = bytesAfterOpen - bytesBeforeOpen
           const memoryDecreaseByClose = bytesAfterOpen - bytesAfterClose
@@ -1173,6 +1172,73 @@ describe('BrowserWindow module', function () {
           done()
         })
       })
+
+      // see #9387
+      it('properly manages remote object references after page reload', (done) => {
+        w.destroy()
+        w = new BrowserWindow({
+          show: false,
+          webPreferences: {
+            preload: preload,
+            sandbox: true
+          }
+        })
+        w.loadURL('file://' + path.join(fixtures, 'api', 'sandbox.html?reload-remote'))
+
+        ipcMain.on('get-remote-module-path', (event) => {
+          event.returnValue = path.join(fixtures, 'module', 'hello.js')
+        })
+
+        let reload = false
+        ipcMain.on('reloaded', (event) => {
+          event.returnValue = reload
+          reload = !reload
+        })
+
+        ipcMain.once('reload', (event) => {
+          event.sender.reload()
+        })
+
+        ipcMain.once('answer', (event, arg) => {
+          ipcMain.removeAllListeners('reloaded')
+          ipcMain.removeAllListeners('get-remote-module-path')
+          assert.equal(arg, 'hi')
+          done()
+        })
+      })
+
+      it('properly manages remote object references after page reload in child window', (done) => {
+        w.destroy()
+        w = new BrowserWindow({
+          show: false,
+          webPreferences: {
+            preload: preload,
+            sandbox: true
+          }
+        })
+        w.loadURL('file://' + path.join(fixtures, 'api', 'sandbox.html?reload-remote-child'))
+
+        ipcMain.on('get-remote-module-path', (event) => {
+          event.returnValue = path.join(fixtures, 'module', 'hello-child.js')
+        })
+
+        let reload = false
+        ipcMain.on('reloaded', (event) => {
+          event.returnValue = reload
+          reload = !reload
+        })
+
+        ipcMain.once('reload', (event) => {
+          event.sender.reload()
+        })
+
+        ipcMain.once('answer', (event, arg) => {
+          ipcMain.removeAllListeners('reloaded')
+          ipcMain.removeAllListeners('get-remote-module-path')
+          assert.equal(arg, 'hi child window')
+          done()
+        })
+      })
     })
 
     describe('nativeWindowOpen option', () => {

spec/fixtures/api/sandbox.html

@@ -13,13 +13,28 @@
       await timeout(100)
     }
   }
-  if (window.opener) {
+
+  const [,test] = window.location.href.split('?')
+  if (window.opener && test !== 'reload-remote') {
     window.callback = () => {
       opener.require('electron').ipcRenderer.send('answer', document.body.innerHTML)
     }
   } else {
-    const {ipcRenderer} = require('electron')
+    const {ipcRenderer, remote} = require('electron')
     const tests = {
+      'reload-remote-child': () => {
+        open(`${location.protocol}//${location.pathname}?reload-remote`)
+      },
+      'reload-remote': async () => {
+        const p = ipcRenderer.sendSync('get-remote-module-path')
+        const Hello = remote.require(p)
+        if (!ipcRenderer.sendSync('reloaded')) {
+          ipcRenderer.send('reload')
+          return
+        }
+        await invokeGc()
+        ipcRenderer.send('answer', new Hello().say())
+      },
       'allocate-memory': async () => {
         await invokeGc()
         const {privateBytes: bytesBeforeOpen} = process.getProcessMemoryInfo()
@@ -95,7 +110,6 @@
         popup.close()
     }, false)
 
-    let [,test] = window.location.href.split('?')
     if (tests.hasOwnProperty(test))
       tests[test]()
   }

spec/fixtures/module/hello-child.js

@@ -0,0 +1,6 @@
+class Hello {
+  say () {
+    return 'hi child window'
+  }
+}
+module.exports = Hello

spec/fixtures/module/hello.js

@@ -0,0 +1,6 @@
+class Hello {
+  say () {
+    return 'hi'
+  }
+}
+module.exports = Hello

spec/fixtures/module/preload-sandbox.js

@@ -3,9 +3,9 @@
   const {ipcRenderer} = require('electron')
   window.ipcRenderer = ipcRenderer
   window.setImmediate = setImmediate
+  window.require = require
   if (location.protocol === 'file:') {
     window.test = 'preload'
-    window.require = require
     window.process = process
   } else if (location.href !== 'about:blank') {
     addEventListener('DOMContentLoaded', () => {