Bug 1919294 - Send about:restartrequired telemetry from the place buildid mismatch is detected r=jld

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

browser/base/content/test/tabcrashed/browser_aboutRestartRequired_basic.js

@@ -7,15 +7,17 @@ requestLongerTimeout(5);
 
 SimpleTest.expectChildProcessCrash();
 
-add_task(async function test_browser_crashed_basic_event() {
-  info("Waiting for oop-browser-crashed event.");
-
-  Services.telemetry.clearScalars();
+async function execTest() {
   is(
-    getFalsePositiveTelemetry(),
+    await getFalsePositiveTelemetry(),
     undefined,
     "Build ID mismatch false positive count should be undefined"
   );
+  is(
+    await getTrueMismatchTelemetry(),
+    undefined,
+    "Build ID true mismatch count should be undefined"
+  );
 
   await forceCleanProcesses();
   let eventPromise = getEventPromise("oop-browser-crashed", "basic");
@@ -23,9 +25,28 @@ add_task(async function test_browser_crashed_basic_event() {
   await eventPromise;
 
   is(
-    getFalsePositiveTelemetry(),
+    await getTrueMismatchTelemetry(),
+    undefined,
+    "Build ID true mismatch count should be undefined"
+  );
+  is(
+    await getFalsePositiveTelemetry(),
     undefined,
     "Build ID mismatch false positive count should be undefined"
   );
+
   await closeTab(tab);
+}
+
+add_task(async function test_telemetry_restartrequired_no_mismatch() {
+  // Do not clear telemetry's scalars, otherwise --verify will break because
+  // the parent process will have kept memory of sent telemetry but that test
+  // will not be aware
+
+  info("Waiting for oop-browser-crashed event.");
+
+  // Run once
+  await execTest();
+  // Run a second time and make sure it has not increased
+  await execTest();
 });

browser/base/content/test/tabcrashed/browser_aboutRestartRequired_buildid_false-positive.js

@@ -7,16 +7,7 @@ requestLongerTimeout(2);
 
 SimpleTest.expectChildProcessCrash();
 
-add_task(async function test_browser_crashed_false_positive_event() {
-  info("Waiting for oop-browser-crashed event.");
-
-  Services.telemetry.clearScalars();
-  is(
-    getFalsePositiveTelemetry(),
-    undefined,
-    "Build ID mismatch false positive count should be undefined"
-  );
-
+async function execTest(expectedValueAfter) {
   setBuildidMatchDontSendEnv();
   await forceCleanProcesses();
   let eventPromise = getEventPromise("oop-browser-crashed", "false-positive");
@@ -25,10 +16,25 @@ add_task(async function test_browser_crashed_false_positive_event() {
   unsetBuildidMatchDontSendEnv();
 
   is(
-    getFalsePositiveTelemetry(),
-    1,
-    "Build ID mismatch false positive count should be 1"
+    await getFalsePositiveTelemetry(),
+    expectedValueAfter,
+    `Build ID mismatch false positive count should be ${expectedValueAfter}`
   );
 
   await closeTab(tab);
-});
+}
+
+add_task(
+  async function test_telemetry_restartrequired_falsepositive_mismatch() {
+    // Do not clear telemetry's scalars, otherwise --verify will break because
+    // the parent process will have kept memory of sent telemetry but that test
+    // will not be aware
+
+    info("Waiting for oop-browser-crashed event.");
+
+    // Run once
+    await execTest(1);
+    // Run a second time and make sure it has not increased
+    await execTest(1);
+  }
+);

browser/base/content/test/tabcrashed/browser_aboutRestartRequired_buildid_true.js

@@ -0,0 +1,43 @@
+"use strict";
+
+// On debug builds, crashing tabs results in much thinking, which
+// slows down the test and results in intermittent test timeouts,
+// so we'll pump up the expected timeout for this test.
+requestLongerTimeout(5);
+
+SimpleTest.expectChildProcessCrash();
+
+async function execTest(expectedValueAfter) {
+  setBuildidMismatchEnv();
+  setBuildidMatchDontSendEnv();
+  await forceCleanProcesses();
+  let eventPromise = getEventPromise(
+    "oop-browser-buildid-mismatch",
+    "real-mismatch"
+  );
+  let tab = await openNewTab(false);
+  await eventPromise;
+  unsetBuildidMatchDontSendEnv();
+  unsetBuildidMismatchEnv();
+
+  is(
+    await getTrueMismatchTelemetry(),
+    expectedValueAfter,
+    `Build ID true mismatch count should be ${expectedValueAfter}`
+  );
+
+  await closeTab(tab);
+}
+
+add_task(async function test_telemetry_restartrequired_real_mismatch() {
+  // Do not clear telemetry's scalars, otherwise --verify will break because
+  // the parent process will have kept memory of sent telemetry but that test
+  // will not be aware
+
+  info("Waiting for oop-browser-buildid-mismatch event.");
+
+  // Run once
+  await execTest(1);
+  // Run a second time and make sure it has not increased
+  await execTest(1);
+});

browser/base/content/test/tabcrashed/browser_aboutRestartRequired_noForkServer.toml

@@ -13,3 +13,5 @@ prefs = [
 
 ["browser_aboutRestartRequired_buildid_false-positive.js"]
 skip-if = ["win11_2009 && msix && debug"]  # bug 1823581
+
+["browser_aboutRestartRequired_buildid_true.js"]

browser/base/content/test/tabcrashed/head.js

@@ -4,6 +4,11 @@ const { TelemetryTestUtils } = ChromeUtils.importESModule(
   "resource://testing-common/TelemetryTestUtils.sys.mjs"
 );
 
+/* eslint-disable mozilla/no-redeclare-with-import-autofix */
+const { ContentTaskUtils } = ChromeUtils.importESModule(
+  "resource://testing-common/ContentTaskUtils.sys.mjs"
+);
+
 /**
  * Returns a Promise that resolves once a crash report has
  * been submitted. This function will also test the crash
@@ -161,10 +166,22 @@ function setBuildidMatchDontSendEnv() {
 }
 
 function unsetBuildidMatchDontSendEnv() {
-  info("Setting " + kBuildidMatchEnv + "=0");
+  info("Unsetting " + kBuildidMatchEnv);
   Services.env.set(kBuildidMatchEnv, "0");
 }
 
+const kBuildidMismatchEnv = "MOZ_FORCE_BUILDID_MISMATCH";
+
+function setBuildidMismatchEnv() {
+  info("Setting " + kBuildidMismatchEnv + "=1");
+  Services.env.set(kBuildidMismatchEnv, "1");
+}
+
+function unsetBuildidMismatchEnv() {
+  info("Unsetting " + kBuildidMismatchEnv);
+  Services.env.set(kBuildidMismatchEnv, "0");
+}
+
 function getEventPromise(eventName, eventKind) {
   return new Promise(function (resolve) {
     info("Installing event listener (" + eventKind + ")");
@@ -212,9 +229,47 @@ async function closeTab(tab) {
   BrowserTestUtils.removeTab(tab);
 }
 
-function getFalsePositiveTelemetry() {
-  const scalars = TelemetryTestUtils.getProcessScalars("parent");
-  return scalars["dom.contentprocess.buildID_mismatch_false_positive"];
+const kInterval = 100; /* ms */
+const kRetries = 5;
+
+/**
+ * This function waits until utility scalars are reported into the
+ * scalar snapshot.
+ */
+async function waitForProcessScalars(name) {
+  await ContentTaskUtils.waitForCondition(
+    () => {
+      const scalars = TelemetryTestUtils.getProcessScalars("parent");
+      return Object.keys(scalars).includes(name);
+    },
+    `Waiting for ${name} scalars to have been set`,
+    kInterval,
+    kRetries
+  );
+}
+
+async function getTelemetry(name) {
+  try {
+    await waitForProcessScalars(name);
+    const scalars = TelemetryTestUtils.getProcessScalars("parent");
+    return scalars[name];
+  } catch (ex) {
+    const msg = `Waiting for ${name} scalars to have been set`;
+    if (ex.indexOf(msg) === 0) {
+      return undefined;
+    }
+    throw ex;
+  }
+}
+
+async function getFalsePositiveTelemetry() {
+  return await getTelemetry(
+    "dom.contentprocess.buildID_mismatch_false_positive"
+  );
+}
+
+async function getTrueMismatchTelemetry() {
+  return await getTelemetry("dom.contentprocess.buildID_mismatch");
 }
 
 // The logic bound to dom.ipc.processPrelaunch.enabled will react to value

browser/modules/ContentCrashHandlers.sys.mjs

@@ -518,12 +518,6 @@ export var TabCrashHandler = {
     browser.docShell.displayLoadError(Cr.NS_ERROR_BUILDID_MISMATCH, uri, null);
     tab.setAttribute("crashed", true);
     gBrowser.tabContainer.updateTabIndicatorAttr(tab);
-
-    // Make sure to only count once even if there are multiple windows
-    // that will all show about:restartrequired.
-    if (this._crashedTabCount == 1) {
-      Glean.domContentprocess.buildIdMismatch.add(1);
-    }
   },
 
   /**

browser/modules/test/browser/browser.toml

@@ -55,8 +55,6 @@ run-if = ["crashreporter"]
 ["browser_UsageTelemetry.js"]
 https_first_disabled = true
 
-["browser_UsageTelemetry_content_aboutRestartRequired.js"]
-
 ["browser_UsageTelemetry_domains.js"]
 https_first_disabled = true
 

browser/modules/test/browser/browser_UsageTelemetry_content_aboutRestartRequired.js

@@ -1,33 +0,0 @@
-"use strict";
-
-const SCALAR_BUILDID_MISMATCH = "dom.contentprocess.buildID_mismatch";
-
-add_task(async function test_aboutRestartRequired() {
-  const { TabCrashHandler } = ChromeUtils.importESModule(
-    "resource:///modules/ContentCrashHandlers.sys.mjs"
-  );
-
-  // Let's reset the counts.
-  Services.telemetry.clearScalars();
-
-  let scalars = TelemetryTestUtils.getProcessScalars("parent");
-
-  // Check preconditions
-  is(
-    scalars[SCALAR_BUILDID_MISMATCH],
-    undefined,
-    "Build ID mismatch count should be undefined"
-  );
-
-  // Simulate buildID mismatch
-  TabCrashHandler._crashedTabCount = 1;
-  TabCrashHandler.sendToRestartRequiredPage(gBrowser.selectedTab.linkedBrowser);
-
-  scalars = TelemetryTestUtils.getProcessScalars("parent");
-
-  is(
-    scalars[SCALAR_BUILDID_MISMATCH],
-    1,
-    "Build ID mismatch count should be 1."
-  );
-});

dom/base/nsFrameLoader.cpp

@@ -3620,6 +3620,13 @@ static mozilla::Result<bool, nsresult> BuildIDMismatchMemoryAndDisk() {
   nsresult rv;
   nsCOMPtr<nsIFile> file;
 
+  if (const char* forceMismatch = PR_GetEnv("MOZ_FORCE_BUILDID_MISMATCH")) {
+    if (forceMismatch[0] == '1') {
+      NS_WARNING("Forcing a buildid mismatch");
+      return true;
+    }
+  }
+
 #if defined(ANDROID)
   // Android packages on installation will stop existing instance, so we
   // cannot run into this problem.
@@ -3692,7 +3699,10 @@ void nsFrameLoader::MaybeNotifyCrashed(BrowsingContext* aBrowsingContext,
   }
 
 #if defined(MOZ_TELEMETRY_REPORTING)
-  bool sendTelemetry = false;
+  bool sendTelemetryFalsePositive = false, sendTelemetryTrueMismatch = false;
+
+  static bool haveSentTelemetryFalsePositive = false,
+              haveSentTelemetryTrueMismatch = false;
 #endif  // defined(MOZ_TELEMETRY_REPORTING)
 
   // Fire the actual crashed event.
@@ -3701,17 +3711,20 @@ void nsFrameLoader::MaybeNotifyCrashed(BrowsingContext* aBrowsingContext,
     auto changedOrError = BuildIDMismatchMemoryAndDisk();
     if (changedOrError.isErr()) {
       NS_WARNING("Error while checking buildid mismatch");
-      eventName = u"oop-browser-buildid-mismatch"_ns;
+      eventName = u"oop-browser-crashed"_ns;
     } else {
       bool aChanged = changedOrError.unwrap();
       if (aChanged) {
         NS_WARNING("True build ID mismatch");
         eventName = u"oop-browser-buildid-mismatch"_ns;
+#if defined(MOZ_TELEMETRY_REPORTING)
+        sendTelemetryTrueMismatch = true;
+#endif  // defined(MOZ_TELEMETRY_REPORTING)
       } else {
         NS_WARNING("build ID mismatch false alarm");
         eventName = u"oop-browser-crashed"_ns;
 #if defined(MOZ_TELEMETRY_REPORTING)
-        sendTelemetry = true;
+        sendTelemetryFalsePositive = true;
 #endif  // defined(MOZ_TELEMETRY_REPORTING)
       }
     }
@@ -3721,8 +3734,14 @@ void nsFrameLoader::MaybeNotifyCrashed(BrowsingContext* aBrowsingContext,
   }
 
 #if defined(MOZ_TELEMETRY_REPORTING)
-  if (sendTelemetry) {
+  if (sendTelemetryFalsePositive && !haveSentTelemetryFalsePositive) {
     glean::dom_contentprocess::build_id_mismatch_false_positive.Add(1);
+    haveSentTelemetryFalsePositive = true;
+  }
+
+  if (sendTelemetryTrueMismatch && !haveSentTelemetryTrueMismatch) {
+    glean::dom_contentprocess::build_id_mismatch.Add(1);
+    haveSentTelemetryTrueMismatch = true;
   }
 #endif  // defined(MOZ_TELEMETRY_REPORTING)