Enterprise: Handle browser crash

.github/workflows/enterprise.yml

@@ -250,6 +250,16 @@ jobs:
           . venv_tests/bin/activate
           MOZ_LOG=console:5 python3 test_felt_browser_signout.py ${{ steps.unpack.outputs.runtime_path }} $PWD/geckodriver $PWD/profiles/
 
+      - name: "test browser crash handled"
+        run: |
+          . venv_tests/bin/activate
+          MOZ_LOG=console:5 python3 test_felt_browser_crash_abort_restart.py ${{ steps.unpack.outputs.runtime_path }} $PWD/geckodriver $PWD/profiles/
+
+      - name: "test browser crash restart"
+        run: |
+          . venv_tests/bin/activate
+          MOZ_LOG=console:5 python3 test_felt_browser_crash_restart.py ${{ steps.unpack.outputs.runtime_path }} $PWD/geckodriver $PWD/profiles/
+
       - name: "test browser safe mode"
         run: |
           . venv_tests/bin/activate
@@ -554,6 +564,18 @@ jobs:
           $Env:MOZ_LOG="console:5"
           python3 test_felt_browser_fxa.py "${{ steps.unpack.outputs.firefox_path }}" "${{ steps.unpack.outputs.geckodriver_path }}" "${{ steps.unpack.outputs.profiles_path }}"
 
+      - name: "test browser crash handled"
+        run: |
+          . venv_tests\Scripts\activate
+          $Env:MOZ_LOG="console:5"
+          python3 test_felt_browser_crash_abort_restart.py "${{ steps.unpack.outputs.firefox_path }}" "${{ steps.unpack.outputs.geckodriver_path }}" "${{ steps.unpack.outputs.profiles_path }}"
+
+      - name: "test browser crash restart"
+        run: |
+          . venv_tests\Scripts\activate
+          $Env:MOZ_LOG="console:5"
+          python3 test_felt_browser_crash_restart.py "${{ steps.unpack.outputs.firefox_path }}" "${{ steps.unpack.outputs.geckodriver_path }}" "${{ steps.unpack.outputs.profiles_path }}"
+
       ### This is failing on our GCP workers in a non debuggable way.
       ### GitHub Actions Windows workers are fine and TaskCluster.
       # - name: "test browser safe mode"
@@ -993,6 +1015,16 @@ jobs:
           . venv_tests/bin/activate
           MOZ_LOG=console:5 python3 test_felt_browser_fxa.py "${{ steps.attach.outputs.runtime_path }}" $PWD/geckodriver $PWD/profiles/
 
+      - name: "test browser crash handled"
+        run: |
+          . venv_tests/bin/activate
+          MOZ_LOG=console:5 python3 test_felt_browser_crash_abort_restart.py "${{ steps.attach.outputs.runtime_path }}" $PWD/geckodriver $PWD/profiles/
+
+      - name: "test browser crash restart"
+        run: |
+          . venv_tests/bin/activate
+          MOZ_LOG=console:5 python3 test_felt_browser_crash_restart.py "${{ steps.attach.outputs.runtime_path }}" $PWD/geckodriver $PWD/profiles/
+
       - name: "test browser safe mode"
         run: |
           . venv_tests/bin/activate

browser/components/enterprise/modules/ConsoleClient.sys.mjs

@@ -326,7 +326,7 @@ export const ConsoleClient = {
     }
 
     const text = await res.text().catch(() => "");
