Change when we enable Apple's Crash reporter.

Enable Apple Crash Reporter for Browser process in Non-Debug builds.

* Introduce mac_util::IsBackgroundProcess().
* Invert logic around Breakpad Enabling/Disabling to make it clearer.
* Honor the kDisableBreakpad switch.

Our new policy:
* Breakpad is enabled/disabled based on the user stats reporting opt-in flag - this hasn't changed.
* Always disable Apple Crash in background processes (plug-in, renderer, etc).
* If Breakpad is enabled browser crashes will be passed to Crash Reporter.
* If Breakpad is NOT enabled browser [or any other foreground app] crashes will be passed to Crash Reporter in Release builds.

Effectively this means that we now enable Crash Reporter for the browser process in Chromium release builds so these will no longer crash silently with no trace.

Review URL: http://codereview.chromium.org/180048

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@24959 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 50 insertions, 15 deletions

diff --git a/base/mac_util.h b/base/mac_util.h
index 220d68e..96d77b2 100644
--- a/base/mac_util.h
+++ b/base/mac_util.h
@@ -24,6 +24,9 @@ bool FSRefFromPath(const std::string& path, FSRef* ref);
 // Returns true if the application is running from a bundle
 bool AmIBundled();
 
+// Returns true if this process is marked as a "Background only process".
+bool IsBackgroundOnlyProcess();
+
 // Returns the main bundle or the override, used for code that needs
 // to fetch resources from bundles, but work within a unittest where we
 // aren't a bundle.
diff --git a/base/mac_util.mm b/base/mac_util.mm
index 6711257..b2e9b6e 100644
--- a/base/mac_util.mm
+++ b/base/mac_util.mm
@@ -43,6 +43,13 @@ bool AmIBundled() {
   return info.nodeFlags & kFSNodeIsDirectoryMask;
 }
 
+bool IsBackgroundOnlyProcess() {
+  NSBundle* main_bundle = MainAppBundle();
+  NSDictionary* info_dictionary = [main_bundle infoDictionary];
+  return [[info_dictionary objectForKey:@"LSUIElement"]
+                                   isEqualToString:@"1"] ? true : false;
+}
+
 // No threading worries since NSBundle isn't thread safe.
 static NSBundle* g_override_app_bundle = nil;
 
diff --git a/chrome/app/breakpad_mac.h b/chrome/app/breakpad_mac.h
index ce6f25f..d80f580 100644
--- a/chrome/app/breakpad_mac.h
+++ b/chrome/app/breakpad_mac.h
@@ -15,8 +15,8 @@ void InitCrashReporter();
 // CommandLine::Init has been called.
 void InitCrashProcessInfo();
 
-// Is Breakpad disabled?
-bool IsCrashReporterDisabled();
+// Is Breakpad enabled?
+bool IsCrashReporterEnabled();
 
 // Call on clean process shutdown.
 void DestructCrashReporter();
diff --git a/chrome/app/breakpad_mac.mm b/chrome/app/breakpad_mac.mm
index 6acb3f4..d295d43 100644
--- a/chrome/app/breakpad_mac.mm
+++ b/chrome/app/breakpad_mac.mm
@@ -24,8 +24,8 @@ BreakpadRef gBreakpadRef = NULL;
 
 }  // namespace
 
-bool IsCrashReporterDisabled() {
-  return gBreakpadRef == NULL;
+bool IsCrashReporterEnabled() {
+  return gBreakpadRef != NULL;
 }
 
 void DestructCrashReporter() {
@@ -47,13 +47,16 @@ void InitCrashReporter() {
   // command line.
   NSBundle* main_bundle = mac_util::MainAppBundle();
   NSDictionary* info_dictionary = [main_bundle infoDictionary];
-  bool is_browser = [[info_dictionary objectForKey:@"LSUIElement"]
-                                   isEqualToString:@"1"] ? false : true;
+  bool is_browser = !mac_util::IsBackgroundOnlyProcess();
   bool enable_breakpad =
       is_browser ? GoogleUpdateSettings::GetCollectStatsConsent() :
                    CommandLine::ForCurrentProcess()->
                        HasSwitch(switches::kEnableCrashReporter);
 
+  if (parsed_command_line.HasSwitch(switches::kDisableBreakpad)) {
+    enable_breakpad = false;
+  }
+
   if (!enable_breakpad) {
     LOG(WARNING) << "Breakpad disabled";
     return;
diff --git a/chrome/app/breakpad_mac_stubs.mm b/chrome/app/breakpad_mac_stubs.mm
index 258e02a..d1f4cac 100644
--- a/chrome/app/breakpad_mac_stubs.mm
+++ b/chrome/app/breakpad_mac_stubs.mm
@@ -7,8 +7,8 @@
 // Stubbed out versions of breakpad integration functions so we can compile
 // without linking in Breakpad.
 
-bool IsCrashReporterDisabled() {
-  return true;
+bool IsCrashReporterEnabled() {
+  return false;
 }
 
 void InitCrashProcessInfo() {
diff --git a/chrome/app/chrome_dll_main.cc b/chrome/app/chrome_dll_main.cc
index 7562c1f..5f7e37f 100644
--- a/chrome/app/chrome_dll_main.cc
+++ b/chrome/app/chrome_dll_main.cc
@@ -433,12 +433,34 @@ int ChromeMain(int argc, const char** argv) {
   // dylib is even loaded, to catch potential early crashes.
   InitCrashReporter();
 
-  // If Breakpad is not present, turn off OS crash dumps to avoid having
-  // to wait eons for Apple's Crash Reporter to generate dumps for builds
-  // where debugging symbols are present.
-  if (IsCrashReporterDisabled())
-    DebugUtil::DisableOSCrashDumps();
-  else
+#if defined(NDEBUG)
+  bool is_debug_build = false;
+#else
+  bool is_debug_build = true;
+#endif
+
+  // Details on when we enable Apple's Crash reporter.
+  //
+  // Motivation:
+  //    In debug mode it takes Apple's crash reporter eons to generate a crash
+  // dump.
+  //
+  // What we do:
+  // * We only pass crashes for foreground processes to Apple's Crash reporter.
+  //    At the time of this writing, that means just the Browser process.
+  // * If Breakpad is enabled, it will pass browser crashes to Crash Reporter
+  //    itself.
+  // * If Breakpad is disabled, we only turn on Crash Reporter for the
+  //    Browser process in release mode.
+  if (!parsed_command_line.HasSwitch(switches::kDisableBreakpad)) {
+    bool disable_apple_crash_reporter = is_debug_build
+                                        || mac_util::IsBackgroundOnlyProcess();
+    if (!IsCrashReporterEnabled() && disable_apple_crash_reporter) {
+      DebugUtil::DisableOSCrashDumps();
+    }
+  }
+
+  if (IsCrashReporterEnabled())
     InitCrashProcessInfo();
 #endif  // OS_MACOSX
 
diff --git a/chrome/browser/first_run_mac.mm b/chrome/browser/first_run_mac.mm
index 23186b9..32ea144 100644
--- a/chrome/browser/first_run_mac.mm
+++ b/chrome/browser/first_run_mac.mm
@@ -154,7 +154,7 @@ bool FirstRunController::DoFirstRun(Profile* profile,
   // but display a confirmation dialog before sending a crash report so we
   // respect a user's privacy while still getting any crashes that might happen
   // before this point.  Then remove the need for that dialog here.
-  DCHECK(IsCrashReporterDisabled());
+  DCHECK(!IsCrashReporterEnabled());
 
   //------------------ Start Temporary Code ---------------------
   // Migrate old first run format.