Change breakpads on the helper processes to keep our rimZ clean.

Initialize crash reporting in helper processes such as the renderer process.
Renderer crash reporting stopped working in r23006 when multiple .app bundles
were introduced, because the stats collection and crash reporting preference
is presently accessed via NSUserDefaults, keyed on the bundle ID.  The main
browser process and helper processes have distinct bundle IDs.  In the new
scheme, only the main browser process consults this preference, and passes it
to helper processes in their command lines.

BUG=19204
TEST=When reporting is enabled, Breakpad should pick up browser and renderer
     process crashes;
     When reporting is enabled, renderer should not log messages like
         [mmdd/hhmmss:WARNING:/path/to/breakpad_mac.mm(47)] Breakpad disabled;
     When reporting is disabled, browser and renderer should both log these
         messages.
Review URL: http://codereview.chromium.org/165546

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

1 files changed, 46 insertions, 52 deletions

diff --git a/chrome/app/breakpad_mac.mm b/chrome/app/breakpad_mac.mm
index 3bad98d..8a36f16 100644
--- a/chrome/app/breakpad_mac.mm
+++ b/chrome/app/breakpad_mac.mm
@@ -14,15 +14,14 @@
 #include "base/sys_string_conversions.h"
 #import "breakpad/src/client/mac/Framework/Breakpad.h"
 #include "chrome/common/child_process_logging.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/installer/util/google_update_settings.h"
 
