From 26e3ad3c2ef38ee5169922f8310f3bb52ead7a4a Mon Sep 17 00:00:00 2001 From: Charlie Hess Date: Fri, 6 Jan 2017 13:09:48 -0800 Subject: [PATCH 01/14] Display more information about remote event crashes. If possible, we'll dig into the function args and print the ones that are attached remotely. --- lib/browser/rpc-server.js | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index b12b518888a2..f04a9c17078c 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -146,6 +146,28 @@ const throwRPCError = function (message) { throw error } +const rendererMissingErrorMessage = function (meta, args, callIntoRenderer) { + let message = `Attempting to call a function in a renderer window that has been closed or released.` + + `Function provided here: ${meta.location}.`; + + if (!args || args.length === 0) return message; + if (!args[0].sender || !args[0].sender._events) return message; + + const eventsAttached = args[0].sender._events; + const remoteEvents = Object.keys(eventsAttached).filter((eventName) => { + return Array.isArray(eventsAttached[eventName]) ? + eventsAttached[eventName].includes(callIntoRenderer) : + eventsAttached[eventName] === callIntoRenderer + }); + + if (remoteEvents && remoteEvents.length > 0) { + message += `\nRemote event names:` + remoteEvents.forEach((eventName) => message += ` ${eventName} `); + } + + return message; +} + // Convert array of meta data from renderer into array of real values. const unwrapArgs = function (sender, args) { const metaToValue = function (meta) { @@ -196,7 +218,7 @@ const unwrapArgs = function (sender, args) { if (!sender.isDestroyed() && webContentsId === sender.getId()) { sender.send('ELECTRON_RENDERER_CALLBACK', meta.id, valueToMeta(sender, args)) } else { - throw new Error(`Attempting to call a function in a renderer window that has been closed or released. Function provided here: ${meta.location}.`) + throw new Error(rendererMissingErrorMessage(meta, args, callIntoRenderer)); } } From 49c64462671c845da8a098dac0b74065e7526d02 Mon Sep 17 00:00:00 2001 From: Charlie Hess Date: Fri, 6 Jan 2017 14:14:52 -0800 Subject: [PATCH 02/14] L I N T I N G --- lib/browser/rpc-server.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index f04a9c17078c..c67e7efb93de 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -148,24 +148,24 @@ const throwRPCError = function (message) { const rendererMissingErrorMessage = function (meta, args, callIntoRenderer) { let message = `Attempting to call a function in a renderer window that has been closed or released.` + - `Function provided here: ${meta.location}.`; + `\nFunction provided here: ${meta.location}` - if (!args || args.length === 0) return message; - if (!args[0].sender || !args[0].sender._events) return message; + if (!args || args.length === 0) return message + if (!args[0].sender || !args[0].sender._events) return message - const eventsAttached = args[0].sender._events; + const eventsAttached = args[0].sender._events const remoteEvents = Object.keys(eventsAttached).filter((eventName) => { return Array.isArray(eventsAttached[eventName]) ? eventsAttached[eventName].includes(callIntoRenderer) : eventsAttached[eventName] === callIntoRenderer - }); + }) if (remoteEvents && remoteEvents.length > 0) { message += `\nRemote event names:` - remoteEvents.forEach((eventName) => message += ` ${eventName} `); + remoteEvents.forEach((eventName) => message += ` ${eventName} `) } - return message; + return message } // Convert array of meta data from renderer into array of real values. @@ -218,7 +218,7 @@ const unwrapArgs = function (sender, args) { if (!sender.isDestroyed() && webContentsId === sender.getId()) { sender.send('ELECTRON_RENDERER_CALLBACK', meta.id, valueToMeta(sender, args)) } else { - throw new Error(rendererMissingErrorMessage(meta, args, callIntoRenderer)); + throw new Error(rendererMissingErrorMessage(meta, args, callIntoRenderer)) } } From 41ea1697842043de5dd814b6beddc8b31600abf4 Mon Sep 17 00:00:00 2001 From: Charlie Hess Date: Fri, 6 Jan 2017 14:18:14 -0800 Subject: [PATCH 03/14] L I N T I N G P T I I --- lib/browser/rpc-server.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index c67e7efb93de..38dc0b8711f2 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -155,14 +155,16 @@ const rendererMissingErrorMessage = function (meta, args, callIntoRenderer) { const eventsAttached = args[0].sender._events const remoteEvents = Object.keys(eventsAttached).filter((eventName) => { - return Array.isArray(eventsAttached[eventName]) ? - eventsAttached[eventName].includes(callIntoRenderer) : - eventsAttached[eventName] === callIntoRenderer + return Array.isArray(eventsAttached[eventName]) + ? eventsAttached[eventName].includes(callIntoRenderer) + : eventsAttached[eventName] === callIntoRenderer }) if (remoteEvents && remoteEvents.length > 0) { message += `\nRemote event names:` - remoteEvents.forEach((eventName) => message += ` ${eventName} `) + remoteEvents.forEach((eventName) => { + message += ` ${eventName} ` + }) } return message From 67f7a60524f7126bee657cdd2f4eb5485681359e Mon Sep 17 00:00:00 2001 From: Charlie Hess Date: Wed, 11 Jan 2017 15:32:43 -0800 Subject: [PATCH 04/14] Review CommentZ --- lib/browser/rpc-server.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index 38dc0b8711f2..f0b7a2a60d0c 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -146,11 +146,11 @@ const throwRPCError = function (message) { throw error } -const rendererMissingErrorMessage = function (meta, args, callIntoRenderer) { +const rendererMissingErrorMessage = (meta, args, callIntoRenderer) => { let message = `Attempting to call a function in a renderer window that has been closed or released.` + `\nFunction provided here: ${meta.location}` - if (!args || args.length === 0) return message + if (args.length === 0) return message if (!args[0].sender || !args[0].sender._events) return message const eventsAttached = args[0].sender._events @@ -160,7 +160,7 @@ const rendererMissingErrorMessage = function (meta, args, callIntoRenderer) { : eventsAttached[eventName] === callIntoRenderer }) - if (remoteEvents && remoteEvents.length > 0) { + if (remoteEvents.length > 0) { message += `\nRemote event names:` remoteEvents.forEach((eventName) => { message += ` ${eventName} ` From a0b24bd155437d7a85d0238e572abbabf8ce0f42 Mon Sep 17 00:00:00 2001 From: Charlie Hess Date: Wed, 11 Jan 2017 15:33:44 -0800 Subject: [PATCH 05/14] Simplify. --- lib/browser/rpc-server.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index f0b7a2a60d0c..d097d9103dd8 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -161,10 +161,7 @@ const rendererMissingErrorMessage = (meta, args, callIntoRenderer) => { }) if (remoteEvents.length > 0) { - message += `\nRemote event names:` - remoteEvents.forEach((eventName) => { - message += ` ${eventName} ` - }) + message += `\nRemote event names: ${remoteEvents.join(' ')}` } return message From b04db2e546c3f56ff9b2fef5f664e49896ba4e8c Mon Sep 17 00:00:00 2001 From: Charlie Hess Date: Thu, 12 Jan 2017 15:02:24 -0800 Subject: [PATCH 06/14] Add a spec for the "renderer window closed" error. --- spec/api-ipc-spec.js | 12 ++++++++++++ spec/fixtures/api/reload-page.html | 17 +++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 spec/fixtures/api/reload-page.html diff --git a/spec/api-ipc-spec.js b/spec/api-ipc-spec.js index 76310f47812a..eab88033bcca 100644 --- a/spec/api-ipc-spec.js +++ b/spec/api-ipc-spec.js @@ -494,6 +494,18 @@ describe('ipc module', function () { w.removeListener('test', listener) assert.equal(w.listenerCount('test'), 0) }) + + it('throws an error when a function is called in a destroyed renderer', (done) => { + w = new BrowserWindow({ + show: false + }) + w.loadURL('file://' + path.join(fixtures, 'api', 'reload-page.html')) + setTimeout(() => { + assert.throws(() => w.webContents.emit('remote-handler'), + /Attempting to call a function in a renderer window that has been closed or released./) + done() + }, 400) + }) }) it('throws an error when removing all the listeners', () => { diff --git a/spec/fixtures/api/reload-page.html b/spec/fixtures/api/reload-page.html new file mode 100644 index 000000000000..240548cb8d8f --- /dev/null +++ b/spec/fixtures/api/reload-page.html @@ -0,0 +1,17 @@ + + + + + + + + + + + From 45986405b85c663d18a3ac30c966c3eee1101d6e Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 12 Jan 2017 16:31:02 -0800 Subject: [PATCH 07/14] Assert remote event names --- lib/browser/rpc-server.js | 2 +- spec/api-ipc-spec.js | 20 +++++++++++++------ ...ad-page.html => remote-event-handler.html} | 5 +++-- spec/static/main.js | 10 ++++++++++ 4 files changed, 28 insertions(+), 9 deletions(-) rename spec/fixtures/api/{reload-page.html => remote-event-handler.html} (60%) diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index d097d9103dd8..5a871709ecea 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -161,7 +161,7 @@ const rendererMissingErrorMessage = (meta, args, callIntoRenderer) => { }) if (remoteEvents.length > 0) { - message += `\nRemote event names: ${remoteEvents.join(' ')}` + message += `\nRemote event names: ${remoteEvents.join(', ')}` } return message diff --git a/spec/api-ipc-spec.js b/spec/api-ipc-spec.js index eab88033bcca..47f9b98bf52b 100644 --- a/spec/api-ipc-spec.js +++ b/spec/api-ipc-spec.js @@ -499,12 +499,20 @@ describe('ipc module', function () { w = new BrowserWindow({ show: false }) - w.loadURL('file://' + path.join(fixtures, 'api', 'reload-page.html')) - setTimeout(() => { - assert.throws(() => w.webContents.emit('remote-handler'), - /Attempting to call a function in a renderer window that has been closed or released./) - done() - }, 400) + w.webContents.once('did-finish-load', () => { + w.webContents.once('did-finish-load', () => { + const expectedMessage = [ + 'Attempting to call a function in a renderer window that has been closed or released.', + 'Function provided here: remote-event-handler.html:11:33', + 'Remote event names: remote-handler, other-remote-handler' + ].join('\n') + const errorMessage = ipcRenderer.sendSync('try-emit-web-contents-event', w.webContents.id, 'remote-handler') + assert.equal(errorMessage, expectedMessage) + done() + }) + w.webContents.reload() + }) + w.loadURL('file://' + path.join(fixtures, 'api', 'remote-event-handler.html')) }) }) diff --git a/spec/fixtures/api/reload-page.html b/spec/fixtures/api/remote-event-handler.html similarity index 60% rename from spec/fixtures/api/reload-page.html rename to spec/fixtures/api/remote-event-handler.html index 240548cb8d8f..30c3cfb36ad7 100644 --- a/spec/fixtures/api/reload-page.html +++ b/spec/fixtures/api/remote-event-handler.html @@ -7,8 +7,9 @@ const {remote} = require('electron') const browserWindow = remote.getCurrentWindow() - browserWindow.webContents.on('remote-handler', () => { }) - browserWindow.reload() + const handler = () => {} + browserWindow.webContents.on('remote-handler', handler) + browserWindow.webContents.on('other-remote-handler', handler) diff --git a/spec/static/main.js b/spec/static/main.js index 9612ed25cf05..cb14f9d9aec5 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -249,3 +249,13 @@ ipcMain.on('create-window-with-options-cycle', (event) => { ipcMain.on('prevent-next-new-window', (event, id) => { webContents.fromId(id).once('new-window', event => event.preventDefault()) }) + +ipcMain.on('try-emit-web-contents-event', (event, id, eventName) => { + const contents = webContents.fromId(id) + try { + contents.emit(eventName, {sender: contents}) + event.returnValue = null + } catch (error) { + event.returnValue = error.message + } +}) From 63d8137da2b9b63260b525fb999e28daa118f6bc Mon Sep 17 00:00:00 2001 From: Charlie Hess Date: Thu, 12 Jan 2017 17:20:50 -0800 Subject: [PATCH 08/14] Use EventEmitter public methods instead of _events. --- lib/browser/rpc-server.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index 5a871709ecea..42e91ab290cd 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -151,13 +151,11 @@ const rendererMissingErrorMessage = (meta, args, callIntoRenderer) => { `\nFunction provided here: ${meta.location}` if (args.length === 0) return message - if (!args[0].sender || !args[0].sender._events) return message + if (!args[0].sender || !args[0].sender.eventNames) return message - const eventsAttached = args[0].sender._events - const remoteEvents = Object.keys(eventsAttached).filter((eventName) => { - return Array.isArray(eventsAttached[eventName]) - ? eventsAttached[eventName].includes(callIntoRenderer) - : eventsAttached[eventName] === callIntoRenderer + const sender = args[0].sender + const remoteEvents = sender.eventNames().filter((eventName) => { + return sender.listeners(eventName).includes(callIntoRenderer); }) if (remoteEvents.length > 0) { From bc2f1e81990719743876105bfebd655fd6bb5c42 Mon Sep 17 00:00:00 2001 From: Charlie Hess Date: Thu, 12 Jan 2017 18:23:04 -0800 Subject: [PATCH 09/14] Argh. --- lib/browser/rpc-server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index 42e91ab290cd..d6b9e8a61c3f 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -155,7 +155,7 @@ const rendererMissingErrorMessage = (meta, args, callIntoRenderer) => { const sender = args[0].sender const remoteEvents = sender.eventNames().filter((eventName) => { - return sender.listeners(eventName).includes(callIntoRenderer); + return sender.listeners(eventName).includes(callIntoRenderer) }) if (remoteEvents.length > 0) { From c213971a2dc87554ce70b9019d04e93cd5311c0f Mon Sep 17 00:00:00 2001 From: Charlie Hess Date: Fri, 13 Jan 2017 12:28:11 -0800 Subject: [PATCH 10/14] Write a warning instead of crashing. --- lib/browser/rpc-server.js | 5 ++++- spec/api-ipc-spec.js | 6 +++--- spec/static/main.js | 14 ++++++++------ 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index d6b9e8a61c3f..257c8ac575d9 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -160,6 +160,9 @@ const rendererMissingErrorMessage = (meta, args, callIntoRenderer) => { if (remoteEvents.length > 0) { message += `\nRemote event names: ${remoteEvents.join(', ')}` + remoteEvents.forEach((eventName) => { + sender.removeListener(eventName, callIntoRenderer) + }) } return message @@ -215,7 +218,7 @@ const unwrapArgs = function (sender, args) { if (!sender.isDestroyed() && webContentsId === sender.getId()) { sender.send('ELECTRON_RENDERER_CALLBACK', meta.id, valueToMeta(sender, args)) } else { - throw new Error(rendererMissingErrorMessage(meta, args, callIntoRenderer)) + console.warn(rendererMissingErrorMessage(meta, args, callIntoRenderer)) } } diff --git a/spec/api-ipc-spec.js b/spec/api-ipc-spec.js index 47f9b98bf52b..9f4ce0fb1ebc 100644 --- a/spec/api-ipc-spec.js +++ b/spec/api-ipc-spec.js @@ -495,7 +495,7 @@ describe('ipc module', function () { assert.equal(w.listenerCount('test'), 0) }) - it('throws an error when a function is called in a destroyed renderer', (done) => { + it('shows a warning when a function is called in a destroyed renderer', (done) => { w = new BrowserWindow({ show: false }) @@ -506,8 +506,8 @@ describe('ipc module', function () { 'Function provided here: remote-event-handler.html:11:33', 'Remote event names: remote-handler, other-remote-handler' ].join('\n') - const errorMessage = ipcRenderer.sendSync('try-emit-web-contents-event', w.webContents.id, 'remote-handler') - assert.equal(errorMessage, expectedMessage) + const warningMessage = ipcRenderer.sendSync('try-emit-web-contents-event', w.webContents.id, 'remote-handler') + assert.equal(warningMessage, expectedMessage) done() }) w.webContents.reload() diff --git a/spec/static/main.js b/spec/static/main.js index cb14f9d9aec5..a0ce1c24d380 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -251,11 +251,13 @@ ipcMain.on('prevent-next-new-window', (event, id) => { }) ipcMain.on('try-emit-web-contents-event', (event, id, eventName) => { - const contents = webContents.fromId(id) - try { - contents.emit(eventName, {sender: contents}) - event.returnValue = null - } catch (error) { - event.returnValue = error.message + const consoleWarn = console.warn + let lastWarning = null + console.warn = (message) => { + lastWarning = message } + const contents = webContents.fromId(id) + contents.emit(eventName, {sender: contents}) + event.returnValue = lastWarning + console.warn = consoleWarn }) From f6410d3b77e3bc149564b9e5c84a1f80ed3d6555 Mon Sep 17 00:00:00 2001 From: Charlie Hess Date: Wed, 18 Jan 2017 12:44:26 -0800 Subject: [PATCH 11/14] Assert that listener count decreases after a remove event. --- lib/browser/rpc-server.js | 4 ++-- spec/api-ipc-spec.js | 6 ++++-- spec/static/main.js | 14 +++++++++++--- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index 257c8ac575d9..306388f340d3 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -146,7 +146,7 @@ const throwRPCError = function (message) { throw error } -const rendererMissingErrorMessage = (meta, args, callIntoRenderer) => { +const removeRemoteListenersAndLogWarning = (meta, args, callIntoRenderer) => { let message = `Attempting to call a function in a renderer window that has been closed or released.` + `\nFunction provided here: ${meta.location}` @@ -218,7 +218,7 @@ const unwrapArgs = function (sender, args) { if (!sender.isDestroyed() && webContentsId === sender.getId()) { sender.send('ELECTRON_RENDERER_CALLBACK', meta.id, valueToMeta(sender, args)) } else { - console.warn(rendererMissingErrorMessage(meta, args, callIntoRenderer)) + console.warn(removeRemoteListenersAndLogWarning(meta, args, callIntoRenderer)) } } diff --git a/spec/api-ipc-spec.js b/spec/api-ipc-spec.js index 9f4ce0fb1ebc..32cc25041c52 100644 --- a/spec/api-ipc-spec.js +++ b/spec/api-ipc-spec.js @@ -495,7 +495,7 @@ describe('ipc module', function () { assert.equal(w.listenerCount('test'), 0) }) - it('shows a warning when a function is called in a destroyed renderer', (done) => { + it('detaches listeners subscribed to destroyed renderers, and shows a warning', (done) => { w = new BrowserWindow({ show: false }) @@ -506,8 +506,10 @@ describe('ipc module', function () { 'Function provided here: remote-event-handler.html:11:33', 'Remote event names: remote-handler, other-remote-handler' ].join('\n') - const warningMessage = ipcRenderer.sendSync('try-emit-web-contents-event', w.webContents.id, 'remote-handler') + const {warningMessage, listenerCountBefore, listenerCountAfter} = + ipcRenderer.sendSync('try-emit-web-contents-event', w.webContents.id, 'remote-handler') assert.equal(warningMessage, expectedMessage) + assert.equal(listenerCountAfter, listenerCountBefore - 1) done() }) w.webContents.reload() diff --git a/spec/static/main.js b/spec/static/main.js index a0ce1c24d380..50fe5352eb5f 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -252,12 +252,20 @@ ipcMain.on('prevent-next-new-window', (event, id) => { ipcMain.on('try-emit-web-contents-event', (event, id, eventName) => { const consoleWarn = console.warn - let lastWarning = null + let warningMessage = null console.warn = (message) => { - lastWarning = message + warningMessage = message } + const contents = webContents.fromId(id) + const listenerCountBefore = contents.listenerCount(eventName) contents.emit(eventName, {sender: contents}) - event.returnValue = lastWarning + const listenerCountAfter = contents.listenerCount(eventName) + + event.returnValue = { + warningMessage, + listenerCountBefore, + listenerCountAfter + } console.warn = consoleWarn }) From ddedcf22d1d9893ea404616ecb904577cdaccc67 Mon Sep 17 00:00:00 2001 From: Charlie Hess Date: Wed, 18 Jan 2017 16:09:46 -0800 Subject: [PATCH 12/14] Move console.warn inside the helper method. --- lib/browser/rpc-server.js | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index 306388f340d3..1277e51ff439 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -150,22 +150,21 @@ const removeRemoteListenersAndLogWarning = (meta, args, callIntoRenderer) => { let message = `Attempting to call a function in a renderer window that has been closed or released.` + `\nFunction provided here: ${meta.location}` - if (args.length === 0) return message - if (!args[0].sender || !args[0].sender.eventNames) return message - - const sender = args[0].sender - const remoteEvents = sender.eventNames().filter((eventName) => { - return sender.listeners(eventName).includes(callIntoRenderer) - }) - - if (remoteEvents.length > 0) { - message += `\nRemote event names: ${remoteEvents.join(', ')}` - remoteEvents.forEach((eventName) => { - sender.removeListener(eventName, callIntoRenderer) + if (args.length > 0 && args[0].sender && args[0].sender.eventNames) { + const sender = args[0].sender + const remoteEvents = sender.eventNames().filter((eventName) => { + return sender.listeners(eventName).includes(callIntoRenderer) }) + + if (remoteEvents.length > 0) { + message += `\nRemote event names: ${remoteEvents.join(', ')}` + remoteEvents.forEach((eventName) => { + sender.removeListener(eventName, callIntoRenderer) + }) + } } - return message + console.warn(message) } // Convert array of meta data from renderer into array of real values. @@ -218,7 +217,7 @@ const unwrapArgs = function (sender, args) { if (!sender.isDestroyed() && webContentsId === sender.getId()) { sender.send('ELECTRON_RENDERER_CALLBACK', meta.id, valueToMeta(sender, args)) } else { - console.warn(removeRemoteListenersAndLogWarning(meta, args, callIntoRenderer)) + removeRemoteListenersAndLogWarning(meta, args, callIntoRenderer) } } From 56a8eb3a946c2855aef425575cba5391ea8c1e42 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 24 Jan 2017 15:05:34 -0800 Subject: [PATCH 13/14] Check that sender is a WebContents instance --- lib/browser/rpc-server.js | 6 ++++-- spec/api-ipc-spec.js | 10 ++++++---- spec/static/main.js | 14 ++++++++------ 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index 1277e51ff439..1dac51645926 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -3,6 +3,8 @@ const {Buffer} = require('buffer') const electron = require('electron') const v8Util = process.atomBinding('v8_util') +const {WebContents} = process.atomBinding('web_contents') + const {ipcMain, isPromise, webContents} = electron const fs = require('fs') @@ -150,8 +152,8 @@ const removeRemoteListenersAndLogWarning = (meta, args, callIntoRenderer) => { let message = `Attempting to call a function in a renderer window that has been closed or released.` + `\nFunction provided here: ${meta.location}` - if (args.length > 0 && args[0].sender && args[0].sender.eventNames) { - const sender = args[0].sender + if (args.length > 0 && (args[0].sender instanceof WebContents)) { + const {sender} = args[0] const remoteEvents = sender.eventNames().filter((eventName) => { return sender.listeners(eventName).includes(callIntoRenderer) }) diff --git a/spec/api-ipc-spec.js b/spec/api-ipc-spec.js index 32cc25041c52..b1ca29c6a158 100644 --- a/spec/api-ipc-spec.js +++ b/spec/api-ipc-spec.js @@ -506,10 +506,12 @@ describe('ipc module', function () { 'Function provided here: remote-event-handler.html:11:33', 'Remote event names: remote-handler, other-remote-handler' ].join('\n') - const {warningMessage, listenerCountBefore, listenerCountAfter} = - ipcRenderer.sendSync('try-emit-web-contents-event', w.webContents.id, 'remote-handler') - assert.equal(warningMessage, expectedMessage) - assert.equal(listenerCountAfter, listenerCountBefore - 1) + const results = ipcRenderer.sendSync('try-emit-web-contents-event', w.webContents.id, 'remote-handler') + assert.deepEqual(results, { + warningMessage: expectedMessage, + listenerCountBefore: 2, + listenerCountAfter: 1 + }) done() }) w.webContents.reload() diff --git a/spec/static/main.js b/spec/static/main.js index 50fe5352eb5f..ab16e19c9e3c 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -253,13 +253,16 @@ ipcMain.on('prevent-next-new-window', (event, id) => { ipcMain.on('try-emit-web-contents-event', (event, id, eventName) => { const consoleWarn = console.warn let warningMessage = null - console.warn = (message) => { - warningMessage = message - } - const contents = webContents.fromId(id) const listenerCountBefore = contents.listenerCount(eventName) - contents.emit(eventName, {sender: contents}) + + try { + console.warn = (message) => warningMessage = message + contents.emit(eventName, {sender: contents}) + } finally { + console.warn = consoleWarn + } + const listenerCountAfter = contents.listenerCount(eventName) event.returnValue = { @@ -267,5 +270,4 @@ ipcMain.on('try-emit-web-contents-event', (event, id, eventName) => { listenerCountBefore, listenerCountAfter } - console.warn = consoleWarn }) From 330ac5f2667d564ab66cbf4ea44437a47a446091 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 24 Jan 2017 15:09:41 -0800 Subject: [PATCH 14/14] Fix standard linting error --- spec/static/main.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/static/main.js b/spec/static/main.js index ab16e19c9e3c..eaa5dd0f3059 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -257,7 +257,9 @@ ipcMain.on('try-emit-web-contents-event', (event, id, eventName) => { const listenerCountBefore = contents.listenerCount(eventName) try { - console.warn = (message) => warningMessage = message + console.warn = (message) => { + warningMessage = message + } contents.emit(eventName, {sender: contents}) } finally { console.warn = consoleWarn