-    throw new Error(`Fetch failed (${res.status}): ${text}`);
+    throw new Error(`Fetch ${method} ${path} failed (${res.status}): ${text}`);
   },
 
   /**

browser/extensions/felt/api.js

@@ -144,13 +144,12 @@ this.felt = class extends ExtensionAPI {
         );
         break;
 
-      case "FeltParent:FirefoxAbnormalExit":
-        Services.ppmm.removeMessageListener(
-          "FeltParent:FirefoxAbnormalExit",
-          this
-        );
-        // TODO: What should we do, restart Firefox?
+      case "FeltParent:FirefoxAbnormalExit": {
+        const success = Services.felt.makeBackgroundProcess(false);
+        console.debug(`FeltExtension: makeBackgroundProcess? ${success}`);
+        this.showWindow("felt-browser-error-multiple-crashes");
         break;
+      }
 
       case "FeltParent:FirefoxLogoutExit": {
         const success = Services.felt.makeBackgroundProcess(false);
@@ -188,13 +187,17 @@ this.felt = class extends ExtensionAPI {
     }
   }
 
-  showWindow() {
+  showWindow(errorMessage = "") {
     // Height and width are for now set to fit the sso.mozilla.com without the need to resize the window
     let flags =
       "chrome,private,centerscreen,titlebar,resizable,width=727,height=744";
+    const queryString = errorMessage
+      ? `?error=${encodeURIComponent(errorMessage)}`
+      : "";
+
     this._win = Services.ww.openWindow(
       null,
-      "chrome://felt/content/felt.xhtml",
+      `chrome://felt/content/felt.xhtml${queryString}`,
       "_blank",
       flags,
       null

browser/extensions/felt/content/FeltProcessParent.sys.mjs

@@ -55,6 +55,26 @@ export class FeltProcessParent extends JSProcessActorParent {
     // Track extension ready state (extension must register its observer)
     this.extensionReady = false;
 
+    this.abnormalExitCounter = 0;
+
+    // Amount of abnormal exit to allow over abnormal_exit_period
+    this.abnormalExitLimit = Services.prefs.getIntPref(
+      "enterprise.browser.abnormal_exit_limit",
+      3
+    );
+
+    /* Time period (in seconds) considered for checking the amount of abnormal
+     * exits. Hitting the limit defined above within this period will stop
+     * automatic restart and show user an error.
+     *
+     * confere shouldAbortRestarting()
+     */
+    this.abnormalExitPeriod = Services.prefs.getIntPref(
+      "enterprise.browser.abnormal_exit_period",
+      120
+    );
+    this.abnormalExitFirstTime = 0;
+
     this.restartObserver = {
       observe(aSubject, aTopic) {
         console.debug(`FeltExtension: ParentProcess: Received ${aTopic}`);
@@ -253,21 +273,65 @@ export class FeltProcessParent extends JSProcessActorParent {
           );
           if (!this.restartReported && !this.logoutReported) {
             if (this.proc.exitCode === 0) {
+              this.abnormalExitCounter = 0;
+              this.abnormalExitFirstTime = 0;
               Services.cpmm.sendAsyncMessage(
                 "FeltParent:FirefoxNormalExit",
                 {}
               );
             } else {
-              Services.cpmm.sendAsyncMessage(
-                "FeltParent:FirefoxAbnormalExit",
-                {}
-              );
+              this.handleRestartAfterAbnormalExit();
             }
           }
         });
       });
   }
 
