diff --git a/dom/ipc/CrashReporterParent.cpp b/dom/ipc/CrashReporterParent.cpp index 0dc926f9a02..cb00667d5e9 100644 --- a/dom/ipc/CrashReporterParent.cpp +++ b/dom/ipc/CrashReporterParent.cpp @@ -39,14 +39,13 @@ CrashReporterParent::RecvAddLibraryMappings(const InfallibleTArray& map return true; } -bool -CrashReporterParent::RecvAnnotateCrashReport(const nsCString& key, - const nsCString& data) +void +CrashReporterParent::AnnotateCrashReport(const nsCString& key, + 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) diff --git a/dom/ipc/CrashReporterParent.h b/dom/ipc/CrashReporterParent.h index caf621e5933..bbbf1d5d8a1 100644 --- a/dom/ipc/CrashReporterParent.h +++ b/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& 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 childDump; - nsCOMPtr parentDump; if (CrashReporter::CreatePairedMinidumps(child, mMainThread, - &mHangID, - getter_AddRefs(childDump), - getter_AddRefs(parentDump)) && - CrashReporter::GetIDFromMinidump(childDump, mChildDumpID) && - CrashReporter::GetIDFromMinidump(parentDump, mParentDumpID)) { + getter_AddRefs(childDump)) && + CrashReporter::GetIDFromMinidump(childDump, mChildDumpID)) { return true; } return false; diff --git a/dom/plugins/ipc/PluginModuleParent.cpp b/dom/plugins/ipc/PluginModuleParent.cpp index 410b8c477ba..c4bc2ef9028 100755 --- a/dom/plugins/ipc/PluginModuleParent.cpp +++ b/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) { - notes.Put(CS("NumberOfProcessors"), - nsPrintfCString("%d", PR_GetNumberOfProcessors())); + if (mPluginCpuUsageOnHang.Length() > 0) { + notes.Put(CS("NumberOfProcessors"), + nsPrintfCString("%d", PR_GetNumberOfProcessors())); - nsCString cpuUsageStr; - cpuUsageStr.AppendFloat(std::ceil(mPluginCpuUsageOnHang[0] * 100) / 100); - notes.Put(CS("PluginCpuUsage"), cpuUsageStr); + nsCString cpuUsageStr; + cpuUsageStr.AppendFloat(std::ceil(mPluginCpuUsageOnHang[0] * 100) / 100); + notes.Put(CS("PluginCpuUsage"), cpuUsageStr); #ifdef MOZ_CRASHREPORTER_INJECTOR - for (uint32_t i=1; iAnnotateCrashReport(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)", - NS_ConvertUTF16toUTF8(mBrowserDumpID).get(), - NS_ConvertUTF16toUTF8(mPluginDumpID).get(), - NS_ConvertUTF16toUTF8(crashReporter->HangID()).get())); + ("generated paired browser/plugin minidumps: %s)", + NS_ConvertUTF16toUTF8(mPluginDumpID).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()) { - crashReporter->GenerateHangCrashReport(¬es); + + if (!mPluginDumpID.IsEmpty()) { + crashReporter->GenerateChildData(¬es); return; } diff --git a/dom/plugins/test/mochitest/hang_test.js b/dom/plugins/test/mochitest/hang_test.js index 47871b5bc72..8160009ec67 100644 --- a/dom/plugins/test/mochitest/hang_test.js +++ b/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 dump and extra files + + // check plugin 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) { diff --git a/dom/plugins/test/mochitest/test_crashing.html b/dom/plugins/test/mochitest/test_crashing.html index aa4706dc8ef..c9f5f04228d 100644 --- a/dom/plugins/test/mochitest/test_crashing.html +++ b/dom/plugins/test/mochitest/test_crashing.html @@ -6,14 +6,15 @@