-extern "C" {
-
 namespace {
 
 BreakpadRef gBreakpadRef = NULL;
 
-} // namespace
+}  // namespace
 
 bool IsCrashReporterDisabled() {
   return gBreakpadRef == NULL;
@@ -40,66 +39,65 @@ void InitCrashReporter() {
   DCHECK(gBreakpadRef == NULL);
   base::ScopedNSAutoreleasePool autorelease_pool;
 
-  // Check for Send stats preference. If preference is not specifically turned
-  // on then disable crash reporting.
-  bool user_consented = GoogleUpdateSettings::GetCollectStatsConsent();
-  if (!user_consented) {
+  // Check whether the user has consented to stats and crash reporting.  The
+  // browser process can make this determination directly.  Helper processes
+  // may not have access to the disk or to the same data as the browser
+  // process, so the browser passes the consent preference to them on the
+  // command line.
+  NSBundle* main_bundle = [NSBundle mainBundle];
+  NSDictionary* info_dictionary = [main_bundle infoDictionary];
+  bool is_browser = [[info_dictionary objectForKey:@"LSUIElement"]
+                                   isEqualToString:@"1"] ? true : false;
+  bool enable_breakpad =
+      is_browser ? GoogleUpdateSettings::GetCollectStatsConsent() :
+                   CommandLine::ForCurrentProcess()->
+                       HasSwitch(switches::kEnableCrashReporter);
+
+  if (!enable_breakpad) {
     LOG(WARNING) << "Breakpad disabled";
     return;
   }
 
-  NSBundle* main_bundle = [NSBundle mainBundle];
+  // Tell Breakpad where crash_inspector and crash_report_sender are.
   NSString* resource_path = [main_bundle resourcePath];
-
-  NSDictionary* info_dictionary = [main_bundle infoDictionary];
-  NSMutableDictionary *breakpad_config = [info_dictionary
-                                             mutableCopy];
-
-  // Tell Breakpad where inspector & crash_reporter are.
-  NSString *inspector_location = [resource_path
-      stringByAppendingPathComponent:@"crash_inspector"];
-  NSString *reporter_bundle_location = [resource_path
-      stringByAppendingPathComponent:@"crash_report_sender.app"];
-  NSString *reporter_location = [[NSBundle
-                                      bundleWithPath:reporter_bundle_location]
-                                     executablePath];
-
+  NSString *inspector_location =
+      [resource_path stringByAppendingPathComponent:@"crash_inspector"];
+  NSString *reporter_bundle_location =
+      [resource_path stringByAppendingPathComponent:@"crash_report_sender.app"];
+  NSString *reporter_location =
+      [[NSBundle bundleWithPath:reporter_bundle_location] executablePath];
+
+  NSMutableDictionary *breakpad_config =
+      [[info_dictionary mutableCopy] autorelease];
   [breakpad_config setObject:inspector_location
                       forKey:@BREAKPAD_INSPECTOR_LOCATION];
   [breakpad_config setObject:reporter_location
                       forKey:@BREAKPAD_REPORTER_EXE_LOCATION];
 
-  // Pass crash to Crash Reporter if we're a foreground application [the
-  // browser process].  This is so the user gets notification when Chrome
-  // crashes and also since we get "restart ui" for free.
-  BOOL is_background_app = [[info_dictionary objectForKey:@"LSUIElement"]
-                                isEqualToString:@"1"];
-  if (!is_background_app) {
+  // In the main application (the browser process), crashes can be passed to
+  // the system's Crash Reporter.  This allows the system to notify the user
+  // when the application crashes, and provide the user with the option to
+  // restart it.
+  if (is_browser)
     [breakpad_config setObject:@"NO" forKey:@BREAKPAD_SEND_AND_EXIT];
-  }
 
-  // Init breakpad
-  BreakpadRef breakpad = NULL;
-  breakpad = BreakpadCreate(breakpad_config);
-  if (!breakpad) {
-    LOG(ERROR) << "Breakpad init failed.";
+  // Initialize Breakpad.
+  gBreakpadRef = BreakpadCreate(breakpad_config);
+  if (!gBreakpadRef) {
+    LOG(ERROR) << "Breakpad initializaiton failed";
     return;
   }
 
-  // This needs to be set before calling SetCrashKeyValue().
-  gBreakpadRef = breakpad;
-
-  // Set breakpad MetaData values.
-  // These values are added to the plist when building a branded Chrome.app.
-  NSString* version_str = [info_dictionary objectForKey:@BREAKPAD_VERSION];
-  SetCrashKeyValue(@"ver", version_str);
-  NSString* prod_name_str = [info_dictionary objectForKey:@BREAKPAD_PRODUCT];
-  SetCrashKeyValue(@"prod", prod_name_str);
+  // Set Breakpad metadata values.  These values are added to Info.plist during
+  // the branded Google Chrome.app build.
+  SetCrashKeyValue(@"ver", [info_dictionary objectForKey:@BREAKPAD_VERSION]);
+  SetCrashKeyValue(@"prod", [info_dictionary objectForKey:@BREAKPAD_PRODUCT]);
   SetCrashKeyValue(@"plat", @"OS X");
 
-  // Enable child process crashes to include the page url.
-  child_process_logging::SetCrashKeyFunctions(
-      SetCrashKeyValue, ClearCrashKeyValue);
+  // Enable child process crashes to include the page URL.
+  // TODO: Should this only be done for certain process types?
+  child_process_logging::SetCrashKeyFunctions(SetCrashKeyValue,
+                                              ClearCrashKeyValue);
 }
 
 void InitCrashProcessInfo() {
@@ -108,10 +106,9 @@ void InitCrashProcessInfo() {
   }
 
   // Determine the process type.
-  NSString *process_type = @"browser";
-  const CommandLine& parsed_command_line = *CommandLine::ForCurrentProcess();
+  NSString* process_type = @"browser";
   std::wstring process_type_switch =
-      parsed_command_line.GetSwitchValue(switches::kProcessType);
+      CommandLine::ForCurrentProcess()->GetSwitchValue(switches::kProcessType);
   if (!process_type_switch.empty()) {
     process_type = base::SysWideToNSString(process_type_switch);
   }
@@ -139,6 +136,3 @@ void ClearCrashKeyValue(NSString* key) {
 
   BreakpadRemoveUploadParameter(gBreakpadRef, key);
 }
-
-}
-