Bug 1824420 - r=nalexander

Differential Revision: https://phabricator.services.mozilla.com/D182143

browser/installer/windows/nsis/installer.nsi

@@ -108,7 +108,7 @@ VIAddVersionKey "OriginalFilename" "setup.exe"
 !insertmacro ChangeMUIHeaderImage
 !insertmacro ChangeMUISidebarImage
 !insertmacro CheckForFilesInUse
-!insertmacro CleanUpdateDirectories
+!insertmacro CleanMaintenanceServiceLogs
 !insertmacro CopyFilesFromDir
 !insertmacro CopyPostSigningData
 !insertmacro CopyProvenanceData
@@ -311,8 +311,8 @@ Section "-InstallStartCleanup"
   ; setup the application model id registration value
   ${InitHashAppModelId} "$INSTDIR" "Software\Mozilla\${AppName}\TaskBarIDs"
 
-  ; Remove the updates directory
+  ; Clean up old maintenance service logs
-  ${CleanUpdateDirectories} "Mozilla\Firefox" "Mozilla\updates"
+  ${CleanMaintenanceServiceLogs} "Mozilla\Firefox"
 
   ${RemoveDeprecatedFiles}
   ${RemovePrecompleteEntries} "false"
@@ -776,6 +776,11 @@ Section "-Application" APP_IDX
   WriteRegDWORD HKCU "Software\Mozilla\${AppName}\Installer\$AppUserModelID" \
                      "DidRegisterDefaultBrowserAgent" $RegisterDefaultAgent
 !endif
+
+; Return value is saved to an unused variable to prevent the the error flag
+; from being set.
+Var /GLOBAL UnusedExecCatchReturn
+ExecWait '"$INSTDIR\${FileMainEXE}" --backgroundtask install' $UnusedExecCatchReturn
 SectionEnd
 
 ; Cleanup operations to perform at the end of the installation.

browser/installer/windows/nsis/uninstaller.nsi

@@ -114,7 +114,7 @@ VIAddVersionKey "OriginalFilename" "helper.exe"
 !insertmacro un.ChangeMUIHeaderImage
 !insertmacro un.ChangeMUISidebarImage
 !insertmacro un.CheckForFilesInUse
-!insertmacro un.CleanUpdateDirectories
+!insertmacro un.CleanMaintenanceServiceLogs
 !insertmacro un.CleanVirtualStore
 !insertmacro un.DeleteShortcuts
 !insertmacro un.GetCommonDirectory
@@ -460,8 +460,8 @@ Section "Uninstall"
     ${EndIf}
   ${EndIf}
 
-  ; Remove the updates directory
+  ; Clean up old maintenance service logs
-  ${un.CleanUpdateDirectories} "Mozilla\Firefox" "Mozilla\updates"
+  ${un.CleanMaintenanceServiceLogs} "Mozilla\Firefox"
 
   ; Remove any app model id's stored in the registry for this install path
   DeleteRegValue HKCU "Software\Mozilla\${AppName}\TaskBarIDs" "$INSTDIR"

browser/modules/BackgroundTask_install.sys.mjs

@@ -0,0 +1,29 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*-
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/**
+ * This task ought to have an ephemeral profile and should not apply updates.
+ * These settings are controlled externally, by
+ * `BackgroundTasks::IsUpdatingTaskName` and
+ * `BackgroundTasks::IsEphemeralProfileTaskName`.
+ */
+
+// This happens synchronously during installation. It shouldn't take that long
+// and if something goes wrong we really don't want to sit around waiting for
+// it.
+export const backgroundTaskTimeoutSec = 30;
+
+export async function runBackgroundTask(commandLine) {
+  console.log("Running BackgroundTask_install.");
+
+  console.log("Cleaning up update files.");
+  try {
+    Cc["@mozilla.org/updates/update-manager;1"]
+      .getService(Ci.nsIUpdateManager)
+      .doInstallCleanup();
+  } catch (ex) {
+    console.error(ex);
+  }
+}

browser/modules/BackgroundTask_uninstall.sys.mjs

