From 8b793b03dd9bfa862b49c9c160548efbf1c22e74 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 29 Oct 2018 16:25:44 +0900 Subject: [PATCH 1/2] fix: report wrong context error based on contextId --- lib/browser/rpc-server.js | 22 ++++++++++++++----- lib/renderer/api/remote.js | 2 ++ spec/api-remote-spec.js | 20 +++++++++++++++++ .../module/preload-remote-function.js | 5 +++++ 4 files changed, 43 insertions(+), 6 deletions(-) create mode 100644 spec/fixtures/module/preload-remote-function.js diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index 09ffb13dcb497..f3de32a3f4e14 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -148,9 +148,10 @@ const throwRPCError = function (message) { throw error } -const removeRemoteListenersAndLogWarning = (sender, meta, callIntoRenderer) => { +const removeRemoteListenersAndLogWarning = (sender, callIntoRenderer) => { + const location = v8Util.getHiddenValue(callIntoRenderer, 'location') let message = `Attempting to call a function in a renderer window that has been closed or released.` + - `\nFunction provided here: ${meta.location}` + `\nFunction provided here: ${location}` if (sender instanceof EventEmitter) { const remoteEvents = sender.eventNames().filter((eventName) => { @@ -213,14 +214,14 @@ const unwrapArgs = function (sender, contextId, args) { return rendererFunctions.get(objectId) } - const webContentsId = sender.getId() - let callIntoRenderer = function (...args) { - if (!sender.isDestroyed() && webContentsId === sender.getId()) { + const callIntoRenderer = function (...args) { + if (!sender.isDestroyed()) { sender.send('ELECTRON_RENDERER_CALLBACK', contextId, meta.id, valueToMeta(sender, contextId, args)) } else { - removeRemoteListenersAndLogWarning(this, meta, callIntoRenderer) + removeRemoteListenersAndLogWarning(this, callIntoRenderer) } } + v8Util.setHiddenValue(callIntoRenderer, 'location', meta.location) Object.defineProperty(callIntoRenderer, 'length', { value: meta.length }) v8Util.setRemoteCallbackFreer(callIntoRenderer, contextId, meta.id, sender) @@ -261,6 +262,15 @@ const callFunction = function (event, contextId, func, caller, args) { } } +ipcMain.on('ELECTRON_BROWSER_WRONG_CONTEXT_ERROR', function (event, contextId, passedContextId, id) { + const objectId = [passedContextId, id] + if (!rendererFunctions.has(objectId)) { + // Do nothing if the error has already been reported before. + return + } + removeRemoteListenersAndLogWarning(event.sender, rendererFunctions.get(objectId)) +}) + ipcMain.on('ELECTRON_BROWSER_REQUIRE', function (event, contextId, module) { try { event.returnValue = valueToMeta(event.sender, contextId, process.mainModule.require(module)) diff --git a/lib/renderer/api/remote.js b/lib/renderer/api/remote.js index 59117f8f2dd97..64d901202986d 100644 --- a/lib/renderer/api/remote.js +++ b/lib/renderer/api/remote.js @@ -279,6 +279,7 @@ function metaToException (meta) { ipcRenderer.on('ELECTRON_RENDERER_CALLBACK', (event, passedContextId, id, args) => { if (passedContextId !== contextId) { // The invoked callback belongs to an old page in this renderer. + ipcRenderer.send('ELECTRON_BROWSER_WRONG_CONTEXT_ERROR', contextId, passedContextId, id) return } callbacksRegistry.apply(id, metaToValue(args)) @@ -288,6 +289,7 @@ ipcRenderer.on('ELECTRON_RENDERER_CALLBACK', (event, passedContextId, id, args) ipcRenderer.on('ELECTRON_RENDERER_RELEASE_CALLBACK', (event, passedContextId, id) => { if (passedContextId !== contextId) { // The freed callback belongs to an old page in this renderer. + ipcRenderer.send('ELECTRON_BROWSER_WRONG_CONTEXT_ERROR', contextId, passedContextId, id) return } callbacksRegistry.remove(id) diff --git a/spec/api-remote-spec.js b/spec/api-remote-spec.js index 69b45ac8aa711..7285d90b5993f 100644 --- a/spec/api-remote-spec.js +++ b/spec/api-remote-spec.js @@ -5,6 +5,7 @@ const path = require('path') const {closeWindow} = require('./window-helpers') const {remote} = require('electron') +const {ipcMain, BrowserWindow} = remote const comparePaths = (path1, path2) => { if (process.platform === 'win32') { @@ -484,4 +485,23 @@ describe('remote module', () => { } }) }) + + describe('remote function in renderer', () => { + afterEach(() => { + ipcMain.removeAllListeners('done') + }) + + it('works when created in preload script', (done) => { + ipcMain.once('done', () => w.close()) + const preload = path.join(fixtures, 'module', 'preload-remote-function.js') + w = new BrowserWindow({ + show: false, + webPreferences: { + preload: preload + } + }) + w.once('closed', () => done()) + w.loadURL('about:blank') + }) + }) }) diff --git a/spec/fixtures/module/preload-remote-function.js b/spec/fixtures/module/preload-remote-function.js new file mode 100644 index 0000000000000..e9d5c3311c3ef --- /dev/null +++ b/spec/fixtures/module/preload-remote-function.js @@ -0,0 +1,5 @@ +const { remote, ipcRenderer } = require('electron') +remote.getCurrentWindow().rendererFunc = () => { + ipcRenderer.send('done') +} +remote.getCurrentWindow().rendererFunc() From 39422ad53e402d5fc32940eca456fbb53c3f6f1c Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 30 Oct 2018 09:25:24 +0900 Subject: [PATCH 2/2] fix: destroyed remote renderer warning is now async --- spec/static/main.js | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/spec/static/main.js b/spec/static/main.js index 4d5023e7af2cb..9abfb11c0a3a8 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -313,26 +313,21 @@ ipcMain.on('disable-preload-on-next-will-attach-webview', (event, id) => { ipcMain.on('try-emit-web-contents-event', (event, id, eventName) => { const consoleWarn = console.warn - let warningMessage = null const contents = webContents.fromId(id) const listenerCountBefore = contents.listenerCount(eventName) - try { - console.warn = (message) => { - warningMessage = message - } - contents.emit(eventName, {sender: contents}) - } finally { + console.warn = (warningMessage) => { console.warn = consoleWarn - } - - const listenerCountAfter = contents.listenerCount(eventName) - event.returnValue = { - warningMessage, - listenerCountBefore, - listenerCountAfter + const listenerCountAfter = contents.listenerCount(eventName) + event.returnValue = { + warningMessage, + listenerCountBefore, + listenerCountAfter + } } + + contents.emit(eventName, {sender: contents}) }) ipcMain.on('handle-uncaught-exception', (event, message) => {