don't use EventEmitter for invokable ipcs

electron · May 28, 2019 · 79c3c92 · 79c3c92
1 parent 545d34c
commit 79c3c92
Show file tree

Hide file tree

Showing 5 changed files with 130 additions and 88 deletions.
diff --git a/docs/api/ipc-main.md b/docs/api/ipc-main.md
@@ -88,6 +88,54 @@ Removes the specified `listener` from the listener array for the specified
 
 Removes listeners of the specified `channel`.
 
+### `ipcMain.handle(channel, listener)`
+
+* `channel` String
+* `listener` Function
+  * `event` IpcMainEvent
+  * `...args` any[]
+
+Adds a handler for an `invoke`able IPC. This handler will be called whenever a
+renderer calls `ipcRenderer.invoke(channel, ...args)`.
+
+If `listener` returns a Promise, the eventual result of the promise will be
+returned as a reply to the remote caller. Otherwise, the return value of the
+listener will be used as the value of the reply.
+
+```js
+// main process
+ipcMain.handle('my-invokable-ipc', async (event, ...args) => {
+  const result = await somePromise(...args)
+  return result
+})
+
+// renderer process
+async () => {
+  const result = await ipcRenderer.invoke('my-invokable-ipc', arg1, arg2)
+  // ...
+}
+```
+
+The `event` that is passed as the first argument to the handler is the same as
+that passed to a regular event listener. It includes information about which
+WebContents is the source of the invoke request.
+
+### `ipcMain.handleOnce(channel, listener)`
+
+* `channel` String
+* `listener` Function
+  * `event` IpcMainEvent
+  * `...args` any[]
+
+Handles a single `invoke`able IPC message, then removes the listener. See
+`ipcMain.handle(channel, listener)`.
+
+### `ipcMain.removeHandler(channel)`
+
+* `channel` String
+
+Removes any handler for `channel`, if present.
+
 ## Event object
 
 The documentation for the `event` object passed to the `callback` can be found

diff --git a/docs/api/structures/ipc-main-event.md b/docs/api/structures/ipc-main-event.md
@@ -3,7 +3,3 @@
 * `frameId` Integer - The ID of the renderer frame that sent this message
 * `returnValue` any - Set this to the value to be returned in a syncronous message
 * `sender` WebContents - Returns the `webContents` that sent the message
-* `reply` Function - A function that will send an IPC message to the renderer frame that sent the original message that you are currently handling.  You should use this method to "reply" to the sent message in order to guaruntee the reply will go to the correct process and frame.
-  * `...args` any[]
-* `throw` Function - (only for 'invoke' events) Reject the waiting promise on the caller's end with the given string
-IpcRenderer
diff --git a/lib/browser/api/ipc-main.ts b/lib/browser/api/ipc-main.ts
@@ -1,18 +1,39 @@
 import { EventEmitter } from 'events'
