Bug 1642039 - Part 3: make updateInstalledAtStartup and lastUpdateInstalled async r=bytesized,application-update-reviewers,firefox-desktop-core-reviewers ,mossop

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

browser/components/BrowserContentHandler.sys.mjs

@@ -270,7 +270,11 @@ function openBrowserWindow(
   let args;
   if (!urlOrUrlList) {
     // Just pass in the defaultArgs directly. We'll use system principal on the other end.
-    args = [gBrowserContentHandler.getArgs(isStartup)];
+    if (isStartup) {
+      args = [gBrowserContentHandler.getFirstWindowArgs()];
+    } else {
+      args = [gBrowserContentHandler.getNewWindowArgs()];
+    }
   } else if (Array.isArray(urlOrUrlList)) {
     // There isn't an explicit way to pass a principal here, so we load multiple URLs
     // with system principal when we get to actually loading them.
@@ -424,6 +428,43 @@ async function doSearch(searchTerm, cmdLine) {
   ).catch(console.error);
 }
 
+function spinForLastUpdateInstalled() {
+  return spinResolve(lazy.UpdateManager.lastUpdateInstalled());
+}
+
+function spinForUpdateInstalledAtStartup() {
+  return spinResolve(lazy.UpdateManager.updateInstalledAtStartup());
+}
+
+function spinResolve(promise) {
+  if (!(promise instanceof Promise)) {
+    return promise;
+  }
+  let done = false;
+  let result = null;
+  let error = null;
+  promise
+    .catch(e => {
+      error = e;
+    })
+    .then(r => {
+      result = r;
+      done = true;
+    });
+
+  Services.tm.spinEventLoopUntil(
+    "BrowserContentHandler.sys.mjs:BCH_spinResolve",
+    () => done
+  );
+  if (!done) {
+    throw new Error("Forcefully exited event loop.");
+  } else if (error) {
+    throw error;
+  } else {
+    return result;
+  }
+}
+
 export function nsBrowserContentHandler() {
   if (!gBrowserContentHandler) {
     gBrowserContentHandler = this;
@@ -691,10 +732,52 @@ nsBrowserContentHandler.prototype = {
   /* nsIBrowserHandler */
 
   get defaultArgs() {
-    return this.getArgs();
+    return this.getNewWindowArgs();
   },
 
-  getArgs(isStartup = false) {
+  // This function is expected to be called in non-startup cases,
+  // a WNP will not be retrieved within this function, but it will retrieve
+  // any new profile override page(s) or regular startup page(s).
+  // For the startup version of this function, please use getFirstWindowArgs().
+  // See Bug 1642039 for more information.
+  getNewWindowArgs(skipStartPage = false) {
+    var page = lazy.LaterRun.getURL();
+    if (page == "about:blank") {
+      page = "";
+    }
+    var startPage = "";
+    var prefb = Services.prefs;
+    try {
+      var choice = prefb.getIntPref("browser.startup.page");
+      if (choice == 1 || choice == 3) {
+        startPage = lazy.HomePage.get();
+      }
+    } catch (e) {
+      console.error(e);
+    }
+
+    if (startPage == "about:blank") {
+      startPage = "";
+    }
+
+    if (!skipStartPage && startPage) {
+      if (page) {
+        page += "|" + startPage;
+      } else {
+        page = startPage;
+      }
+    } else if (!page) {
+      page = startPage;
+    }
+
+    return page || "about:blank";
+  },
+
+  // This function is expected to be called very early during Firefox startup,
+  // It will retrieve a WNP if avaliable, before calling getNewWindowsArg()
+  // to retrieve any other startup pages that needs to be displayed.
+  // See Bug 1642039 for more information.
+  getFirstWindowArgs() {
     var prefb = Services.prefs;
 
     if (!gFirstWindow) {
@@ -752,7 +835,17 @@ nsBrowserContentHandler.prototype = {
             overridePage = Services.urlFormatter.formatURLPref(
               "startup.homepage_override_url"
             );
-            let update = lazy.UpdateManager.lastUpdateInstalled;
+
+            /*
+            The update manager loads its data asynchronously, off of the main thread.
+            However, making this function asynchronous would be very difficult and
+            wouldn't provide any benefit. This code is part of the sequence of operations
+            that must run before the first tab and its contents can be displayed.
+            The user has to wait for this to complete regardless of the method or thread of execution,
+            and the browser will be practically unusable until it finishes.
+            Therefore, asynchronous execution does not offer any real advantages in this context.
+            */
+            let update = spinForLastUpdateInstalled();
 
             // Make sure the update is newer than the last WNP version
             // and the update is not newer than the current Firefox version.
@@ -856,20 +949,32 @@ nsBrowserContentHandler.prototype = {
 
             // Send the update ping to signal that the update was successful.
             // Only do this if the update is installed right now.
-            if (lazy.UpdateManager.updateInstalledAtStartup) {
-              lazy.UpdatePing.handleUpdateSuccess(old_mstone, old_buildId);
-            }
+            // The following code is ran asynchronously, but we won't await on it
+            // since the user may be still waiting for the browser to start up at this point.
+            lazy.UpdateManager.updateInstalledAtStartup().then(
+              async updateInstalledAtStartup => {
+                if (updateInstalledAtStartup) {
+                  await lazy.UpdatePing.handleUpdateSuccess(
+                    old_mstone,
+                    old_buildId
+                  );
+                }
+              }
+            );
 
             overridePage = overridePage.replace("%OLD_VERSION%", old_mstone);
             break;
           }
-          case OVERRIDE_NEW_BUILD_ID:
-            if (lazy.UpdateManager.updateInstalledAtStartup) {
+          case OVERRIDE_NEW_BUILD_ID: {
+            let updateInstalledAtStartup = spinForUpdateInstalledAtStartup();
+            if (updateInstalledAtStartup) {
               // Send the update ping to signal that the update was successful.
+              // This is asynchronous, but we are just going to kick it off because we can't easily `await` on it here.
               lazy.UpdatePing.handleUpdateSuccess(old_mstone, old_buildId);
               lazy.LaterRun.enable(lazy.LaterRun.ENABLE_REASON_UPDATE_APPLIED);
             }
             break;
+          }
         }
       }
     } catch (ex) {}
@@ -880,7 +985,7 @@ nsBrowserContentHandler.prototype = {
     }
 
     // Allow showing a one-time startup override if we're not showing one
-    if (isStartup && overridePage == "" && prefb.prefHasUserValue(ONCE_PREF)) {
+    if (overridePage == "" && prefb.prefHasUserValue(ONCE_PREF)) {
       try {
         // Show if we haven't passed the expiration or there's no expiration
         const { expire, url } = JSON.parse(
@@ -935,23 +1040,16 @@ nsBrowserContentHandler.prototype = {
       }
     }
 
-    var startPage = "";
-    try {
-      var choice = prefb.getIntPref("browser.startup.page");
-      if (choice == 1 || choice == 3) {
-        startPage = lazy.HomePage.get();
-      }
-    } catch (e) {
-      console.error(e);
-    }
+    let skipStartPage =
+      override == OVERRIDE_NEW_PROFILE &&
+      prefb.getBoolPref("browser.startup.firstrunSkipsHomepage");
+
+    var startPage = this.getNewWindowArgs(skipStartPage && !willRestoreSession);
 
     if (startPage == "about:blank") {
       startPage = "";
     }
 
-    let skipStartPage =
-      override == OVERRIDE_NEW_PROFILE &&
-      prefb.getBoolPref("browser.startup.firstrunSkipsHomepage");
     // Only show the startPage if we're not restoring an update session and are
     // not set to skip the start page on this profile
     if (overridePage && startPage && !willRestoreSession && !skipStartPage) {

browser/components/enterprisepolicies/tests/browser/browser_policy_override_postupdatepage.js

@@ -65,8 +65,9 @@ add_task(async function test_override_postupdate_page() {
 
 function getPostUpdatePage() {
   Services.prefs.setCharPref(PREF_MSTONE, "PreviousMilestone");
-  return Cc["@mozilla.org/browser/clh;1"].getService(Ci.nsIBrowserHandler)
-    .defaultArgs;
+  return Cc["@mozilla.org/browser/clh;1"]
+    .getService(Ci.nsIBrowserHandler)
+    .getFirstWindowArgs();
 }
 
 /**

browser/components/nsIBrowserHandler.idl

@@ -11,6 +11,7 @@ interface nsIBrowserHandler : nsISupports
 {
   attribute AUTF8String startPage;
   attribute AUTF8String defaultArgs;
+  AUTF8String getFirstWindowArgs();
   attribute boolean kiosk;
   attribute boolean majorUpgrade;
   attribute boolean firstRunProfile;

browser/components/tests/browser/browser_browserGlue_upgradeDialog_trigger.js

@@ -26,7 +26,7 @@ async function forceMajorUpgrade() {
     set: [["browser.startup.homepage_override.mstone", "88.0"]],
   });
 
-  void BrowserHandler.defaultArgs;
+  void BrowserHandler.getFirstWindowArgs();
 
   return async () => {
     await SpecialPowers.popPrefEnv();

browser/components/tests/browser/browser_bug538331.js

@@ -128,9 +128,9 @@ add_task(async function test_bug538331() {
 
     await reloadUpdateManagerData(false);
 
-    let noOverrideArgs = Cc["@mozilla.org/browser/clh;1"].getService(
-      Ci.nsIBrowserHandler
-    ).defaultArgs;
+    let noOverrideArgs = Cc["@mozilla.org/browser/clh;1"]
+      .getService(Ci.nsIBrowserHandler)
+      .getFirstWindowArgs();
 
     let overrideArgs = "";
     if (testCase.prefURL) {
@@ -149,9 +149,9 @@ add_task(async function test_bug538331() {
       Services.prefs.setCharPref(PREF_MSTONE, "PreviousMilestone");
     }
 
-    let defaultArgs = Cc["@mozilla.org/browser/clh;1"].getService(
-      Ci.nsIBrowserHandler
-    ).defaultArgs;
+    let defaultArgs = Cc["@mozilla.org/browser/clh;1"]
+      .getService(Ci.nsIBrowserHandler)
+      .getFirstWindowArgs();
     is(defaultArgs, overrideArgs, "correct value returned by defaultArgs");
 
     if (testCase.noMstoneChange === undefined || !testCase.noMstoneChange) {

browser/components/tests/browser/browser_startup_homepage.js

@@ -18,7 +18,7 @@ async function checkArgs(message, expect, prefs = {}) {
   Assert.equal(
     Cc["@mozilla.org/browser/clh;1"]
       .getService(Ci.nsIBrowserHandler)
-      .wrappedJSObject.getArgs(true),
+      .getFirstWindowArgs(),
     expect,
     message
   );

toolkit/components/telemetry/pings/UpdatePing.sys.mjs

@@ -53,7 +53,7 @@ export var UpdatePing = {
    * @param {String} aPreviousVersion The browser version we updated from.
    * @param {String} aPreviousBuildId The browser build id we updated from.
    */
-  handleUpdateSuccess(aPreviousVersion, aPreviousBuildId) {
+  async handleUpdateSuccess(aPreviousVersion, aPreviousBuildId) {
     if (!this._enabled) {
       return;
     }
@@ -70,7 +70,9 @@ export var UpdatePing = {
     let updateManager = Cc["@mozilla.org/updates/update-manager;1"].getService(
       Ci.nsIUpdateManager
     );
-    let update = updateManager ? updateManager.updateInstalledAtStartup : null;
+    let update = updateManager
+      ? await updateManager.updateInstalledAtStartup()
+      : null;
 
     const payload = {
       reason: "success",

toolkit/components/telemetry/tests/browser/browser_UpdatePingSuccess.js

@@ -55,7 +55,9 @@ add_task(async function test_updatePing() {
   // Manually call the BrowserContentHandler: this automatically gets called when
   // the browser is started and an update was applied successfully in order to
   // display the "update" info page.
-  Cc["@mozilla.org/browser/clh;1"].getService(Ci.nsIBrowserHandler).defaultArgs;
+  Cc["@mozilla.org/browser/clh;1"]
+    .getService(Ci.nsIBrowserHandler)
+    .getFirstWindowArgs();
 
   // We cannot control when the ping will be generated/archived after we trigger
   // an update, so let's make sure to have one before moving on with validation.

toolkit/mozapps/update/UpdateService.sys.mjs

@@ -4581,16 +4581,18 @@ export class UpdateManager {
   /**
    * See nsIUpdateService.idl
    */
-  get updateInstalledAtStartup() {
+  async updateInstalledAtStartup() {
+    await lazy.AUS.init();
     return this.#updateInstalledAtStartup;
   }
 
   /**
    * See nsIUpdateService.idl
    */
-  get lastUpdateInstalled() {
-    if (this.updateInstalledAtStartup) {
-      return this.updateInstalledAtStartup;
+  async lastUpdateInstalled() {
+    await lazy.AUS.init();
+    if (this.#updateInstalledAtStartup) {
+      return this.#updateInstalledAtStartup;
     }
     return this._getUpdates().find(u => u.state == STATE_SUCCEEDED) ?? null;
   }
@@ -4889,8 +4891,9 @@ export class UpdateManager {
   /**
    * See nsIUpdateService.idl
    */
-  elevationOptedIn() {
+  async elevationOptedIn() {
     // The user has been been made aware that the update requires elevation.
+    await lazy.AUS.init();
     let update = this._readyUpdate;
     if (!update) {
       return;

toolkit/mozapps/update/content/updateElevation.js

@@ -89,7 +89,7 @@ const gUpdateElevationDialog = {
     }
     window.close();
   },
-  onRestartNow() {
+  async onRestartNow() {
     // disable the "finish" (Restart) and "extra1" (Later) buttons
     // because the Software Update wizard is still up at the point,
     // and will remain up until we return and we close the
@@ -108,7 +108,7 @@ const gUpdateElevationDialog = {
     let um = Cc["@mozilla.org/updates/update-manager;1"].getService(
       Ci.nsIUpdateManager
     );
-    um.elevationOptedIn();
+    await um.elevationOptedIn();
 
     // Notify all windows that an application quit has been requested.
     let cancelQuit = Cc["@mozilla.org/supports-PRBool;1"].createInstance(

toolkit/mozapps/update/content/updateElevation.xhtml

@@ -74,7 +74,7 @@
 	        oncommand="gUpdateElevationDialog.onNoThanks();" />
     <spacer flex="1"/>
     <button id="elevateAccept" dlgtype="accept" label="" class="dialog-button"
-	        oncommand="gUpdateElevationDialog.onRestartNow();" default="true"/>
+	        oncommand="gUpdateElevationDialog.onRestartNow();" default="true"/> <!-- note that onRestartNow runs asynchronously -->
   </hbox>
 </dialog>
 </window>

toolkit/mozapps/update/nsIUpdateService.idl

@@ -894,16 +894,18 @@ interface nsIUpdateManager : nsISupports
   Promise getDownloadingUpdate();
 
   /**
-   * If Firefox installed an update at the launch of the current session, this
-   * will contain that update. If not, this will be `null`.
+   * Returns a Promise that resolves with the update that Firefox installed at the
+   * launch of the current session. If no update was installed, null will be returned
+   * @returns Promise<nsIUpdate>
    */
-  readonly attribute nsIUpdate updateInstalledAtStartup;
+  Promise updateInstalledAtStartup();
 
   /**
-   * Returns the most recent update that has been installed,
-   * null if it does not exist.
+   * Returns a Promise that resolves with the most recent update that has been installed,
+   * the value will be null if it does not exist
+   * @returns Promise<nsIUpdate>
    */
-  readonly attribute nsIUpdate lastUpdateInstalled;
+  Promise lastUpdateInstalled();
 
   /**
    * Adds the specified update to the update history. The update history is
@@ -930,8 +932,10 @@ interface nsIUpdateManager : nsISupports
   /**
    * The user agreed to proceed with an elevated update and we are now
    * permitted to show an elevation prompt.
+   *
+   * @returns Promise<undefined>
    */
-  void elevationOptedIn();
+  Promise elevationOptedIn();
 
   /**
    * These functions clean up and remove an active update without applying

toolkit/mozapps/update/tests/browser/browser_showWhatsNewPageTest.js

@@ -179,9 +179,9 @@ async function WnpTest({
   await reloadUpdateManagerData(false);
 
   if (expectedPostUpdatePage) {
-    const postUpdatePage = Cc["@mozilla.org/browser/clh;1"].getService(
-      Ci.nsIBrowserHandler
-    ).defaultArgs;
+    const postUpdatePage = Cc["@mozilla.org/browser/clh;1"]
+      .getService(Ci.nsIBrowserHandler)
+      .getFirstWindowArgs();
     is(
       postUpdatePage,
       expectedPostUpdatePage,

toolkit/mozapps/update/tests/unit_aus_update/cleanupSuccessLogMove.js

@@ -37,11 +37,24 @@ async function run_test() {
     1,
     "the update manager update count" + MSG_SHOULD_EQUAL
   );
+
+  // As of Bug 1642039, updateInstalledAtStartup returns a promise
+  // instead of being an attribute. Therefore we cannot directly
+  // compare it with history[0] which is a XPCOM wrapped object.
+  history[0]
+    .QueryInterface(Ci.nsIWritablePropertyBag)
+    .setProperty("cleanupSuccessLogMoveTestValue", "value1");
+
   Assert.equal(
-    gUpdateManager.updateInstalledAtStartup,
-    history[0],
+    (await gUpdateManager.updateInstalledAtStartup())
+      .QueryInterface(Ci.nsIWritablePropertyBag)
+      .getProperty("cleanupSuccessLogMoveTestValue"),
+    history[0]
+      .QueryInterface(Ci.nsIWritablePropertyBag)
+      .getProperty("cleanupSuccessLogMoveTestValue"),
     "the update installed at startup should be the update from the history"
   );
+
   await waitForUpdateXMLFiles();
 
   let cancelations = Services.prefs.getIntPref(PREF_APP_UPDATE_CANCELATIONS, 0);
@@ -87,7 +100,7 @@ async function run_test() {
   await testPostUpdateProcessing();
 
   Assert.equal(
-    gUpdateManager.updateInstalledAtStartup,
+    await gUpdateManager.updateInstalledAtStartup(),
     null,
     "updateInstalledAtStartup should be cleared on next browser start"
   );