Bug 1282776 - Finalize crash reports for child process crashes happening too early r=froydnj

This changes the way crash reports for child processes happening too early
during the child process' startup. Before bug 1547698 we wrote a partial
.extra file with those crashes that lacked the process type. The user would
not be notified of those crashes until she restarted Firefox and even when
submitted those crashes would be erroneously labeled as browser crashes.

After bug 1547698 we stopped writing .extra files entirely for those crashes
which left orphaned .dmp files among the pending crash reports.

This patch does three things to improve the situation:

* It writes a partial .extra file so that the crashes are detected at the next
  startup. So the user is still not notified directly of these crashes but she
  can report them later.
* It adds the process type to the .extra file so that the crash reporters are
  labelled correctly.
* It fixes a leak in the `pidToMinidump` hash-map. Since the crashes were
  not finalized the `ChildProcessData` strucutre associated with them would
  never be fred.

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

--HG--
extra : moz-landing-system : lando

dom/ipc/ContentParent.cpp

@@ -1643,6 +1643,9 @@ void ContentParent::ActorDestroy(ActorDestroyReason why) {
           dumpID = mCrashReporter->MinidumpID();
         }
         props->SetPropertyAsAString(NS_LITERAL_STRING("dumpID"), dumpID);
+      } else {
+        CrashReporter::FinalizeOrphanedMinidump(OtherPid(),
+                                                GeckoProcessType_Content);
       }
     }
     nsAutoString cpId;

dom/media/gmp/GMPParent.cpp

@@ -423,7 +423,7 @@ bool GMPParent::EnsureProcessLoaded() {
   return NS_SUCCEEDED(rv);
 }
 
