From ef9b34759e64fc6a3dc075aec22cee05b6f10973 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Wed, 16 Feb 2022 01:36:56 +0100 Subject: [PATCH] fix: macos autoupdate flow (#1979) * fix: macos autoupdate flow This removes macOS-specific code paths and replaces them with one unified logic for all three platforms (Windows/macOS/AppImage). It also fixes a bug where tray dissapears due to GC (it only worked because we had reference in ctx, but it could breat at any time) * style: improved update dialog copy * refactor: unobtrusive autoupdate notification This does not block main process nor interrupt user in any way. Notification can be safely ignored, in which case the update will be installed on exit. --- assets/locales/en.json | 11 ++-- src/auto-updater/index.js | 74 +++++++++++++--------- src/auto-updater/macos-quit-and-install.js | 21 ------ src/tray.js | 7 +- 4 files changed, 56 insertions(+), 57 deletions(-) delete mode 100644 src/auto-updater/macos-quit-and-install.js diff --git a/assets/locales/en.json b/assets/locales/en.json index fb65bdd55..0cc80df56 100644 --- a/assets/locales/en.json +++ b/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", diff --git a/src/auto-updater/index.js b/src/auto-updater/index.js index 26f4dd52e..889a8a62d 100644 --- a/src/auto-updater/index.js +++ b/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 () { diff --git a/src/auto-updater/macos-quit-and-install.js b/src/auto-updater/macos-quit-and-install.js deleted file mode 100644 index dc981eacb..000000000 --- a/src/auto-updater/macos-quit-and-install.js +++ /dev/null @@ -1,21 +0,0 @@ -const { app, BrowserWindow } = require('electron') -const { autoUpdater } = require('electron-updater') -const logger = require('../common/logger') - -// adapted from https://github.com/electron-userland/electron-builder/issues/1604#issuecomment-372091881 -module.exports = async function quitAndInstall ({ stopIpfs }) { - app.removeAllListeners('window-all-closed') - const browserWindows = BrowserWindow.getAllWindows() - browserWindows.forEach(function (browserWindow) { - browserWindow.removeAllListeners('close') - }) - - try { - const status = await stopIpfs() - logger.info(`[quit-and-install] stopIpfs had finished with status: ${status}`) - } catch (err) { - logger.error('[quit-and-install] stopIpfs had an error', err) - } - - autoUpdater.quitAndInstall(true, true) -} diff --git a/src/tray.js b/src/tray.js index 518c738a9..108acd7b2 100644 --- a/src/tray.js +++ b/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 = {