Bug 1651133 - Double-check the build ID to avoid spurious about:restartrequired r=jld

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

browser/base/content/test/tabcrashed/browser.ini

@@ -5,6 +5,13 @@ support-files =
   file_contains_emptyiframe.html
   file_iframe.html
 
+[browser_aboutRestartRequired_basic.js]
+skip-if =
+  !debug
+[browser_aboutRestartRequired_buildid.js]
+skip-if =
+  !debug
+  (os == 'win')
 [browser_autoSubmitRequest.js]
 skip-if =
   apple_silicon  # crash bug 1707182

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

@@ -0,0 +1,24 @@
+"use strict";
+
+const kTimeout = 5 * 1000;
+
+add_task(async function test_browser_crashed_basic_event() {
+  info("Waiting for oop-browser-crashed event.");
+
+  Services.telemetry.clearScalars();
+  is(
+    getFalsePositiveTelemetry(),
+    undefined,
+    "Build ID mismatch false positive count should be undefined"
+  );
+
+  let eventPromise = getEventPromise("oop-browser-crashed", "basic", kTimeout);
+  await openNewTab(true);
+  await eventPromise;
+
+  is(
+    getFalsePositiveTelemetry(),
+    undefined,
+    "Build ID mismatch false positive count should be undefined"
+  );
+});

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