-void GMPParent::WriteExtraDataForMinidump() {
+void GMPParent::AddCrashAnnotations() {
   mCrashReporter->AddAnnotation(CrashReporter::Annotation::GMPPlugin, true);
   mCrashReporter->AddAnnotation(CrashReporter::Annotation::PluginFilename,
                                 NS_ConvertUTF16toUTF8(mName));
@@ -435,10 +435,12 @@ void GMPParent::WriteExtraDataForMinidump() {
 
 bool GMPParent::GetCrashID(nsString& aResult) {
   if (!mCrashReporter) {
+    CrashReporter::FinalizeOrphanedMinidump(OtherPid(),
+                                            GeckoProcessType_GMPlugin);
     return false;
   }
 
-  WriteExtraDataForMinidump();
+  AddCrashAnnotations();
   if (!mCrashReporter->GenerateCrashReport(OtherPid())) {
     return false;
   }

dom/media/gmp/GMPParent.h

@@ -153,7 +153,7 @@ class GMPParent final : public PGMPParent {
       const nsAString& aJSON);  // Main thread.
   RefPtr<GenericPromise> ReadChromiumManifestFile(
       nsIFile* aFile);  // GMP thread.
-  void WriteExtraDataForMinidump();
+  void AddCrashAnnotations();
   bool GetCrashID(nsString& aResult);
   void ActorDestroy(ActorDestroyReason aWhy) override;
 

dom/media/ipc/RDDChild.cpp

@@ -122,6 +122,8 @@ void RDDChild::ActorDestroy(ActorDestroyReason aWhy) {
     if (mCrashReporter) {
       mCrashReporter->GenerateCrashReport(OtherPid());
       mCrashReporter = nullptr;
+    } else {
+      CrashReporter::FinalizeOrphanedMinidump(OtherPid(), GeckoProcessType_RDD);
     }
   }
 

dom/plugins/ipc/PluginModuleParent.cpp

@@ -663,7 +663,7 @@ PluginModuleChromeParent::~PluginModuleChromeParent() {
   mozilla::BackgroundHangMonitor::UnregisterAnnotator(*this);
 }
 
-void PluginModuleChromeParent::WriteExtraDataForMinidump() {
+void PluginModuleChromeParent::AddCrashAnnotations() {
   // mCrashReporterMutex is already held by the caller
   mCrashReporterMutex.AssertCurrentThreadOwns();
 
@@ -1278,9 +1278,13 @@ static void RemoveMinidump(nsIFile* minidump) {
 void PluginModuleChromeParent::ProcessFirstMinidump() {
   mozilla::MutexAutoLock lock(mCrashReporterMutex);
 
-  if (!mCrashReporter) return;
+  if (!mCrashReporter) {
+    CrashReporter::FinalizeOrphanedMinidump(OtherPid(),
+                                            GeckoProcessType_Plugin);
+    return;
+  }
 
-  WriteExtraDataForMinidump();
+  AddCrashAnnotations();
 
   if (mCrashReporter->HasMinidump()) {
     // A minidump may be set in TerminateChildProcess, which means the

dom/plugins/ipc/PluginModuleParent.h

@@ -420,7 +420,7 @@ class PluginModuleChromeParent : public PluginModuleParent,
   virtual bool ShouldContinueFromReplyTimeout() override;
 
   void ProcessFirstMinidump();
-  void WriteExtraDataForMinidump();
+  void AddCrashAnnotations();
 
   PluginProcessParent* Process() const { return mSubprocess; }
   base::ProcessHandle ChildProcessHandle() {

gfx/ipc/GPUChild.cpp

@@ -240,6 +240,8 @@ void GPUChild::ActorDestroy(ActorDestroyReason aWhy) {
     if (mCrashReporter) {
       mCrashReporter->GenerateCrashReport(OtherPid());
       mCrashReporter = nullptr;
+    } else {
+      CrashReporter::FinalizeOrphanedMinidump(OtherPid(), GeckoProcessType_GPU);
     }
 
     Telemetry::Accumulate(

gfx/vr/ipc/VRChild.cpp

@@ -94,6 +94,8 @@ void VRChild::ActorDestroy(ActorDestroyReason aWhy) {
     if (mCrashReporter) {
       mCrashReporter->GenerateCrashReport(OtherPid());
       mCrashReporter = nullptr;
+    } else {
+      CrashReporter::FinalizeOrphanedMinidump(OtherPid(), GeckoProcessType_VR);
     }
 
     Telemetry::Accumulate(
@@ -210,4 +212,4 @@ void VRChild::Destroy(UniquePtr<VRChild>&& aChild) {
 }
 
 }  // namespace gfx
-}  // namespace mozilla
+}  // namespace mozilla

ipc/glue/CrashReporterHost.cpp

@@ -118,29 +118,8 @@ bool CrashReporterHost::FinalizeCrashReport() {
 
   CrashReporter::AnnotationTable annotations;
 
-  nsAutoCString type;
-  // The gecko media plugin and normal plugin processes are lumped together
-  // as a historical artifact.
-  if (mProcessType == GeckoProcessType_GMPlugin) {
-    type.AssignLiteral("plugin");
-  } else if (mProcessType == GeckoProcessType_Content) {
-    type.AssignLiteral("content");
-  } else {
-    // This check will pick up some cases that will never happen (e.g. IPDL
-    // unit tests), but that's OK.
-    switch (mProcessType) {
-#define GECKO_PROCESS_TYPE(enum_name, string_name, xre_name, bin_type) \
-  case GeckoProcessType_##enum_name:                                   \
-    type.AssignLiteral(string_name);                                   \
-    break;
-#include "mozilla/GeckoProcessTypes.h"
-#undef GECKO_PROCESS_TYPE
-      default:
-        NS_ERROR("unknown process type");
-        break;
-    }
-  }
-  annotations[CrashReporter::Annotation::ProcessType] = type;
+  annotations[CrashReporter::Annotation::ProcessType] =
+      XRE_ChildProcessTypeToAnnotation(mProcessType);
 
   char startTime[32];
   SprintfLiteral(startTime, "%lld", static_cast<long long>(mStartTime));

netwerk/ipc/SocketProcessParent.cpp

@@ -57,6 +57,9 @@ void SocketProcessParent::ActorDestroy(ActorDestroyReason aWhy) {
     if (mCrashReporter) {
       mCrashReporter->GenerateCrashReport(OtherPid());
       mCrashReporter = nullptr;
+    } else {
+      CrashReporter::FinalizeOrphanedMinidump(OtherPid(),
+                                              GeckoProcessType_Content);
     }
   }
 

security/sandbox/win/src/remotesandboxbroker/RemoteSandboxBrokerParent.cpp

@@ -66,6 +66,9 @@ void RemoteSandboxBrokerParent::ActorDestroy(ActorDestroyReason aWhy) {
     if (mCrashReporter) {
       mCrashReporter->GenerateCrashReport(OtherPid());
       mCrashReporter = nullptr;
+    } else {
+      CrashReporter::FinalizeOrphanedMinidump(
+          OtherPid(), GeckoProcessType_RemoteSandboxBroker);
     }
   }
   Shutdown();

toolkit/crashreporter/nsDummyExceptionHandler.cpp

@@ -211,6 +211,10 @@ bool TakeMinidumpForChild(uint32_t childPid, nsIFile** dump,
   return false;
 }
 
+bool FinalizeOrphanedMinidump(uint32_t aChildPid, GeckoProcessType aType) {
+  return false;
+}
+
 ThreadId CurrentThreadId() { return -1; }
 
 bool TakeMinidump(nsIFile** aResult, bool aMoveToPending) { return false; }

toolkit/crashreporter/nsExceptionHandler.cpp

@@ -26,7 +26,6 @@
 #include "mozilla/ipc/CrashReporterClient.h"
 
 #include "nsThreadUtils.h"
-#include "nsXULAppAPI.h"
 #include "jsfriendapi.h"
 #include "ThreadAnnotation.h"
 #include "private/pprio.h"
@@ -3345,6 +3344,25 @@ bool TakeMinidumpForChild(uint32_t childPid, nsIFile** dump,
   return !!*dump;
 }
 
+bool FinalizeOrphanedMinidump(uint32_t aChildPid, GeckoProcessType aType) {
+  AnnotationTable annotations;
+  nsCOMPtr<nsIFile> minidump;
+
+  if (!TakeMinidumpForChild(aChildPid, getter_AddRefs(minidump), annotations)) {
+    return false;
+  }
+
+  nsAutoString id;
+  if (!GetIDFromMinidump(minidump, id)) {
+    return false;
+  }
+
+  annotations[Annotation::ProcessType] =
+      XRE_ChildProcessTypeToAnnotation(aType);
+
+  return WriteExtraFile(id, annotations);
+}
+
 //-----------------------------------------------------------------------------
 // CreatePairedMinidumps() and helpers
 //

toolkit/crashreporter/nsExceptionHandler.h

@@ -21,6 +21,7 @@
 #include <stdint.h>
 #include "nsError.h"
 #include "nsString.h"
+#include "nsXULAppAPI.h"
 #include "prio.h"
 
 #if defined(XP_WIN)
@@ -172,6 +173,18 @@ bool TakeMinidumpForChild(uint32_t childPid, nsIFile** dump,
                           AnnotationTable& aAnnotations,
                           uint32_t* aSequence = nullptr);
 
+/**
+ * If a dump was found for |childPid| then write a minimal .extra file to
+ * complete it and remove it from the list of pending crash dumps. It's
+ * required to call this method after a non-main process crash if the crash
+ * report could not be finalized via the CrashReporterHost (for example because
+ * it wasn't instanced yet).
+ *
+ * @param aChildPid The pid of the crashed child process
+ * @param aType The type of the crashed process
+ */
+bool FinalizeOrphanedMinidump(uint32_t aChildPid, GeckoProcessType aType);
+
 #if defined(XP_WIN)
 typedef HANDLE ProcessHandle;
 typedef DWORD ProcessId;

toolkit/xre/nsEmbedFunctions.cpp

@@ -230,6 +230,21 @@ const char* XRE_ChildProcessTypeToString(GeckoProcessType aProcessType) {
              : "invalid";
 }
 
+const char* XRE_ChildProcessTypeToAnnotation(GeckoProcessType aProcessType) {
+  switch (aProcessType) {
+    case GeckoProcessType_GMPlugin:
+      // The gecko media plugin and normal plugin processes are lumped together
+      // as a historical artifact.
+      return "plugin";
+    case GeckoProcessType_Default:
+      return "";
+    case GeckoProcessType_Content:
+      return "content";
+    default:
+      return XRE_ChildProcessTypeToString(aProcessType);
+  }
+}
+
 namespace mozilla {
 namespace startup {
 GeckoProcessType sChildProcessType = GeckoProcessType_Default;

xpcom/build/nsXULAppAPI.h

@@ -385,6 +385,8 @@ static_assert(MOZ_ARRAY_LENGTH(kGeckoProcessTypeString) == GeckoProcessType_End,
 
 XRE_API(const char*, XRE_ChildProcessTypeToString,
         (GeckoProcessType aProcessType))
+XRE_API(const char*, XRE_ChildProcessTypeToAnnotation,
+        (GeckoProcessType aProcessType))
 
 #if defined(MOZ_WIDGET_ANDROID)
 struct XRE_AndroidChildFds {