fix: macos autoupdate flow

assets/locales/en.json

@@ -129,13 +129,14 @@
     "message": "It was not possible to download the update. Please check your Internet connection and try again."
   },
   "updateDownloadedDialog": {
-    "title": "Update downloaded",
-    "message": "Update for version { version } downloaded. To install the update, please restart IPFS Desktop.",
-    "action": "Restart"
+    "title": "IPFS Desktop Update",
+    "message": "An update to IPFS Desktop { version } is available. Would you like to install it now?",
+    "later": "Later",
+    "now": "Install now"
   },
   "updateDownloadedNotification": {
-    "title": "Update downloaded",
-    "message": "Update for version { version } of IPFS Desktop downloaded. Click this notification to install."
+    "title": "IPFS Desktop Update",
+    "message": "An update to IPFS Desktop { version } is available."
   },
   "runGarbageCollectorWarning": {
     "title": "Garbage collector",

src/auto-updater/index.js

@@ -1,11 +1,9 @@
-const { shell } = require('electron')
+const { shell, app, BrowserWindow, Notification } = require('electron')
 const { autoUpdater } = require('electron-updater')
 const i18n = require('i18next')
 const { ipcMain } = require('electron')
 const logger = require('../common/logger')
-const { notify } = require('../common/notify')
 const { showDialog } = require('../dialogs')
-const macQuitAndInstall = require('./macos-quit-and-install')
 const { IS_MAC, IS_WIN, IS_APPIMAGE } = require('../common/consts')
 
 function isAutoUpdateSupported () {
@@ -14,14 +12,13 @@ function isAutoUpdateSupported () {
   return IS_MAC || IS_WIN || IS_APPIMAGE
 }
 
+let updateNotification = null // must be a global to avoid gc
 let feedback = false
 
 function setup (ctx) {
   // we download manually in 'update-available'
   autoUpdater.autoDownload = false
-
-  // mac requires manual upgrade, other platforms work out of the box
-  autoUpdater.autoInstallOnAppQuit = !IS_MAC
+  autoUpdater.autoInstallOnAppQuit = true
 
   autoUpdater.on('error', err => {
     logger.error(`[updater] ${err.toString()}`)
@@ -93,36 +90,53 @@ function setup (ctx) {
   autoUpdater.on('update-downloaded', ({ version }) => {
     logger.info(`[updater] update to ${version} downloaded`)
 
-    const { autoInstallOnAppQuit } = autoUpdater
-    const doIt = () => {
-      // Do nothing if install is handled by upstream logic
-      if (autoInstallOnAppQuit) return
-      // Else, do custom install handling
-      setImmediate(() => {
-        if (IS_MAC) macQuitAndInstall(ctx)
+    const feedbackDialog = () => {
+      const opt = showDialog({
+        title: i18n.t('updateDownloadedDialog.title'),
+        message: i18n.t('updateDownloadedDialog.message', { version }),
+        type: 'info',
+        buttons: [
+          i18n.t('updateDownloadedDialog.later'),
+          i18n.t('updateDownloadedDialog.now')
+        ]
       })
+      if (opt === 1) { // now
+        setImmediate(async () => {
+          await beforeQuitCleanup() // just to be sure (we had regressions before)
+          autoUpdater.quitAndInstall()
+        })
+      }
     }
-
-    if (!feedback) {
-      notify({
+    if (feedback) {
+      feedback = false
+      // when in instant feedback mode, show dialog immediatelly
+      feedbackDialog()
+    } else {
+      // show unobtrusive notification + dialog on click
+      updateNotification = new Notification({
         title: i18n.t('updateDownloadedNotification.title'),
         body: i18n.t('updateDownloadedNotification.message', { version })
-      }, doIt)
+      })
+      updateNotification.on('click', feedbackDialog)
+      updateNotification.show()
     }
-
-    feedback = false
-
-    showDialog({
-      title: i18n.t('updateDownloadedDialog.title'),
-      message: i18n.t('updateDownloadedDialog.message', { version }),
-      type: 'info',
-      buttons: [
-        (autoInstallOnAppQuit ? i18n.t('ok') : i18n.t('updateDownloadedDialog.action'))
-      ]
-    })
-
-    doIt()
   })
+
+  // In some cases before-quit event is not emitted before all windows are closed,
+  // and we need to do cleanup here
+  const beforeQuitCleanup = async () => {
+    BrowserWindow.getAllWindows().forEach(w => w.removeAllListeners('close'))
+    app.removeAllListeners('window-all-closed')
+    try {
+      const s = await ctx.stopIpfs()
+      logger.info(`[beforeQuitCleanup] stopIpfs had finished with status: ${s}`)
+    } catch (err) {
+      logger.error('[beforeQuitCleanup] stopIpfs had an error', err)
+    }
+  }
+  // built-in updater != electron-updater
+  // Added in https://github.com/electron-userland/electron-builder/pull/6395
+  require('electron').autoUpdater.on('before-quit-for-update', beforeQuitCleanup)
 }
 
 async function checkForUpdates () {

src/tray.js

@@ -239,9 +239,14 @@ function icon (color) {
   return path.join(dir, `${color}-22Template.png`)
 }
 
+// Ok this one is pretty ridiculous:
+// Tray must be global or it will break due to GC:
+// https://www.electronjs.org/docs/faq#my-apps-tray-disappeared-after-a-few-minutes
+let tray = null
+
 module.exports = function (ctx) {
   logger.info('[tray] starting')
-  const tray = new Tray(icon(off))
+  tray = new Tray(icon(off))
   let menu = null
 
   const state = {