From 1e2e461e5e6a0814863b6eb698ed75732720af6d Mon Sep 17 00:00:00 2001
From: Max Brunsfeld <maxbrunsfeld@gmail.com>
Date: Thu, 26 Jul 2018 10:20:01 -0700
Subject: [PATCH] Avoid excessive numbers of IPC event listeners in
 ApplicationDelegate

---
 spec/auto-update-manager-spec.js |  7 +---
 src/application-delegate.js      | 71 ++++++++------------------------
 2 files changed, 20 insertions(+), 58 deletions(-)

diff --git a/spec/auto-update-manager-spec.js b/spec/auto-update-manager-spec.js
index 780c9816b..ab9e0ed70 100644
--- a/spec/auto-update-manager-spec.js
+++ b/spec/auto-update-manager-spec.js
@@ -1,11 +1,8 @@
-'use babel'
-
-import AutoUpdateManager from '../src/auto-update-manager'
-import {remote} from 'electron'
+const AutoUpdateManager = require('../src/auto-update-manager')
+const {remote} = require('electron')
 const electronAutoUpdater = remote.require('electron').autoUpdater
 
 describe('AutoUpdateManager (renderer)', () => {
-
   if (process.platform !== 'darwin') return // Tests are tied to electron autoUpdater, we use something else on Linux and Win32
 
   let autoUpdateManager
diff --git a/src/application-delegate.js b/src/application-delegate.js
index ec6a37454..b78c62aae 100644
--- a/src/application-delegate.js
+++ b/src/application-delegate.js
@@ -1,12 +1,16 @@
 const {ipcRenderer, remote, shell} = require('electron')
 const ipcHelpers = require('./ipc-helpers')
-const {Disposable} = require('event-kit')
+const {Emitter, Disposable} = require('event-kit')
 const getWindowLoadSettings = require('./get-window-load-settings')
 
 module.exports =
 class ApplicationDelegate {
   constructor () {
     this.pendingSettingsUpdateCount = 0
+    this.ipcMessageEmitter = new Emitter()
+    ipcRenderer.on('message', (event, message, detail) => {
+      this.ipcMessageEmitter.emit(message, detail)
+    })
   }
 
   getWindowLoadSettings () { return getWindowLoadSettings() }
@@ -189,21 +193,13 @@ class ApplicationDelegate {
   }
 
   onDidChangeUserSettings (callback) {
-    const outerCallback = (event, message, detail) => {
-      if (message === 'did-change-user-settings') {
-        if (this.pendingSettingsUpdateCount === 0) callback(detail)
-      }
-    }
-    ipcRenderer.on('message', outerCallback)
-    return new Disposable(() => ipcRenderer.removeListener('message', outerCallback))
+    return this.ipcMessageEmitter.on('did-change-user-settings', detail => {
+      if (this.pendingSettingsUpdateCount === 0) callback(detail)
+    })
   }
 
   onDidFailToReadUserSettings (callback) {
-    const outerCallback = (event, message, detail) => {
-      if (message === 'did-fail-to-read-user-settings') callback(detail)
-    }
-    ipcRenderer.on('message', outerCallback)
-    return new Disposable(() => ipcRenderer.removeListener('message', outerCallback))
+    return this.ipcMessageEmitter.on('did-fail-to-read-user-setting', callback)
   }
 
   confirm (options, callback) {
@@ -261,24 +257,14 @@ class ApplicationDelegate {
   }
 
   onDidOpenLocations (callback) {
-    const outerCallback = (event, message, detail) => {
-      if (message === 'open-locations') callback(detail)
-    }
-
-    ipcRenderer.on('message', outerCallback)
-    return new Disposable(() => ipcRenderer.removeListener('message', outerCallback))
+    return this.ipcMessageEmitter.on('open-locations', callback)
   }
 
   onUpdateAvailable (callback) {
-    const outerCallback = (event, message, detail) => {
-      // TODO: Yes, this is strange that `onUpdateAvailable` is listening for
-      // `did-begin-downloading-update`. We currently have no mechanism to know
-      // if there is an update, so begin of downloading is a good proxy.
-      if (message === 'did-begin-downloading-update') callback(detail)
-    }
-
-    ipcRenderer.on('message', outerCallback)
-    return new Disposable(() => ipcRenderer.removeListener('message', outerCallback))
+    // TODO: Yes, this is strange that `onUpdateAvailable` is listening for
+    // `did-begin-downloading-update`. We currently have no mechanism to know
+    // if there is an update, so begin of downloading is a good proxy.
+    return this.ipcMessageEmitter.on('did-begin-downloading-update', callback)
   }
 
   onDidBeginDownloadingUpdate (callback) {
@@ -286,40 +272,19 @@ class ApplicationDelegate {
   }
 
   onDidBeginCheckingForUpdate (callback) {
-    const outerCallback = (event, message, detail) => {
-      if (message === 'checking-for-update') callback(detail)
-    }
-
-    ipcRenderer.on('message', outerCallback)
-    return new Disposable(() => ipcRenderer.removeListener('message', outerCallback))
+    return this.ipcMessageEmitter.on('checking-for-update', callback)
   }
 
   onDidCompleteDownloadingUpdate (callback) {
-    const outerCallback = (event, message, detail) => {
-      // TODO: We could rename this event to `did-complete-downloading-update`
-      if (message === 'update-available') callback(detail)
-    }
-
-    ipcRenderer.on('message', outerCallback)
-    return new Disposable(() => ipcRenderer.removeListener('message', outerCallback))
+    return this.ipcMessageEmitter.on('update-available', callback)
   }
 
   onUpdateNotAvailable (callback) {
-    const outerCallback = (event, message, detail) => {
-      if (message === 'update-not-available') callback(detail)
-    }
-
-    ipcRenderer.on('message', outerCallback)
-    return new Disposable(() => ipcRenderer.removeListener('message', outerCallback))
+    return this.ipcMessageEmitter.on('update-not-available', callback)
   }
 
   onUpdateError (callback) {
-    const outerCallback = (event, message, detail) => {
-      if (message === 'update-error') callback(detail)
-    }
-
-    ipcRenderer.on('message', outerCallback)
-    return new Disposable(() => ipcRenderer.removeListener('message', outerCallback))
+    return this.ipcMessageEmitter.on('update-error', callback)
   }
 
   onApplicationMenuCommand (handler) {