bug 444103 - add ability to add string-formatted notes to crash report. r=bsmedberg
authorTed Mielczarek <ted.mielczarek@gmail.com>
Sun, 27 Jul 2008 13:08:03 -0400
changeset 16248 61c0a4ca72e4fb6de76fee7ce4521a5b721e2cdd
parent 16247 409570e09968032028bad082dfb7ae5d62fbe6e5
child 16249 a953aef66b5bfffb920874e25830d985ad8d1cc6
push idunknown
push userunknown
push dateunknown
reviewersbsmedberg
bugs444103
milestone1.9.1a2pre
bug 444103 - add ability to add string-formatted notes to crash report. r=bsmedberg
toolkit/crashreporter/nsExceptionHandler.cpp
toolkit/crashreporter/nsExceptionHandler.h
toolkit/crashreporter/test/TestCrashReporterAPI.cpp
toolkit/xre/nsAppRunner.cpp
xpcom/system/nsICrashReporter.idl
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@@ -131,16 +131,17 @@ static const int kCrashTimeParameterLen 
 
 static const char kTimeSinceLastCrashParameter[] = "SecondsSinceLastCrash=";
 static const int kTimeSinceLastCrashParameterLen =
                                      sizeof(kTimeSinceLastCrashParameter)-1;
 
 // this holds additional data sent via the API
 static nsDataHashtable<nsCStringHashKey,nsCString>* crashReporterAPIData_Hash;
 static nsCString* crashReporterAPIData = nsnull;
+static nsCString* notesField = nsnull;
 
 static XP_CHAR*
 Concat(XP_CHAR* str, const XP_CHAR* toAppend, int* size)
 {
   int appendLen = XP_STRLEN(toAppend);
   if (appendLen >= *size) appendLen = *size - 1;
 
   memcpy(str, toAppend, appendLen * sizeof(XP_CHAR));
@@ -335,16 +336,19 @@ nsresult SetExceptionHandler(nsILocalFil
 
   crashReporterAPIData_Hash =
     new nsDataHashtable<nsCStringHashKey,nsCString>();
   NS_ENSURE_TRUE(crashReporterAPIData_Hash, NS_ERROR_OUT_OF_MEMORY);
 
   rv = crashReporterAPIData_Hash->Init();
   NS_ENSURE_SUCCESS(rv, rv);
 
+  notesField = new nsCString();
+  NS_ENSURE_TRUE(notesField, NS_ERROR_OUT_OF_MEMORY);
+
   // locate crashreporter executable
   nsCOMPtr<nsIFile> exePath;
   rv = aXREDirectory->Clone(getter_AddRefs(exePath));
   NS_ENSURE_SUCCESS(rv, rv);
 
 #if defined(XP_MACOSX)
   exePath->Append(NS_LITERAL_STRING("crashreporter.app"));
   exePath->Append(NS_LITERAL_STRING("Contents"));
@@ -691,16 +695,21 @@ nsresult UnsetExceptionHandler()
     crashReporterAPIData_Hash = nsnull;
   }
 
   if (crashReporterAPIData) {
     delete crashReporterAPIData;
     crashReporterAPIData = nsnull;
   }
 
+  if (notesField) {
+    delete notesField;
+    notesField = nsnull;
+  }
+
   if (crashReporterPath) {
     NS_Free(crashReporterPath);
     crashReporterPath = nsnull;
   }
 
   if (!gExceptionHandler)
     return NS_ERROR_NOT_INITIALIZED;
 
@@ -741,17 +750,17 @@ static PLDHashOperator PR_CALLBACK Enume
                                                     nsCString entry,
                                                     void* userData)
 {
   crashReporterAPIData->Append(key + NS_LITERAL_CSTRING("=") + entry +
                                NS_LITERAL_CSTRING("\n"));
   return PL_DHASH_NEXT;
 }
 
