Bug 784145 - When submitting hang reports, submit the browser report as a field of the plugin report instead of as a completely separate report. r=ted

dom/ipc/CrashReporterParent.cpp

@@ -39,14 +39,13 @@ CrashReporterParent::RecvAddLibraryMappings(const InfallibleTArray<Mapping>& map
   return true;
 }
 
-bool
+void
-CrashReporterParent::RecvAnnotateCrashReport(const nsCString& key,
+CrashReporterParent::AnnotateCrashReport(const nsCString& key,
-                                             const nsCString& data)
+                                         const nsCString& data)
 {
 #ifdef MOZ_CRASHREPORTER
     mNotes.Put(key, data);
 #endif
-    return true;
 }
 
 bool
@@ -82,22 +81,6 @@ CrashReporterParent::SetChildData(const NativeThreadId& tid,
 }
 
 #ifdef MOZ_CRASHREPORTER
-bool
-CrashReporterParent::GenerateHangCrashReport(const AnnotationTable* processNotes)
-{
-    if (mChildDumpID.IsEmpty())
-        return false;
-
-    GenerateChildData(processNotes);
-
-    CrashReporter::AnnotationTable notes;
-    notes.Init(4);
-    notes.Put(nsDependentCString("HangID"), NS_ConvertUTF16toUTF8(mHangID));
-    if (!CrashReporter::AppendExtraData(mParentDumpID, notes))
-        NS_WARNING("problem appending parent data to .extra");
-    return true;
-}
-
 bool
 CrashReporterParent::GenerateCrashReportForMinidump(nsIFile* minidump,
     const AnnotationTable* processNotes)

dom/ipc/CrashReporterParent.h

@@ -34,13 +34,6 @@ public:
   bool
   GeneratePairedMinidump(Toplevel* t);
 
-  /* Attempt to create a bare-bones crash report for a hang, along with extra
-     process-specific annotations present in the given AnnotationTable. Returns
-     true if successful, false otherwise.
-  */
-  bool
-  GenerateHangCrashReport(const AnnotationTable* processNotes);
-
   /* Attempt to create a bare-bones crash report, along with extra process-
      specific annotations present in the given AnnotationTable. Returns true if
      successful, false otherwise.
@@ -49,6 +42,12 @@ public:
   bool
   GenerateCrashReport(Toplevel* t, const AnnotationTable* processNotes);
 
+  /**
+   * Add the .extra data for an existing crash report.
+   */
+  bool
+  GenerateChildData(const AnnotationTable* processNotes);
+
   bool
   GenerateCrashReportForMinidump(nsIFile* minidump,
                                  const AnnotationTable* processNotes);
@@ -63,18 +62,6 @@ public:
   void
     SetChildData(const NativeThreadId& id, const uint32_t& processType);
 
-  /* Returns the shared hang ID of a parent/child paired minidump.
-     GeneratePairedMinidump must be called first.
-  */
-  const nsString& HangID() {
-    return mHangID;
-  }
-  /* Returns the ID of the parent minidump.
-     GeneratePairedMinidump must be called first.
-  */
-  const nsString& ParentDumpID() {
-    return mParentDumpID;
-  }
   /* Returns the ID of the child minidump.
      GeneratePairedMinidump or GenerateCrashReport must be called first.
   */
@@ -82,26 +69,27 @@ public:
     return mChildDumpID;
   }
 
+  void
+  AnnotateCrashReport(const nsCString& key, const nsCString& data);
+
  protected:
   virtual void ActorDestroy(ActorDestroyReason why);
 
   virtual bool
     RecvAddLibraryMappings(const InfallibleTArray<Mapping>& m);
   virtual bool
-    RecvAnnotateCrashReport(const nsCString& key, const nsCString& data);
+    RecvAnnotateCrashReport(const nsCString& key, const nsCString& data) {
+    AnnotateCrashReport(key, data);
+    return true;
+  }
   virtual bool
     RecvAppendAppNotes(const nsCString& data);
 
 #ifdef MOZ_CRASHREPORTER
-  bool
-  GenerateChildData(const AnnotationTable* processNotes);
-
   AnnotationTable mNotes;
 #endif
   nsCString mAppNotes;
-  nsString mHangID;
   nsString mChildDumpID;
-  nsString mParentDumpID;
   NativeThreadId mMainThread;
   time_t mStartTime;
   uint32_t mProcessType;
@@ -120,14 +108,10 @@ CrashReporterParent::GeneratePairedMinidump(Toplevel* t)
   child = t->OtherProcess();
 #endif
   nsCOMPtr<nsIFile> childDump;
-  nsCOMPtr<nsIFile> parentDump;
   if (CrashReporter::CreatePairedMinidumps(child,
                                            mMainThread,
-                                           &mHangID,
+                                           getter_AddRefs(childDump)) &&
-                                           getter_AddRefs(childDump),
+      CrashReporter::GetIDFromMinidump(childDump, mChildDumpID)) {
-                                           getter_AddRefs(parentDump)) &&
-      CrashReporter::GetIDFromMinidump(childDump, mChildDumpID) &&
-      CrashReporter::GetIDFromMinidump(parentDump, mParentDumpID)) {
     return true;
   }
   return false;