+  /**
+   * Handles the abnormal exit and decides whether to restart the Firefox
+   * again or to inform the user of the set of crashes.
+   */
+  handleRestartAfterAbnormalExit() {
+    if (this.abnormalExitCounter === 0) {
+      this.abnormalExitFirstTime =
+        Services.telemetry.msSinceProcessStart() / 1000;
+    }
+    this.abnormalExitCounter += 1;
+    if (this.shouldAbortRestarting()) {
+      console.debug(
+        "Abort restarting Firefox and inform the user of the crashes."
+      );
+      Services.cpmm.sendAsyncMessage("FeltParent:FirefoxAbnormalExit", {});
+    } else {
+      console.debug("Trying to restart Firefox again.");
+      this.startFirefox([]);
+    }
+  }
+  /**
+   * Checks the state of the recent abnormal exits, meaning whether the crashes
+   * counter exceeds a pre-set counter limit within a pre-set time period.
+   *
+   * @returns {boolean} Whether these "abnormal" thresholds are exceeded.
+   */
+  shouldAbortRestarting() {
+    console.debug(
+      `Firefox AbnormalExit abnormalExitLimit=${this.abnormalExitLimit} abnormalExitCounter=${this.abnormalExitCounter} ; firstTime=${this.abnormalExitFirstTime} abnormalExitPeriod=${this.abnormalExitPeriod}`
+    );
+    // Have we reached the limit of allowed crashes ?
+    const isExceedingCrashCounterLimit =
+      this.abnormalExitCounter >= this.abnormalExitLimit;
+    // How much time since the first crash we recorded in this session ?
+    const timeSinceFirstCrash =
+      Services.telemetry.msSinceProcessStart() / 1000 -
+      this.abnormalExitFirstTime;
+    // Is the time since first crash too recent ?
+    const isWithinCrashPeriod = timeSinceFirstCrash <= this.abnormalExitPeriod;
+    console.debug(
+      `Firefox AbnormalExit crashLimitHit=${isExceedingCrashCounterLimit} timeSinceFirstCrash=${timeSinceFirstCrash} crashedNotLongAgoEnough=${isWithinCrashPeriod}`
+    );
+    return isExceedingCrashCounterLimit && isWithinCrashPeriod;
+  }
+
   async startFirefoxProcess() {
     let socket = Services.felt.oneShotIpcServer();
 

browser/extensions/felt/content/felt.xhtml

@@ -81,6 +81,13 @@
             </div>
           </form>
         </div>
+        <div class="felt-browser-error is-hidden">
+          <span
+            class="felt-browser-error-multiple-crashes is-hidden"
+            data-l10n-id="felt-browser-error-multiple-crashes"
+          >
+          </span>
+        </div>
         <span
           class="felt-powered-by text-deemphasized"
           data-l10n-id="felt-powered-by"

browser/extensions/felt/content/locales/felt.ftl

@@ -12,6 +12,7 @@ felt-sso-input-email =
     .accesskey = E
 felt-sso-continue-btn =
     .label = Continue
+felt-browser-error-multiple-crashes = { -brand-short-name } crashed multiple times
 felt-powered-by =
     Powered by { -vendor-short-name }
 # $version is the version of Felt, not Firefox/Enterprise Browser.

browser/extensions/felt/content/styles/felt.css

@@ -160,3 +160,15 @@ body:has(#notification-popup-box[open]) .felt:not(#notification-popup-box .felt)
   opacity: 0.5;
   pointer-events: none;
 }
+
+.felt-browser-error {
+  position: absolute;
+  /* stylelint-disable-next-line */
+  bottom: 32px;
+  /* stylelint-disable-next-line */
+  left: 8px;
+  font-size: medium;
+  /* stylelint-disable-next-line stylelint-plugin-mozilla/use-design-tokens */
+  color: var(--icon-color-critical);
+  font-weight: var(--font-weight-bold);
+}

browser/extensions/felt/content/window.js

@@ -97,6 +97,18 @@ async function listenFormEmailSubmission() {
   });
 }
 
+function informAboutPotentialStartupFailure() {
+  if (window.location.search) {
+    const errorClass = new URLSearchParams(window.location.search).get("error");
+    if (errorClass) {
+      document
+        .querySelector(".felt-browser-error")
+        .classList.remove("is-hidden");
+      document.querySelector(`.${errorClass}`).classList.remove("is-hidden");
+    }
+  }
+}
+
 function setupMarionetteEnvironment() {
   window.fullScreen = false;
 
@@ -186,6 +198,7 @@ window.addEventListener(
     setupMarionetteEnvironment();
     setupPopupNotifications();
     listenFormEmailSubmission();
+    informAboutPotentialStartupFailure();
   },
   true
 );

taskcluster/kinds/enterprise-test/kind.yml

@@ -170,6 +170,18 @@ tasks:
             enterprise_test_file: test_felt_browser_fxa.py
             enterprise_test_slug: feltFxa
 
+    felt-browser-crashAbortRestart:
+        description: Testing proper cancellation of restarts after too many crashes
+        attributes:
+            enterprise_test_file: test_felt_browser_crash_abort_restart.py
+            enterprise_test_slug: feltCrashAbortRestart
+
+    felt-browser-crashRestart:
+        description: Testing proper restart by FELT after crash of browser
+        attributes:
+            enterprise_test_file: test_felt_browser_crash_restart.py
+            enterprise_test_slug: feltCrashRestart
+
     felt-browser-safemode:
         description: Testing passing safe mode from FELT to Browser
         attributes:

testing/enterprise/felt_browser_crash_abort_restart.json

