Bring OpenBot in front as it first shows after an in-app update on Windows

desktop/scripts/installer-update.test.ts

@@ -261,3 +261,57 @@ test("the uninstaller deletes the data only when it is not an update", () => {
   expect(remove).toContain('RmDir /r "$APPDATA\\${BUNDLEID}"');
   expect(remove).toContain('RmDir /r "$LOCALAPPDATA\\${BUNDLEID}"');
 });
+
+/**
+ * Box H: after an in-app update the new OpenBot opened behind the maximised Edge that Windows put in
+ * front as the old one closed, and Starting OpenBot ran there unseen for four minutes. Every OpenBot
+ * the installer starts, after an update (/R) and from its last page, is started the same way: the
+ * installer first passes on its right to bring a window to the front (AllowSetForegroundWindow with
+ * ASFW_ANY, -1), then starts OpenBot with the flag that has it come forward as its window first shows
+ * (STARTED_BY_INSTALLER in src-tauri/src/main.rs), added once to the arguments an update hands on.
+ */
+test("every OpenBot the installer starts may come to the front and is told the installer started it", () => {
+  expect(hooks()).toContain(
+    '!define OPENBOT_STARTED_BY_INSTALLER "--started-by-installer"',
+  );
+  const run = code(
+    between(hooks(), "!macro OPENBOT_RUN_AFTER_INSTALL args\n", "!macroend"),
+  );
+  const at = (line: string) => {
+    const index = run.indexOf(line);
+    expect(index).toBeGreaterThan(-1);
+    return index;
+  };
+  const allowed = at("System::Call 'user32::AllowSetForegroundWindow(i -1)'");
+  const given = at('StrCpy $OpenBotRunArgs "${args}"');
+  const looked = at(
+    '${StrLoc} $OpenBotFlagAt "$OpenBotRunArgs" "${OPENBOT_STARTED_BY_INSTALLER}" ">"',
+  );
+  const started = at(
+    'nsis_tauri_utils::RunAsUser "$INSTDIR\\${MAINBINARYNAME}.exe" "$OpenBotRunArgs"',
+  );
+  expect(allowed).toBeLessThan(started);
+  expect(given).toBeLessThan(looked);
+  expect(looked).toBeLessThan(started);
+  // Exactly one launch in the macro.
+  expect(run.filter((line) => line.includes("RunAsUser"))).toHaveLength(1);
+
+  // After an update: the arguments the updater handed on (/ARGS), then nothing of Tauri's runs.
+  const success = code(
+    between(template(), "Function .onInstSuccess", "FunctionEnd"),
+  );
+  const handed = success.indexOf('${GetOptions} $CMDLINE "/ARGS" $R0');
+  expect(handed).toBeGreaterThan(-1);
+  expect(success.slice(handed + 1, handed + 3)).toEqual([
+    '!insertmacro OPENBOT_RUN_AFTER_INSTALL "$R0"',
+    "Return",
+  ]);
+  // From the last page: no arguments, then nothing of Tauri's runs.
+  const finish = code(
+    between(template(), "Function RunMainBinary", "FunctionEnd"),
+  );
+  expect(finish.slice(0, 2)).toEqual([
+    '!insertmacro OPENBOT_RUN_AFTER_INSTALL ""',
+    "Return",
+  ]);
+});

desktop/src-tauri/src/main.rs