@@ -0,0 +1,191 @@
+"use strict";
+
+const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");
+
+const kTimeout = 60 * 1000;
+
+// 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);
+
+async function waitForObserver(topic) {
+  return new Promise(function(resolve, reject) {
+    info("Setting up observer for '" + topic + "'");
+    let interval = undefined;
+
+    let obs = {
+      observe() {
+        info("Observed '" + topic + "'");
+        clearInterval(interval);
+        Services.obs.removeObserver(obs, topic);
+        resolve();
+      },
+    };
+
+    Services.obs.addObserver(obs, topic);
+
+    let times = 0;
+    let intervalMs = 1000;
+    interval = setInterval(async () => {
+      info("Opening and closing tab '" + topic + "'");
+      await openNewTab(false);
+      times += intervalMs;
+      if (times > kTimeout) {
+        clearInterval(interval);
+        reject();
+      }
+    }, intervalMs);
+  });
+}
+
+async function forceCleanProcesses() {
+  const origPrefValue = SpecialPowers.getBoolPref(
+    "dom.ipc.processPrelaunch.enabled"
+  );
+  info(
+    "forceCleanProcesses: get dom.ipc.processPrelaunch.enabled=" + origPrefValue
+  );
+  await SpecialPowers.setBoolPref("dom.ipc.processPrelaunch.enabled", false);
+  info("forceCleanProcesses: set dom.ipc.processPrelaunch.enabled=false");
+  await SpecialPowers.setBoolPref(
+    "dom.ipc.processPrelaunch.enabled",
+    origPrefValue
+  );
+  info(
+    "forceCleanProcesses: set dom.ipc.processPrelaunch.enabled=" + origPrefValue
+  );
+  const currPrefValue = SpecialPowers.getBoolPref(
+    "dom.ipc.processPrelaunch.enabled"
+  );
+  ok(currPrefValue === origPrefValue, "processPrelaunch properly re-enabled");
+}
+
+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"
+  );
+
+  try {
+    setBuildidMatchDontSendEnv();
+    await forceCleanProcesses();
+    let eventPromise = getEventPromise(
+      "oop-browser-crashed",
+      "false-positive",
+      kTimeout
+    );
+    await openNewTab(false);
+    await eventPromise;
+  } finally {
+    unsetBuildidMatchDontSendEnv();
+  }
+
+  is(
+    getFalsePositiveTelemetry(),
+    undefined,
+    "Build ID mismatch false positive count should be undefined"
+  );
+});
+
+add_task(async function test_browser_restartrequired_event() {
+  info("Waiting for oop-browser-buildid-mismatch event.");
+
+  Services.telemetry.clearScalars();
+  is(
+    getFalsePositiveTelemetry(),
+    undefined,
+    "Build ID mismatch false positive count should be undefined"
+  );
+
+  let profD = Services.dirsvc.get("GreD", Ci.nsIFile);
+  let platformIniOrig = await OS.File.read(
+    OS.Path.join(profD.path, "platform.ini"),
+    { encoding: "utf-8" }
+  );
+  let buildID = Services.appinfo.platformBuildID;
+  let platformIniNew = platformIniOrig.replace(buildID, "1234");
+
+  await OS.File.writeAtomic(
+    OS.Path.join(profD.path, "platform.ini"),
+    platformIniNew,
+    { encoding: "utf-8" }
+  );
+
+  try {
+    setBuildidMatchDontSendEnv();
+    await forceCleanProcesses();
+    let eventPromise = getEventPromise(
+      "oop-browser-buildid-mismatch",
+      "buildid-mismatch",
+      kTimeout
+    );
+    await openNewTab(false);
+
+    await eventPromise;
+  } finally {
+    await OS.File.writeAtomic(
+      OS.Path.join(profD.path, "platform.ini"),
+      platformIniOrig,
+      { encoding: "utf-8" }
+    );
+
+    unsetBuildidMatchDontSendEnv();
+  }
+
+  is(
+    getFalsePositiveTelemetry(),
+    1,
+    "Build ID mismatch false positive count should be 1"
+  );
+});
+
+add_task(async function test_browser_crashed_no_platform_ini_event() {
+  info("Waiting for oop-browser-crashed event.");
+
+  Services.telemetry.clearScalars();
+  is(
+    getFalsePositiveTelemetry(),
+    undefined,
+    "Build ID mismatch false positive count should be undefined"
+  );
+
+  let profD = Services.dirsvc.get("GreD", Ci.nsIFile);
+  let platformIniOrig = await OS.File.read(
+    OS.Path.join(profD.path, "platform.ini"),
+    { encoding: "utf-8" }
+  );
+
+  await OS.File.remove(OS.Path.join(profD.path, "platform.ini"));
+
+  try {
+    setBuildidMatchDontSendEnv();
+    await forceCleanProcesses();
+    let eventPromise = getEventPromise(
+      "oop-browser-buildid-mismatch",
+      "no-platform-ini",
+      kTimeout
+    );
+    await openNewTab(false);
+
+    await eventPromise;
+  } finally {
+    await OS.File.writeAtomic(
+      OS.Path.join(profD.path, "platform.ini"),
+      platformIniOrig,
+      { encoding: "utf-8" }
+    );
+
+    unsetBuildidMatchDontSendEnv();
+  }
+
+  is(
+    getFalsePositiveTelemetry(),
+    1,
+    "Build ID mismatch false positive count should be 1"
+  );
+});

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

