Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix missing remote object error when calling remote function created in preload script #15444

Merged
merged 2 commits into from Oct 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 15 additions & 5 deletions lib/browser/rpc-server.js
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 5 additions & 2 deletions lib/renderer/api/remote.js
Expand Up @@ -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)
}
})
}
Expand Down
20 changes: 20 additions & 0 deletions spec/api-remote-spec.js
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: preload: preload can just be preload

}
})
w.once('closed', () => done())
w.loadURL('about:blank')
})
})
})
5 changes: 5 additions & 0 deletions 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()
23 changes: 9 additions & 14 deletions spec/static/main.js
Expand Up @@ -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) => {
Expand Down