-nsresult AnnotateCrashReport(const nsACString &key, const nsACString &data)
+nsresult AnnotateCrashReport(const nsACString& key, const nsACString& data)
 {
   if (!gExceptionHandler)
     return NS_ERROR_NOT_INITIALIZED;
 
   if (DoFindInReadable(key, NS_LITERAL_CSTRING("=")) ||
       DoFindInReadable(key, NS_LITERAL_CSTRING("\n")))
     return NS_ERROR_INVALID_ARG;
 
@@ -773,18 +782,44 @@ nsresult AnnotateCrashReport(const nsACS
   // now rebuild the file contents
   crashReporterAPIData->Truncate(0);
   crashReporterAPIData_Hash->EnumerateRead(EnumerateEntries,
                                            crashReporterAPIData);
 
   return NS_OK;
 }
 
+nsresult AppendAppNotesToCrashReport(const nsACString& data)
+{
+  if (!gExceptionHandler)
+    return NS_ERROR_NOT_INITIALIZED;
+
+  if (DoFindInReadable(data, NS_LITERAL_CSTRING("\0")))
+    return NS_ERROR_INVALID_ARG;
+
+  notesField->Append(data);
+  return AnnotateCrashReport(NS_LITERAL_CSTRING("Notes"), *notesField);
+}
+
+// Returns true if found, false if not found.
+bool GetAnnotation(const nsACString& key, nsACString& data)
+{
+  if (!gExceptionHandler)
+    return NS_ERROR_NOT_INITIALIZED;
+
+  nsCAutoString entry;
+  if (!crashReporterAPIData_Hash->Get(key, &entry))
+    return false;
+
+  data = entry;
+  return true;
+}
+
 nsresult
-SetRestartArgs(int argc, char **argv)
+SetRestartArgs(int argc, char** argv)
 {
   if (!gExceptionHandler)
     return NS_OK;
 
   int i;
   nsCAutoString envVar;
   char *env;
   for (i = 0; i < argc; i++) {
--- a/toolkit/crashreporter/nsExceptionHandler.h
+++ b/toolkit/crashreporter/nsExceptionHandler.h
@@ -49,18 +49,19 @@
 #include <windows.h>
 #endif
 
 namespace CrashReporter {
 nsresult SetExceptionHandler(nsILocalFile* aXREDirectory,
                              const char* aServerURL);
 nsresult SetMinidumpPath(const nsAString& aPath);
 nsresult UnsetExceptionHandler();
-nsresult AnnotateCrashReport(const nsACString &key, const nsACString &data);
-nsresult SetRestartArgs(int argc, char **argv);
+nsresult AnnotateCrashReport(const nsACString& key, const nsACString& data);
+nsresult AppendAppNotesToCrashReport(const nsACString& data);
+nsresult SetRestartArgs(int argc, char** argv);
 nsresult SetupExtraData(nsILocalFile* aAppDataDirectory,
                         const nsACString& aBuildID);
 #ifdef XP_WIN32
   nsresult WriteMinidumpForException(EXCEPTION_POINTERS* aExceptionInfo);
 #endif
 }
 
 #endif /* nsExceptionHandler_h__ */
--- a/toolkit/crashreporter/test/TestCrashReporterAPI.cpp
+++ b/toolkit/crashreporter/test/TestCrashReporterAPI.cpp
@@ -44,123 +44,178 @@
 #include "nsIServiceManager.h"
 #include "nsServiceManagerUtils.h"
 #include "nsDirectoryServiceDefs.h"
 #include "nsIProperties.h"
 #include "nsCOMPtr.h"
 #include "nsILocalFile.h"
 
 #include "nsExceptionHandler.h"
-#include "nsICrashReporter.h"
+
+// Defined in nsExceptionHandler.cpp, but not normally exposed
+namespace CrashReporter {
+  bool GetAnnotation(const nsACString& key, nsACString& data);
+};
 
-#define mu_assert(message, test) do { if (NS_FAILED(test)) \
-                                       return message; } while (0)
-#define mu_assert_failure(message, test) do { if (NS_SUCCEEDED(test)) \
-                                               return message; } while (0)
-#define mu_run_test(test) do { char *message = test(); tests_run++; \
-                                if (message) return message; } while (0)
+#define ok(message, test) do {                   \
+                               if (!(test))      \
+                                 return message; \
+                             } while (0)
+#define equals(message, a, b) ok(message, a == b)
+#define ok_nsresult(message, rv) ok(message, NS_SUCCEEDED(rv))
+#define fail_nsresult(message, rv) ok(message, NS_FAILED(rv))
+#define run_test(test) do { char *message = test(); tests_run++; \
+                            if (message) return message; } while (0)
 int tests_run;
 