@@ -1099,7 +1099,8 @@ fn report_running_row<R: tauri::Runtime>(
 
 /// OpenBot's window in front of whatever is there, at a moment setup needs the person: Docker
 /// Desktop has answered, the system's password prompt has closed, the installation failed or is
-/// done, OpenBot has started. `why` goes in the log.
+/// done, OpenBot has started, a launch the installer started first shows (`forward_at_first_show`).
+/// `why` goes in the log.
 ///
 /// On macOS that means activating the app, not only ordering its window to the front. A window an
 /// inactive app orders front stays behind the active app's. Tauri's `set_focus` does both: tao
@@ -1235,6 +1236,70 @@ fn bring_setup_to_front(app: tauri::AppHandle, why: String) {
     bring_to_front(&app, &why);
 }
 
+/// The argument OpenBot's installer starts it with, after an in-app update and from the installer's
+/// last page (`OPENBOT_RUN_AFTER_INSTALL` in `windows/hooks.nsh`).
+const STARTED_BY_INSTALLER: &str = "--started-by-installer";
+
+/// Whether OpenBot's installer started this launch, from the program's arguments (the program
+/// first).
+fn started_by_installer(args: &[std::ffi::OsString]) -> bool {
+    args.iter().skip(1).any(|arg| arg == STARTED_BY_INSTALLER)
+}
+
+/**
+The window's first show, for a launch OpenBot's installer started (`started_by_installer`): after an
+in-app update, or from the installer's last page. OpenBot comes in front once (`come_forward`, which
+is `bring_to_front`), so the person sees Starting OpenBot and its progress. On box H (Windows Server
+2022, an in-app update from 1.0.1 to 1.0.2) Windows put the maximised Edge in front as 1.0.1 closed,
+1.0.2 opened behind it, and the person saw only Edge for the four minutes of Starting OpenBot, until
+"[front] OpenBot is showing" at home. That one came forward because it came four and a half minutes
+after the person's last input, the click on Update and restart: past the foreground lock time-out,
+the time after input during which Windows keeps apps from coming forward
+(`SPI_GETFOREGROUNDLOCKTIMEOUT`, 200 seconds by default).
+
+Windows lets a window come to the front only from a process it allows ("SetForegroundWindow
+function", Remarks, Microsoft Learn). The right comes from the updating version, which passes it on
+while its window is still up (`pass_the_foreground_on`), and the installer, which passes it on again
+before it starts OpenBot. Where Windows still refuses, as after an update from a version before
+this or once the person has typed or clicked elsewhere, `bring_to_front` flashes the taskbar button.
+
+Any other launch is left to Windows: from the Start menu or the taskbar it is in front already. Held
+back, as every moment is, while the browser sign-in is open (`held_back`).
+*/
+fn forward_at_first_show(started_by_installer: bool, come_forward: impl FnOnce(&str)) {
+    if started_by_installer {
+        come_forward("the installer started OpenBot");
+    }
+}
+
+/**
+Windows, as the updater is about to start the installer: pass OpenBot's right to set the foreground
+window on to every process, so the installer and the new version it starts may come to the front
+(`forward_at_first_show`). Called while OpenBot's window is still up, just before the updater's
+cleanup takes it down: until then OpenBot holds the foreground, from the person's click on Update and
+restart. Once the window is gone, Windows activates the one behind it, and nothing started after that
+may take the foreground from it by itself.
+
+"A process that can set the foreground window can enable another process to set the foreground
+window by calling AllowSetForegroundWindow", and with `ASFW_ANY` "all processes will be enabled". The
+right lasts until "the user generates input" or another process calls it ("AllowSetForegroundWindow
+function", Microsoft Learn). So if the person types or clicks in another window meanwhile, Windows
+keeps that window in front, as it means to, and the new version flashes its taskbar button instead.
+*/
+#[cfg(windows)]
+fn pass_the_foreground_on() {
+    use windows::Win32::UI::WindowsAndMessaging::{AllowSetForegroundWindow, ASFW_ANY};
+    // SAFETY: takes no pointers; it only widens which processes may set the foreground window.
+    match unsafe { AllowSetForegroundWindow(ASFW_ANY) } {
+        Ok(()) => log::info!(
+            "[app update] passed the foreground on, for the installer and the new version"
+        ),
+        Err(error) => log::info!(
+            "[app update] could not pass the foreground on ({error}); OpenBot was no longer in front"
+        ),
+    }
+}
+
 /// The person said yes to pausing their Podman while OpenBot runs. See `own_machine`. False when
 /// nothing was waiting for it.
 #[tauri::command]
@@ -2746,6 +2811,9 @@ fn stop_before_installer<R: tauri::Runtime>(app: &tauri::AppHandle<R>) {
         // The installer runs either way: the new version's Start finds whatever is left running.
         log::warn!("[app update] stopping OpenBot before the installer: {error}");
     }
+    // While the window is still up and in front: the new version comes forward as it first shows.
+    #[cfg(windows)]
+    pass_the_foreground_on();
     app.cleanup_before_exit();
 }
 
