Bug 2026616 - Fix session loss on browser restart

testing/enterprise/manifest.toml

@@ -143,6 +143,8 @@ run-if = [
 
 ["test_felt_restart_is_quit.py"]
 
+["test_felt_restart_persists_session.py"]
+
 ["test_felt_restart_works.py"]
 
 ["test_felt_starts_missing_bin.py"]

testing/enterprise/test_felt_restart_persists_session.py

@@ -0,0 +1,152 @@
+#!/usr/bin/env python3
+# 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 https://mozilla.org/MPL/2.0/.
+
+import os
+import sys
+
+sys.path.append(os.path.dirname(__file__))
+
+from felt_tests import FeltTests
+from marionette_driver.errors import (
+    NoSuchWindowException,
+    UnknownException,
+)
+
+
+class AppRestartPersistsSession(FeltTests):
+    """A FELT restart must preserve the open tabs (bug 2026616 symptom).
+
+    Seeds tabs, restarts, and reads the session the old browser flushed to disk
+    on its clean shutdown (Firefox writes the "clean" sessionstore.jsonlz4 during
+    a completed shutdown and renames it to sessionstore-backups/previous.jsonlz4
+    once the relaunched browser loads it) to confirm those tabs were persisted.
+
+    This guards against session loss on restart, but note it does not by itself
+    reproduce the pre-fix force-kill race: the browser flushes SessionStore
+    during its own clean shutdown before FELT's kill can land, so the session is
+    persisted regardless of FELT's kill-vs-wait timing for a fast shutdown. The
+    core wait-for-exit behaviour is guarded by AppRestartWorks (repeated restarts
+    must not be miscounted as crashes).
+    """
+
+    SESSION_TABS = ["about:support", "about:policies"]
+
+    def test_restart_persists_session(self):
+        super().run_felt_base()
+
+        self.connect_child_browser()
+        self._browser_pid = self._child_driver.session_capabilities["moz:processID"]
+
+        self._open_child_tabs(self.SESSION_TABS)
+        opened = self._child_tab_urls()
+        assert set(self.SESSION_TABS).issubset(opened), (
+            f"Seeded tabs were not open before the restart: {opened}"
+        )
+
+        # Tab state reaches SessionStore's parent side asynchronously; wait so
+        # the shutdown write captures the tabs and not empty entries.
+        self._child_longwait.until(
+            lambda _: set(self.SESSION_TABS).issubset(self._child_session_urls()),
+            message="SessionStore did not record the seeded tabs before restart",
+        )
+
+        self.issue_child_restart()
+        self.wait_process_exit(self._browser_pid)
+
+        self._logger.info("Connecting to new browser")
+        self.connect_child_browser()
+        new_browser_pid = self._child_driver.session_capabilities["moz:processID"]
+        assert new_browser_pid != self._browser_pid, (
+            f"Expected a new process, still {new_browser_pid}"
+        )
+
+        # A force-killed shutdown (the pre-fix behaviour) writes no clean
+        # session, so this waits out and fails.
+        self._child_longwait.until(
+            lambda _: set(self.SESSION_TABS).issubset(
+                set(self._persisted_session_urls())
+            ),
+            message="Clean shutdown did not persist the open tabs to sessionstore",
+        )
+
+        self._logger.info(f"Closing new browser with PID {new_browser_pid}")
+        self._child_driver.set_context("chrome")
+        self._child_driver.execute_script(
+            "Services.startup.quit(Ci.nsIAppStartup.eForceQuit);"
+        )
+
+    def issue_child_restart(self):
+        try:
+            self._logger.info("Issuing restart being done by felt")
+            self._child_driver.set_context("chrome")
+            self._child_driver.execute_script(
+                "Services.startup.quit(Ci.nsIAppStartup.eRestart | Ci.nsIAppStartup.eAttemptQuit);"
+            )
+        except UnknownException:
+            self._logger.info("Received expected UnknownException")
+        except NoSuchWindowException:
+            self._logger.info("Received expected NoSuchWindowException")
+        except OSError:
+            self._logger.info(
+                "Firefox quit before execute_script returned, no data received over Marionette socket"
+            )
+        finally:
+            self._manually_closed_child = True
+
+    def _open_child_tabs(self, urls):
+        for url in urls:
+            self.open_tab_child(url)
+
+    def _child_tab_urls(self):
+        self._child_driver.set_context("chrome")
+        return self._child_driver.execute_script(
+            "return Array.from(gBrowser.tabs).map(t => t.linkedBrowser.currentURI.spec);"
+        )
+
+    def _child_session_urls(self):
+        self._child_driver.set_context("chrome")
+        return self._child_driver.execute_script(
+            """
+            const { SessionStore } = ChromeUtils.importESModule(
+              "resource:///modules/sessionstore/SessionStore.sys.mjs"
+            );
+            const state = JSON.parse(SessionStore.getBrowserState());
+            return state.windows.flatMap(w =>
+              w.tabs.flatMap(t => (t.entries || []).map(e => e.url))
+            );
+            """
+        )
+
+    def _persisted_session_urls(self):
+        # clean is renamed to previous.jsonlz4 once the relaunch loads it.
+        candidates = [
+            os.path.join(self._child_profile_path, "sessionstore.jsonlz4"),
+            os.path.join(
+                self._child_profile_path, "sessionstore-backups", "previous.jsonlz4"
+            ),
+        ]
+        self._child_driver.set_context("chrome")
+        return self._child_driver.execute_async_script(
+            """
+            const paths = arguments[0];
+            const resolve = arguments[arguments.length - 1];
+            (async () => {
+              for (const path of paths) {
+                try {
+                  const text = await IOUtils.readUTF8(path, { decompress: true });
+                  const state = JSON.parse(text);
+                  resolve(
+                    (state.windows || []).flatMap(w =>
+                      (w.tabs || []).flatMap(t => (t.entries || []).map(e => e.url))
+                    )
+                  );
+                  return;
+                } catch (e) {}
+              }
+              resolve([]);
+            })();
+            """,
+            script_args=[candidates],
+        )

toolkit/components/felt/content/FeltProcessParent.sys.mjs

@@ -108,6 +108,7 @@ const kBrowserObserverTopics = [
 ];
 
 let gObserversRegistered = false;
+let gRestartInProgress = false;
 
 /**
  * Manages the SSO login and launching Firefox
@@ -157,6 +158,12 @@ export class FeltProcessParent extends JSProcessActorParent {
           }
 
           case "felt-firefox-restarting": {
+            gRestartInProgress = true;
+            if (gFeltProcessParentInstance) {
+              gFeltProcessParentInstance.restartReported = true;
+              gFeltProcessParentInstance.firefox = null;
+            }
+
             const restartDisabled = Services.prefs.getBoolPref(
               "enterprise.disable_restart",
               false
@@ -188,22 +195,30 @@ export class FeltProcessParent extends JSProcessActorParent {
                 lazy.log.debug(
                   `ParentProcess: restart notification, restartDisabled=${restartDisabled}`
                 );
-                // Kill Firefox directly instead of broadcasting to receiveMessage()
+                // Handle the restart directly instead of broadcasting to receiveMessage()
                 // since gFeltProcessParentInstance is accessible here
-                if (gFeltProcessParentInstance?.proc) {
-                  gFeltProcessParentInstance.restartReported = true;
-                  gFeltProcessParentInstance.firefox = null;
-                  lazy.log.debug(
-                    `ParentProcess: Killing Firefox PID=${gFeltProcessParentInstance.proc.pid}`
-                  );
-                  gFeltProcessParentInstance.proc
-                    .kill()
+                const proc = gFeltProcessParentInstance?.proc;
+                if (proc) {
+                  const willRelaunch = !restartDisabled && !pendingUpdate;
+
+                  if (!willRelaunch && proc.exitCode === null) {
+                    lazy.log.debug(
+                      `ParentProcess: no relaunch, stopping Firefox PID=${proc.pid} to prevent self-relaunch`
+                    );
+                    proc.kill(0);
+                  } else {
+                    lazy.log.debug(
+                      `ParentProcess: Waiting for Firefox PID=${proc.pid} to exit for restart`
+                    );
+                  }
+
+                  proc.exitPromise
                     .then(() => {
                       lazy.log.debug(
-                        `ParentProcess: Killed Firefox, restartDisabled=${restartDisabled}`
+                        `ParentProcess: Firefox exited for restart, restartDisabled=${restartDisabled}`
                       );
 
-                      if (!restartDisabled && !pendingUpdate) {
+                      if (willRelaunch) {
                         lazy.log.debug(`ParentProcess: Starting new Firefox`);
                         gFeltProcessParentInstance.startFirefox(
                           PROCESS_START_REASON.RESTART
@@ -227,11 +242,21 @@ export class FeltProcessParent extends JSProcessActorParent {
                       }
                     })
                     .catch(err => {
-                      lazy.log.debug(`ParentProcess: Kill failed: ${err}`);
+                      lazy.log.debug(
+                        `ParentProcess: Wait for exit failed: ${err}`
+                      );
+                    })
+                    .finally(() => {
+                      gRestartInProgress = false;
                     });
                 } else {
-                  lazy.log.debug(`ParentProcess: No proc to kill!`);
+                  gRestartInProgress = false;
+                  lazy.log.debug(`ParentProcess: No proc to wait for!`);
                 }
+              })
+              .catch(err => {
+                gRestartInProgress = false;
+                lazy.log.debug(`ParentProcess: Restart failed: ${err}`);
               });
             break;
           }
@@ -543,7 +568,11 @@ export class FeltProcessParent extends JSProcessActorParent {
             `firefox exit: PID:${this.proc.pid} exitCode:${JSON.stringify(this.proc.exitCode)}`
           );
 
-          if (!this.restartReported && !this.logoutReported) {
+          if (
+            !this.restartReported &&
+            !this.logoutReported &&
+            !gRestartInProgress
+          ) {
             if (this.proc.exitCode === 0) {
               this.abnormalExitCounter = 0;
               this.abnormalExitFirstTime = 0;