@@ -3,16 +3,36 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+/**
+ * This task ought to have an ephemeral profile and should not apply updates.
+ * These settings are controlled externally, by
+ * `BackgroundTasks::IsUpdatingTaskName` and
+ * `BackgroundTasks::IsEphemeralProfileTaskName`.
+ */
+
 import { AppConstants } from "resource://gre/modules/AppConstants.sys.mjs";
 
 export async function runBackgroundTask(commandLine) {
-  if (AppConstants.platform !== "win") {
-    console.log("Not a Windows install, skipping `uninstall` background task.");
-    return;
-  }
   console.log("Running BackgroundTask_uninstall.");
 
-  removeNotifications();
+  if (AppConstants.platform === "win") {
+    try {
+      removeNotifications();
+    } catch (ex) {
+      console.error(ex);
+    }
+  } else {
+    console.log("Not a Windows install. Skipping notification removal.");
+  }
+
+  console.log("Cleaning up update files.");
+  try {
+    Cc["@mozilla.org/updates/update-manager;1"]
+      .getService(Ci.nsIUpdateManager)
+      .doUninstallCleanup();
+  } catch (ex) {
+    console.error(ex);
+  }
 }
 
 function removeNotifications() {

browser/modules/moz.build

@@ -160,5 +160,6 @@ if CONFIG["MOZ_WIDGET_TOOLKIT"] == "windows":
     ]
 
     EXTRA_JS_MODULES.backgroundtasks += [
+        "BackgroundTask_install.sys.mjs",
         "BackgroundTask_uninstall.sys.mjs",
     ]

toolkit/mozapps/installer/windows/nsis/common.nsh

@@ -3541,31 +3541,19 @@
 !macroend
 
 /**
- * If present removes the updates directory located in the profile's local
+ * Cleans up some old logs in the Maintenance Service directory.
- * directory for this installation.
  *
  * @param   _OLD_REL_PATH
  *          The relative path to the profile directory from Local AppData.
  *          Calculated for the old update directory not based on a hash.
- * @param   _NEW_REL_PATH
- *          The relative path to the profile directory from Local AppData.
- *          Calculated for the new update directory based on a hash.
  *
- * $R9 = Local AppData
- * $R8 = _NEW_REL_PATH
  * $R7 = _OLD_REL_PATH
  * $R1 = taskBar ID hash located in registry at SOFTWARE\_OLD_REL_PATH\TaskBarIDs
- * $R2 = various path values.
- * $R3 = length of the long path to $PROGRAMFILES
- * $R4 = length of the long path to $INSTDIR
- * $R5 = long path to $PROGRAMFILES
  * $R6 = long path to $INSTDIR
- * $R0 = path to the new update directory built from _NEW_REL_PATH and
- *       the taskbar ID.
  */
-!macro CleanUpdateDirectories
+!macro CleanMaintenanceServiceLogs
 
-  !ifndef ${_MOZFUNC_UN}CleanUpdateDirectories
+  !ifndef ${_MOZFUNC_UN}CleanMaintenanceServiceLogs
     !define _MOZFUNC_UN_TMP ${_MOZFUNC_UN}
     !insertmacro ${_MOZFUNC_UN_TMP}GetLongPath
     !insertmacro ${_MOZFUNC_UN_TMP}GetCommonDirectory
@@ -3575,154 +3563,25 @@
 
     !verbose push
     !verbose ${_MOZFUNC_VERBOSE}
-    !define ${_MOZFUNC_UN}CleanUpdateDirectories "!insertmacro ${_MOZFUNC_UN}CleanUpdateDirectoriesCall"
+    !define ${_MOZFUNC_UN}CleanMaintenanceServiceLogs "!insertmacro ${_MOZFUNC_UN}CleanMaintenanceServiceLogsCall"
 
-    Function ${_MOZFUNC_UN}CleanUpdateDirectories
+    Function ${_MOZFUNC_UN}CleanMaintenanceServiceLogs
-      Exch $R8
-      Exch 1
       Exch $R7
       Push $R6
-      Push $R5
-      Push $R4
-      Push $R3
-      Push $R2
       Push $R1
-      Push $R0
-      Push $R9
 
       ${${_MOZFUNC_UN}GetLongPath} "$INSTDIR" $R6
-      StrLen $R4 "$R6"
-
-!ifdef HAVE_64BIT_BUILD
-      ${${_MOZFUNC_UN}GetLongPath} "$PROGRAMFILES64" $R5
-!else
-      ${${_MOZFUNC_UN}GetLongPath} "$PROGRAMFILES" $R5
-!endif
-      StrLen $R3 "$R5"
-      ${GetLocalAppDataFolder} $R9
 
       ${If} $R7 != "" ; _OLD_REL_PATH was passed
       ${AndIf} $R6 != "" ; We have the install dir path
