From 41a6b3a7bd539414d627b75e30db36acbffe499c 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 | 20 ++++++++++++++----- lib/renderer/api/remote.js | 7 +++++-- spec/api-remote-spec.js | 20 +++++++++++++++++++ .../module/preload-remote-function.js | 5 +++++ 4 files changed, 45 insertions(+), 7 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 68bc90b617c93..2bc0af4c66b75 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -152,9 +152,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) => { @@ -214,14 +215,14 @@ const unwrapArgs = function (sender, contextId, args) { return rendererFunctions.get(objectId) } - const processId = sender.getProcessId() const callIntoRenderer = function (...args) { - if (!sender.isDestroyed() && processId === sender.getProcessId()) { + if (!sender.isDestroyed()) { sender._sendInternal('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) @@ -281,6 +282,15 @@ const handleRemoteCommand = function (channel, handler) { }) } +handleRemoteCommand('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)) +}) + handleRemoteCommand('ELECTRON_BROWSER_REQUIRE', function (event, contextId, moduleName) { const customEvent = eventBinding.createWithSender(event.sender) event.sender.emit('remote-require', customEvent, moduleName) diff --git a/lib/renderer/api/remote.js b/lib/renderer/api/remote.js index 20562992dcdf4..036ebd8c360e6 100644 --- a/lib/renderer/api/remote.js +++ b/lib/renderer/api/remote.js @@ -270,9 +270,12 @@ function metaToPlainObject (meta) { } function handleMessage (channel, handler) { - ipcRenderer.on(channel, (event, passedContextId, ...args) => { + ipcRenderer.on(channel, (event, passedContextId, id, ...args) => { if (passedContextId === contextId) { - handler(...args) + handler(id, ...args) + } else { + // Message sent to an un-exist context, notify the error to main process. + ipcRenderer.send('ELECTRON_BROWSER_WRONG_CONTEXT_ERROR', contextId, passedContextId, id) } }) } diff --git a/spec/api-remote-spec.js b/spec/api-remote-spec.js index 080cea1f2c01a..1d0bf1a6ca845 100644 --- a/spec/api-remote-spec.js +++ b/spec/api-remote-spec.js @@ -8,6 +8,7 @@ const { closeWindow } = require('./window-helpers') const { resolveGetters } = require('./assert-helpers') const { remote, ipcRenderer } = require('electron') +const { ipcMain, BrowserWindow } = remote const { expect } = chai chai.use(dirtyChai) @@ -512,4 +513,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 a57726f5e23267cf5a55ec95f2e0f74977935b9c 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 16c0f202ee124..415d96b0c57f7 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -350,26 +350,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) => {