bug 842728 - init Breakpad from C++ in xpcshell, remove ability to init from script. r=bholley,gps

js/xpconnect/shell/xpcshell.cpp

@@ -1601,6 +1601,13 @@ main(int argc, char **argv, char **envp)
         argv += 2;
     }
 
+#ifdef MOZ_CRASHREPORTER
+    // This is needed during startup and also shutdown, so keep it out
+    // of the nested scope.
+    // Special exception: will remain usable after NS_ShutdownXPCOM
+    nsCOMPtr<nsICrashReporter> crashReporter;
+#endif
+
     {
         if (argc > 1 && !strcmp(argv[1], "--greomni")) {
             nsCOMPtr<nsIFile> greOmni;
@@ -1626,6 +1633,14 @@ main(int argc, char **argv, char **envp)
             return 1;
         }
 
+#ifdef MOZ_CRASHREPORTER
+        const char *val = getenv("MOZ_CRASHREPORTER");
+        crashReporter = do_GetService("@mozilla.org/toolkit/crash-reporter;1");
+        if (val && *val) {
+            crashReporter->SetEnabled(true);
+        }
+#endif
+
         nsCOMPtr<nsIJSRuntimeService> rtsvc = do_GetService("@mozilla.org/js/xpc/RuntimeService;1");
         // get the JSRuntime from the runtime svc
         if (!rtsvc) {
@@ -1771,13 +1786,6 @@ main(int argc, char **argv, char **envp)
     if (!XRE_ShutdownTestShell())
         NS_ERROR("problem shutting down testshell");
 
-#ifdef MOZ_CRASHREPORTER
-    // Get the crashreporter service while XPCOM is still active.
-    // This is a special exception: it will remain usable after NS_ShutdownXPCOM().
-    nsCOMPtr<nsICrashReporter> crashReporter =
-        do_GetService("@mozilla.org/toolkit/crash-reporter;1");
-#endif
-
     // no nsCOMPtrs are allowed to be alive when you call NS_ShutdownXPCOM
     rv = NS_ShutdownXPCOM( NULL );
     MOZ_ASSERT(NS_SUCCEEDED(rv), "NS_ShutdownXPCOM failed");

testing/xpcshell/head.js

@@ -73,20 +73,17 @@ try {
 }
 catch (e) { }
 
-// Enable crash reporting, if possible
+// Configure crash reporting, if possible
-// We rely on the Python harness to set MOZ_CRASHREPORTER_NO_REPORT
+// We rely on the Python harness to set MOZ_CRASHREPORTER,
-// and handle checking for minidumps.
+// MOZ_CRASHREPORTER_NO_REPORT, and handle checking for minidumps.
 // Note that if we're in a child process, we don't want to init the
 // crashreporter component.
-try { // nsIXULRuntime is not available in some configurations.
+try {
   if (runningInParent &&
       "@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)) {
-      crashReporter.enabled = true;
       crashReporter.minidumpPath = do_get_tempdir();
     }
   }

testing/xpcshell/runxpcshelltests.py

@@ -683,6 +683,8 @@ class XPCShellTests(object):
         """
         # Make assertions fatal
         self.env["XPCOM_DEBUG_BREAK"] = "stack-and-abort"
+        # Enable crash reporting
+        self.env["MOZ_CRASHREPORTER"] = "1"
         # Don't launch the crash reporter client
         self.env["MOZ_CRASHREPORTER_NO_REPORT"] = "1"
         # Capturing backtraces is very slow on some platforms, and it's

toolkit/crashreporter/test/unit/crasher_subprocess_head.js

@@ -12,12 +12,10 @@ let crashReporter =
   Components.classes["@mozilla.org/toolkit/crash-reporter;1"]
     .getService(Components.interfaces.nsICrashReporter);
 
-// the crash reporter is already enabled in content processes,
+// Setting the minidump path is not allowed in content processes
-// and setting the minidump path is not allowed
 let processType = Components.classes["@mozilla.org/xre/runtime;1"].
       getService(Components.interfaces.nsIXULRuntime).processType;
 if (processType == Components.interfaces.nsIXULRuntime.PROCESS_TYPE_DEFAULT) {
-  crashReporter.enabled = true;
   crashReporter.minidumpPath = _tmpd;
 }
 

toolkit/crashreporter/test/unit/test_crashreporter.js

@@ -1,70 +1,10 @@
 function run_test()
 {
-  if (!("@mozilla.org/toolkit/crash-reporter;1" in Components.classes)) {
-    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.
-   * XXX: don't run these tests right now, there's a race condition in
-   * Breakpad with the handler thread. See:
-   * http://code.google.com/p/google-breakpad/issues/detail?id=334
-   **********
-
-  // 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 {
@@ -140,25 +80,6 @@ function run_test()
   // Add more data.
   cr.appendAppNotesToCrashReport("additional testData4");
 
-  /*
-   * These tests are also disabled, see comment above.
-  // 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

@@ -14,13 +14,20 @@ interface nsIURL;
  *                    future releases.
  */
 
-[scriptable, uuid(56761088-57ad-4f5c-bd61-f678c2807fe0)]
+[scriptable, uuid(4b525eab-d4b8-4f71-bf43-b698e306631b)]
 interface nsICrashReporter : nsISupports
 {
   /**
-   * Enable or disable the crashreporter at runtime.
+   * Get the enabled status of the crash reporter.
    */
-  attribute boolean enabled;
+  readonly attribute boolean enabled;
+
+  /**
+   * Enable or disable crash reporting at runtime. Not available to script
+   * because the JS engine relies on proper exception handler chaining.
+   */
+  [noscript]
+  void setEnabled(in bool enabled);
 
   /**
    * Get or set the URL to which crash reports will be submitted.