@@ -1,3 +1,9 @@
+"use strict";
+
+const { TelemetryTestUtils } = ChromeUtils.import(
+  "resource://testing-common/TelemetryTestUtils.jsm"
+);
+
 /**
  * Returns a Promise that resolves once a crash report has
  * been submitted. This function will also test the crash
@@ -138,3 +144,79 @@ function prepareNoDump() {
     TabCrashHandler.getDumpID = originalGetDumpID;
   });
 }
+
+const kBuildidMatchEnv = "MOZ_BUILDID_MATCH_DONTSEND";
+
+function setBuildidMatchDontSendEnv() {
+  const envService = Cc["@mozilla.org/process/environment;1"].getService(
+    Ci.nsIEnvironment
+  );
+  info("Setting " + kBuildidMatchEnv + "=1");
+  envService.set(kBuildidMatchEnv, "1");
+  info("Set " + kBuildidMatchEnv + "=1");
+}
+
+function unsetBuildidMatchDontSendEnv() {
+  const envService = Cc["@mozilla.org/process/environment;1"].getService(
+    Ci.nsIEnvironment
+  );
+  info("Setting " + kBuildidMatchEnv + "=0");
+  envService.set(kBuildidMatchEnv, "0");
+  info("Set " + kBuildidMatchEnv + "=0");
+}
+
+function getEventPromise(eventName, eventKind, kTimeout) {
+  return new Promise(function(resolve, reject) {
+    /* eslint-disable mozilla/no-arbitrary-setTimeout */
+    let maybeTimeout = setTimeout(() => {
+      ok(
+        false,
+        "Timed out waiting " + eventName + " (" + eventKind + ") event"
+      );
+      reject();
+    }, kTimeout);
+
+    info("Installing event listener (" + eventKind + ")");
+    window.addEventListener(
+      eventName,
+      event => {
+        info("Clear timeout for " + eventKind + " event");
+        clearTimeout(maybeTimeout);
+        ok(true, "Received " + eventName + " (" + eventKind + ") event");
+        info("Call resolve() for " + eventKind + " event");
+        resolve();
+      },
+      { once: true }
+    );
+    info("Installed event listener (" + eventKind + ")");
+  });
+}
+
+async function openNewTab(forceCrash) {
+  const PAGE =
+    "data:text/html,<html><body>A%20regular,%20everyday,%20normal%20page.";
+
+  let options = {
+    gBrowser,
+    PAGE,
+    waitForLoad: false,
+    waitForStateStop: false,
+    forceNewProcess: true,
+  };
+
+  let tab = await BrowserTestUtils.openNewForegroundTab(options);
+
+  if (forceCrash === true) {
+    let browser = tab.linkedBrowser;
+    await BrowserTestUtils.crashFrame(browser, true, true, null);
+  }
+
+  // Since we crashed early, we expect to have some about:blank
+  // Remove it to clean up
+  BrowserTestUtils.removeTab(tab);
+}
+
+function getFalsePositiveTelemetry() {
+  const scalars = TelemetryTestUtils.getProcessScalars("parent");
+  return scalars["dom.contentprocess.buildID_mismatch_false_positive"];
+}

dom/base/nsFrameLoader.cpp

@@ -56,6 +56,10 @@
 #include "nsIXULRuntime.h"
 #include "nsNetUtil.h"
 #include "nsFocusManager.h"
+#include "nsIINIParser.h"
+#include "nsAppRunner.h"
+#include "nsDirectoryService.h"
+#include "nsDirectoryServiceDefs.h"
 
 #include "nsGkAtoms.h"
 #include "nsNameSpaceManager.h"
@@ -137,6 +141,10 @@
 #  include "nsIWebBrowserPrint.h"
 #endif
 
+#if defined(MOZ_TELEMETRY_REPORTING)
+#  include "mozilla/Telemetry.h"
+#endif  // defined(MOZ_TELEMETRY_REPORTING)
+
 using namespace mozilla;
 using namespace mozilla::hal;
 using namespace mozilla::dom;
@@ -3676,6 +3684,32 @@ void nsFrameLoader::SetWillChangeProcess() {
   docshell->SetWillChangeProcess();
 }
 