-      ${AndIf} $R5 != "" ; We the program files path
-      ${AndIf} $R4 > $R3 ; The length of $INSTDIR > the length of $PROGRAMFILES
-
-        ; Copy from the start of $INSTDIR the length of $PROGRAMFILES
-        StrCpy $R2 "$R6" $R3
-
-        ; Check if $INSTDIR is under $PROGRAMFILES
-        ${If} $R2 == $R5
-
-          ; Copy the relative path to $INSTDIR from $PROGRAMFILES
-          StrCpy $R2 "$R6" "" $R3
-
-          ; Concatenate the local AppData path ($R9) to the relative profile path and
-          ; the relative path to $INSTDIR from $PROGRAMFILES
-          StrCpy $R2 "$R9\$R7$R2"
-          ${${_MOZFUNC_UN}GetLongPath} "$R2" $R2
-
-          ${If} $R2 != ""
-            ; Backup the old update directory logs and delete the directory
-            ${If} ${FileExists} "$R2\updates\last-update.log"
-              Rename "$R2\updates\last-update.log" "$TEMP\moz-update-old-1-last-update.log"
-            ${EndIf}
-
-            ${If} ${FileExists} "$R2\updates\backup-update.log"
-              Rename "$R2\updates\backup-update.log" "$TEMP\moz-update-old-1-backup-update.log"
-            ${EndIf}
-
-            ${If} ${FileExists} "$R2\updates"
-                RmDir /r "$R2"
-            ${EndIf}
-          ${EndIf}
-        ${EndIf}
-
         ; Get the taskbar ID hash for this installation path
         ReadRegStr $R1 HKLM "SOFTWARE\$R7\TaskBarIDs" $R6
         ${If} $R1 == ""
           ReadRegStr $R1 HKCU "SOFTWARE\$R7\TaskBarIDs" $R6
         ${EndIf}
 
-        ; If the taskbar ID hash exists then delete the new update directory
-        ; Backup its logs before deleting it.
         ${If} $R1 != ""
-          StrCpy $R0 "$R9\$R8\$R1"
+          ; Remove the secure log files that our updater may have created
-
-          ${If} ${FileExists} "$R0\updates\last-update.log"
-            Rename "$R0\updates\last-update.log" "$TEMP\moz-update-old-2-last-update.log"
-          ${EndIf}
-
-          ${If} ${FileExists} "$R0\updates\backup-update.log"
-            Rename "$R0\updates\backup-update.log" "$TEMP\moz-update-old-2-backup-update.log"
-          ${EndIf}
-
-          ; Remove the old updates directory, located in the user's Windows profile directory
-          ${If} ${FileExists} "$R0\updates"
-            RmDir /r "$R0"
-          ${EndIf}
-
-          ${GetCommonAppDataFolder} $R0
-          StrCpy $R0 "$R0\$R8\$R1"
-
-          ${If} ${FileExists} "$R0\updates\last-update.log"
-            Rename "$R0\updates\last-update.log" "$TEMP\moz-update-old-3-last-update.log"
-          ${EndIf}
-
-          ${If} ${FileExists} "$R0\updates\backup-update.log"
-            Rename "$R0\updates\backup-update.log" "$TEMP\moz-update-old-3-backup-update.log"
-          ${EndIf}
-
-          ; Even though this is an old update directory, completely clear it out
-          ; on uninstall only, not on installation. If this is an installation,
-          ; it may be a paveover install and there may be un-migrated settings
-          ; in the update directory that we don't want to lose.
-          ; On install though, we should still remove pending updates and update
-          ; metadata since migrating that data could potentially confuse Firefox
-          ; into thinking that it failed to apply an update.
-          !if "${_MOZFUNC_UN}" == "un."
-            ${If} ${FileExists} "$R0"
-              RmDir /r "$R0"
-            ${EndIf}
-          !else
-            ${If} ${FileExists} "$R0\updates"
-              RmDir /r "$R0\updates"
-            ${EndIf}
-            Delete "$R0\active-update.xml"
-          !endif
-
-          ${${_MOZFUNC_UN}GetCommonDirectory} $R0
-          StrCpy $R0 "$R0\updates\$R1"
-
-          ${If} ${FileExists} "$R0\updates\last-update.log"
-            Rename "$R0\updates\last-update.log" "$TEMP\moz-update-newest-last-update.log"
-          ${EndIf}
-
-          ${If} ${FileExists} "$R0\updates\backup-update.log"
-            Rename "$R0\updates\backup-update.log" "$TEMP\moz-update-newest-backup-update.log"
-          ${EndIf}
-
-          ; The update directory is shared across all users of this
-          ; installation, and it contains a number of things. Which files we
-          ; want to keep and which we want to delete depends on if we are
-          ; installing or uninstalling.
-          ; If we are installing, we want to clear out any in-progress updates.
-          ; Otherwise we could potentially install an old, pending update when
-          ; Firefox first launches. The updates themselves live in the "updates"
-          ; subdirectory, and the update metadata lives in active-update.xml.
-          ; If we are uninstalling, we want to clear out the updates, the
-          ; update history, and the per-installation update configuration data.
-          ; In this case, we can just delete the whole update directory.
-          !if "${_MOZFUNC_UN}" == "un."
-            ${If} ${FileExists} "$R0"
-              RmDir /r "$R0"
-            ${EndIf}
-          !else
-            ${If} ${FileExists} "$R0\updates"
-              RmDir /r "$R0\updates"
-            ${EndIf}
-            Delete "$R0\active-update.xml"
-          !endif
-
-          ; Also remove the secure log files that our updater may have created
           ; inside the maintenance service path. There are several files named
           ; with the install hash and an extension indicating the kind of file.
           ; so use a wildcard to delete them all.
