Bug 495730 - xpcshell-tests: each test leaks 3 nsStringBuffer, after (harness) bug 483062 landing; (Bv2a) Fix and extend test_crashreporter.js; r=ted.mielczarek

testing/xpcshell/head.js

@@ -62,6 +62,8 @@ let (ios = Components.classes["@mozilla.org/network/io-service;1"]
 // We rely on the Python harness to set MOZ_CRASHREPORTER_NO_REPORT
 // and handle checking for minidumps.
 if ("@mozilla.org/toolkit/crash-reporter;1" in Components.classes) {
+  // Remember to update </toolkit/crashreporter/test/unit/test_crashreporter.js>
+  // too if you change this initial setting.
   let (crashReporter =
         Components.classes["@mozilla.org/toolkit/crash-reporter;1"]
         .getService(Components.interfaces.nsICrashReporter)) {

toolkit/crashreporter/test/unit/test_crashreporter.js

@@ -1,19 +1,75 @@
 function run_test()
 {
   if (!("@mozilla.org/toolkit/crash-reporter;1" in Components.classes)) {
-    do_check_true(true, "Can't test this in a non-libxul build");
+    dump("INFO | test_crashreporter.js | Can't test crashreporter in a non-libxul build.\n");
     return;
   }
 
+  dump("INFO | test_crashreporter.js | Get crashreporter service.\n");
   var cr = Components.classes["@mozilla.org/toolkit/crash-reporter;1"]
                      .getService(Components.interfaces.nsICrashReporter);
   do_check_neq(cr, null);
 
+  /**********
+   * Check behavior when disabled.
+   **********/
+
+  // Crash reporting is enabled by default in </testing/xpcshell/head.js>.
+  dump("INFO | test_crashreporter.js | Disable crashreporter.\n");
+  cr.enabled = false;
+  do_check_false(cr.enabled);
+
+  try {
+    let su = cr.serverURL;
+    do_throw("Getting serverURL when disabled should have thrown!");
+  }
+  catch (ex) {
+    do_check_eq(ex.result, Components.results.NS_ERROR_NOT_INITIALIZED);
+  }
+
+  try {
+    let mdp = cr.minidumpPath;
+    do_throw("Getting minidumpPath when disabled should have thrown!");
+  }
+  catch (ex) {
+    do_check_eq(ex.result, Components.results.NS_ERROR_NOT_INITIALIZED);
+  }
+
+  try {
+    cr.annotateCrashReport(null, null);
+    do_throw("Calling annotateCrashReport() when disabled should have thrown!");
+  }
+  catch (ex) {
+    do_check_eq(ex.result, Components.results.NS_ERROR_NOT_INITIALIZED);
+  }
+
+  try {
+    cr.appendAppNotesToCrashReport(null);
+    do_throw("Calling appendAppNotesToCrashReport() when disabled should have thrown!");
+  }
+  catch (ex) {
+    do_check_eq(ex.result, Components.results.NS_ERROR_NOT_INITIALIZED);
+  }
+
+  /**********
+   * Check behavior when enabled.
+   **********/
+
+  dump("INFO | test_crashreporter.js | Re-enable crashreporter (in default state).\n");
   // check that we can enable the crashreporter
   cr.enabled = true;
   do_check_true(cr.enabled);
   // ensure that double-enabling doesn't error
   cr.enabled = true;
+  do_check_true(cr.enabled);
+
+  try {
+    let su = cr.serverURL;
+    do_throw("Getting serverURL when not set should have thrown!");
+  }
+  catch (ex) {
+    do_check_eq(ex.result, Components.results.NS_ERROR_FAILURE);
+  }
 
   // check setting/getting serverURL
   var ios = Components.classes["@mozilla.org/network/io-service;1"]
@@ -22,35 +78,76 @@ function run_test()
   // try it with two different URLs, just for kicks
   var testspecs = ["http://example.com/submit",
                    "https://example.org/anothersubmit"];
-  for (var i=0; i<testspecs.length; i++) {
-    var u = ios.newURI(testspecs[i], null, null);
-    cr.serverURL = u;
+  for (var i = 0; i < testspecs.length; ++i) {
+    cr.serverURL = ios.newURI(testspecs[i], null, null);
     do_check_eq(cr.serverURL.spec, testspecs[i]);
   }
 
   // should not allow setting non-http/https URLs
   try {
-    u = ios.newURI("ftp://example.com/submit", null, null);
-    cr.serverURL = u;
-    do_throw("Setting serverURL to a non-http URL should have thrown!");
+    cr.serverURL = ios.newURI("ftp://example.com/submit", null, null);
+    do_throw("Setting serverURL to a non-http(s) URL should have thrown!");
   }
   catch (ex) {
     do_check_eq(ex.result, Components.results.NS_ERROR_INVALID_ARG);
   }
 
   // check getting/setting minidumpPath
-  // it should be $TEMP initially, but I'm not sure if we can exactly test that
+  // it should be $TEMP by default, but I'm not sure if we can exactly test that
   // this will at least test that it doesn't throw
   do_check_neq(cr.minidumpPath.path, "");
   var cwd = do_get_cwd();
   cr.minidumpPath = cwd;
   do_check_eq(cr.minidumpPath.path, cwd.path);
 
+  try {
+    cr.annotateCrashReport("=", "");
+    do_throw("Calling annotateCrashReport() with an '=' key should have thrown!");
+  }
+  catch (ex) {
+    do_check_eq(ex.result, Components.results.NS_ERROR_INVALID_ARG);
+  }
+  try {
+    cr.annotateCrashReport("\n", "");
+    do_throw("Calling annotateCrashReport() with a '\\n' key should have thrown!");
+  }
+  catch (ex) {
+    do_check_eq(ex.result, Components.results.NS_ERROR_INVALID_ARG);
+  }
+  try {
+    cr.annotateCrashReport("", "\0");
+    do_throw("Calling annotateCrashReport() with a '\\0' data should have thrown!");
+  }
+  catch (ex) {
+    do_check_eq(ex.result, Components.results.NS_ERROR_INVALID_ARG);
+  }
+  cr.annotateCrashReport("testKey", "testData");
+
+  try {
+    cr.appendAppNotesToCrashReport("\0");
+    do_throw("Calling appendAppNotesToCrashReport() with a '\\0' data should have thrown!");
+  }
+  catch (ex) {
+    do_check_eq(ex.result, Components.results.NS_ERROR_INVALID_ARG);
+  }
+  cr.appendAppNotesToCrashReport("additional testData");
+
   // check that we can disable the crashreporter
   cr.enabled = false;
   do_check_false(cr.enabled);
   // ensure that double-disabling doesn't error
   cr.enabled = false;
+  do_check_false(cr.enabled);
+
+  /**********
+   * Reset to initial state.
+   **********/
+
   // leave it enabled at the end in case of shutdown crashes
+  dump("INFO | test_crashreporter.js | Reset crashreporter to its initial state.\n");
+  // (Values as initially set by </testing/xpcshell/head.js>.)
   cr.enabled = true;
+  do_check_true(cr.enabled);
+  cr.minidumpPath = cwd;
+  do_check_eq(cr.minidumpPath.path, cwd.path);
 }

xpcom/system/nsICrashReporter.idl

@@ -42,6 +42,7 @@ interface nsIURL;
 
 /**
  * Provides access to crash reporting functionality.
+ *
  * @status UNSTABLE - This interface is not frozen and will probably change in
  *                    future releases.
  */
@@ -75,6 +76,7 @@ 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
@@ -95,7 +97,7 @@ interface nsICrashReporter : nsISupports
    *        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.
+   * @throw NS_ERROR_INVALID_ARG if data contains invalid characters.
    *                             The only invalid character is '\0'.
    */
   void appendAppNotesToCrashReport(in ACString data);
@@ -104,12 +106,14 @@ interface nsICrashReporter : nsISupports
    * 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);
   
   /**
    * Append note containing an Obj-C exception's info.
+   *
    * @param aException  NSException object to append note for
    */
   [noscript] void appendObjCExceptionInfoToAppNotes(in voidPtr aException);