Incorporate feedback from first Breakpad integration.

This upload fixes five issues:

1) Preston's email was hardcoded in the xib :-(
2) Changed from xib to NIB to facilitate Tiger building
3) Changed the logs location to be user specifiable by BreakpadMinidumpLocation
key, or ~/Library/Breakpad/<BREAKPAD_PRODUCT> by default
4) Fixed GTM Defines problem in order to build on Tiger
5) Also set CFBundleIcon in the sender program correctly, and updated plist, and
renamed ReporterIcons to crash_report_sendER.ICNS.  However the rietveld upload
script doesn't appear to pick up renamed files correctly, so that file doesn't
show up in the patch upload.

Also various comments were updated for accuracy.



git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@323 4c0a9323-5329-0410-9bdc-e9ce6186880e

2 files changed, 53 insertions, 19 deletions

diff --git a/src/client/mac/Framework/Breakpad.h b/src/client/mac/Framework/Breakpad.h
index 05005a99..5c6c2154 100644
--- a/src/client/mac/Framework/Breakpad.h
+++ b/src/client/mac/Framework/Breakpad.h
@@ -54,6 +54,11 @@ extern "C" {
 #define kReporterMinidumpDirectoryKey "MinidumpDir"
 #define kReporterMinidumpIDKey        "MinidumpID"
 
+// The default subdirectory of the Library to put crash dumps in
+// The subdirectory is
+//  ~/Library/<kDefaultLibrarySubdirectory>/<GoogleBreakpadProduct>
+#define kDefaultLibrarySubdirectory   "Breakpad"
+
 // Specify some special keys to be used in the configuration file that is
 // generated by Breakpad and consumed by the crash_sender.
 #define BREAKPAD_PRODUCT_DISPLAY       "BreakpadProductDisplay"
@@ -64,6 +69,7 @@ extern "C" {
 #define BREAKPAD_REPORT_INTERVAL       "BreakpadReportInterval"
 #define BREAKPAD_SKIP_CONFIRM          "BreakpadSkipConfirm"
 #define BREAKPAD_SEND_AND_EXIT         "BreakpadSendAndExit"
+#define BREAKPAD_DUMP_DIRECTORY        "BreakpadMinidumpLocation"
 #define BREAKPAD_INSPECTOR_LOCATION    "BreakpadInspectorLocation"
 
 #define BREAKPAD_REPORTER_EXE_LOCATION \
@@ -94,41 +100,66 @@ typedef bool (*BreakpadFilterCallback)(int exception_type,
 // BREAKPAD_PRODUCT               Product name (e.g., "MyAwesomeProduct")
 //                                This one is used as the key to identify
 //                                the product when uploading
+//                                REQUIRED
+//
 // BREAKPAD_PRODUCT_DISPLAY       This is the display name, e.g. a pretty
 //                                name for the product when the crash_sender
 //                                pops up UI for the user.  Falls back to
 //                                BREAKPAD_PRODUCT if not specified.
+//
 // BREAKPAD_VERSION               Product version (e.g., 1.2.3), used
 //                                as metadata for crash report
+//                                REQUIRED
+//
 // BREAKPAD_VENDOR                Vendor named, used in UI (e.g. the Xxxx
 //                                foo bar company product widget has crashed)
+//
 // BREAKPAD_URL                   URL destination for reporting
+//                                REQUIRED
+//
 // BREAKPAD_REPORT_INTERVAL       # of seconds between sending
 //                                reports.  If an additional report is
 //                                generated within this time, it will
 //                                be ignored.  Default: 3600sec.
 //                                Specify 0 to send all reports.
+//
 // BREAKPAD_SKIP_CONFIRM          If true, the reporter will send the report
 //                                without any user intervention.
+//                                Defaults to NO
+//
 // BREAKPAD_SEND_AND_EXIT         If true, the handler will exit after sending.
 //                                This will prevent any other handler (e.g.,
 //                                CrashReporter) from getting the crash.
+//                                Defaults TO YES
+//
+// BREAKPAD_DUMP_DIRECTORY        The directory to store crash-dumps
+//                                in. By default, we use
+//                                ~/Library/Breakpad/<BREAKPAD_PRODUCT>
+//                                The path you specify here is tilde-expanded.
+//
 // BREAKPAD_INSPECTOR_LOCATION    The full path to the Inspector executable.
+//                                Defaults to <Framework resources>/Inspector
+//
 // BREAKPAD_REPORTER_EXE_LOCATION The full path to the Reporter/sender
 //                                executable.
+//                                Default:
+//                                <Framework Resources>/crash_report_sender.app
+//
 // BREAKPAD_LOGFILES              Indicates an array of log file paths that
-//                                should be uploaded at crash time
+//                                should be uploaded at crash time.
+//
 // BREAKPAD_REQUEST_COMMENTS      If true, the message dialog will have a
 //                                text box for the user to enter comments as
 //                                well as a name and email address.
-// BREAKPAD_COMMENTS              The text the user provided as comments.
-//
-// The BREAKPAD_PRODUCT and BREAKPAD_VERSION are required to have non-
-// NULL values.  By default, the BREAKPAD_PRODUCT will be the
-// CFBundleName and the BREAKPAD_VERSION will be the CFBundleVersion
-// when these keys are present in the bundle's Info.plist.  If the
-// BREAKPAD_PRODUCT or BREAKPAD_VERSION are ultimately undefined,
-// BreakpadCreate() will fail.  You have been warned.
+//                                Default: NO
+//
+// The BREAKPAD_PRODUCT, BREAKPAD_VERSION, and BREAKPAD_URL are
+// required to have non-NULL values.  By default, the BREAKPAD_PRODUCT
+// will be the CFBundleName and the BREAKPAD_VERSION will be the
+// CFBundleVersion when these keys are present in the bundle's
+// Info.plist.  If the BREAKPAD_PRODUCT or BREAKPAD_VERSION are
+// ultimately undefined, BreakpadCreate() will fail.  You have been
+// warned.
 //
 // If you are running in a debugger, breakpad will not install, unless the
 // BREAKPAD_IGNORE_DEBUGGER envionment variable is set and/or non-zero.
diff --git a/src/client/mac/Framework/Breakpad.mm b/src/client/mac/Framework/Breakpad.mm
index 52301c72..9de0a94b 100644
--- a/src/client/mac/Framework/Breakpad.mm
+++ b/src/client/mac/Framework/Breakpad.mm
@@ -407,7 +407,9 @@ bool Breakpad::ExtractParameters(NSDictionary *parameters) {
                 [parameters objectForKey:@BREAKPAD_REQUEST_COMMENTS];
   NSString *vendor =
     [parameters objectForKey:@BREAKPAD_VENDOR];
-
+  NSString *dumpSubdirectory =
+    [parameters objectForKey:@BREAKPAD_DUMP_DIRECTORY];
+  
   // If these two are not already set(skipConfirm and sendAndExit can
   // come from user defaults and take priority)
   if (!skipConfirm) {
@@ -509,9 +511,14 @@ bool Breakpad::ExtractParameters(NSDictionary *parameters) {
     return false;
   }
 
-  // The product and version are required values
+  if (!dumpSubdirectory) {
+    dumpSubdirectory = @"";
+  }
+
+  // The product and version are required values.
   if (![product length] || ![version length]) {
-    DEBUGLOG(stderr, "Missing required product and/or version keys\n");
+    DEBUGLOG(stderr,
+             "Missing required product or version subdirectory keys\n");
     return false;
   }
 
@@ -537,7 +544,9 @@ bool Breakpad::ExtractParameters(NSDictionary *parameters) {
                          [requestUserText UTF8String]);
   dictionary.SetKeyValue(BREAKPAD_VENDOR,
                          [vendor UTF8String]);
-
+  dictionary.SetKeyValue(BREAKPAD_DUMP_DIRECTORY,
+                         [dumpSubdirectory UTF8String]);
+  
   if (logFilePaths) {
     char logFileKey[255];
     for(unsigned int i = 0; i < [logFilePaths count]; i++) {
@@ -550,12 +559,6 @@ bool Breakpad::ExtractParameters(NSDictionary *parameters) {
     dictionary.SetKeyValue(BREAKPAD_EMAIL,
                            [reportEmail UTF8String]);
   }
-#if 0 // for testing
-  BreakpadSetKeyValue(this, @"UserKey1", @"User Value 1");
-  BreakpadSetKeyValue(this, @"UserKey2", @"User Value 2");
-  BreakpadSetKeyValue(this, @"UserKey3", @"User Value 3");
-  BreakpadSetKeyValue(this, @"UserKey4", @"User Value 4");
-#endif
 
   return true;
 }