+import { IpcMainEvent } from 'electron'
 
 class IpcMain extends EventEmitter {
-  handle (method: string, fn: Function) {
-    if (this.listenerCount(method) !== 0) {
+  _invokeHandlers: {[channel: string]: (e: IpcMainEvent, ...args: any[]) => void};
+  constructor () {
+    super()
+    this._invokeHandlers = {}
+  }
+
+  handle (method: string, fn: (e: IpcMainEvent, ...args: any[]) => any) {
+    if (method in this._invokeHandlers) {
       throw new Error(`Attempted to register a second handler for '${method}'`)
     }
-    this.on(method, async (e, ...args) => {
+    if (typeof fn !== 'function') {
+      throw new Error(`Expected handler to be a function, but found type '${typeof fn}'`)
+    }
+    this._invokeHandlers[method] = async (e, ...args) => {
       try {
-        e.reply(await Promise.resolve(fn(...args)))
+        (e as any).reply(await Promise.resolve(fn(e, ...args)))
       } catch (err) {
-        e.throw(err)
+        (e as any).throw(err)
       }
+    }
+  }
+
+  handleOnce (method: string, fn: Function) {
+    this.handle(method, (e, ...args) => {
+      this.removeHandler(method)
+      return fn(e, ...args)
     })
   }
+
+  removeHandler (method: string) {
+    delete this._invokeHandlers[method]
+  }
 }
 
 const ipcMain = new IpcMain()

diff --git a/lib/browser/api/web-contents.js b/lib/browser/api/web-contents.js
@@ -330,7 +330,11 @@ WebContents.prototype._init = function () {
     event.reply = (result) => event.sendReply({ result })
     event.throw = (error) => event.sendReply({ error: error.toString() })
     this.emit('ipc-invoke', event, channel, ...args)
-    ipcMain.emit(channel, event, ...args)
+    if (channel in ipcMain._invokeHandlers) {
+      ipcMain._invokeHandlers[channel](event, ...args)
+    } else {
+      event.throw(`No handler registered for '${channel}'`)
+    }
   })
 
   this.on('-ipc-message-sync', function (event, internal, channel, args) {

diff --git a/spec-main/api-ipc-spec.ts b/spec-main/api-ipc-spec.ts
@@ -1,13 +1,23 @@
 import * as chai from 'chai'
 import * as chaiAsPromised from 'chai-as-promised'
-import { BrowserWindow, ipcMain } from 'electron'
+import { BrowserWindow, ipcMain, IpcMainEvent } from 'electron'
 
 const { expect } = chai
 
 chai.use(chaiAsPromised)
 
 describe('ipc module', () => {
   describe('invoke', () => {
+    let w = (null as unknown as BrowserWindow);
+
+    before(async () => {
+      w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } })
+      await w.loadURL('about:blank')
+    })
+    after(async () => {
+      w.destroy()
+    })
+
     async function rendererInvoke(...args: any[]) {
       const {ipcRenderer} = require('electron')
       try {
@@ -18,67 +28,21 @@ describe('ipc module', () => {
       }
     }
 
-    it('receives responses', async () => {
-      ipcMain.once('test', (e, arg) => {
-        expect(arg).to.equal(123)
-        e.reply('456')
-      })
-      const done = new Promise(resolve => {
-        ipcMain.once('result', (e, arg) => {
-          expect(arg).to.deep.equal({result: '456'})
-          resolve()
-        })
-      })
-      const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } })
-      try {
-        await w.loadURL(`data:text/html,<script>(${rendererInvoke})(123)</script>`)
-        await done
-      } finally {
-        w.destroy()
-      }
-    })
-
-    it('receives errors', async () => {
-      ipcMain.once('test', (e) => {
-        e.throw('some error')
-      })
-      const done = new Promise(resolve => {
-        ipcMain.once('result', (e, arg) => {
-          expect(arg.error).to.match(/some error/)
-          resolve()
-        })
-      })
-      const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } })
-      try {
-        await w.loadURL(`data:text/html,<script>(${rendererInvoke})()</script>`)
-        await done
-      } finally {
-        w.destroy()
-      }
-    })
-
-    it('registers a synchronous handler', async () => {
-      (ipcMain as any).handle('test', (arg: number) => {
-        ipcMain.removeAllListeners('test')
+    it('receives a response from a synchronous handler', async () => {
+      ipcMain.handleOnce('test', (e: IpcMainEvent, arg: number) => {
         expect(arg).to.equal(123)
         return 3
       })
       const done = new Promise(resolve => ipcMain.once('result', (e, arg) => {
         expect(arg).to.deep.equal({result: 3})
         resolve()
       }))
-      const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } })
-      try {
-        await w.loadURL(`data:text/html,<script>(${rendererInvoke})(123)</script>`)
-        await done
-      } finally {
-        w.destroy()
-      }
+      await w.webContents.executeJavaScript(`(${rendererInvoke})(123)`)
+      await done
     })
 
-    it('registers an asynchronous handler', async () => {
-      (ipcMain as any).handle('test', async (arg: number) => {
-        ipcMain.removeAllListeners('test')
+    it('receives a response from an asynchronous handler', async () => {
+      ipcMain.handleOnce('test', async (e: IpcMainEvent, arg: number) => {
         expect(arg).to.equal(123)
         await new Promise(resolve => setImmediate(resolve))
         return 3
@@ -87,50 +51,59 @@ describe('ipc module', () => {
         expect(arg).to.deep.equal({result: 3})
         resolve()
       }))
-      const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } })
-      try {
-        await w.loadURL(`data:text/html,<script>(${rendererInvoke})(123)</script>`)
-        await done
-      } finally {
-        w.destroy()
-      }
+      await w.webContents.executeJavaScript(`(${rendererInvoke})(123)`)
+      await done
     })
 
     it('receives an error from a synchronous handler', async () => {
-      (ipcMain as any).handle('test', () => {
-        ipcMain.removeAllListeners('test')
+      ipcMain.handleOnce('test', () => {
         throw new Error('some error')
       })
       const done = new Promise(resolve => ipcMain.once('result', (e, arg) => {
         expect(arg.error).to.match(/some error/)
         resolve()
       }))
-      const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } })
-      try {
-        await w.loadURL(`data:text/html,<script>(${rendererInvoke})()</script>`)
-        await done
-      } finally {
-        w.destroy()
-      }
+      await w.webContents.executeJavaScript(`(${rendererInvoke})()`)
+      await done
     })
 
     it('receives an error from an asynchronous handler', async () => {
-      (ipcMain as any).handle('test', async () => {
-        ipcMain.removeAllListeners('test')
+      ipcMain.handleOnce('test', async () => {
         await new Promise(resolve => setImmediate(resolve))
         throw new Error('some error')
       })
       const done = new Promise(resolve => ipcMain.once('result', (e, arg) => {
         expect(arg.error).to.match(/some error/)
         resolve()
       }))
-      const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } })
-      try {
-        await w.loadURL(`data:text/html,<script>(${rendererInvoke})()</script>`)
-        await done
-      } finally {
-        w.destroy()
-      }
+      await w.webContents.executeJavaScript(`(${rendererInvoke})()`)
+      await done
+    })
+
+    it('throws an error if no handler is registered', async () => {
+      const done = new Promise(resolve => ipcMain.once('result', (e, arg) => {
+        expect(arg.error).to.match(/No handler registered/)
+        resolve()
+      }))
+      await w.webContents.executeJavaScript(`(${rendererInvoke})()`)
+      await done
+    })
+
+    it('throws an error when invoking a handler that was removed', async () => {
+      ipcMain.handle('test', () => {})
+      ipcMain.removeHandler('test')
+      const done = new Promise(resolve => ipcMain.once('result', (e, arg) => {
+        expect(arg.error).to.match(/No handler registered/)
+        resolve()
+      }))
+      await w.webContents.executeJavaScript(`(${rendererInvoke})()`)
+      await done
+    })
+
+    it('forbids multiple handlers', async () => {
+      ipcMain.handle('test', () => {})
+      expect(() => { ipcMain.handle('test', () => {}) }).to.throw(/second handler/)
+      ipcMain.removeHandler('test')
     })
   })
 })