Bug 771251 - OOP crash reporting accesses the directory service off the main thread. In addition, the first design of the crash injector callback meant that we're dropping some set of Flash crashes which happen during an RPC call. r=ted

* Fix the directory service usage by saving the pending directory path from OOPInit. * Force clients to call OOPInit on the main thread. * Make injected crashes available via TakeMinidumpForChild and give each crash a sequence number so that we can pick the earliest crash from the three possibilities; delete the other two to avoid polluting about:crashes
2012-07-10 22:20:05 -04:00 · 2012-07-10 22:20:05 -04:00 · a1ac513b61
--- a/dom/ipc/CrashReporterParent.cpp
+++ b/dom/ipc/CrashReporterParent.cpp
@ -98,6 +98,15 @@ CrashReporterParent::GenerateHangCrashReport(const AnnotationTable* processNotes
    return true;
 }

+bool
+CrashReporterParent::GenerateCrashReportForMinidump(nsIFile* minidump,
+    const AnnotationTable* processNotes)
+{
+    if (!CrashReporter::GetIDFromMinidump(minidump, mChildDumpID))
+        return false;
+    return GenerateChildData(processNotes);
+}
+
 bool
 CrashReporterParent::GenerateChildData(const AnnotationTable* processNotes)
 {
--- a/dom/ipc/CrashReporterParent.h
+++ b/dom/ipc/CrashReporterParent.h
@ -49,6 +49,10 @@ public:
  bool
  GenerateCrashReport(Toplevel* t, const AnnotationTable* processNotes);

+  bool
+  GenerateCrashReportForMinidump(nsIFile* minidump,
+                                 const AnnotationTable* processNotes);
+
  /* Instantiate a new crash reporter actor from a given parent that manages
     the protocol.
  */
@ -135,7 +139,7 @@ CrashReporterParent::GenerateCrashReport(Toplevel* t,
                                         const AnnotationTable* processNotes)
 {
  nsCOMPtr<nsIFile> crashDump;
-  if (t->TakeMinidump(getter_AddRefs(crashDump)) &&
+  if (t->TakeMinidump(getter_AddRefs(crashDump), NULL) &&
      CrashReporter::GetIDFromMinidump(crashDump, mChildDumpID)) {
    return GenerateChildData(processNotes);
  }
--- a/dom/plugins/ipc/PluginModuleParent.cpp
+++ b/dom/plugins/ipc/PluginModuleParent.cpp
@ -27,9 +27,6 @@

 #include "nsAutoPtr.h"
 #include "nsCRT.h"
-#ifdef MOZ_CRASHREPORTER
-#include "mozilla/dom/CrashReporterParent.h"
-#endif
 #include "nsNPAPIPlugin.h"
 #include "nsIFile.h"

@ -49,6 +46,12 @@ using namespace mozilla;
 using namespace mozilla::plugins;
 using namespace mozilla::plugins::parent;

+#ifdef MOZ_CRASHREPORTER
+#include "mozilla/dom/CrashReporterParent.h"
+
+using namespace CrashReporter;
+#endif
+
 static const char kChildTimeoutPref[] = "dom.ipc.plugins.timeoutSecs";
 static const char kParentTimeoutPref[] = "dom.ipc.plugins.parentTimeoutSecs";
 static const char kLaunchTimeoutPref[] = "dom.ipc.plugins.processLaunchTimeoutSecs";
@ -133,9 +136,9 @@ PluginModuleParent::~PluginModuleParent()

 #ifdef MOZ_CRASHREPORTER_INJECTOR
    if (mFlashProcess1)
-        CrashReporter::UnregisterInjectorCallback(mFlashProcess1);
+        UnregisterInjectorCallback(mFlashProcess1);
    if (mFlashProcess2)
-        CrashReporter::UnregisterInjectorCallback(mFlashProcess2);
+        UnregisterInjectorCallback(mFlashProcess2);
 #endif

    Preferences::UnregisterCallback(TimeoutChanged, kChildTimeoutPref, this);
@ -144,7 +147,7 @@ PluginModuleParent::~PluginModuleParent()

 #ifdef MOZ_CRASHREPORTER
 void
-PluginModuleParent::WriteExtraDataForMinidump(CrashReporter::AnnotationTable& notes)
+PluginModuleParent::WriteExtraDataForMinidump(AnnotationTable& notes)
 {
    typedef nsDependentCString CS;

@ -200,19 +203,17 @@ bool
 PluginModuleParent::ShouldContinueFromReplyTimeout()
 {
 #ifdef MOZ_CRASHREPORTER
-    if (mPluginDumpID.IsEmpty()) {
-        CrashReporterParent* crashReporter = CrashReporter();
-        if (crashReporter->GeneratePairedMinidump(this)) {
-            mBrowserDumpID = crashReporter->ParentDumpID();
-            mPluginDumpID = crashReporter->ChildDumpID();
-            PLUGIN_LOG_DEBUG(
-                    ("generated paired browser/plugin minidumps: %s/%s (ID=%s)",
-                     NS_ConvertUTF16toUTF8(mBrowserDumpID).get(),
-                     NS_ConvertUTF16toUTF8(mPluginDumpID).get(),
-                     NS_ConvertUTF16toUTF8(crashReporter->HangID()).get()));
-        } else {
-            NS_WARNING("failed to capture paired minidumps from hang");
-        }
+    CrashReporterParent* crashReporter = CrashReporter();
+    if (crashReporter->GeneratePairedMinidump(this)) {
+        mBrowserDumpID = crashReporter->ParentDumpID();
+        mPluginDumpID = crashReporter->ChildDumpID();
+        PLUGIN_LOG_DEBUG(
+                ("generated paired browser/plugin minidumps: %s/%s (ID=%s)",
+                 NS_ConvertUTF16toUTF8(mBrowserDumpID).get(),
+                 NS_ConvertUTF16toUTF8(mPluginDumpID).get(),
+                 NS_ConvertUTF16toUTF8(crashReporter->HangID()).get()));
+    } else {
+        NS_WARNING("failed to capture paired minidumps from hang");
    }
 #endif

@ -237,33 +238,99 @@ PluginModuleParent::CrashReporter()
 }
 #endif

+#ifdef MOZ_CRASHREPORTER
+static void
+RemoveMinidump(nsIFile* minidump)
+{
+    if (!minidump)
+        return;
+
+    minidump->Remove(false);
+    nsCOMPtr<nsIFile> extraFile;
+    if (GetExtraFileForMinidump(minidump,
+                                getter_AddRefs(extraFile))) {
+        extraFile->Remove(true);
+    }
+}
+
+void
+PluginModuleParent::ProcessFirstMinidump()
+{
+    CrashReporterParent* crashReporter = CrashReporter();
+    if (!crashReporter)
+        return;
+
+    AnnotationTable notes;
+    notes.Init(4);
+    WriteExtraDataForMinidump(notes);
+        
+    if (!mPluginDumpID.IsEmpty() && !mBrowserDumpID.IsEmpty()) {
+        crashReporter->GenerateHangCrashReport(&notes);
+        return;
+    }
+
+    PRUint32 sequence = PR_UINT32_MAX;
+    nsCOMPtr<nsIFile> dumpFile;
+    nsCAutoString flashProcessType;
+    TakeMinidump(getter_AddRefs(dumpFile), &sequence);
+
+#ifdef MOZ_CRASHREPORTER_INJECTOR
+    nsCOMPtr<nsIFile> childDumpFile;
+    PRUint32 childSequence;
+
+    if (mFlashProcess1 &&
+        TakeMinidumpForChild(mFlashProcess1,
+                             getter_AddRefs(childDumpFile),
+                             &childSequence)) {
+        if (childSequence < sequence) {
+            RemoveMinidump(dumpFile);
+            dumpFile = childDumpFile;
+            sequence = childSequence;
+            flashProcessType.AssignLiteral("Broker");
+        }
+        else {
+            RemoveMinidump(childDumpFile);
+        }
+    }
+    if (mFlashProcess2 &&
+        TakeMinidumpForChild(mFlashProcess2,
+                             getter_AddRefs(childDumpFile),
+                             &childSequence)) {
+        if (childSequence < sequence) {
+            RemoveMinidump(dumpFile);
+            dumpFile = childDumpFile;
+            sequence = childSequence;
+            flashProcessType.AssignLiteral("Sandbox");
+        }
+        else {
+            RemoveMinidump(childDumpFile);
+        }
+    }
+#endif
+
+    if (!dumpFile) {
+        NS_WARNING("[PluginModuleParent::ActorDestroy] abnormal shutdown without minidump!");
+        return;
+    }
+
+    PLUGIN_LOG_DEBUG(("got child minidump: %s",
+                      NS_ConvertUTF16toUTF8(mPluginDumpID).get()));
+
+    GetIDFromMinidump(dumpFile, mPluginDumpID);
+    if (!flashProcessType.IsEmpty()) {
+        notes.Put(NS_LITERAL_CSTRING("FlashProcessDump"), flashProcessType);
+        crashReporter->GenerateCrashReportForMinidump(dumpFile, &notes);
+    }
+}
+#endif
+
 void
 PluginModuleParent::ActorDestroy(ActorDestroyReason why)
 {
    switch (why) {
    case AbnormalShutdown: {
 #ifdef MOZ_CRASHREPORTER
-        CrashReporterParent* crashReporter = CrashReporter();
-
-        CrashReporter::AnnotationTable notes;
-        notes.Init(4);
-        WriteExtraDataForMinidump(notes);
-        
-        if (!mPluginDumpID.IsEmpty() && !mBrowserDumpID.IsEmpty()) {
-            crashReporter->GenerateHangCrashReport(&notes);
-        }
-        else if (!mPluginDumpID.IsEmpty()) {
-            // Nothing to do, we've already written this minidump in
-            // PluginModuleParent::OnCrash
-        }
-        else if (crashReporter->GenerateCrashReport(this, &notes)) {
-            mPluginDumpID = crashReporter->ChildDumpID();
-            PLUGIN_LOG_DEBUG(("got child minidump: %s",
-                              NS_ConvertUTF16toUTF8(mPluginDumpID).get()));
-        }
-        else {
-            NS_WARNING("[PluginModuleParent::ActorDestroy] abnormal shutdown without minidump!");
-        }
+        ProcessFirstMinidump();
 #endif

        mShutdown = true;
@ -1248,33 +1315,8 @@ PluginModuleParent::InitializeInjector()
 }

 void
-PluginModuleParent::OnCrash(DWORD processID, const nsAString& aDumpID)
+PluginModuleParent::OnCrash(DWORD processID)
 {
-    if (!mPluginDumpID.IsEmpty()) {
-        // One process has already crashed: we assume that the first-to-crash
-        // is the interesting one
-        return;
-    }
-
-    mPluginDumpID = aDumpID;
-
-    CrashReporter::AnnotationTable notes;
-    notes.Init(4);
-    WriteExtraDataForMinidump(notes);
-    notes.Put(NS_LITERAL_CSTRING("ProcessType"), NS_LITERAL_CSTRING("plugin"));
-    if (processID == mFlashProcess1) {
-        notes.Put(NS_LITERAL_CSTRING("FlashProcessDump"),
-                  NS_LITERAL_CSTRING("Broker"));
-    }
-    else if (processID == mFlashProcess2) {
-        notes.Put(NS_LITERAL_CSTRING("FlashProcessDump"),
-                  NS_LITERAL_CSTRING("Sandbox"));
-    }
-    else {
-        NS_ERROR("Got minidump for Flash process neither broker nor sandbox.");
-    }
-    CrashReporter::AppendExtraData(aDumpID, notes);
-
    GetIPCChannel()->CloseWithError();
    KillProcess(OtherProcess(), 1, false);
 }
--- a/dom/plugins/ipc/PluginModuleParent.h
+++ b/dom/plugins/ipc/PluginModuleParent.h
@ -288,6 +288,7 @@ private:
    CrashReporterParent* CrashReporter();

 #ifdef MOZ_CRASHREPORTER
+    void ProcessFirstMinidump();
    void WriteExtraDataForMinidump(CrashReporter::AnnotationTable& notes);
 #endif
    void CleanupFromTimeout();
@ -319,7 +320,7 @@ private:
 #ifdef MOZ_CRASHREPORTER_INJECTOR
    void InitializeInjector();
    
-    NS_OVERRIDE void OnCrash(DWORD processID, const nsAString& aDumpID);
+    NS_OVERRIDE void OnCrash(DWORD processID);

    DWORD mFlashProcess1;
    DWORD mFlashProcess2;
--- a/ipc/glue/GeckoChildProcessHost.cpp
+++ b/ipc/glue/GeckoChildProcessHost.cpp
@ -250,6 +250,10 @@ void GeckoChildProcessHost::InitWindowsGroupID()
 bool
 GeckoChildProcessHost::SyncLaunch(std::vector<std::string> aExtraOpts, int aTimeoutMs, base::ProcessArchitecture arch)
 {
+#ifdef MOZ_CRASHREPORTER
+  CrashReporter::OOPInit();
+#endif
+
 #ifdef XP_WIN
  InitWindowsGroupID();
 #endif
@ -291,6 +295,10 @@ GeckoChildProcessHost::SyncLaunch(std::vector<std::string> aExtraOpts, int aTime
 bool
 GeckoChildProcessHost::AsyncLaunch(std::vector<std::string> aExtraOpts)
 {
+#ifdef MOZ_CRASHREPORTER
+  CrashReporter::OOPInit();
+#endif
+
 #ifdef XP_WIN
  InitWindowsGroupID();
 #endif
--- a/ipc/ipdl/ipdl/cxx/ast.py
+++ b/ipc/ipdl/ipdl/cxx/ast.py
@ -330,6 +330,7 @@ Type.INT = Type('int')
 Type.INT32 = Type('int32')
 Type.INTPTR = Type('intptr_t')
 Type.UINT32 = Type('uint32')
+Type.UINT32PTR = Type('uint32', ptr=1)
 Type.SIZE = Type('size_t')
 Type.VOID = Type('void')
 Type.VOIDPTR = Type('void', ptr=1)
--- a/ipc/ipdl/ipdl/lower.py
+++ b/ipc/ipdl/ipdl/lower.py
@ -3086,16 +3086,18 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
            ])

            dumpvar = ExprVar('aDump')
+            seqvar = ExprVar('aSequence')
            getdump = MethodDefn(MethodDecl(
                'TakeMinidump',
-                params=[ Decl(Type('nsIFile', ptrptr=1), dumpvar.name) ],
+                params=[ Decl(Type('nsIFile', ptrptr=1), dumpvar.name),
+                         Decl(Type.UINT32PTR, seqvar.name)],
                ret=Type.BOOL,
                const=1))
            getdump.addstmts([
                CppDirective('ifdef', 'MOZ_CRASHREPORTER'),
                StmtReturn(ExprCall(
                    ExprVar('XRE_TakeMinidumpForChild'),
-                    args=[ ExprCall(otherpidvar), dumpvar ])),
+                    args=[ ExprCall(otherpidvar), dumpvar, seqvar ])),
                CppDirective('else'),
                StmtReturn.FALSE,
                CppDirective('endif')
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@ -23,6 +23,7 @@
 #include "client/windows/handler/exception_handler.h"
 #include <DbgHelp.h>
 #include <string.h>
+#include "nsDirectoryServiceUtils.h"

 #include "nsWindowsDllInterceptor.h"
 #elif defined(XP_MACOSX)
@ -40,7 +41,6 @@
 #include <unistd.h>
 #include "mac_utils.h"
 #elif defined(XP_LINUX)
-#include "nsDirectoryServiceUtils.h"
 #include "nsDirectoryServiceDefs.h"
 #include "nsIINIParser.h"
 #include "common/linux/linux_libc_support.h"
@ -152,6 +152,7 @@ static const XP_CHAR extraFileExtension[] = {'.', 'e', 'x', 't',

 static google_breakpad::ExceptionHandler* gExceptionHandler = nsnull;

+static XP_CHAR* pendingDirectory;
 static XP_CHAR* crashReporterPath;

 // if this is false, we don't launch the crash reporter
@ -221,13 +222,31 @@ static const int kMagicChildCrashReportFd = 4;

 // |dumpMapLock| must protect all access to |pidToMinidump|.
 static Mutex* dumpMapLock;
-typedef nsInterfaceHashtable<nsUint32HashKey, nsIFile> ChildMinidumpMap;
+struct ChildProcessData : public nsUint32HashKey
+{
+  ChildProcessData(KeyTypePointer aKey)
+    : nsUint32HashKey(aKey)
+    , sequence(0)
+#ifdef MOZ_CRASHREPORTER_INJECTOR
+    , callback(NULL)
+#endif
+  { }
+
+  nsCOMPtr<nsIFile> minidump;
+  // Each crashing process is assigned an increasing sequence number to
+  // indicate which process crashed first.
+  PRUint32 sequence;
+#ifdef MOZ_CRASHREPORTER_INJECTOR
+  InjectorCrashCallback* callback;
+#endif
+};
+
+typedef nsTHashtable<ChildProcessData> ChildMinidumpMap;
 static ChildMinidumpMap* pidToMinidump;
+static PRUint32 crashSequence;

 #ifdef MOZ_CRASHREPORTER_INJECTOR
 static nsIThread* sInjectorThread;
-typedef nsDataHashtable<nsUint32HashKey, InjectorCrashCallback*> InjectorPIDMap;
-static InjectorPIDMap* pidToInjectorCallback;

 class ReportInjectedCrash : public nsRunnable
 {
@ -714,13 +733,13 @@ nsresult SetExceptionHandler(nsIFile* aXREDirectory,

 #ifdef XP_WIN32
  nsString crashReporterPath_temp;
-  exePath->GetPath(crashReporterPath_temp);

+  exePath->GetPath(crashReporterPath_temp);
  crashReporterPath = ToNewUnicode(crashReporterPath_temp);
 #elif !defined(__ANDROID__)
  nsCString crashReporterPath_temp;
-  exePath->GetNativePath(crashReporterPath_temp);

+  exePath->GetNativePath(crashReporterPath_temp);
  crashReporterPath = ToNewCString(crashReporterPath_temp);
 #else
  // On Android, we launch using the application package name
@ -1133,6 +1152,11 @@ nsresult UnsetExceptionHandler()
  delete notesField;
  notesField = nsnull;

+  if (pendingDirectory) {
+    NS_Free(pendingDirectory);
+    pendingDirectory = nsnull;
+  }
+
  if (crashReporterPath) {
    NS_Free(crashReporterPath);
    crashReporterPath = nsnull;
@ -1652,23 +1676,23 @@ nsresult SetSubmitReports(bool aSubmitReports)
 }

 // The "pending" dir is Crash Reports/pending, from which minidumps
-// can be submitted
+// can be submitted. Because this method may be called off the main thread,
+// we store the pending directory as a path.
 static bool
 GetPendingDir(nsIFile** dir)
 {
-  nsCOMPtr<nsIProperties> dirSvc =
-    do_GetService(NS_DIRECTORY_SERVICE_CONTRACTID);
-  if (!dirSvc)
+  if (!pendingDirectory) {
+    NS_ERROR("Not initialized");
    return false;
-  nsCOMPtr<nsIFile> pendingDir;
-  if (NS_FAILED(dirSvc->Get("UAppData",
-                            NS_GET_IID(nsIFile),
-                            getter_AddRefs(pendingDir))) ||
-      NS_FAILED(pendingDir->Append(NS_LITERAL_STRING("Crash Reports"))) ||
-      NS_FAILED(pendingDir->Append(NS_LITERAL_STRING("pending"))))
-    return false;
-  *dir = NULL;
-  pendingDir.swap(*dir);
+  }
+
+  nsCOMPtr<nsIFile> pending = do_CreateInstance(NS_LOCAL_FILE_CONTRACTID);
+#ifdef XP_WIN
+  pending->InitWithPath(nsDependentString(pendingDirectory));
+#else
+  pending->InitWithNativePath(nsDependentCString(pendingDirectory));
+#endif
+  pending.swap(*dir);
  return true;
 }

@ -1933,12 +1957,22 @@ OnChildProcessDumpRequested(void* aContext,
      aClientInfo->pid();
 #endif

+#ifdef MOZ_CRASHREPORTER_INJECTOR
+    bool runCallback;
+#endif
    {
      MutexAutoLock lock(*dumpMapLock);
-      pidToMinidump->Put(pid, minidump);
+      ChildProcessData* pd = pidToMinidump->PutEntry(pid);
+      MOZ_ASSERT(!pd->minidump);
+      pd->minidump = minidump;
+      pd->sequence = ++crashSequence;
+#ifdef MOZ_CRASHREPORTER_INJECTOR
+      runCallback = NULL != pd->callback;
+#endif
    }
 #ifdef MOZ_CRASHREPORTER_INJECTOR
-    NS_DispatchToMainThread(new ReportInjectedCrash(pid));
+    if (runCallback)
+      NS_DispatchToMainThread(new ReportInjectedCrash(pid));
 #endif
  }
 }
@ -1954,11 +1988,14 @@ static bool ChildFilter(void *context) {
  return true;
 }

-static void
+void
 OOPInit()
 {
-  NS_ABORT_IF_FALSE(!OOPInitialized(),
-                    "OOP crash reporter initialized more than once!");
+  if (OOPInitialized())
+    return;
+
+  MOZ_ASSERT(NS_IsMainThread());
+
  NS_ABORT_IF_FALSE(gExceptionHandler != NULL,
                    "attempt to initialize OOP crash reporter before in-process crashreporter!");

@ -2019,6 +2056,26 @@ OOPInit()
  pidToMinidump->Init();

  dumpMapLock = new Mutex("CrashReporter::dumpMapLock");
+
+  nsCOMPtr<nsIFile> pendingDir;
+  nsresult rv = NS_GetSpecialDirectory("UAppData", getter_AddRefs(pendingDir));
+  if (NS_FAILED(rv)) {
+    NS_ERROR("Couldn't get the user appdata directory!");
+    return;
+  }
+
+  pendingDir->Append(NS_LITERAL_STRING("Crash Reports"));
+  pendingDir->Append(NS_LITERAL_STRING("pending"));
+
+#ifdef XP_WIN
+  nsString path;
+  pendingDir->GetPath(path);
+  pendingDirectory = ToNewUnicode(path);
+#else
+  nsCString path;
+  pendingDir->GetNativePath(path);
+  pendingDirectory = ToNewUnicode(path);
+#endif  
 }

 static void
@ -2034,9 +2091,6 @@ OOPDeinit()
    sInjectorThread->Shutdown();
    NS_RELEASE(sInjectorThread);
  }
-
-  delete pidToInjectorCallback;
-  pidToInjectorCallback = NULL;
 #endif

  delete crashServer;
@ -2062,8 +2116,7 @@ GetChildNotificationPipe()
  if (!GetEnabled())
    return kNullNotifyPipe;

-  if (!OOPInitialized())
-    OOPInit();
+  MOZ_ASSERT(OOPInitialized());

  return childCrashNotifyPipe;
 }
@ -2079,17 +2132,17 @@ InjectCrashReporterIntoProcess(DWORD processID, InjectorCrashCallback* cb)
  if (!OOPInitialized())
    OOPInit();

-  if (!pidToInjectorCallback) {
-    pidToInjectorCallback = new InjectorPIDMap;
-    pidToInjectorCallback->Init();
-  }
-
  if (!sInjectorThread) {
    if (NS_FAILED(NS_NewThread(&sInjectorThread)))
      return;
  }

-  pidToInjectorCallback->Put(processID, cb);
+  {
+    MutexAutoLock lock(*dumpMapLock);
+    ChildProcessData* pd = pidToMinidump->PutEntry(processID);
+    MOZ_ASSERT(!pd->minidump && !pd->callback);
+    pd->callback = cb;
+  }

  nsCOMPtr<nsIRunnable> r = new InjectCrashRunnable(processID);
  sInjectorThread->Dispatch(r, nsIEventTarget::DISPATCH_NORMAL);
@ -2099,23 +2152,22 @@ NS_IMETHODIMP
 ReportInjectedCrash::Run()
 {
  // Crash reporting may have been disabled after this method was dispatched
-  if (!pidToInjectorCallback)
+  if (!OOPInitialized())
    return NS_OK;

-  InjectorCrashCallback* cb = pidToInjectorCallback->Get(mPID);
-  if (!cb)
-    return NS_OK;
+  InjectorCrashCallback* cb;
+  {
+    MutexAutoLock lock(*dumpMapLock);
+    ChildProcessData* pd = pidToMinidump->GetEntry(mPID);
+    if (!pd || !pd->callback)
+      return NS_OK;

-  nsCOMPtr<nsIFile> minidump;
-  if (!TakeMinidumpForChild(mPID, getter_AddRefs(minidump))) {
-    NS_WARNING("No minidump for crash notification.");
-    return NS_OK;
+    MOZ_ASSERT(pd->minidump);
+
+    cb = pd->callback;
  }

-  nsString id;
-  GetIDFromMinidump(minidump, id);
-
-  cb->OnCrash(mPID, id);
+  cb->OnCrash(mPID);
  return NS_OK;
 }

@ -2125,7 +2177,8 @@ UnregisterInjectorCallback(DWORD processID)
  if (!OOPInitialized())
    return;

-  pidToInjectorCallback->Remove(processID);
+  MutexAutoLock lock(*dumpMapLock);
+  pidToMinidump->RemoveEntry(processID);
 }

 #endif // MOZ_CRASHREPORTER_INJECTOR
@ -2171,8 +2224,7 @@ CreateNotificationPipeForChild(int* childCrashFd, int* childCrashRemapFd)
    return true;
  }

-  if (!OOPInitialized())
-    OOPInit();
+  MOZ_ASSERT(OOPInitialized());

  *childCrashFd = clientSocketFd;
  *childCrashRemapFd = kMagicChildCrashReportFd;
@ -2232,22 +2284,25 @@ SetRemoteExceptionHandler(const nsACString& crashPipe)


 bool
-TakeMinidumpForChild(PRUint32 childPid, nsIFile** dump)
+TakeMinidumpForChild(PRUint32 childPid, nsIFile** dump, PRUint32* aSequence)
 {
  if (!GetEnabled())
    return false;

  MutexAutoLock lock(*dumpMapLock);

-  nsCOMPtr<nsIFile> d;
-  bool found = pidToMinidump->Get(childPid, getter_AddRefs(d));
-  if (found)
-    pidToMinidump->Remove(childPid);
+  ChildProcessData* pd = pidToMinidump->GetEntry(childPid);
+  if (!pd)
+    return false;

-  *dump = NULL;
-  d.swap(*dump);
+  NS_IF_ADDREF(*dump = pd->minidump);
+  if (aSequence) {
+    *aSequence = pd->sequence;
+  }
+  
+  pidToMinidump->RemoveEntry(childPid);

-  return found;
+  return !!*dump;
 }

 //-----------------------------------------------------------------------------
--- a/toolkit/crashreporter/nsExceptionHandler.h
+++ b/toolkit/crashreporter/nsExceptionHandler.h
@ -69,11 +69,17 @@ nsresult SetSubmitReports(bool aSubmitReport);

 // Out-of-process crash reporter API.

-// Return true iff a dump was found for |childPid|, and return the
+// Initializes out-of-process crash reporting. This method must be called
+// before the platform-specifi notificationpipe APIs are called.
+void OOPInit();
+
+// Return true if a dump was found for |childPid|, and return the
 // path in |dump|.  The caller owns the last reference to |dump| if it
-// is non-NULL.
+// is non-NULL. The sequence parameter will be filled with an ordinal
+// indicating which remote process crashed first.
 bool TakeMinidumpForChild(PRUint32 childPid,
-                          nsIFile** dump NS_OUTPARAM);
+                          nsIFile** dump NS_OUTPARAM,
+                          PRUint32* aSequence = NULL);

 #if defined(XP_WIN)
 typedef HANDLE ProcessHandle;
@ -120,9 +126,17 @@ class InjectorCrashCallback
 public:
  InjectorCrashCallback() { }

-  virtual void OnCrash(DWORD processID, const nsAString& aDumpID) = 0;
+  /**
+   * Inform the callback of a crash. The client code should call
+   * TakeMinidumpForChild to remove it from the PID mapping table.
+   *
+   * The callback will not be fired if the client has already called
+   * TakeMinidumpForChild for this process ID.
+   */
+  virtual void OnCrash(DWORD processID) = 0;
 };

+// This method implies OOPInit
 void InjectCrashReporterIntoProcess(DWORD processID, InjectorCrashCallback* cb);
 void UnregisterInjectorCallback(DWORD processID);
 #endif
--- a/toolkit/xre/nsEmbedFunctions.cpp
+++ b/toolkit/xre/nsEmbedFunctions.cpp
@ -226,9 +226,10 @@ GeckoProcessType sChildProcessType = GeckoProcessType_Default;
 // IPDL wants access to this crashreporter interface, and
 // crashreporter is built in such a way to make that awkward
 bool
-XRE_TakeMinidumpForChild(PRUint32 aChildPid, nsIFile** aDump)
+XRE_TakeMinidumpForChild(PRUint32 aChildPid, nsIFile** aDump,
+                         PRUint32* aSequence)
 {
-  return CrashReporter::TakeMinidumpForChild(aChildPid, aDump);
+  return CrashReporter::TakeMinidumpForChild(aChildPid, aDump, aSequence);
 }

 bool
--- a/xpcom/build/nsXULAppAPI.h
+++ b/xpcom/build/nsXULAppAPI.h
@ -367,7 +367,8 @@ XRE_API(GeckoProcessType,
 #if defined(MOZ_CRASHREPORTER)
 // Used in the "master" parent process hosting the crash server
 XRE_API(bool,
-        XRE_TakeMinidumpForChild, (PRUint32 aChildPid, nsIFile** aDump))
+        XRE_TakeMinidumpForChild, (PRUint32 aChildPid, nsIFile** aDump,
+                                   PRUint32* aSequence))

 // Used in child processes.
 XRE_API(bool,