Bug 1788596 - Use Utility process actor names for crash annotations r=gsvelto

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

ipc/glue/UtilityProcessManager.h

@@ -98,6 +98,16 @@ class UtilityProcessManager final : public UtilityProcessHost::Listener {
     }
   }
 
+  Span<const UtilityActorName> GetActors(
+      const RefPtr<UtilityProcessParent>& aParent) {
+    for (auto& p : mProcesses) {
+      if (p && p->mProcessParent && p->mProcessParent == aParent) {
+        return p->mActors;
+      }
+    }
+    return {};
+  }
+
   Span<const UtilityActorName> GetActors(GeckoChildProcessHost* aHost) {
     for (auto& p : mProcesses) {
       if (p && p->mProcess == aHost) {

ipc/glue/UtilityProcessParent.cpp

@@ -76,9 +76,20 @@ void UtilityProcessParent::ActorDestroy(ActorDestroyReason aWhy) {
 
     if (mCrashReporter) {
 #if defined(MOZ_SANDBOX)
-      mCrashReporter->AddAnnotation(
-          CrashReporter::Annotation::UtilityProcessSandboxingKind,
-          (unsigned int)mHost->mSandbox);
+      RefPtr<mozilla::ipc::UtilityProcessManager> upm =
+          mozilla::ipc::UtilityProcessManager::GetSingleton();
+      if (upm) {
+        Span<const UtilityActorName> actors = upm->GetActors(this);
+        nsAutoCString actorsName;
+        if (!actors.IsEmpty()) {
+          actorsName += GetUtilityActorName(actors.First<1>()[0]);
+          for (const auto& actor : actors.From(1)) {
+            actorsName += ", "_ns + GetUtilityActorName(actor);
+          }
+        }
+        mCrashReporter->AddAnnotation(
+            CrashReporter::Annotation::UtilityActorsName, actorsName);
+      }
 #endif
     }
 

ipc/glue/test/browser/browser_utility_audioDecodeCrash.js

@@ -23,11 +23,11 @@ async function getAudioDecoderPid(expectation) {
 async function crashDecoder(expectation) {
   const audioPid = await getAudioDecoderPid(expectation);
   ok(audioPid > 0, `Found an audio decoder ${audioPid}`);
-
-  const ProcessTools = Cc["@mozilla.org/processtools-service;1"].getService(
-    Ci.nsIProcessToolsService
-  );
-  ProcessTools.kill(audioPid);
+  const actorIsAudioDecoder = actorNames => {
+    return actorNames.startsWith("audio-decoder-");
+  };
+  info(`Crashing audio decoder ${audioPid}`);
+  await crashSomeUtility(audioPid, actorIsAudioDecoder);
 }
 
 async function runTest(src, withClose, expectation) {

ipc/glue/test/browser/browser_utility_crashReporter.js

@@ -3,70 +3,36 @@
 
 "use strict";
 
-add_task(async () => {
-  SimpleTest.expectChildProcessCrash();
-  const utilityPid = await startUtilityProcess();
+async function startAndCrashUtility(actors, actorsCheck) {
+  const utilityPid = await startUtilityProcess(actors);
+  await crashSomeUtility(utilityPid, actorsCheck);
+}
 
-  const crashMan = Services.crashmanager;
-  const utilityProcessGone = TestUtils.topicObserved("ipc:utility-shutdown");
+// When running full suite, previous audio decoding tests might have left some
+// running and this might interfere with our testing
+add_setup(async function ensureNoExistingProcess() {
+  await utilityProcessTest().stopProcess();
+});
 
-  info("prune any previous crashes");
-  const future = new Date(Date.now() + 1000 * 60 * 60 * 24);
-  await crashMan.pruneOldCrashes(future);
+add_task(async function utilityNoActor() {
+  await startAndCrashUtility(0, actorNames => {
+    return actorNames === undefined;
+  });
+});
 
-  info("crash Utility Process");
-  const ProcessTools = Cc["@mozilla.org/processtools-service;1"].getService(
-    Ci.nsIProcessToolsService
-  );
-  ProcessTools.crash(utilityPid);
+add_task(async function utilityOneActor() {
+  await startAndCrashUtility(1, actorNames => {
+    return actorNames === kGenericUtilityActor;
+  });
+});
 
-  info("Waiting for utility process to go away.");
-  let [subject, data] = await utilityProcessGone;
-  ok(
-    parseInt(data, 10) === utilityPid,
-    `Should match the crashed PID ${utilityPid} with ${data}`
-  );
-  ok(
-    subject instanceof Ci.nsIPropertyBag2,
-    "Subject needs to be a nsIPropertyBag2 to clean up properly"
-  );
-
-  const dumpID = subject.getPropertyAsAString("dumpID");
-  ok(dumpID, "There should be a dumpID");
-
-  await crashMan.ensureCrashIsPresent(dumpID);
-  await crashMan.getCrashes().then(crashes => {
-    is(crashes.length, 1, "There should be only one record");
-    const crash = crashes[0];
-    ok(
-      crash.isOfType(
-        crashMan.processTypes[Ci.nsIXULRuntime.PROCESS_TYPE_UTILITY],
-        crashMan.CRASH_TYPE_CRASH
-      ),
-      "Record should be a utility process crash"
-    );
-    ok(crash.id === dumpID, "Record should have an ID");
-    ok(
-      parseInt(crash.metadata.UtilityProcessSandboxingKind, 10) ===
-        kGenericUtility,
-      "Record should have the sandboxing kind value"
+add_task(async function utilityManyActors() {
+  await startAndCrashUtility(42, actorNames => {
+    return (
+      actorNames ===
+      Array(42)
+        .fill("unknown")
+        .join(", ")
     );
   });
-
-  let minidumpDirectory = Services.dirsvc.get("ProfD", Ci.nsIFile);
-  minidumpDirectory.append("minidumps");
-
-  let dumpfile = minidumpDirectory.clone();
-  dumpfile.append(dumpID + ".dmp");
-  if (dumpfile.exists()) {
-    info(`Removal of ${dumpfile.path}`);
-    dumpfile.remove(false);
-  }
-
-  let extrafile = minidumpDirectory.clone();
-  extrafile.append(dumpID + ".extra");
-  info(`Removal of ${extrafile.path}`);
-  if (extrafile.exists()) {
-    extrafile.remove(false);
-  }
 });

ipc/glue/test/browser/browser_utility_memoryReport.js

@@ -3,6 +3,12 @@
 
 "use strict";
 
+// When running full suite, previous audio decoding tests might have left some
+// running and this might interfere with our testing
+add_setup(async function ensureNoExistingProcess() {
+  await utilityProcessTest().stopProcess();
+});
+
 add_task(async () => {
   const utilityPid = await startUtilityProcess();
 
@@ -23,7 +29,7 @@ add_task(async () => {
       aAmount,
       aDescription
     ) {
-      const expectedProcess = `Utility (pid: ${utilityPid}, sandboxingKind: ${kGenericUtility})`;
+      const expectedProcess = `Utility (pid: ${utilityPid}, sandboxingKind: ${kGenericUtilitySandbox})`;
       if (aProcess !== expectedProcess) {
         return;
       }

ipc/glue/test/browser/browser_utility_profiler.js

@@ -10,6 +10,12 @@ Services.scriptloader.loadSubScript(
   this
 );
 
+// When running full suite, previous audio decoding tests might have left some
+// running and this might interfere with our testing
+add_setup(async function ensureNoExistingProcess() {
+  await utilityProcessTest().stopProcess();
+});
+
 add_task(async () => {
   const utilityPid = await startUtilityProcess();
 

ipc/glue/test/browser/head.js

@@ -9,11 +9,12 @@ const utilityProcessTest = () => {
   );
 };
 
-const kGenericUtility = 0x0;
+const kGenericUtilitySandbox = 0;
+const kGenericUtilityActor = "unknown";
 
-async function startUtilityProcess() {
+async function startUtilityProcess(actors) {
   info("Start a UtilityProcess");
-  return utilityProcessTest().startProcess();
+  return utilityProcessTest().startProcess(actors);
 }
 
 async function cleanUtilityProcessShutdown(utilityPid) {
@@ -187,3 +188,76 @@ async function runMochitestUtilityAudio(
   info(`Remove media: ${src}`);
   document.body.removeChild(audio);
 }
+
+async function crashSomeUtility(utilityPid, actorsCheck) {
+  SimpleTest.expectChildProcessCrash();
+
+  const crashMan = Services.crashmanager;
+  const utilityProcessGone = TestUtils.topicObserved(
+    "ipc:utility-shutdown",
+    (subject, data) => {
+      info(`ipc:utility-shutdown: data=${data} subject=${subject}`);
+      return parseInt(data, 10) === utilityPid;
+    }
+  );
+
+  info("prune any previous crashes");
+  const future = new Date(Date.now() + 1000 * 60 * 60 * 24);
+  await crashMan.pruneOldCrashes(future);
+
+  info("crash Utility Process");
+  const ProcessTools = Cc["@mozilla.org/processtools-service;1"].getService(
+    Ci.nsIProcessToolsService
+  );
+  ProcessTools.crash(utilityPid);
+
+  info("Waiting for utility process to go away.");
+  let [subject, data] = await utilityProcessGone;
+  info(`utilityPid=${utilityPid} data=${data}`);
+  ok(
+    parseInt(data, 10) === utilityPid,
+    `Should match the crashed PID ${utilityPid} with ${data}`
+  );
+  ok(
+    subject instanceof Ci.nsIPropertyBag2,
+    "Subject needs to be a nsIPropertyBag2 to clean up properly"
+  );
+
+  const dumpID = subject.getPropertyAsAString("dumpID");
+  ok(dumpID, "There should be a dumpID");
+
+  await crashMan.ensureCrashIsPresent(dumpID);
+  await crashMan.getCrashes().then(crashes => {
+    is(crashes.length, 1, "There should be only one record");
+    const crash = crashes[0];
+    ok(
+      crash.isOfType(
+        crashMan.processTypes[Ci.nsIXULRuntime.PROCESS_TYPE_UTILITY],
+        crashMan.CRASH_TYPE_CRASH
+      ),
+      "Record should be a utility process crash"
+    );
+    ok(crash.id === dumpID, "Record should have an ID");
+    ok(
+      actorsCheck(crash.metadata.UtilityActorsName),
+      `Record should have the correct actors name: ${crash.metadata.UtilityActorsName}`
+    );
+  });
+
+  let minidumpDirectory = Services.dirsvc.get("ProfD", Ci.nsIFile);
+  minidumpDirectory.append("minidumps");
+
+  let dumpfile = minidumpDirectory.clone();
+  dumpfile.append(dumpID + ".dmp");
+  if (dumpfile.exists()) {
+    info(`Removal of ${dumpfile.path}`);
+    dumpfile.remove(false);
+  }
+
+  let extrafile = minidumpDirectory.clone();
+  extrafile.append(dumpID + ".extra");
+  info(`Removal of ${extrafile.path}`);
+  if (extrafile.exists()) {
+    extrafile.remove(false);
+  }
+}

ipc/glue/test/utility_process_xpcom/UtilityProcessTest.cpp

@@ -8,11 +8,12 @@
 #  include "mozilla/ipc/UtilityProcessTest.h"
 #  include "mozilla/ipc/UtilityProcessManager.h"
 #  include "mozilla/dom/Promise.h"
+#  include "mozilla/ProcInfo.h"
 
 namespace mozilla::ipc {
 
 NS_IMETHODIMP
-UtilityProcessTest::StartProcess(JSContext* aCx,
+UtilityProcessTest::StartProcess(int32_t aUnknownActors, JSContext* aCx,
                                  mozilla::dom::Promise** aOutPromise) {
   NS_ENSURE_ARG(aOutPromise);
   *aOutPromise = nullptr;
@@ -34,9 +35,18 @@ UtilityProcessTest::StartProcess(JSContext* aCx,
   utilityProc->LaunchProcess(SandboxingKind::GENERIC_UTILITY)
       ->Then(
           GetCurrentSerialEventTarget(), __func__,
-          [promise, utilityProc]() {
+          [promise, utilityProc, aUnknownActors]() {
             Maybe<int32_t> utilityPid =
                 utilityProc->ProcessPid(SandboxingKind::GENERIC_UTILITY);
+            if (aUnknownActors > 0) {
+              RefPtr<UtilityProcessParent> utilityParent =
+                  utilityProc->GetProcessParent(
+                      SandboxingKind::GENERIC_UTILITY);
+              for (int32_t i = 0; i < aUnknownActors; i++) {
+                utilityProc->RegisterActor(utilityParent,
+                                           UtilityActorName::Unknown);
+              }
+            }
             if (utilityPid.isSome()) {
               promise->MaybeResolve(*utilityPid);
             } else {

ipc/glue/test/utility_process_xpcom/components.conf

@@ -6,7 +6,7 @@
 
 Classes = [
     {
-        'cid': '{4505abd8-2718-44a0-96b6-6af66b07cc9f}',
+        'cid': '{0a4478f4-c5ae-4fb1-8686-d5b09fb99afb}',
         'contract_ids': ['@mozilla.org/utility-process-test;1'],
         'type': 'mozilla::ipc::UtilityProcessTest',
         'headers': ['mozilla/ipc/UtilityProcessTest.h'],

ipc/glue/test/utility_process_xpcom/nsIUtilityProcessTest.idl

@@ -5,7 +5,7 @@
 
 #include "nsISupports.idl"
 
-[scriptable, uuid(4505abd8-2718-44a0-96b6-6af66b07cc9f)]
+[scriptable, uuid(0a4478f4-c5ae-4fb1-8686-d5b09fb99afb)]
 interface nsIUtilityProcessTest : nsISupports
 {
   /**
@@ -14,7 +14,7 @@ interface nsIUtilityProcessTest : nsISupports
    * Allowing to start Utility Process from JS code.
    */
   [implicit_jscontext]
-  Promise startProcess();
+  Promise startProcess([optional] in int32_t unknownActors);
 
   /**
    * ** Test-only Method **

security/sandbox/common/test/SandboxTestingChild.cpp

@@ -88,9 +88,11 @@ void SandboxTestingChild::Bind(Endpoint<PSandboxTestingChild>&& aEndpoint) {
       case ipc::SandboxingKind::GENERIC_UTILITY:
         RunTestsGenericUtility(this);
 #ifdef MOZ_APPLEMEDIA
+        [[fallthrough]];
       case ipc::SandboxingKind::UTILITY_AUDIO_DECODING_APPLE_MEDIA:
 #endif
 #ifdef XP_WIN
+        [[fallthrough]];
       case ipc::SandboxingKind::UTILITY_AUDIO_DECODING_WMF:
 #endif
         RunTestsUtilityAudioDecoder(this, s->mSandbox);

toolkit/components/processtools/ProcInfo.h

@@ -76,6 +76,9 @@ enum class UtilityActorName {
   MfMediaEngineCDM,
 };
 
+// String that will be used e.g. to annotate crash reports
+nsCString GetUtilityActorName(const UtilityActorName aActorName);
+
 /* Get the CPU frequency to use to convert cycle time values to actual time.
  * @returns the TSC (Time Stamp Counter) frequency in MHz, or 0 if converting
  * cycle time values should not be attempted. */

toolkit/components/processtools/ProcInfo_common.cpp

@@ -41,4 +41,19 @@ RefPtr<ProcInfoPromise> GetProcInfo(nsTArray<ProcInfoRequest>&& aRequests) {
   return promise;
 }
 
+nsCString GetUtilityActorName(const UtilityActorName aActorName) {
+  switch (aActorName) {
+    case UtilityActorName::Unknown:
+      return "unknown"_ns;
+    case UtilityActorName::AudioDecoder_Generic:
+      return "audio-decoder-generic"_ns;
+    case UtilityActorName::AudioDecoder_AppleMedia:
+      return "audio-decoder-applemedia"_ns;
+    case UtilityActorName::AudioDecoder_WMF:
+      return "audio-decoder-wmf"_ns;
+    case UtilityActorName::MfMediaEngineCDM:
+      return "mf-media-engine"_ns;
+  }
+}
+
 }  // namespace mozilla

toolkit/crashreporter/CrashAnnotations.yaml

@@ -987,9 +987,9 @@ UtilityProcessStatus:
     Status of the Utility process, can be set to "Running" or "Destroyed"
   type: string
 
-UtilityProcessSandboxingKind:
+UtilityActorsName:
   description: >
-    The SandboxingKind passed for this Utility process instance
+    Comma-separated list of IPC actors name running on this Utility process instance
   type: integer
   ping: true