+static mozilla::Result<bool, nsresult> DidBuildIDChange() {
+  nsresult rv;
+  nsCOMPtr<nsIFile> file;
+
+  rv = NS_GetSpecialDirectory(NS_GRE_DIR, getter_AddRefs(file));
+  MOZ_TRY(rv);
+
+  rv = file->AppendNative("platform.ini"_ns);
+  MOZ_TRY(rv);
+
+  nsCOMPtr<nsIINIParserFactory> iniFactory =
+      do_GetService("@mozilla.org/xpcom/ini-parser-factory;1", &rv);
+  MOZ_TRY(rv);
+
+  nsCOMPtr<nsIINIParser> parser;
+  rv = iniFactory->CreateINIParser(file, getter_AddRefs(parser));
+  MOZ_TRY(rv);
+
+  nsAutoCString installedBuildID;
+  rv = parser->GetString("Build"_ns, "BuildID"_ns, installedBuildID);
+  MOZ_TRY(rv);
+
+  nsDependentCString runningBuildID(PlatformBuildID());
+  return (installedBuildID != runningBuildID);
+}
+
 void nsFrameLoader::MaybeNotifyCrashed(BrowsingContext* aBrowsingContext,
                                        ContentParentId aChildID,
                                        mozilla::ipc::MessageChannel* aChannel) {
@@ -3711,14 +3745,46 @@ void nsFrameLoader::MaybeNotifyCrashed(BrowsingContext* aBrowsingContext,
     return;
   }
 
+#if defined(MOZ_TELEMETRY_REPORTING)
+  bool sendTelemetry = false;
+#endif  // defined(MOZ_TELEMETRY_REPORTING)
+
   // Fire the actual crashed event.
   nsString eventName;
   if (aChannel && !aChannel->DoBuildIDsMatch()) {
-    eventName = u"oop-browser-buildid-mismatch"_ns;
+    auto changedOrError = DidBuildIDChange();
+    if (changedOrError.isErr()) {
+      NS_WARNING("Error while checking buildid mismatch");
+      eventName = u"oop-browser-buildid-mismatch"_ns;
+#if defined(MOZ_TELEMETRY_REPORTING)
+      sendTelemetry = true;
+#endif  // defined(MOZ_TELEMETRY_REPORTING)
+    } 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)
+        sendTelemetry = true;
+#endif  // defined(MOZ_TELEMETRY_REPORTING)
+      } else {
+        NS_WARNING("build ID mismatch false alarm");
+        eventName = u"oop-browser-crashed"_ns;
+      }
+    }
   } else {
+    NS_WARNING("No build ID mismatch");
     eventName = u"oop-browser-crashed"_ns;
   }
 
+#if defined(MOZ_TELEMETRY_REPORTING)
+  if (sendTelemetry) {
+    Telemetry::ScalarAdd(
+        Telemetry::ScalarID::DOM_CONTENTPROCESS_BUILDID_MISMATCH_FALSE_POSITIVE,
+        1);
+  }
+#endif  // defined(MOZ_TELEMETRY_REPORTING)
+
   FrameCrashedEventInit init;
   init.mBubbles = true;
   init.mCancelable = true;

ipc/glue/MessageChannel.cpp

@@ -1077,6 +1077,22 @@ bool MessageChannel::SendBuildIDsMatchMessage(const char* aParentBuildID) {
     ReportConnectionError("MessageChannel", msg.get());
     return false;
   }
+
+#if defined(MOZ_DEBUG) && defined(ENABLE_TESTS)
+  // Technically, the behavior is interesting for any kind of process
+  // but when exercising tests, we want to crash only a content process and
+  // avoid making noise with other kind of processes crashing
+  if (const char* dontSend = PR_GetEnv("MOZ_BUILDID_MATCH_DONTSEND")) {
+    if (dontSend[0] == '1') {
+      if (XRE_IsContentProcess()) {
+        // Make sure we do not die too early, as this causes weird behavior
+        PR_Sleep(PR_MillisecondsToInterval(1000));
+        return false;
+      }
+    }
+  }
+#endif
+
   mLink->SendMessage(std::move(msg));
   return true;
 }

toolkit/components/telemetry/Scalars.yaml

@@ -2163,6 +2163,21 @@ dom.contentprocess:
       - 'fennec'
     record_in_processes:
       - 'main'
+  buildID_mismatch_false_positive:
+    bug_numbers:
+      - 1651133
+    description: >
+      The number of times a process crashed early but we could verify it was not
+      because of buildID mismatch between the parent and the content processes.
+    expires: "95"
+    kind: uint
+    notification_emails:
+      - alissy@mozilla.com
+    release_channel_collection: opt-out
+    products:
+      - 'firefox'
+    record_in_processes:
+      - 'main'
   os_priority_lowered:
     bug_numbers:
       - 1538987