+
+
 char *
 test_init_exception_handler()
 {
   nsCOMPtr<nsILocalFile> lf;
   // we don't plan on launching the crash reporter in this app anyway,
   // so it's ok to pass a bogus nsILocalFile
-  mu_assert("NS_NewNativeLocalFile", NS_NewNativeLocalFile(EmptyCString(),
-                                                           PR_TRUE,
-                                                           getter_AddRefs(lf)));
+  ok_nsresult("NS_NewNativeLocalFile", NS_NewNativeLocalFile(EmptyCString(),
+                                                             PR_TRUE,
+                                                             getter_AddRefs(lf)));
 
-  mu_assert("CrashReporter::SetExceptionHandler",
+  ok_nsresult("CrashReporter::SetExceptionHandler",
             CrashReporter::SetExceptionHandler(lf, nsnull));
   return 0;
 }
 
 char *
 test_set_minidump_path()
 {
   nsresult rv;
   nsCOMPtr<nsIProperties> directoryService = 
     do_GetService(NS_DIRECTORY_SERVICE_CONTRACTID, &rv);
 
-  mu_assert("do_GetService", rv);
+  ok_nsresult("do_GetService", rv);
 
   nsCOMPtr<nsILocalFile> currentDirectory;
   rv = directoryService->Get(NS_XPCOM_CURRENT_PROCESS_DIR,
                              NS_GET_IID(nsILocalFile),
                              getter_AddRefs(currentDirectory));
-  mu_assert("directoryService->Get", rv);
+  ok_nsresult("directoryService->Get", rv);
 
   nsAutoString currentDirectoryPath;
   rv = currentDirectory->GetPath(currentDirectoryPath);
-  mu_assert("currentDirectory->GetPath", rv);
+  ok_nsresult("currentDirectory->GetPath", rv);
   
-  mu_assert("CrashReporter::SetMinidumpPath",
+  ok_nsresult("CrashReporter::SetMinidumpPath",
             CrashReporter::SetMinidumpPath(currentDirectoryPath));
 
   return 0;
 }
 
 char *
 test_annotate_crash_report_basic()
 {
-  mu_assert("CrashReporter::AnnotateCrashReport: basic",
+  ok_nsresult("CrashReporter::AnnotateCrashReport: basic 1",
      CrashReporter::AnnotateCrashReport(NS_LITERAL_CSTRING("test"),
                                         NS_LITERAL_CSTRING("some data")));
 
+
+  nsCAutoString result;
+  ok("CrashReporter::GetAnnotation", CrashReporter::GetAnnotation(NS_LITERAL_CSTRING("test"),
+                                                                  result));
+  nsCString msg = result + NS_LITERAL_CSTRING(" == ") +
+    NS_LITERAL_CSTRING("some data");
+  equals((char*)PromiseFlatCString(msg).get(), result,
+         NS_LITERAL_CSTRING("some data"));
+
+  // now replace it with something else
+  ok_nsresult("CrashReporter::AnnotateCrashReport: basic 2",
+     CrashReporter::AnnotateCrashReport(NS_LITERAL_CSTRING("test"),
+                                        NS_LITERAL_CSTRING("some other data")));
+
+
+  ok("CrashReporter::GetAnnotation", CrashReporter::GetAnnotation(NS_LITERAL_CSTRING("test"),
+                                                                  result));
+  msg = result + NS_LITERAL_CSTRING(" == ") +
+    NS_LITERAL_CSTRING("some other data");
+  equals((char*)PromiseFlatCString(msg).get(), result,
+         NS_LITERAL_CSTRING("some other data"));
+  return 0;
+}
+
+char *
+test_appendnotes_crash_report()
+{
+  // Append two notes
+  ok_nsresult("CrashReporter::AppendAppNotesToCrashReport: 1",
+              CrashReporter::AppendAppNotesToCrashReport(NS_LITERAL_CSTRING("some data")));
+
+  
+  ok_nsresult("CrashReporter::AppendAppNotesToCrashReport: 2",
+              CrashReporter::AppendAppNotesToCrashReport(NS_LITERAL_CSTRING("some other data")));
+
+  // ensure that the result is correct
+  nsCAutoString result;
+  ok("CrashReporter::GetAnnotation",
+     CrashReporter::GetAnnotation(NS_LITERAL_CSTRING("Notes"),
+                                  result));
+
+  nsCString msg = result + NS_LITERAL_CSTRING(" == ") +
+    NS_LITERAL_CSTRING("some datasome other data");
+  equals((char*)PromiseFlatCString(msg).get(), result,
+         NS_LITERAL_CSTRING("some datasome other data"));
   return 0;
 }
 
 char *
 test_annotate_crash_report_invalid_equals()
 {
-  mu_assert_failure("CrashReporter::AnnotateCrashReport: invalid = in key",
+  fail_nsresult("CrashReporter::AnnotateCrashReport: invalid = in key",
      CrashReporter::AnnotateCrashReport(NS_LITERAL_CSTRING("test=something"),
                                         NS_LITERAL_CSTRING("some data")));
   return 0;
 }
 
 char *
 test_annotate_crash_report_invalid_cr()
 {
-  mu_assert_failure("CrashReporter::AnnotateCrashReport: invalid \n in key",
+  fail_nsresult("CrashReporter::AnnotateCrashReport: invalid \n in key",
      CrashReporter::AnnotateCrashReport(NS_LITERAL_CSTRING("test\nsomething"),
                                         NS_LITERAL_CSTRING("some data")));
   return 0;
 }
 
 char *
 test_unset_exception_handler()
 {
-  mu_assert("CrashReporter::UnsetExceptionHandler",
+  ok_nsresult("CrashReporter::UnsetExceptionHandler",
             CrashReporter::UnsetExceptionHandler());
   return 0;
 }
 
 static char* all_tests()
 {
-  mu_run_test(test_init_exception_handler);
-  mu_run_test(test_set_minidump_path);
-  mu_run_test(test_annotate_crash_report_basic);
-  mu_run_test(test_annotate_crash_report_invalid_equals);
-  mu_run_test(test_annotate_crash_report_invalid_cr);
-  mu_run_test(test_unset_exception_handler);
+  run_test(test_init_exception_handler);
+  run_test(test_set_minidump_path);
+  run_test(test_annotate_crash_report_basic);
+  run_test(test_annotate_crash_report_invalid_equals);
+  run_test(test_annotate_crash_report_invalid_cr);
+  run_test(test_appendnotes_crash_report);
+  run_test(test_unset_exception_handler);
   return 0;
 }
 
 int
 main (int argc, char **argv)
 {
   NS_InitXPCOM2(nsnull, nsnull, nsnull);
 
   PR_SetEnv("MOZ_CRASHREPORTER=1");
 
   char* result = all_tests();
   if (result != 0) {
-    printf("FAIL: %s\n", result);
+    printf("TEST-UNEXPECTED-FAIL | %s | %s\n", __FILE__, result);
   }
   else {
-    printf("ALL TESTS PASSED\n");
+    printf("TEST-PASS | %s | all tests passed\n", __FILE__);
   }
   printf("Tests run: %d\n", tests_run);
  
   return result != 0;
 }
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -832,16 +832,22 @@ nsXULAppInfo::GetUserCanElevate(PRBool *
 NS_IMETHODIMP
 nsXULAppInfo::AnnotateCrashReport(const nsACString& key,
                                   const nsACString& data)
 {
   return CrashReporter::AnnotateCrashReport(key, data);
 }
 
 NS_IMETHODIMP
+nsXULAppInfo::AppendAppNotesToCrashReport(const nsACString& data)
+{
+  return CrashReporter::AppendAppNotesToCrashReport(data);
+}
+
+NS_IMETHODIMP
 nsXULAppInfo::WriteMinidumpForException(void* aExceptionInfo)
 {
 #ifdef XP_WIN32
   return CrashReporter::WriteMinidumpForException(static_cast<EXCEPTION_POINTERS*>(aExceptionInfo));
 #else
   return NS_ERROR_NOT_IMPLEMENTED;
 #endif
 }
--- a/xpcom/system/nsICrashReporter.idl
+++ b/xpcom/system/nsICrashReporter.idl
@@ -38,32 +38,45 @@
 #include "nsISupports.idl"
 
 /**
  * Provides access to crash reporting functionality.
  * @status UNSTABLE - This interface is not frozen and will probably change in
  *                    future releases.
  */
 
-[scriptable, uuid(4d6449fe-d642-45e5-9773-41af5793c99a)]
+[scriptable, uuid(189c9392-157c-445f-84db-900eb46d4839)]
 interface nsICrashReporter : nsISupports
 {
   /**
    * Add some extra data to be submitted with a crash report.
    * @param key
    *        Name of the data to be added.
    * @param data
    *        Data to be added.
    *
    * @throw NS_ERROR_NOT_INITIALIZED if crash reporting not initialized
    * @throw NS_ERROR_INVALID_ARG if key or data contain invalid characters.
    *                             Invalid characters for key are '=' and
    *                             '\n'.  Invalid character for data is '\0'.
    */
   void annotateCrashReport(in ACString key, in ACString data);
+
+  /**
+   * Append some data to the "Notes" field, to be submitted with a crash report.
+   * Unlike annotateCrashReport, this method will append to existing data.
+   *
+   * @param data
+   *        Data to be added.
+   *
+   * @throw NS_ERROR_NOT_INITIALIZED if crash reporting not initialized
+   * @throw NS_ERROR_INVALID_ARG if or data contains invalid characters.
+   *                             The only invalid character is '\0'.
+   */
+  void appendAppNotesToCrashReport(in ACString data);
   
   /**
     * Write a minidump immediately, with the user-supplied exception
     * information. This is implemented on Windows only, because
     * SEH (structured exception handling) exists on Windows only.
     * @param aExceptionInfo  EXCEPTION_INFO* provided by Window's SEH
     */
   [noscript] void WriteMinidumpForException(in voidPtr aExceptionInfo);