@@ -10953,6 +11021,8 @@ fn main() {
     // The local time's offset from UTC, for the log's first line. Read now, while this is the only
     // thread: on macOS and Linux the time crate refuses to read it once there are others.
     let local_offset = time::UtcOffset::current_local_offset().ok();
+    // After an in-app update, or from the installer's last page: in front as the window first shows.
+    let from_installer = started_by_installer(&args);
     let context = tauri::generate_context!();
     // A `shell-updater` build with compiled trust supplies `plugins.updater` when no bundled block
     // (such as the signed Mac overlay) already does. Registration itself happens once, in `setup`.
@@ -11138,6 +11208,9 @@ fn main() {
         .setup(move |app| {
             build_main_window(app.handle())?;
             log_launch(app.handle(), local_offset);
+            // The window is built visible, so this is its first show: a launch the installer
+            // started comes forward now, while Starting OpenBot is what it shows.
+            forward_at_first_show(from_installer, |why| bring_to_front(app.handle(), why));
             // Built by `generate_context!`, so the permissions OpenBot's page is granted are
             // compiled in. First, before anything that could trust an app and open it.
             know_app_page_grants(app.handle());
@@ -15532,6 +15605,101 @@ mod tests {
         ));
     }
 
+    /// Box H, an in-app update from 1.0.1 to 1.0.2 on Windows Server 2022 with a maximised Edge
+    /// open: Windows put Edge in front as 1.0.1 closed, and 1.0.2 opened behind it. The person saw
+    /// only Edge through the four minutes of Starting OpenBot, until "[front] OpenBot is showing"
+    /// at home. The updating version passes the foreground on before its window goes, the
+    /// installer passes it on again and starts the new version saying so, and the new version
+    /// comes forward once, as its window first shows. Never while the browser sign-in is open.
+    #[test]
+    fn a_launch_the_installer_started_comes_forward_once_as_its_window_first_shows() {
+        if crate::test_support::isolated_process(
+            "tests::a_launch_the_installer_started_comes_forward_once_as_its_window_first_shows",
+        ) {
+            return;
+        }
+        let args = |given: &[&str]| -> Vec<std::ffi::OsString> {
+            std::iter::once("openbot-desktop.exe")
+                .chain(given.iter().copied())
+                .map(std::ffi::OsString::from)
+                .collect()
+        };
+        // The installer's launch, after an update or from its last page. An update hands the old
+        // version's own arguments on (/ARGS), and the installer adds the flag when they lack it.
+        assert!(started_by_installer(&args(&[STARTED_BY_INSTALLER])));
+        assert!(started_by_installer(&args(&[
+            "--kept",
+            STARTED_BY_INSTALLER
+        ])));
+        // The Start menu, sign-in to Windows, a notification's link: Windows' own rules.
+        assert!(!started_by_installer(&args(&[])));
+        assert!(!started_by_installer(&args(&["openbot://open?path=/"])));
+        assert!(!started_by_installer(&[std::ffi::OsString::from(
+            STARTED_BY_INSTALLER
+        )]));
+
+        // Started by the installer: it asks for the front once, and nothing holds it back.
+        let asked = std::cell::RefCell::new(Vec::new());
+        forward_at_first_show(true, |why| {
+            asked.borrow_mut().push((why.to_owned(), held_back(why)))
+        });
+        assert_eq!(
+            *asked.borrow(),
+            [("the installer started OpenBot".to_owned(), false)]
+        );
+        // Any other launch asks nothing.
+        forward_at_first_show(false, |why| {
+            asked.borrow_mut().push((why.to_owned(), held_back(why)))
+        });
+        assert_eq!(asked.borrow().len(), 1);
+        // While the browser sign-in is open it is held back, as every other moment is.
+        let held = while_signing_in(
+            || {
+                let mut held = None;
+                forward_at_first_show(true, |why| held = Some(held_back(why)));
+                held
+            },
+            || {},
+        );
+        assert_eq!(held, Some(true));
+
+        let main = include_str!("main.rs");
+        // In setup, once, as the window is first built, through `bring_to_front`, which asks
+        // `held_back` first.
+        let setup = &main[main.find(".setup(move |app| {\n").unwrap()..];
+        assert!(
+            setup.starts_with(
+                ".setup(move |app| {\n            build_main_window(app.handle())?;\n            log_launch(app.handle(), local_offset);\n"
+            ),
+            "{}",
+            &setup[..400]
+        );
+        let first_show = &setup[setup.find("log_launch(").unwrap()..];
+        let first_show = &first_show[first_show.find('\n').unwrap() + 1..];
+        let first_show = &first_show[..first_show.find("know_app_page_grants").unwrap()];
+        assert!(
+            first_show.contains(
+                "forward_at_first_show(from_installer, |why| bring_to_front(app.handle(), why));"
+            ),
+            "{first_show}"
+        );
+        let launch = &main[main.find("\nfn main() {\n").unwrap()..];
+        let launch = &launch[..launch.find(".setup(move |app| {\n").unwrap()];
+        assert!(launch.contains("let from_installer = started_by_installer(&args);"));
+        // The updating version passes the foreground on while its window is still up: before the
+        // updater's cleanup takes the windows down and Windows activates the one behind.
+        let before = &main[main.find("fn stop_before_installer<").unwrap()..];
+        let before = &before[..before.find("\n}\n").unwrap()];
+        let passed = before
+            .find("pass_the_foreground_on();")
+            .expect("the foreground passed on before the installer starts");
+        assert!(passed < before.find("app.cleanup_before_exit();").unwrap());
+        // The installer starts OpenBot with the flag read here.
+        assert!(include_str!("../windows/hooks.nsh").contains(&format!(
+            "!define OPENBOT_STARTED_BY_INSTALLER \"{STARTED_BY_INSTALLER}\""
+        )));
+    }
+
     /// Windows cascades a window with no position of its own down and right on every launch. On
     /// a 1080p screen with a 48 px taskbar the third launch put a 1016 by 839 window's top at 234,
     /// its bottom at 1073, past the work area's 1032, and the account row behind the taskbar.

desktop/src-tauri/windows/hooks.nsh

@@ -160,6 +160,33 @@ Var OpenBotRunning
   Pop $0
 !macroend
 
+; Every OpenBot the installer starts, after an in-app update (/R, in .onInstSuccess) and from its last
+; page (RunMainBinary), is started here, so it can come to the front as its window first shows. On
+; box H, after an in-app update, the new OpenBot opened behind the maximised Edge that Windows put in
+; front as the old one closed, and Starting OpenBot ran there unseen for four minutes. Windows lets
+; only some processes set the foreground window, and one that may can let every other process do it
+; (AllowSetForegroundWindow with ASFW_ANY, -1) until the person next gives input. The updating OpenBot
+; passes that on before its window goes (src/main.rs), and the person's click on Finish gives it to
+; the installer, so the installer passes it on again, then starts OpenBot with
+; --started-by-installer, which has it come forward once (`forward_at_first_show` in src/main.rs).
+; Where Windows still says no, OpenBot flashes its taskbar button. An update hands the old version's
+; own arguments on (/ARGS), which carry the flag from the last update, so it is added only when
+; missing. RunAsUser is how Tauri starts the app, as the person rather than an elevated installer.
+!define OPENBOT_STARTED_BY_INSTALLER "--started-by-installer"
+Var OpenBotRunArgs
+Var OpenBotFlagAt
+!macro OPENBOT_RUN_AFTER_INSTALL args
+  System::Call 'user32::AllowSetForegroundWindow(i -1)'
+  StrCpy $OpenBotRunArgs "${args}"
+  ${StrLoc} $OpenBotFlagAt "$OpenBotRunArgs" "${OPENBOT_STARTED_BY_INSTALLER}" ">"
+  ${If} $OpenBotRunArgs == ""
+    StrCpy $OpenBotRunArgs "${OPENBOT_STARTED_BY_INSTALLER}"
+  ${ElseIf} $OpenBotFlagAt == ""
+    StrCpy $OpenBotRunArgs "$OpenBotRunArgs ${OPENBOT_STARTED_BY_INSTALLER}"
+  ${EndIf}
+  nsis_tauri_utils::RunAsUser "$INSTDIR\${MAINBINARYNAME}.exe" "$OpenBotRunArgs"
+!macroend
+
 !macro NSIS_HOOK_PREINSTALL
   ${If} $OpenBotUninstallFirst = 1
     !insertmacro OPENBOT_UNINSTALL_OLD_VERSION

desktop/src-tauri/windows/installer.nsi

@@ -19,6 +19,10 @@
 ; that left nothing on screen for minutes. No hook could change the page, and no setting changes
 ; what it runs. Uninstalling from Windows' Apps runs the uninstaller on its own and is unchanged, so
 ; the person can still delete the data there.
+;
+; And every OpenBot this installer starts, after an in-app update and from the last page, is started
+; by OPENBOT_RUN_AFTER_INSTALL (windows/hooks.nsh), so it comes to the front as its window first
+; shows rather than behind the window Windows put in front when the old version closed.
 ; <<< OpenBot
 Unicode true
 ManifestDPIAware true
@@ -454,6 +458,12 @@ Var AppStartMenuFolder
 !insertmacro MUI_PAGE_FINISH
 
 Function RunMainBinary
+  ; >>> OpenBot
+  ; Started as every OpenBot the installer starts: free to come to the front, and told the
+  ; installer started it (OPENBOT_RUN_AFTER_INSTALL in windows/hooks.nsh).
+  !insertmacro OPENBOT_RUN_AFTER_INSTALL ""
+  Return
+  ; <<< OpenBot
   nsis_tauri_utils::RunAsUser "$INSTDIR\${MAINBINARYNAME}.exe" ""
 FunctionEnd
 
@@ -785,6 +795,13 @@ Function .onInstSuccess
     ${GetOptions} $CMDLINE "/R" $R0
     ${IfNot} ${Errors}
       ${GetOptions} $CMDLINE "/ARGS" $R0
+      ; >>> OpenBot
+      ; After an in-app update: the new version, free to come to the front, and told the installer
+      ; started it (OPENBOT_RUN_AFTER_INSTALL in windows/hooks.nsh), with the arguments the updater
+      ; handed on.
+      !insertmacro OPENBOT_RUN_AFTER_INSTALL "$R0"
+      Return
+      ; <<< OpenBot
       nsis_tauri_utils::RunAsUser "$INSTDIR\${MAINBINARYNAME}.exe" "$R0"
     ${EndIf}
   ${EndIf}