Backed out changeset 1315d653382a (bug 1852133) for causing build bus…

…tages at toolkit/xre/nsUpdateDriver.cpp CLOSED TREE

toolkit/components/backgroundtasks/moz.build

@@ -66,7 +66,6 @@ TESTING_JS_MODULES += [
 ]
 
 TESTING_JS_MODULES.backgroundtasks += [
-    "tests/BackgroundTask_automaticrestart.sys.mjs",
     "tests/BackgroundTask_backgroundtask_specific_pref.sys.mjs",
     "tests/BackgroundTask_crash.sys.mjs",
     "tests/BackgroundTask_file_exists.sys.mjs",

toolkit/components/backgroundtasks/tests/xpcshell/xpcshell.toml

@@ -16,9 +16,6 @@ tags = "remote-settings"
 run-if = ["buildapp == 'browser'"]
 reason = "ASRouter is Firefox-only."
 
-["test_backgroundtask_automaticrestart.js"]
-run-if = ["os == 'win'"]
-
 ["test_backgroundtask_help.js"]
 
 ["test_backgroundtask_localization.js"]

toolkit/mozapps/update/BackgroundTask_backgroundupdate.sys.mjs

@@ -46,6 +46,11 @@ export const backgroundTaskTimeoutSec = Services.prefs.getIntPref(
   10 * 60
 );
 
+// Add 65 second minimum run time to account for restartWithSameArgs
+// having a minimum 60 second run time to properly register a restart upon
+// program exit.
+const MINIMUM_RUN_TIME_BEFORE_RESTART_MS = 65 * 1000;
+
 /**
  * Verify that pre-conditions to update this installation (both persistent and
  * transient) are fulfilled, and if they are all fulfilled, pump the update
@@ -194,13 +199,13 @@ export async function maybeSubmitBackgroundUpdatePing() {
 export async function runBackgroundTask(commandLine) {
   let SLUG = "runBackgroundTask";
   lazy.log.error(`${SLUG}: backgroundupdate`);
-  let automaticRestartFound =
-    -1 != commandLine.findFlag("automatic-restart", false);
+  const taskStartTime = new Date().getTime();
+  let registeredRestartFound =
+    -1 !== commandLine.findFlag("registered-restart", false);
 
-  // Modify Glean metrics for a successful automatic restart.
-  if (automaticRestartFound) {
-    Glean.backgroundUpdate.automaticRestartSuccess.set(true);
-    lazy.log.debug(`${SLUG}: application automatic restart completed`);
+  // Modify Glean metrics for a successful registered restart.
+  if (registeredRestartFound) {
+    Glean.backgroundUpdate.registeredRestartSuccess.set(true);
   }
 
   // Help debugging.  This is a pared down version of
@@ -434,29 +439,36 @@ export async function runBackgroundTask(commandLine) {
   lazy.log.debug(
     `${SLUG}: Checking if staged background update is ready for restart`
   );
-  // If a restart loop is occurring then automaticRestartFound will be true.
+  // If a restart loop is occurring then registeredRestartFound will be true.
   if (
     updateStatus === lazy.AppUpdater.STATUS.READY_FOR_RESTART &&
-    !automaticRestartFound
+    !registeredRestartFound
   ) {
     lazy.log.debug(
-      `${SLUG}: Starting Firefox restart after staged background update`
+      `${SLUG}: Registering Firefox restart after staged background update, waiting for program to have a run time greater than 65 seconds`
     );
 
     // We need to restart Firefox with the same arguments to ensure
     // the background update continues from where it was before the restart.
+    // Wait for at least 65 seconds to ensure that the application
+    // has been open for long enough to correctly register a restart.
+    const taskRunTimeMs = new Date().getTime() - taskStartTime;
+    if (taskRunTimeMs < MINIMUM_RUN_TIME_BEFORE_RESTART_MS) {
+      await lazy.ExtensionUtils.promiseTimeout(
+        MINIMUM_RUN_TIME_BEFORE_RESTART_MS - taskRunTimeMs
+      );
+    }
+
     try {
       Cc["@mozilla.org/updates/update-processor;1"]
         .createInstance(Ci.nsIUpdateProcessor)
-        .attemptAutomaticApplicationRestartWithLaunchArgs([
-          "-automatic-restart",
-        ]);
-      // Report an attempted automatic restart.
-      Glean.backgroundUpdate.automaticRestartAttempted.set(true);
-      lazy.log.debug(`${SLUG}: automatic application restart queued`);
+        .registerApplicationRestartWithLaunchArgs(["-registered-restart"]);
+      lazy.log.debug(`${SLUG}: register application restart succeeded`);
+      // Report an attempted registered restart.
+      Glean.backgroundUpdate.registeredRestartAttempted.set(true);
     } catch (e) {
       lazy.log.error(
-        `${SLUG}: caught exception; failed to queue automatic application restart`,
+        `${SLUG}: caught exception; failed to register application restart`,
         e
       );
     }

toolkit/mozapps/update/metrics.yaml

@@ -205,10 +205,10 @@ background_update:
     send_in_pings:
       - background-update
 
-  automatic_restart_attempted:
+  registered_restart_attempted:
     type: boolean
     description: >
-      True if the background update task successfully attempted an automatic restart.
+      True if the background update task successfully registered a restart.
     bugs:
       - https://bugzilla.mozilla.org/show_bug.cgi?id=1847099
     data_reviews:
@@ -221,11 +221,11 @@ background_update:
     send_in_pings:
       - background-update
 
-  automatic_restart_success:
+  registered_restart_success:
     type: boolean
     description: >
       True if the background update task successfully restarted after
-      an automatic restart.
+      a registered restart.
     bugs:
       - https://bugzilla.mozilla.org/show_bug.cgi?id=1847099
     data_reviews:

toolkit/mozapps/update/nsIUpdateService.idl

@@ -657,52 +657,25 @@ interface nsIUpdateProcessor : nsISupports
   bool getServiceRegKeyExists();
 
   /**
-  * Attempts to restart the application manually on program exit with the same
+  * Registers an application restart upon program exit with the same
   * arguments it was started with, while accepting additional arguments.
+  * The application must have been running for a minimum of 60 seconds
+  * before invoking this function due to limitations in
+  * RegisterApplicationRestart.
   *
   * This function should only be called on Windows.
   *
-  * @param    argvExtra
-  *           An array of strings to be passed to the application upon
-  *           restart as additional arguments.
-  * @returns  pidRet
-  *           Returns the pid of a newly spawned child process. This value
-  *           is only valid if the function returns successfully.
-  * @throws   NS_ERROR_ABORT
-  *           If the child process failed to spawn correctly.
-  * @throws   NS_ERROR_NOT_IMPLEMENTED
-  *           If this is called on a non-Windows platform.
-  * @throws   NS_ERROR_NOT_AVAILABLE
-  *           If the command line cannot be read.
+  * @param  argvExtra
+  *         An array of strings to be passed to the application upon
+  *         restart as additional arguments.
+  * @throws NS_ERROR_ABORT
+  *         If the application is in a restart loop.
+  * @throws NS_ERROR_NOT_IMPLEMENTED
+  *         If this is called on a non-Windows platform.
+  * @throws NS_ERROR_NOT_AVAILABLE
+  *         If the command line cannot be read.
   */
-  long attemptAutomaticApplicationRestartWithLaunchArgs(in Array<AString> argvExtra);
-
-  /**
-  *   This function is meant to be used in conjunction with
-  *   RegisterApplicationRestartWithLaunchArgs() if you want the child process
-  *   that invokes this function to wait for the parent process
-  *   to finish execution. When the application has the argument
-  *   -restart-pid <pid> this function waits for the application with
-  *   <pid> to exit.
-  *
-  * This function should only be called on Windows.
-  *
-  *   @param   pid
-  *            Which process ID to wait for.
-  *   @param   timeoutMS
-  *            How long to wait for the process to exit in milliseconds.
-  *   @throws  NS_OK
-  *            On successful wait.
-  *   @throws  NS_ERROR_NOT_IMPLEMENTED
-  *            If this is called on a non-Windows platform.
-  *   @throws  NS_ERROR_INVALID_ARG
-  *            If -restart-pid has no pid parameter.
-  *   @throws  NS_ERROR_ILLEGAL_VALUE
-  *            If pid cannot be converted into unsigned int.
-  *   @throws  NS_ERROR_FAILURE
-  *            If timeout elapses without process exit.
-  */
-  void waitForProcessExit(in unsigned long pid, in unsigned long timeoutMS);
+  void registerApplicationRestartWithLaunchArgs(in Array<AString> argvExtra);
 };
 
 /**

toolkit/xre/nsAppRunner.cpp

@@ -4882,32 +4882,6 @@ int XREMain::XRE_mainStartup(bool* aExitFlag) {
 #endif
 
 #if defined(MOZ_UPDATER) && !defined(MOZ_WIDGET_ANDROID)
-#  ifdef XP_WIN
-  {
-    // When automatically restarting for a staged background update
-    // we want the child process to wait here so that the updater
-    // does not register two instances running and use that as a
-    // reason to not process updates. This function requires having
-    // -restart-pid <param> in the command line to function properly.
-    const char* restartPidString = nullptr;
-    if (ARG_FOUND == CheckArg("restart-pid", &restartPidString) &&
-        !CheckArg("test-only-automatic-restart-no-wait")) {
-      // pid should be first parameter following -restart-pid.
-      uint32_t pid = nsDependentCString(restartPidString).ToInteger(&rv, 10U);
-      if (NS_SUCCEEDED(rv) && pid > 0) {
-        printf_stderr(
-            "*** MaybeWaitForProcessExit: launched pidDWORD = %u ***\n", pid);
-        RefPtr<nsUpdateProcessor> updater = new nsUpdateProcessor();
-        if (NS_FAILED(
-                updater->WaitForProcessExit(pid, MAYBE_WAIT_TIMEOUT_MS))) {
-          NS_WARNING("Failed to MaybeWaitForProcessExit.");
-        }
-      } else {
-        NS_WARNING("Failed to parse pid from -restart-pid.");
-      }
-    }
-  }
-#  endif
   Maybe<ShouldNotProcessUpdatesReason> shouldNotProcessUpdatesReason =
       ShouldNotProcessUpdates(mDirProvider);
   if (shouldNotProcessUpdatesReason.isNothing()) {

toolkit/xre/nsAppRunner.h

@@ -127,9 +127,6 @@ void UnlockProfile();
 BOOL WinLaunchChild(const wchar_t* exePath, int argc, char** argv,
                     HANDLE userToken = nullptr, HANDLE* hProcess = nullptr);
 
-BOOL WinLaunchChild(const wchar_t* exePath, int argc, wchar_t** argv,
-                    HANDLE userToken = nullptr, HANDLE* hProcess = nullptr);
-
 #  define PREF_WIN_REGISTER_APPLICATION_RESTART \
     "toolkit.winRegisterApplicationRestart"