@@ -3738,49 +3597,39 @@
 
       ClearErrors
 
-      Pop $R9
-      Pop $R0
       Pop $R1
-      Pop $R2
-      Pop $R3
-      Pop $R4
-      Pop $R5
       Pop $R6
       Exch $R7
-      Exch 1
-      Exch $R8
     FunctionEnd
 
     !verbose pop
   !endif
 !macroend
 
-!macro CleanUpdateDirectoriesCall _OLD_REL_PATH _NEW_REL_PATH
+!macro CleanMaintenanceServiceLogsCall _OLD_REL_PATH
   !verbose push
   !verbose ${_MOZFUNC_VERBOSE}
   Push "${_OLD_REL_PATH}"
-  Push "${_NEW_REL_PATH}"
+  Call CleanMaintenanceServiceLogs
-  Call CleanUpdateDirectories
   !verbose pop
 !macroend
 
-!macro un.CleanUpdateDirectoriesCall _OLD_REL_PATH _NEW_REL_PATH
+!macro un.CleanMaintenanceServiceLogsCall _OLD_REL_PATH
   !verbose push
   !verbose ${_MOZFUNC_VERBOSE}
   Push "${_OLD_REL_PATH}"
-  Push "${_NEW_REL_PATH}"
+  Call un.CleanMaintenanceServiceLogs
-  Call un.CleanUpdateDirectories
   !verbose pop
 !macroend
 
-!macro un.CleanUpdateDirectories
+!macro un.CleanMaintenanceServiceLogs
-  !ifndef un.CleanUpdateDirectories
+  !ifndef un.CleanMaintenanceServiceLogs
     !verbose push
     !verbose ${_MOZFUNC_VERBOSE}
     !undef _MOZFUNC_UN
     !define _MOZFUNC_UN "un."
 
-    !insertmacro CleanUpdateDirectories
+    !insertmacro CleanMaintenanceServiceLogs
 
     !undef _MOZFUNC_UN
     !define _MOZFUNC_UN

toolkit/mozapps/update/UpdateService.sys.mjs

@@ -118,8 +118,11 @@ const URI_UPDATES_PROPERTIES =
 const KEY_EXECUTABLE = "XREExeF";
 const KEY_PROFILE_DIR = "ProfD";
 const KEY_UPDROOT = "UpdRootD";
+const KEY_OLD_UPDROOT = "OldUpdRootD";
 
 const DIR_UPDATES = "updates";
+const DIR_UPDATE_READY = "0";
+const DIR_UPDATE_DOWNLOADING = "downloading";
 
 const FILE_ACTIVE_UPDATE_XML = "active-update.xml";
 const FILE_BACKUP_UPDATE_LOG = "backup-update.log";