dom/plugins/ipc/PluginModuleParent.cpp

@@ -174,28 +174,24 @@ PluginModuleParent::WriteExtraDataForMinidump(AnnotationTable& notes)
 
     CrashReporterParent* crashReporter = CrashReporter();
     if (crashReporter) {
-        const nsString& hangID = crashReporter->HangID();
-        if (!hangID.IsEmpty()) {
-            notes.Put(CS("HangID"), NS_ConvertUTF16toUTF8(hangID));
 #ifdef XP_WIN
-            if (mPluginCpuUsageOnHang.Length() > 0) {
+        if (mPluginCpuUsageOnHang.Length() > 0) {
-              notes.Put(CS("NumberOfProcessors"),
+            notes.Put(CS("NumberOfProcessors"),
-                        nsPrintfCString("%d", PR_GetNumberOfProcessors()));
+                      nsPrintfCString("%d", PR_GetNumberOfProcessors()));
 
-              nsCString cpuUsageStr;
+            nsCString cpuUsageStr;
-              cpuUsageStr.AppendFloat(std::ceil(mPluginCpuUsageOnHang[0] * 100) / 100);
+            cpuUsageStr.AppendFloat(std::ceil(mPluginCpuUsageOnHang[0] * 100) / 100);
-              notes.Put(CS("PluginCpuUsage"), cpuUsageStr);
+            notes.Put(CS("PluginCpuUsage"), cpuUsageStr);
 
 #ifdef MOZ_CRASHREPORTER_INJECTOR
-              for (uint32_t i=1; i<mPluginCpuUsageOnHang.Length(); ++i) {
+            for (uint32_t i=1; i<mPluginCpuUsageOnHang.Length(); ++i) {
                 nsCString tempStr;
                 tempStr.AppendFloat(std::ceil(mPluginCpuUsageOnHang[i] * 100) / 100);
                 notes.Put(nsPrintfCString("CpuUsageFlashProcess%d", i), tempStr);
-              }
-#endif
             }
 #endif
         }
+#endif
     }
 }
 #endif  // MOZ_CRASHREPORTER
@@ -297,14 +293,19 @@ PluginModuleParent::ShouldContinueFromReplyTimeout()
 {
 #ifdef MOZ_CRASHREPORTER
     CrashReporterParent* crashReporter = CrashReporter();
+    crashReporter->AnnotateCrashReport(NS_LITERAL_CSTRING("PluginHang"),
+                                       NS_LITERAL_CSTRING("1"));
     if (crashReporter->GeneratePairedMinidump(this)) {
-        mBrowserDumpID = crashReporter->ParentDumpID();
         mPluginDumpID = crashReporter->ChildDumpID();
         PLUGIN_LOG_DEBUG(
-                ("generated paired browser/plugin minidumps: %s/%s (ID=%s)",
+                ("generated paired browser/plugin minidumps: %s)",
-                 NS_ConvertUTF16toUTF8(mBrowserDumpID).get(),
+                 NS_ConvertUTF16toUTF8(mPluginDumpID).get()));
-                 NS_ConvertUTF16toUTF8(mPluginDumpID).get(),
+
-                 NS_ConvertUTF16toUTF8(crashReporter->HangID()).get()));
+        crashReporter->AnnotateCrashReport(
+            NS_LITERAL_CSTRING("additional_minidumps"),
+            NS_LITERAL_CSTRING("browser"));
+
+        // TODO: collect Flash minidumps here
     } else {
         NS_WARNING("failed to capture paired minidumps from hang");
     }
@@ -377,9 +378,9 @@ PluginModuleParent::ProcessFirstMinidump()
     AnnotationTable notes;
     notes.Init(4);
     WriteExtraDataForMinidump(notes);
-        
+
-    if (!mPluginDumpID.IsEmpty() && !mBrowserDumpID.IsEmpty()) {
+    if (!mPluginDumpID.IsEmpty()) {
-        crashReporter->GenerateHangCrashReport(&notes);
+        crashReporter->GenerateChildData(&notes);
         return;
     }
 

dom/plugins/test/mochitest/hang_test.js

@@ -1,47 +1,12 @@
 
+Components.utils.import("resource://gre/modules/KeyValueParser.jsm");
+
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 
 var success = false;
 var observerFired = false;
 
-function parseKeyValuePairs(text) {
-  var lines = text.split('\n');
-  var data = {};
-  for (let i = 0; i < lines.length; i++) {
-    if (lines[i] == '')
-      continue;
-
-    // can't just .split() because the value might contain = characters
-    let eq = lines[i].indexOf('=');
-    if (eq != -1) {
-      let [key, value] = [lines[i].substring(0, eq),
-                          lines[i].substring(eq + 1)];
-      if (key && value)
-        data[key] = value.replace(/\\n/g, "\n").replace(/\\\\/g, "\\");
-    }
-  }
-  return data;
-}
-
-function parseKeyValuePairsFromFile(file) {
-  var fstream = Cc["@mozilla.org/network/file-input-stream;1"].
-                createInstance(Ci.nsIFileInputStream);
-  fstream.init(file, -1, 0, 0);
-  var is = Cc["@mozilla.org/intl/converter-input-stream;1"].
-           createInstance(Ci.nsIConverterInputStream);
-  is.init(fstream, "UTF-8", 1024, Ci.nsIConverterInputStream.DEFAULT_REPLACEMENT_CHARACTER);
-  var str = {};
-  var contents = '';
-  while (is.readString(4096, str) != 0) {
-    contents += str.value;
-  }
-  is.close();
-  fstream.close();
-  return parseKeyValuePairs(contents);
-}
-
-
 var testObserver = {
   idleHang: true,
 
@@ -55,10 +20,8 @@ var testObserver = {
 
     var pluginId = subject.getPropertyAsAString("pluginDumpID");
     isnot(pluginId, "", "got a non-empty plugin crash id");
-    var browserId = subject.getPropertyAsAString("browserDumpID");
+
-    isnot(browserId, "", "got a non-empty browser crash id");
+    // check plugin dump and extra files
-    
-    // check dump and extra files
     let directoryService =
       Cc["@mozilla.org/file/directory_service;1"].getService(Ci.nsIProperties);
     let profD = directoryService.get("ProfD", Ci.nsIFile);
@@ -66,18 +29,31 @@ var testObserver = {
     let pluginDumpFile = profD.clone();
     pluginDumpFile.append(pluginId + ".dmp");
     ok(pluginDumpFile.exists(), "plugin minidump exists");
-    let browserDumpFile = profD.clone();
+
-    browserDumpFile.append(browserId + ".dmp");
-    ok(browserDumpFile.exists(), "browser minidump exists");
     let pluginExtraFile = profD.clone();
     pluginExtraFile.append(pluginId + ".extra");
     ok(pluginExtraFile.exists(), "plugin extra file exists");
-    let browserExtraFile = profD.clone();
+
-    browserExtraFile.append(browserId + ".extra");
-    ok(pluginExtraFile.exists(), "browser extra file exists");
-     
-    // check cpu usage field
     let extraData = parseKeyValuePairsFromFile(pluginExtraFile);
+
+    // check additional dumps
+
+    ok("additional_minidumps" in extraData, "got field for additional minidumps");
+    let additionalDumps = extraData.additional_minidumps.split(',');
+    ok(additionalDumps.indexOf('browser') >= 0, "browser in additional_minidumps");
+
+    let additionalDumpFiles = [];
+    for (let name of additionalDumps) {
+      let file = profD.clone();
+      file.append(pluginId + "-" + name + ".dmp");
+      ok(file.exists(), "additional dump '"+name+"' exists");
+      if (file.exists()) {
+        additionalDumpFiles.push(file);
+      }
+    }
+
+    // check cpu usage field
+
     ok("PluginCpuUsage" in extraData, "got extra field for plugin cpu usage");
     let cpuUsage = parseFloat(extraData["PluginCpuUsage"]);
     if (this.idleHang) {
@@ -85,16 +61,17 @@ var testObserver = {
     } else {
       ok(cpuUsage > 0, "plugin cpu usage is >0%");
     }
-    
+
     // check processor count field
     ok("NumberOfProcessors" in extraData, "got extra field for processor count");
     ok(parseInt(extraData["NumberOfProcessors"]) > 0, "number of processors is >0");
 
     // cleanup, to be nice
     pluginDumpFile.remove(false);
-    browserDumpFile.remove(false);
     pluginExtraFile.remove(false);
-    browserExtraFile.remove(false);
+    for (let file of additionalDumpFiles) {
+      file.remove(false);
+    }
   },
 
   QueryInterface: function(iid) {

dom/plugins/test/mochitest/test_crashing.html

@@ -6,14 +6,15 @@
   <script class="testbody" type="application/javascript">
   SimpleTest.waitForExplicitFinish();
 
-  const isOSXLion = navigator.userAgent.indexOf("Mac OS X 10.7") != -1;
-const isOSXMtnLion = navigator.userAgent.indexOf("Mac OS X 10.8") != -1;
-  if (isOSXLion || isOSXMtnLion) {
-    todo(false, "Can't test plugin crash notification on OS X 10.7 or 10.8, see bug 705047");
-    SimpleTest.finish();
-  }
-
   window.frameLoaded = function frameLoaded_toCrash() {
+    const isOSXLion = navigator.userAgent.indexOf("Mac OS X 10.7") != -1;
+    const isOSXMtnLion = navigator.userAgent.indexOf("Mac OS X 10.8") != -1;
+    if (isOSXLion || isOSXMtnLion) {
+      todo(false, "Can't test plugin crash notification on OS X 10.7 or 10.8, see bug 705047");
+      SimpleTest.finish();
+      return;
+    }
+
     if (!SimpleTest.testPluginIsOOP()) {
       ok(true, "Skipping this test when test plugin is not OOP.");
       SimpleTest.finish();

testing/mochitest/specialpowers/content/specialpowers.js

@@ -47,11 +47,9 @@ SpecialPowers.prototype._messageReceived = function(aMessage) {
   switch (aMessage.name) {
     case "SPProcessCrashService":
       if (aMessage.json.type == "crash-observed") {
-        var self = this;
+        for (let e of aMessage.json.dumpIDs) {
-        aMessage.json.dumpIDs.forEach(function(id) {
+          this._encounteredCrashDumpFiles.push(e.id + "." + e.extension);
-          self._encounteredCrashDumpFiles.push(id + ".dmp");
+        }
-          self._encounteredCrashDumpFiles.push(id + ".extra");
-        });
       }
       break;
 

testing/mochitest/tests/SimpleTest/ChromePowers.js

@@ -54,11 +54,9 @@ ChromePowers.prototype._receiveMessage = function(aMessage) {
         // Hack out register/unregister specifically for browser-chrome leaks
         break;
       } else if (aMessage.type == "crash-observed") {
-        var self = this;
+        for (let e of msg.dumpIDs) {
-        msg.dumpIDs.forEach(function(id) {
+          this._encounteredCrashDumpFiles.push(e.id + "." + e.extension);
-          self._encounteredCrashDumpFiles.push(id + ".dmp");
+        }
-          self._encounteredCrashDumpFiles.push(id + ".extra");
-        });
       }
     default:
       // All calls go here, because we need to handle SPProcessCrashService calls as well

testing/mochitest/tests/SimpleTest/SpecialPowersObserverAPI.js

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+Components.utils.import("resource://gre/modules/Services.jsm");
+
 /**
  * Special Powers Exception - used to throw exceptions nicely
  **/
@@ -19,6 +21,42 @@ function SpecialPowersObserverAPI() {
   this._processCrashObserversRegistered = false;
 }
 
+function parseKeyValuePairs(text) {
+  var lines = text.split('\n');
+  var data = {};
+  for (let i = 0; i < lines.length; i++) {
+    if (lines[i] == '')
+      continue;
+
+    // can't just .split() because the value might contain = characters
+    let eq = lines[i].indexOf('=');
+    if (eq != -1) {
+      let [key, value] = [lines[i].substring(0, eq),
+                          lines[i].substring(eq + 1)];
+      if (key && value)
+        data[key] = value.replace(/\\n/g, "\n").replace(/\\\\/g, "\\");
+    }
+  }
+  return data;
+}
+
+function parseKeyValuePairsFromFile(file) {
+  var fstream = Cc["@mozilla.org/network/file-input-stream;1"].
+                createInstance(Ci.nsIFileInputStream);
+  fstream.init(file, -1, 0, 0);
+  var is = Cc["@mozilla.org/intl/converter-input-stream;1"].
+           createInstance(Ci.nsIConverterInputStream);
+  is.init(fstream, "UTF-8", 1024, Ci.nsIConverterInputStream.DEFAULT_REPLACEMENT_CHARACTER);
+  var str = {};
+  var contents = '';
+  while (is.readString(4096, str) != 0) {
+    contents += str.value;
+  }
+  is.close();
+  fstream.close();
+  return parseKeyValuePairs(contents);
+}
+
 SpecialPowersObserverAPI.prototype = {
 
   _observe: function(aSubject, aTopic, aData) {
@@ -28,15 +66,25 @@ SpecialPowersObserverAPI.prototype = {
         function addDumpIDToMessage(propertyName) {
           var id = aSubject.getPropertyAsAString(propertyName);
           if (id) {
-            message.dumpIDs.push(id);
+            message.dumpIDs.push({id: id, extension: "dmp"});
+            message.dumpIDs.push({id: id, extension: "extra"});
           }
         }
 
         var message = { type: "crash-observed", dumpIDs: [] };
-        aSubject = aSubject.QueryInterface(Components.interfaces.nsIPropertyBag2);
+        aSubject = aSubject.QueryInterface(Ci.nsIPropertyBag2);
         if (aTopic == "plugin-crashed") {
           addDumpIDToMessage("pluginDumpID");
           addDumpIDToMessage("browserDumpID");
+
+          let pluginID = aSubject.getPropertyAsAString("pluginDumpID");
+          let extra = this._getExtraData(pluginID);
+          if (extra && ("additional_minidumps" in extra)) {
+            let dumpNames = extra.additional_minidumps.split(',');
+            for (let name of dumpNames) {
+              message.dumpIDs.push({id: pluginID + "-" + name, extension: "dmp"});
+            }
+          }
         } else { // ipc:content-shutdown
           addDumpIDToMessage("dumpID");
         }
@@ -47,14 +95,21 @@ SpecialPowersObserverAPI.prototype = {
 
   _getCrashDumpDir: function() {
     if (!this._crashDumpDir) {
-      var directoryService = Components.classes["@mozilla.org/file/directory_service;1"]
+      this._crashDumpDir = Services.dirsvc.get("ProfD", Ci.nsIFile);
-                             .getService(Components.interfaces.nsIProperties);
-      this._crashDumpDir = directoryService.get("ProfD", Components.interfaces.nsIFile);
       this._crashDumpDir.append("minidumps");
     }
     return this._crashDumpDir;
   },
 
+  _getExtraData: function(dumpId) {
+    let extraFile = this._getCrashDumpDir().clone();
+    extraFile.append(dumpId + ".extra");
+    if (!extraFile.exists()) {
+      return null;
+    }
+    return parseKeyValuePairsFromFile(extraFile);
+  },
+
   _deleteCrashDumpFiles: function(aFilenames) {
     var crashDumpDir = this._getCrashDumpDir();
     if (!crashDumpDir.exists()) {
@@ -83,7 +138,7 @@ SpecialPowersObserverAPI.prototype = {
 
     var crashDumpFiles = [];
     while (entries.hasMoreElements()) {
-      var file = entries.getNext().QueryInterface(Components.interfaces.nsIFile);
+      var file = entries.getNext().QueryInterface(Ci.nsIFile);
       var path = String(file.path);
       if (path.match(/\.(dmp|extra)$/) && !aToIgnore[path]) {
         crashDumpFiles.push(path);
@@ -93,9 +148,7 @@ SpecialPowersObserverAPI.prototype = {
   },
 
   _getURI: function (url) {
-    return Components.classes["@mozilla.org/network/io-service;1"]
+    return Services.io.newURI(url, null, null);
-                     .getService(Components.interfaces.nsIIOService)
-                     .newURI(url, null, null);
   },
 
   /**
@@ -105,8 +158,7 @@ SpecialPowersObserverAPI.prototype = {
   _receiveMessageAPI: function(aMessage) {
     switch(aMessage.name) {
       case "SPPrefService":
-        var prefs = Components.classes["@mozilla.org/preferences-service;1"].
+        var prefs = Services.prefs;
-                    getService(Components.interfaces.nsIPrefBranch);
         var prefType = aMessage.json.prefType.toUpperCase();
         var prefName = aMessage.json.prefName;
         var prefValue = "prefValue" in aMessage.json ? aMessage.json.prefValue : null;
@@ -172,20 +224,17 @@ SpecialPowersObserverAPI.prototype = {
         break;
 
       case "SPPermissionManager":
-        let perms =
-          Components.classes["@mozilla.org/permissionmanager;1"]
-                    .getService(Components.interfaces.nsIPermissionManager);
         let msg = aMessage.json;
 
-        let secMan = Cc["@mozilla.org/scriptsecuritymanager;1"].getService(Ci.nsIScriptSecurityManager);
+        let secMan = Services.scriptSecurityManager;
         let principal = secMan.getAppCodebasePrincipal(this._getURI(msg.url), msg.appId, msg.isInBrowserElement);
 
         switch (msg.op) {
           case "add":
-            perms.addFromPrincipal(principal, msg.type, msg.permission);
+            Services.perms.addFromPrincipal(principal, msg.type, msg.permission);
             break;
           case "remove":
-            perms.removeFromPrincipal(principal, msg.type);
+            Services.perms.removeFromPrincipal(principal, msg.type);
             break;
           default:
             throw new SpecialPowersException("Invalid operation for " +

toolkit/crashreporter/CrashSubmit.jsm

@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 Components.utils.import("resource://gre/modules/Services.jsm");
+Components.utils.import("resource://gre/modules/KeyValueParser.jsm");
 
 let EXPORTED_SYMBOLS = [
   "CrashSubmit"
@@ -21,42 +22,6 @@ let reportURL = null;
 let strings = null;
 let myListener = null;
 
-function parseKeyValuePairs(text) {
-  var lines = text.split('\n');
-  var data = {};
-  for (let i = 0; i < lines.length; i++) {
-    if (lines[i] == '')
-      continue;
-
-    // can't just .split() because the value might contain = characters
-    let eq = lines[i].indexOf('=');
-    if (eq != -1) {
-      let [key, value] = [lines[i].substring(0, eq),
-                          lines[i].substring(eq + 1)];
-      if (key && value)
-        data[key] = value.replace(/\\n/g, "\n").replace(/\\\\/g, "\\");
-    }
-  }
-  return data;
-}
-
-function parseKeyValuePairsFromFile(file) {
-  var fstream = Cc["@mozilla.org/network/file-input-stream;1"].
-                createInstance(Ci.nsIFileInputStream);
-  fstream.init(file, -1, 0, 0);
-  var is = Cc["@mozilla.org/intl/converter-input-stream;1"].
-           createInstance(Ci.nsIConverterInputStream);
-  is.init(fstream, "UTF-8", 1024, Ci.nsIConverterInputStream.DEFAULT_REPLACEMENT_CHARACTER);
-  var str = {};
-  var contents = '';
-  while (is.readString(4096, str) != 0) {
-    contents += str.value;
-  }
-  is.close();
-  fstream.close();
-  return parseKeyValuePairs(contents);
-}
-
 function parseINIStrings(file) {
   var factory = Cc["@mozilla.org/xpcom/ini-parser-factory;1"].
                 getService(Ci.nsIINIParserFactory);
@@ -159,6 +124,7 @@ function Submitter(id, submitSuccess, submitError, noThrottle) {
   this.successCallback = submitSuccess;
   this.errorCallback = submitError;
   this.noThrottle = noThrottle;
+  this.additionalDumps = [];
 }
 
 Submitter.prototype = {
@@ -177,6 +143,9 @@ Submitter.prototype = {
     try {
       this.dump.remove(false);
       this.extra.remove(false);
+      for (let i of this.additionalDumps) {
+        i.dump.remove(false);
+      }
     }
     catch (ex) {
       // report an error? not much the user can do here.
@@ -193,6 +162,7 @@ Submitter.prototype = {
     this.iframe = null;
     this.dump = null;
     this.extra = null;
+    this.additionalDumps = null;
     // remove this object from the list of active submissions
     let idx = CrashSubmit._activeSubmissions.indexOf(this);
     if (idx != -1)
@@ -221,8 +191,19 @@ Submitter.prototype = {
       // tell the server not to throttle this, since it was manually submitted
       formData.append("Throttleable", "0");
     }
-    // add the minidump
+    // add the minidumps
     formData.append("upload_file_minidump", File(this.dump.path));
+    if (this.additionalDumps.length > 0) {
+      let names = [];
+      for (let i of this.additionalDumps) {
+        names.push(i.name);
+        formData.append("upload_file_minidump_"+i.name,
+                        File(i.dump.path));
+      }
+
+      formData.append("additional_minidumps", names.join(","));
+    }
+
     let self = this;
     xhr.addEventListener("readystatechange", function (aEvt) {
       if (xhr.readyState == 4) {
@@ -274,10 +255,26 @@ Submitter.prototype = {
       return false;
     }
 
+    let reportData = parseKeyValuePairsFromFile(extra);
+    let additionalDumps = [];
+    if ("additional_minidumps" in reportData) {
+      let names = extraData.additional_minidumps.split(',');
+      for (let name in names) {
+        let [dump, extra] = getPendingMiniDump(this.id + "-" + name);
+        if (!dump.exists()) {
+          this.notifyStatus(FAILED);
+          this.cleanup();
+          return false;
+        }
+        additionalDumps.push({'name': name, 'dump': dump});
+      }
+    }
+
     this.notifyStatus(SUBMITTING);
 
     this.dump = dump;
     this.extra = extra;
+    this.additionalDumps = additionalDumps;
 
     if (!this.submitForm()) {
        this.notifyStatus(FAILED);

toolkit/crashreporter/KeyValueParser.jsm

@@ -0,0 +1,49 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+Components.utils.import("resource://gre/modules/Services.jsm");
+
+let EXPORTED_SYMBOLS = [
+  "parseKeyValuePairs",
+  "parseKeyValuePairsFromFile"
+];
+
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+
+function parseKeyValuePairs(text) {
+  let lines = text.split('\n');
+  let data = {};
+  for (let i = 0; i < lines.length; i++) {
+    if (lines[i] == '')
+      continue;
+
+    // can't just .split() because the value might contain = characters
+    let eq = lines[i].indexOf('=');
+    if (eq != -1) {
+      let [key, value] = [lines[i].substring(0, eq),
+                          lines[i].substring(eq + 1)];
+      if (key && value)
+        data[key] = value.replace(/\\n/g, "\n").replace(/\\\\/g, "\\");
+    }
+  }
+  return data;
+}
+
+function parseKeyValuePairsFromFile(file) {
+  let fstream = Cc["@mozilla.org/network/file-input-stream;1"].
+                createInstance(Ci.nsIFileInputStream);
+  fstream.init(file, -1, 0, 0);
+  let is = Cc["@mozilla.org/intl/converter-input-stream;1"].
+           createInstance(Ci.nsIConverterInputStream);
+  is.init(fstream, "UTF-8", 1024, Ci.nsIConverterInputStream.DEFAULT_REPLACEMENT_CHARACTER);
+  let str = {};
+  let contents = '';
+  while (is.readString(4096, str) != 0) {
+    contents += str.value;
+  }
+  is.close();
+  fstream.close();
+  return parseKeyValuePairs(contents);
+}

toolkit/crashreporter/Makefile.in

@@ -98,6 +98,7 @@ FORCE_STATIC_LIB = 1
 
 EXTRA_JS_MODULES = \
   CrashSubmit.jsm \
+  KeyValueParser.jsm \
   $(NULL)
 
 ifdef ENABLE_TESTS

toolkit/crashreporter/nsExceptionHandler.cpp

@@ -2046,8 +2046,15 @@ MoveToPending(nsIFile* dumpFile, nsIFile* extraFile)
   if (!GetPendingDir(getter_AddRefs(pendingDir)))
     return false;
 
-  return NS_SUCCEEDED(dumpFile->MoveTo(pendingDir, EmptyString())) &&
+  if (NS_FAILED(dumpFile->MoveTo(pendingDir, EmptyString()))) {
-    NS_SUCCEEDED(extraFile->MoveTo(pendingDir, EmptyString()));
+    return false;
+  }
+
+  if (extraFile && NS_FAILED(extraFile->MoveTo(pendingDir, EmptyString()))) {
+    return false;
+  }
+
+  return true;
 }
 
 static void
@@ -2513,11 +2520,24 @@ TakeMinidumpForChild(uint32_t childPid, nsIFile** dump, uint32_t* aSequence)
 //-----------------------------------------------------------------------------
 // CreatePairedMinidumps() and helpers
 //
-struct PairedDumpContext {
+
-  nsCOMPtr<nsIFile>* minidump;
+void
-  nsCOMPtr<nsIFile>* extra;
+RenameAdditionalHangMinidump(nsIFile* minidump, nsIFile* childMinidump,
-  const Blacklist& blacklist;
+                           const nsACString& name)
-};
+{
+  nsCOMPtr<nsIFile> directory;
+  childMinidump->GetParent(getter_AddRefs(directory));
+  if (!directory)
+    return;
+
+  nsAutoCString leafName;
+  childMinidump->GetNativeLeafName(leafName);
+
+  // turn "<id>.dmp" into "<id>-<name>.dmp
+  leafName.Insert(NS_LITERAL_CSTRING("-") + name, leafName.Length() - 4);
+
+  minidump->MoveToNative(directory, leafName);
+}
 
 static bool
 PairedDumpCallback(const XP_CHAR* dump_path,
@@ -2529,10 +2549,7 @@ PairedDumpCallback(const XP_CHAR* dump_path,
 #endif
                    bool succeeded)
 {
-  PairedDumpContext* ctx = static_cast<PairedDumpContext*>(context);
+  nsCOMPtr<nsIFile>& minidump = *static_cast< nsCOMPtr<nsIFile>* >(context);
-  nsCOMPtr<nsIFile>& minidump = *ctx->minidump;
-  nsCOMPtr<nsIFile>& extra = *ctx->extra;
-  const Blacklist& blacklist = ctx->blacklist;
 
   xpstring dump(dump_path);
   dump += XP_PATH_SEPARATOR;
@@ -2540,7 +2557,29 @@ PairedDumpCallback(const XP_CHAR* dump_path,
   dump += dumpFileExtension;
 
   CreateFileFromPath(dump, getter_AddRefs(minidump));
-  return WriteExtraForMinidump(minidump, blacklist, getter_AddRefs(extra));
+  return true;
+}
+
+static bool
+PairedDumpCallbackExtra(const XP_CHAR* dump_path,
+                        const XP_CHAR* minidump_id,
+                        void* context,
+#ifdef XP_WIN32
+                        EXCEPTION_POINTERS* /*unused*/,
+                        MDRawAssertionInfo* /*unused*/,
+#endif
+                        bool succeeded)
+{
+  PairedDumpCallback(dump_path, minidump_id, context,
+#ifdef XP_WIN32
+                     nullptr, nullptr,
+#endif
+                     succeeded);
+
+  nsCOMPtr<nsIFile>& minidump = *static_cast< nsCOMPtr<nsIFile>* >(context);
+
+  nsCOMPtr<nsIFile> extra;
+  return WriteExtraForMinidump(minidump, Blacklist(), getter_AddRefs(extra));
 }
 
 ThreadId
@@ -2571,31 +2610,11 @@ CurrentThreadId()
 bool
 CreatePairedMinidumps(ProcessHandle childPid,
                       ThreadId childBlamedThread,
-                      nsAString* pairGUID,
+                      nsIFile** childDump)
-                      nsIFile** childDump,
-                      nsIFile** parentDump)
 {
   if (!GetEnabled())
     return false;
 
-  // create the UUID for the hang dump as a pair
-  nsresult rv;
-  nsCOMPtr<nsIUUIDGenerator> uuidgen =
-    do_GetService("@mozilla.org/uuid-generator;1", &rv);
-  NS_ENSURE_SUCCESS(rv, false);  
-
-  nsID id;
-  rv = uuidgen->GenerateUUIDInPlace(&id);
-  NS_ENSURE_SUCCESS(rv, false);
-  
-  char chars[NSID_LENGTH];
-  id.ToProvidedString(chars);
-  CopyASCIItoUTF16(chars, *pairGUID);
-
-  // trim off braces
-  pairGUID->Cut(0, 1);
-  pairGUID->Cut(pairGUID->Length()-1, 1);
-
 #ifdef XP_MACOSX
   mach_port_t childThread = MACH_PORT_NULL;
   thread_act_port_array_t   threads_for_task;
@@ -2611,43 +2630,41 @@ CreatePairedMinidumps(ProcessHandle childPid,
 
   // dump the child
   nsCOMPtr<nsIFile> childMinidump;
-  nsCOMPtr<nsIFile> childExtra;
-  Blacklist childBlacklist(kSubprocessBlacklist,
-                           ArrayLength(kSubprocessBlacklist));
-  PairedDumpContext childCtx =
-    { &childMinidump, &childExtra, childBlacklist };
   if (!google_breakpad::ExceptionHandler::WriteMinidumpForChild(
          childPid,
          childThread,
          gExceptionHandler->dump_path(),
-         PairedDumpCallback,
+         PairedDumpCallbackExtra,
-         &childCtx))
+         static_cast<void*>(&childMinidump)))
     return false;
 
+  nsCOMPtr<nsIFile> childExtra;
+  GetExtraFileForMinidump(childMinidump, getter_AddRefs(childExtra));
+
   // dump the parent
   nsCOMPtr<nsIFile> parentMinidump;
-  nsCOMPtr<nsIFile> parentExtra;
-  // nothing's blacklisted for this process
-  Blacklist parentBlacklist;
-  PairedDumpContext parentCtx =
-    { &parentMinidump, &parentExtra, parentBlacklist };
   if (!google_breakpad::ExceptionHandler::WriteMinidump(
          gExceptionHandler->dump_path(),
          true,                  // write exception stream
          PairedDumpCallback,
-         &parentCtx))
+         static_cast<void*>(&parentMinidump))) {
-    return false;
 
-  // success
+    childMinidump->Remove(false);
-  if (ShouldReport()) {
+    childExtra->Remove(false);
-    MoveToPending(childMinidump, childExtra);
+
-    MoveToPending(parentMinidump, parentExtra);
+    return false;
   }
 
-  *childDump = NULL;
+  // success
-  *parentDump = NULL;
+  RenameAdditionalHangMinidump(parentMinidump, childMinidump,
-  childMinidump.swap(*childDump);
+                               NS_LITERAL_CSTRING("browser"));
-  parentMinidump.swap(*parentDump);
+
+  if (ShouldReport()) {
+    MoveToPending(childMinidump, childExtra);
+    MoveToPending(parentMinidump, nullptr);
+  }
+
+  childMinidump.forget(childDump);
 
   return true;
 }

toolkit/crashreporter/nsExceptionHandler.h

@@ -60,6 +60,8 @@ bool GetExtraFileForID(const nsAString& id, nsIFile** extraFile);
 bool GetExtraFileForMinidump(nsIFile* minidump, nsIFile** extraFile);
 bool AppendExtraData(const nsAString& id, const AnnotationTable& data);
 bool AppendExtraData(nsIFile* extraFile, const AnnotationTable& data);
+void RenameAdditionalHangMinidump(nsIFile* minidump, nsIFile* childMinidump,
+                                  const nsACString& name);
 
 #ifdef XP_WIN32
   nsresult WriteMinidumpForException(EXCEPTION_POINTERS* aExceptionInfo);
@@ -103,18 +105,19 @@ typedef int ThreadId;
 // hoops for us.
 ThreadId CurrentThreadId();
 
-// Create new minidumps that are snapshots of the state of this parent
+// Create a hang report with two minidumps that are snapshots of the state
-// process and |childPid|.  Return true on success along with the
+// of this parent process and |childPid|. The "main" minidump will be the
-// minidumps and a new UUID that can be used to correlate the dumps.
+// child process, and this parent process will have the _browser extension.
 //
-// If this function fails, it's the caller's responsibility to clean
+// Returns true on success. If this function fails, it will attempt to delete
-// up |childDump| and |parentDump|.  Either or both can be created and
+// any files that were created.
-// returned non-null on failure.
+//
+// The .extra information created will not include an additional_minidumps
+// annotation: the caller should annotate additional_minidumps with
+// at least "browser" and perhaps other minidumps attached to this report.
 bool CreatePairedMinidumps(ProcessHandle childPid,
                            ThreadId childBlamedThread,
-                           nsAString* pairGUID,
+                           nsIFile** childDump);
-                           nsIFile** childDump,
-                           nsIFile** parentDump);
 
 #  if defined(XP_WIN32) || defined(XP_MACOSX)
 // Parent-side API for children

toolkit/crashreporter/test/unit/head_crashreporter.js

@@ -144,42 +144,6 @@ function do_content_crash(setup, callback)
   );
 }
 
-// Utility functions for parsing .extra files
-function parseKeyValuePairs(text) {
-  var lines = text.split('\n');
-  var data = {};
-  for (let i = 0; i < lines.length; i++) {
-    if (lines[i] == '')
-      continue;
-
-    // can't just .split() because the value might contain = characters
-    let eq = lines[i].indexOf('=');
-    if (eq != -1) {
-      let [key, value] = [lines[i].substring(0, eq),
-                          lines[i].substring(eq + 1)];
-      if (key && value)
-        data[key] = value.replace("\\n", "\n", "g").replace("\\\\", "\\", "g");
-    }
-  }
-  return data;
-}
-
-function parseKeyValuePairsFromFile(file) {
-  var fstream = Components.classes["@mozilla.org/network/file-input-stream;1"].
-                createInstance(Components.interfaces.nsIFileInputStream);
-  fstream.init(file, -1, 0, 0);
-  var is = Components.classes["@mozilla.org/intl/converter-input-stream;1"].
-           createInstance(Components.interfaces.nsIConverterInputStream);
-  is.init(fstream, "UTF-8", 1024, Components.interfaces.nsIConverterInputStream.DEFAULT_REPLACEMENT_CHARACTER);
-  var str = {};
-  var contents = '';
-  while (is.readString(4096, str) != 0) {
-    contents += str.value;
-  }
-  is.close();
-  fstream.close();
-  return parseKeyValuePairs(contents);
-}
-
 // Import binary APIs via js-ctypes.
 Components.utils.import("resource://test/CrashTestUtils.jsm");
+Components.utils.import("resource://gre/modules/KeyValueParser.jsm");