From 093323defd7dca13c838d06a78c4cb2b5cdd412e Mon Sep 17 00:00:00 2001 From: Kirk Steuber Date: Fri, 18 Dec 2020 20:57:22 +0000 Subject: [PATCH] Bug 353804 - Add update swap handling to UpdateListener and AppUpdater r=mhowell UpdateListener and AppUpdater need to know when a downloading update is transitioning to being a ready update so they don't prompt the user to restart Firefox while updates are still staging (which would result in them not being installed). Differential Revision: https://phabricator.services.mozilla.com/D95821 --- browser/components/BrowserGlue.jsm | 1 + browser/modules/AppUpdater.jsm | 93 +++++++++++++++++------ toolkit/mozapps/update/UpdateListener.jsm | 73 +++++++++++++++--- 3 files changed, 133 insertions(+), 34 deletions(-) diff --git a/browser/components/BrowserGlue.jsm b/browser/components/BrowserGlue.jsm index ec189d100f9e..7908d169c5eb 100644 --- a/browser/components/BrowserGlue.jsm +++ b/browser/components/BrowserGlue.jsm @@ -871,6 +871,7 @@ const listeners = { "update-downloaded": ["UpdateListener"], "update-available": ["UpdateListener"], "update-error": ["UpdateListener"], + "update-swap": ["UpdateListener"], "gmp-plugin-crash": ["PluginManager"], "plugin-crashed": ["PluginManager"], }, diff --git a/browser/modules/AppUpdater.jsm b/browser/modules/AppUpdater.jsm index 8a96acef8569..866769760d4f 100644 --- a/browser/modules/AppUpdater.jsm +++ b/browser/modules/AppUpdater.jsm @@ -46,9 +46,12 @@ class AppUpdater { "nsIUpdateManager" ); this.QueryInterface = ChromeUtils.generateQI([ + "nsIObserver", "nsIProgressEventSink", "nsIRequestObserver", + "nsISupportsWeakReference", ]); + Services.obs.addObserver(this, "update-swap", /* ownsWeak */ true); } /** @@ -389,32 +392,41 @@ class AppUpdater { _awaitStagingComplete() { let observer = (aSubject, aTopic, aData) => { // Update the UI when the background updater is finished - let status = aData; - if ( - status == "applied" || - status == "applied-service" || - status == "pending" || - status == "pending-service" || - status == "pending-elevate" - ) { - // If the update is successfully applied, or if the updater has - // fallen back to non-staged updates, show the "Restart to Update" - // button. - this._setStatus(AppUpdater.STATUS.READY_FOR_RESTART); - } else if (status == "failed") { - // Background update has failed, let's show the UI responsible for - // prompting the user to update manually. - this._setStatus(AppUpdater.STATUS.DOWNLOAD_FAILED); - } else if (status == "downloading") { - // We've fallen back to downloading the complete update because the - // partial update failed to get staged in the background. - // Therefore we need to keep our observer. - this._setupDownloadListener(); - return; + switch (aTopic) { + case "update-staged": + let status = aData; + if ( + status == "applied" || + status == "applied-service" || + status == "pending" || + status == "pending-service" || + status == "pending-elevate" + ) { + // If the update is successfully applied, or if the updater has + // fallen back to non-staged updates, show the "Restart to Update" + // button. + this._setStatus(AppUpdater.STATUS.READY_FOR_RESTART); + } else if (status == "failed") { + // Background update has failed, let's show the UI responsible for + // prompting the user to update manually. + this._setStatus(AppUpdater.STATUS.DOWNLOAD_FAILED); + } else if (status == "downloading") { + // We've fallen back to downloading the complete update because the + // partial update failed to get staged in the background. + // Therefore we need to keep our observer. + this._setupDownloadListener(); + return; + } + break; + case "update-error": + this._setStatus(AppUpdater.STATUS.DOWNLOAD_FAILED); + break; } Services.obs.removeObserver(observer, "update-staged"); + Services.obs.removeObserver(observer, "update-error"); }; Services.obs.addObserver(observer, "update-staged"); + Services.obs.addObserver(observer, "update-error"); } /** @@ -489,6 +501,43 @@ class AppUpdater { } return status; } + + observe(subject, topic, status) { + switch (topic) { + case "update-swap": + this._handleUpdateSwap(); + break; + } + } + + _handleUpdateSwap() { + // This function exists to deal with the fact that we support handling 2 + // updates at once: a ready update and a downloading update. But AppUpdater + // only ever really considers a single update at a time. + // We see an update swap just when the downloading update has finished + // downloading and is being swapped into UpdateManager.readyUpdate. At this + // point, we are in one of two states. Either: + // a) The update that is being swapped in is the update that this + // AppUpdater has already been tracking, or + // b) We've been tracking the ready update. Now that the downloading + // update is about to be swapped into the place of the ready update, we + // need to switch over to tracking the new update. + if ( + this._status == AppUpdater.STATUS.DOWNLOADING || + this._status == AppUpdater.STATUS.STAGING + ) { + // We are already tracking the correct update. + return; + } + + if (this.updateStagingEnabled) { + this._setStatus(AppUpdater.STATUS.STAGING); + this._awaitStagingComplete(); + } else { + this._setStatus(AppUpdater.STATUS.DOWNLOADING); + this._awaitDownloadComplete(); + } + } } AppUpdater.STATUS = { diff --git a/toolkit/mozapps/update/UpdateListener.jsm b/toolkit/mozapps/update/UpdateListener.jsm index eef37288ee1d..4b8a34af7701 100644 --- a/toolkit/mozapps/update/UpdateListener.jsm +++ b/toolkit/mozapps/update/UpdateListener.jsm @@ -33,6 +33,13 @@ const PREF_APP_UPDATE_UNSUPPORTED_URL = "app.update.unsupported.url"; var UpdateListener = { timeouts: [], + restartDoorhangerShown: false, + // Once a restart badge/doorhanger is scheduled, these store the time that + // they were scheduled at (as milliseconds elapsed since the UNIX epoch). This + // allows us to resume the badge/doorhanger timers rather than restarting + // them from the beginning when a new update comes along. + updateFirstReadyTime: null, + get badgeWaitTime() { return Services.prefs.getIntPref("app.update.badgeWaitTime", 4 * 24 * 3600); // 4 days }, @@ -53,6 +60,12 @@ var UpdateListener = { }, reset() { + this.clearPendingAndActiveNotifications(); + this.restartDoorhangerShown = false; + this.updateFirstReadyTime = null; + }, + + clearPendingAndActiveNotifications() { AppMenuNotifications.removeNotification(/^update-/); this.clearCallbacks(); }, @@ -166,6 +179,9 @@ var UpdateListener = { let notification = AppUpdateService.isOtherInstanceHandlingUpdates ? "other-instance" : "restart"; + if (!dismissed) { + this.restartDoorhangerShown = true; + } this.showUpdateNotification(notification, true, dismissed, () => this.requestRestart() ); @@ -260,8 +276,22 @@ var UpdateListener = { case "success": this.clearCallbacks(); - let badgeWaitTimeMs = this.badgeWaitTime * 1000; - let doorhangerWaitTimeMs = update.promptWaitTime * 1000; + let initialBadgeWaitTimeMs = this.badgeWaitTime * 1000; + let initialDoorhangerWaitTimeMs = update.promptWaitTime * 1000; + let now = Date.now(); + + if (!this.updateFirstReadyTime) { + this.updateFirstReadyTime = now; + } + + let badgeWaitTimeMs = Math.max( + 0, + this.updateFirstReadyTime + initialBadgeWaitTimeMs - now + ); + let doorhangerWaitTimeMs = Math.max( + 0, + this.updateFirstReadyTime + initialDoorhangerWaitTimeMs - now + ); if (badgeWaitTimeMs < doorhangerWaitTimeMs) { this.addTimeout(badgeWaitTimeMs, () => { @@ -270,17 +300,19 @@ var UpdateListener = { this.showRestartNotification(update, true); } - // doorhangerWaitTimeMs is relative to when we initially received - // the event. Since we've already waited badgeWaitTimeMs, subtract - // that from doorhangerWaitTimeMs. - let remainingTime = doorhangerWaitTimeMs - badgeWaitTimeMs; - this.addTimeout(remainingTime, () => { - this.showRestartNotification(update, false); - }); + if (!this.restartDoorhangerShown) { + // doorhangerWaitTimeMs is relative to when we initially received + // the event. Since we've already waited badgeWaitTimeMs, subtract + // that from doorhangerWaitTimeMs. + let remainingTime = doorhangerWaitTimeMs - badgeWaitTimeMs; + this.addTimeout(remainingTime, () => { + this.showRestartNotification(update, false); + }); + } }); } else { this.addTimeout(doorhangerWaitTimeMs, () => { - this.showRestartNotification(update, false); + this.showRestartNotification(update, this.restartDoorhangerShown); }); } break; @@ -292,7 +324,6 @@ var UpdateListener = { case "show-prompt": // If an update is available and the app.update.auto preference is // false, then show an update available doorhanger. - this.clearCallbacks(); this.showUpdateAvailableNotification(update, false); break; case "cant-apply": @@ -312,11 +343,26 @@ var UpdateListener = { this.showUpdateDownloadingNotification(); break; case "idle": - this.reset(); + this.clearPendingAndActiveNotifications(); break; } }, + handleUpdateSwap() { + // This function is called because we just finished downloading an update + // (possibly) when another update was already ready. + // At some point, we may want to have some sort of intermediate + // notification to display here so that the badge doesn't just disappear. + // Currently, this function just hides update notifications and clears + // the callback timers so that notifications will not be shown. We want to + // clear the restart notification so the user doesn't try to restart to + // update during staging. We want to clear any other notifications too, + // since none of them make sense to display now. + // Our observer will fire again when the update is either ready to install + // or an error has been encountered. + this.clearPendingAndActiveNotifications(); + }, + observe(subject, topic, status) { let update = subject && subject.QueryInterface(Ci.nsIUpdate); @@ -342,6 +388,9 @@ var UpdateListener = { case "update-error": this.handleUpdateError(update, status); break; + case "update-swap": + this.handleUpdateSwap(); + break; } }, };