@@ -1163,7 +1166,7 @@ function getStatusTextFromCode(code, defaultCode) {
  * @return The ready updates directory, as a nsIFile object
  */
 function getReadyUpdateDir() {
-  return getUpdateDirCreate([DIR_UPDATES, "0"]);
+  return getUpdateDirCreate([DIR_UPDATES, DIR_UPDATE_READY]);
 }
 
 /**
@@ -1173,7 +1176,7 @@ function getReadyUpdateDir() {
  * @return The downloading update directory, as a nsIFile object
  */
 function getDownloadingUpdateDir() {
-  return getUpdateDirCreate([DIR_UPDATES, "downloading"]);
+  return getUpdateDirCreate([DIR_UPDATES, DIR_UPDATE_DOWNLOADING]);
 }
 
 /**
@@ -4909,6 +4912,69 @@ UpdateManager.prototype = {
     cleanupReadyUpdate();
   },
 
+  /**
+   * See nsIUpdateService.idl
+   */
+  doInstallCleanup: async function UM_doInstallCleanup(isUninstall) {
+    LOG("UpdateManager:doInstallCleanup - cleaning up");
+    let completionPromises = [];
+
+    const delete_or_log = path =>
+      IOUtils.remove(path).catch(ex =>
+        console.error(`Failed to delete ${path}`, ex)
+      );
+
+    for (const key of [KEY_OLD_UPDROOT, KEY_UPDROOT]) {
+      const root = Services.dirsvc.get(key, Ci.nsIFile);
+
+      const activeUpdateXml = root.clone();
+      activeUpdateXml.append(FILE_ACTIVE_UPDATE_XML);
+      completionPromises.push(delete_or_log(activeUpdateXml.path));
+
+      const downloadingMar = root.clone();
+      downloadingMar.append(DIR_UPDATES);
+      downloadingMar.append(DIR_UPDATE_DOWNLOADING);
+      downloadingMar.append(FILE_UPDATE_MAR);
+      completionPromises.push(delete_or_log(downloadingMar.path));
+
+      const readyDir = root.clone();
+      readyDir.append(DIR_UPDATES);
+      readyDir.append(DIR_UPDATE_READY);
+      const readyMar = readyDir.clone();
+      readyMar.append(FILE_UPDATE_MAR);
+      completionPromises.push(delete_or_log(readyMar.path));
+      const readyStatus = readyDir.clone();
+      readyStatus.append(FILE_UPDATE_STATUS);
+      completionPromises.push(delete_or_log(readyStatus.path));
+      const versionFile = readyDir.clone();
+      versionFile.append(FILE_UPDATE_VERSION);
+      completionPromises.push(delete_or_log(versionFile.path));
+    }
+
+    return Promise.allSettled(completionPromises);
+  },
+
+  /**
+   * See nsIUpdateService.idl
+   */
+  doUninstallCleanup: async function UM_doUninstallCleanup(isUninstall) {
+    LOG("UpdateManager:doUninstallCleanup - cleaning up.");
+    let completionPromises = [];
+
+    completionPromises.push(
+      IOUtils.remove(Services.dirsvc.get(KEY_UPDROOT, Ci.nsIFile).path, {
+        recursive: true,
+      }).catch(ex => console.error("Failed to remove update directory", ex))
+    );
+    completionPromises.push(
+      IOUtils.remove(Services.dirsvc.get(KEY_OLD_UPDROOT, Ci.nsIFile).path, {
+        recursive: true,
+      }).catch(ex => console.error("Failed to remove old update directory", ex))
+    );
+
+    return Promise.allSettled(completionPromises);
+  },
+
   classID: Components.ID("{093C2356-4843-4C65-8709-D7DBCBBE7DFB}"),
   QueryInterface: ChromeUtils.generateQI(["nsIUpdateManager", "nsIObserver"]),
 };

toolkit/mozapps/update/nsIUpdateService.idl

@@ -749,4 +749,25 @@ interface nsIUpdateManager : nsISupports
    */
   void cleanupDownloadingUpdate();
   void cleanupReadyUpdate();
+
+  /**
+   * Runs cleanup that ought to happen on a Firefox paveover install to
+   * prevent a stale update from being processed when Firefox is first
+   * launched.
+   * This is best-effort. It will not throw on cleanup failure.
+   *
+   * The returned promise does not resolve with any particular value. It simply
+   * conveys that the cleanup has completed.
+   */
+  Promise doInstallCleanup();
+
+  /**
+   * Runs cleanup that ought to happen when Firefox is uninstalled to clean up
+   * old update data that is no longer needed.
+   * This is best-effort. It will not throw on cleanup failure.
+   *
+   * The returned promise does not resolve with any particular value. It simply
+   * conveys that the cleanup has completed.
+   */
+  Promise doUninstallCleanup();
 };