@@ -0,0 +1,14 @@
+{
+  "test_felt_00_chrome_on_email_submit": {},
+  "test_felt_0_load_sso": {
+    "elements": [
+      ["#login", "login"],
+      ["#password", "password"]
+    ]
+  },
+  "test_felt_1_perform_sso_auth": {},
+  "test_felt_2_crash_parent_once": {},
+  "test_felt_3_proper_restart": {},
+  "test_felt_4_crash_parent_twice": {},
+  "test_felt_5_check_error_message": {}
+}

testing/enterprise/felt_browser_crash_restart.json

@@ -0,0 +1,14 @@
+{
+  "test_felt_00_chrome_on_email_submit": {},
+  "test_felt_0_load_sso": {
+    "elements": [
+      ["#login", "login"],
+      ["#password", "password"]
+    ]
+  },
+  "test_felt_1_perform_sso_auth": {},
+  "test_felt_2_crash_parent_once": {},
+  "test_felt_3_proper_restart": {},
+  "test_felt_4_crash_parent_twice": {},
+  "test_felt_5_proper_restart_again": {}
+}

testing/enterprise/felt_browser_crashes.py

@@ -0,0 +1,46 @@
+#!/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/.
+
+
+from felt_tests import FeltTests
+from selenium.webdriver.support import expected_conditions as EC
+
+
+class BrowserCrashes(FeltTests):
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+    def record_felt_no_window(self):
+        self._window_handles = self._driver.window_handles
+
+    def await_felt_auth_window(self):
+        if len(self._window_handles) == 1 and self._window_handles[0] is None:
+            self._window_handles = []
+        self._wait.until(EC.new_window_is_opened(self._window_handles))
+
+    def force_window(self):
+        self._driver.set_context("chrome")
+        assert len(self._driver.window_handles) == 1, "One window exists"
+        self._driver.switch_to.window(self._driver.window_handles[0])
+        self._driver.set_context("content")
+
+    def crash_parent(self):
+        self._browser_pid = self._child_driver.capabilities["moz:processID"]
+        self._logger.info(f"Crashing browser at {self._browser_pid}")
+        try:
+            # This is going to trigger exception for sure
+            self._logger.info("Crashing main process")
+            self.open_tab_child("about:crashparent")
+        except Exception as ex:
+            self._logger.info(f"Caught exception {ex}")
+            pass
+
+    def connect_and_crash(self):
+        # Make sure we record the proper state of window handles of FELT before
+        # we may re-open the window
+        self.record_felt_no_window()
+
+        self.connect_child_browser()
+        self.crash_parent()

testing/enterprise/felt_tests.py

@@ -16,6 +16,7 @@
 from http.server import BaseHTTPRequestHandler, HTTPServer
 from multiprocessing import Manager, Process, Value
 
+import psutil
 import requests
 from base_test import EnterpriseTestsBase
 from selenium.webdriver.common.by import By
@@ -539,6 +540,31 @@ def get_elem_child(self, e):
                 EC.visibility_of_element_located((By.CSS_SELECTOR, e))
             )
 
+    def wait_process_exit(self):
+        self._logger.info("Waiting a few seconds ...")
+        if sys.platform == "win32":
+            time.sleep(8)
+        else:
+            time.sleep(3)
+        self._logger.info(f"Checking PID {self._browser_pid}")
+
+        if not psutil.pid_exists(self._browser_pid):
+            self._logger.info(f"No more PID {self._browser_pid}")
+        else:
+            try:
+                process = psutil.Process(pid=self._browser_pid)
+                process_name = process.name()
+                process_exe = process.exe()
+                process_basename = os.path.basename(process_name)
+                process_cmdline = process.cmdline()
+                self._logger.info(
+                    f"Found PID {self._browser_pid}: EXE:{process_exe} :: NAME:{process_name} :: CMDLINE:{process_cmdline} :: BASENAME:'{process_basename}'"
+                )
+                assert process_basename != "firefox", "Process is not Firefox"
+            except psutil.ZombieProcess:
+                self._logger.info(f"Zombie found as {self._browser_pid}")
+                return True
+
     def test_felt_00_chrome_on_email_submit(self, exp):
         self._driver.set_context("chrome")
         self._logger.info("Submitting email in chrome context ...")