Bug 1408806 - Gracefully deal with errors when killing the minidump-analyzer on shutdown; r=mconley

This solves two problems that were causing tests to fail intermittently. The
first issue is that calling nsIProcess.kill() on a process that had already
terminated would throw an exception which wouldn't be caught. Since this might
happen in cases where the minidump-analyzer run significantly faster than the
main event loop during the test. The second issue is that we tried to
unconditionally escape both the 'TelemetryEnvironment' and 'StackTraces'
field, but the latter would not be present in cases where the
minidump-analyzer would fail or be killed. This led to another spurious
exception.

MozReview-Commit-ID: 7srQtzig7xw

--HG--
extra : source : d10c5245136bc3bddb0275729f6bb90e9e527473

toolkit/components/crashes/CrashService.js

@@ -64,6 +64,10 @@ function runMinidumpAnalyzer(minidumpPath, allThreads) {
             gRunningProcesses.delete(process);
             resolve();
             break;
+          case "process-failed":
+            gRunningProcesses.delete(process);
+            reject();
+            break;
           default:
             reject(new Error("Unexpected topic received " + topic));
             break;
@@ -131,7 +135,9 @@ function processExtraFile(extraPath) {
       // to re-escape them into '\\n' again so that the fields holding JSON
       // strings are valid.
       [ "TelemetryEnvironment", "StackTraces" ].forEach(field => {
-        keyValuePairs[field] = keyValuePairs[field].replace(/\n/g, "n");
+        if (field in keyValuePairs) {
+          keyValuePairs[field] = keyValuePairs[field].replace(/\n/g, "n");
+        }
       });
 
       return keyValuePairs;
@@ -234,7 +240,12 @@ CrashService.prototype = Object.freeze({
       case "quit-application":
         gQuitting = true;
         gRunningProcesses.forEach((process) => {
-          process.kill();
+          try {
+            process.kill();
+          } catch (e) {
+            // If the process has already quit then kill() fails, but since
+            // this failure is benign it is safe to silently ignore it.
+          }
           Services.obs.notifyObservers(null, "test-minidump-analyzer-killed");
         });
         break;