Support custom URL parameters. Added unit tests for Breakpad. Added

a way to specify server parameters in app plist file, as well.

R=stuartmorgan, jeremy
A=nealsid



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

src/client/mac/Breakpad.xcodeproj/project.pbxproj

@@ -30,6 +30,8 @@
 		33880C800F9E097100817F82 /* InfoPlist.strings in Resources */ = {isa = PBXBuildFile; fileRef = 33880C7E0F9E097100817F82 /* InfoPlist.strings */; };
 		4084699D0F5D9CF900FDCA37 /* crash_report_sender.icns in Resources */ = {isa = PBXBuildFile; fileRef = 4084699C0F5D9CF900FDCA37 /* crash_report_sender.icns */; };
 		8DC2EF570486A6940098B216 /* Cocoa.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 1058C7B1FEA5585E11CA2CBB /* Cocoa.framework */; };
+		F91AF5D00FD60393009D8BE2 /* BreakpadFramework_Test.mm in Sources */ = {isa = PBXBuildFile; fileRef = F91AF5CF0FD60393009D8BE2 /* BreakpadFramework_Test.mm */; };
+		F91AF6210FD60784009D8BE2 /* Breakpad.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 8DC2EF5B0486A6940098B216 /* Breakpad.framework */; };
 		F9286B3A0F7EB25800A4DCC8 /* InspectorMain.mm in Sources */ = {isa = PBXBuildFile; fileRef = F9286B390F7EB25800A4DCC8 /* InspectorMain.mm */; };
 		F92C53B80ECCE7B3009BE4BA /* Inspector.mm in Sources */ = {isa = PBXBuildFile; fileRef = F92C53B70ECCE7B3009BE4BA /* Inspector.mm */; };
 		F92C554C0ECCF534009BE4BA /* Foundation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 0867D69BFE84028FC02AAC07 /* Foundation.framework */; };
@@ -105,6 +107,13 @@
 /* End PBXBuildFile section */
 
 /* Begin PBXContainerItemProxy section */
+		F91AF6370FD60A74009D8BE2 /* PBXContainerItemProxy */ = {
+			isa = PBXContainerItemProxy;
+			containerPortal = 0867D690FE84028FC02AAC07 /* Project object */;
+			proxyType = 1;
+			remoteGlobalIDString = 8DC2EF4F0486A6940098B216;
+			remoteInfo = Breakpad;
+		};
 		F92C564D0ECD10E5009BE4BA /* PBXContainerItemProxy */ = {
 			isa = PBXContainerItemProxy;
 			containerPortal = 0867D690FE84028FC02AAC07 /* Project object */;
@@ -133,13 +142,6 @@
 			remoteGlobalIDString = F93803BD0F80820F004D428B;
 			remoteInfo = generator_test;
 		};
-		F93DE2FD0F82C3C900608B94 /* PBXContainerItemProxy */ = {
-			isa = PBXContainerItemProxy;
-			containerPortal = 0867D690FE84028FC02AAC07 /* Project object */;
-			proxyType = 1;
-			remoteGlobalIDString = F93DE2D00F82A67300608B94;
-			remoteInfo = minidump_file_writer_unittest;
-		};
 		F93DE36F0F82CC1300608B94 /* PBXContainerItemProxy */ = {
 			isa = PBXContainerItemProxy;
 			containerPortal = 0867D690FE84028FC02AAC07 /* Project object */;
@@ -228,6 +230,7 @@
 		33880C7F0F9E097100817F82 /* English */ = {isa = PBXFileReference; fileEncoding = 10; lastKnownFileType = text.plist.strings; name = English; path = sender/English.lproj/InfoPlist.strings; sourceTree = "<group>"; };
 		4084699C0F5D9CF900FDCA37 /* crash_report_sender.icns */ = {isa = PBXFileReference; lastKnownFileType = image.icns; name = crash_report_sender.icns; path = sender/crash_report_sender.icns; sourceTree = "<group>"; };
 		8DC2EF5B0486A6940098B216 /* Breakpad.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Breakpad.framework; sourceTree = BUILT_PRODUCTS_DIR; };
+		F91AF5CF0FD60393009D8BE2 /* BreakpadFramework_Test.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = BreakpadFramework_Test.mm; path = tests/BreakpadFramework_Test.mm; sourceTree = "<group>"; };
 		F9286B380F7EB25800A4DCC8 /* Inspector.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = Inspector.h; path = crash_generation/Inspector.h; sourceTree = "<group>"; };
 		F9286B390F7EB25800A4DCC8 /* InspectorMain.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = InspectorMain.mm; path = crash_generation/InspectorMain.mm; sourceTree = "<group>"; };
 		F92C53540ECCE349009BE4BA /* Inspector */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.executable"; includeInIndex = 0; path = Inspector; sourceTree = BUILT_PRODUCTS_DIR; };
@@ -375,6 +378,7 @@
 			isa = PBXFrameworksBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
+				F91AF6210FD60784009D8BE2 /* Breakpad.framework in Frameworks */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};
@@ -564,6 +568,7 @@
 			children = (
 				F9C77DE00F7DD7E30045F7DB /* SimpleStringDictionaryTest.h */,
 				F9C77DE10F7DD7E30045F7DB /* SimpleStringDictionaryTest.mm */,
+				F91AF5CF0FD60393009D8BE2 /* BreakpadFramework_Test.mm */,
 			);
 			name = tests;
 			sourceTree = "<group>";
@@ -754,8 +759,8 @@
 			);
 			dependencies = (
 				F93DE2FC0F82C3C600608B94 /* PBXTargetDependency */,
-				F93DE2FE0F82C3C900608B94 /* PBXTargetDependency */,
 				F93DE3700F82CC1300608B94 /* PBXTargetDependency */,
+				F91AF6380FD60A74009D8BE2 /* PBXTargetDependency */,
 			);
 			name = UnitTests;
 			productName = UnitTests;
@@ -871,7 +876,7 @@
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 			shellPath = /bin/sh;
-			shellScript = "# Run the unit tests in this test bundle.\n\"${SYSTEM_DEVELOPER_DIR}/Tools/RunUnitTests\"\n\necho running minidump generator tests...\n\"${BUILT_PRODUCTS_DIR}/generator_test\"\necho Running minidump file writer tests...\n\"${BUILT_PRODUCTS_DIR}/minidump_file_writer_unittest\"\necho Running exception handler tests...\n\"${BUILT_PRODUCTS_DIR}/handler_test\"\n";
+			shellScript = "# Run the unit tests in this test bundle.\n\"${SYSTEM_DEVELOPER_DIR}/Tools/RunUnitTests\"\n\necho running minidump generator tests...\n\"${BUILT_PRODUCTS_DIR}/generator_test\"\necho Running exception handler tests...\n\"${BUILT_PRODUCTS_DIR}/handler_test\"\n";
 		};
 /* End PBXShellScriptBuildPhase section */
 
@@ -991,12 +996,18 @@
 				F9C77DE40F7DD82F0045F7DB /* SimpleStringDictionary.mm in Sources */,
 				F9C77DE20F7DD7E30045F7DB /* SimpleStringDictionaryTest.mm in Sources */,
 				F9C77E130F7DDF810045F7DB /* GTMSenTestCase.m in Sources */,
+				F91AF5D00FD60393009D8BE2 /* BreakpadFramework_Test.mm in Sources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};
 /* End PBXSourcesBuildPhase section */
 
 /* Begin PBXTargetDependency section */
+		F91AF6380FD60A74009D8BE2 /* PBXTargetDependency */ = {
+			isa = PBXTargetDependency;
+			target = 8DC2EF4F0486A6940098B216 /* Breakpad */;
+			targetProxy = F91AF6370FD60A74009D8BE2 /* PBXContainerItemProxy */;
+		};
 		F92C564E0ECD10E5009BE4BA /* PBXTargetDependency */ = {
 			isa = PBXTargetDependency;
 			target = F92C563B0ECD10B3009BE4BA /* breakpadUtilities */;
@@ -1017,11 +1028,6 @@
 			target = F93803BD0F80820F004D428B /* generator_test */;
 			targetProxy = F93DE2FB0F82C3C600608B94 /* PBXContainerItemProxy */;
 		};
-		F93DE2FE0F82C3C900608B94 /* PBXTargetDependency */ = {
-			isa = PBXTargetDependency;
-			target = F93DE2D00F82A67300608B94 /* minidump_file_writer_unittest */;
-			targetProxy = F93DE2FD0F82C3C900608B94 /* PBXContainerItemProxy */;
-		};
 		F93DE3700F82CC1300608B94 /* PBXTargetDependency */ = {
 			isa = PBXTargetDependency;
 			target = F93DE32B0F82C55600608B94 /* handler_test */;

src/client/mac/Framework/Breakpad.h

@@ -81,15 +81,15 @@ extern "C" {
 #define BREAKPAD_REQUEST_EMAIL         "BreakpadRequestEmail"
 #define BREAKPAD_EMAIL                 "BreakpadEmail"
 #define BREAKPAD_SERVER_TYPE           "BreakpadServerType"
-// TODO(nealsid) find a better way to support server-specific
+#define BREAKPAD_SERVER_PARAMETER_DICT "BreakpadServerParameters"
-// parameters without having to rebuild Breakpad
-#define BREAKPAD_BUILD_ID              "BreakpadBuildID"
 
 // The keys below are NOT user supplied, and are used internally.
-#define BREAKPAD_PROCESS_START_TIME    "BreakpadProcStartTime"
+#define BREAKPAD_PROCESS_START_TIME       "BreakpadProcStartTime"
-#define BREAKPAD_PROCESS_UP_TIME       "BreakpadProcessUpTime"
+#define BREAKPAD_PROCESS_UP_TIME          "BreakpadProcessUpTime"
-#define BREAKPAD_PROCESS_CRASH_TIME    "BreakpadProcessCrashTime"
+#define BREAKPAD_PROCESS_CRASH_TIME       "BreakpadProcessCrashTime"
-#define BREAKPAD_LOGFILE_KEY_PREFIX    "BreakpadAppLogFile"
+#define BREAKPAD_LOGFILE_KEY_PREFIX       "BreakpadAppLogFile"
+#define BREAKPAD_SERVER_PARAMETER_PREFIX  "BreakpadServerParameterPrefix_"
+
 // Optional user-defined function to dec to decide if we should handle
 // this crash or forward it along.
 // Return true if you want Breakpad to handle it.
@@ -98,7 +98,8 @@ extern "C" {
 // (which means the next exception handler will take the exception)
 typedef bool (*BreakpadFilterCallback)(int exception_type,
                                        int exception_code,
-                                       mach_port_t crashing_thread);
+                                       mach_port_t crashing_thread,
+                                       void *context);
 
 // Create a new BreakpadRef object and install it as an exception
 // handler.  The |parameters| will typically be the contents of your
@@ -179,8 +180,15 @@ typedef bool (*BreakpadFilterCallback)(int exception_type,
 //                                other types, see the function in
 //                                crash_report_sender.m that maps parameters to
 //                                URL parameters.  Defaults to 'google'.
-// BREAKPAD_BUILD_ID              A string parameter indicating build id.
+//
-//                                Optional.
+// BREAKPAD_SERVER_PARAMETER_DICT A plist dictionary of static
+//                                parameters that are uploaded to the
+//                                server.  The parameters are sent as
+//                                is to the crash server.  Their
+//                                content isn't added to the minidump
+//                                but pass as URL parameters when
+//                                uploading theminidump to the crash
+//                                server.
 //=============================================================================
 // The BREAKPAD_PRODUCT, BREAKPAD_VERSION and BREAKPAD_URL are
 // required to have non-NULL values.  By default, the BREAKPAD_PRODUCT
@@ -209,17 +217,31 @@ typedef bool (*BreakpadFilterCallback)(int exception_type,
 // completeness.  They are calculated by Breakpad during initialization &
 // crash-dump generation.
 //
-// BREAKPAD_PROCESS_START_TIME    The time the process started.
+// BREAKPAD_PROCESS_START_TIME       The time the process started.
 //
-// BREAKPAD_PROCESS_CRASH_TIME    The time the process crashed.
+// BREAKPAD_PROCESS_CRASH_TIME       The time the process crashed.
 //
-// BREAKPAD_PROCESS_UP_TIME       The total time the process has been running.
+// BREAKPAD_PROCESS_UP_TIME          The total time the process has been
-//                                This parameter is not set until the
+//                                   running.  This parameter is not set
-//                                crash-dump-generation phase.
+//                                   until the crash-dump-generation phase.
+//
+// BREAKPAD_LOGFILE_KEY_PREFIX       Used to find out which parameters in the
+//                                   parameter dictionary correspond to log
+//                                   file paths.
+//
+// BREAKPAD_SERVER_PARAMETER_PREFIX  This prefix is used by Breakpad
+//                                   internally, because Breakpad uses
+//                                   the same dictionary internally to
+//                                   track both its internal
+//                                   configuration parameters and
+//                                   parameters meant to be uploaded
+//                                   to the server.  This string is
+//                                   used internally by Breakpad to
+//                                   prefix user-supplied parameter
+//                                   names so those can be sent to the
+//                                   server without leaking Breakpad's
+//                                   internal values.
 //
-// BREAKPAD_LOGFILE_KEY_PREFIX    Used to find out which parameters in the
-//                                parameter dictionary correspond to log file
-//                                paths.
 
 // Returns a new BreakpadRef object on success, NULL otherwise.
 BreakpadRef BreakpadCreate(NSDictionary *parameters);
@@ -227,27 +249,49 @@ BreakpadRef BreakpadCreate(NSDictionary *parameters);
 // Uninstall and release the data associated with |ref|.
 void BreakpadRelease(BreakpadRef ref);
 
-// Clients may set an optional callback which gets called when a crash occurs.
+// Clients may set an optional callback which gets called when a crash
-// The callback function should return |true| if we should handle the crash,
+// occurs.  The callback function should return |true| if we should
-// generate a crash report, etc. or |false| if we should ignore it and forward
+// handle the crash, generate a crash report, etc. or |false| if we
-// the crash (normally to CrashReporter)
+// should ignore it and forward the crash (normally to CrashReporter).
+// Context is a pointer to arbitrary data to make the callback with.
 void BreakpadSetFilterCallback(BreakpadRef ref,
-                               BreakpadFilterCallback callback);
+                               BreakpadFilterCallback callback,
+                               void *context);
 
-// User defined key and value string storage
+// User defined key and value string storage.  Generally this is used
-// All set keys will be uploaded with the minidump if a crash occurs
+// to configure Breakpad's internal operation, such as whether the
-// Keys and Values are limited to 255 bytes (256 - 1 for terminator).
+// crash_sender should prompt the user, or the filesystem location for
-// NB this is BYTES not GLYPHS.
+// the minidump file.  See Breakpad.h for some parameters that can be
-// Anything longer than 255 bytes will be truncated. Note that the string is
+// set.  Anything longer than 255 bytes will be truncated. Note that
-// converted to UTF8 before truncation, so any multibyte character that
+// the string is converted to UTF8 before truncation, so any multibyte
-// straddles the 255 byte limit will be mangled.
+// character that straddles the 255(256 - 1 for terminator) byte limit
+// will be mangled.
 //
-// A maximum number of 64 key/value pairs are supported.  An assert() will fire
+// A maximum number of 64 key/value pairs are supported.  An assert()
-// if more than this number are set.
+// will fire if more than this number are set.  Unfortunately, right
+// now, the same dictionary is used for both Breakpad's parameters AND
+// the Upload parameters.
+//
+// TODO (nealsid): Investigate how necessary this is if we don't
+// automatically upload parameters to the server anymore.
+// TODO (nealsid): separate server parameter dictionary from the
+// dictionary used to configure Breakpad, and document limits for each
+// independently.
 void BreakpadSetKeyValue(BreakpadRef ref, NSString *key, NSString *value);
 NSString *BreakpadKeyValue(BreakpadRef ref, NSString *key);
 void BreakpadRemoveKeyValue(BreakpadRef ref, NSString *key);
 
+// You can use this method to specify parameters that will be uploaded
+// to the crash server.  They will be automatically encoded as
+// necessary.  Note that as mentioned above there are limits on both
+// the number of keys and their length.
+void BreakpadAddUploadParameter(BreakpadRef ref, NSString *key,
+				NSString *value);
+
+// This method will remove a previously-added parameter from the
+// upload parameter set.
+void BreakpadRemoveUploadParameter(BreakpadRef ref, NSString *key);
+
 // Add a log file for Breakpad to read and send upon crash dump
 void BreakpadAddLogFile(BreakpadRef ref, NSString *logPathname);
 

src/client/mac/Framework/Breakpad.mm

@@ -154,8 +154,9 @@ class Breakpad {
 
   void GenerateAndSendReport();
 
-  void SetFilterCallback(BreakpadFilterCallback callback) {
+  void SetFilterCallback(BreakpadFilterCallback callback, void *context) {
     filter_callback_ = callback;
+    filter_callback_context_ = context;
   }
 
  private:
@@ -163,7 +164,8 @@ class Breakpad {
     : handler_(NULL),
       config_params_(NULL),
       send_and_exit_(true),
-      filter_callback_(NULL) {
+      filter_callback_(NULL), 
+      filter_callback_context_(NULL) {
     inspector_path_[0] = 0;
   }
 
@@ -194,8 +196,7 @@ class Breakpad {
   bool                    send_and_exit_;  // Exit after sending, if true
 
   BreakpadFilterCallback  filter_callback_;
-
+  void                    *filter_callback_context_;
-  unsigned int            logFileCounter;
 };
 
 #pragma mark -
@@ -412,8 +413,10 @@ bool Breakpad::ExtractParameters(NSDictionary *parameters) {
     [parameters objectForKey:@BREAKPAD_VENDOR];
   NSString *dumpSubdirectory =
     [parameters objectForKey:@BREAKPAD_DUMP_DIRECTORY];
-  NSString *buildId =
+
-    [parameters objectForKey:@BREAKPAD_BUILD_ID];
+  NSDictionary *serverParameters = 
+    [parameters objectForKey:@BREAKPAD_SERVER_PARAMETER_DICT];
+
   // These may have been set above as user prefs, which take priority.
   if (!skipConfirm) {
     skipConfirm = [parameters objectForKey:@BREAKPAD_SKIP_CONFIRM];
@@ -521,10 +524,19 @@ bool Breakpad::ExtractParameters(NSDictionary *parameters) {
     dumpSubdirectory = @"";
   }
 
-  // The product and version are required values.
+  // The product, version, and URL are required values.
-  if (![product length] || ![version length]) {
+  if (![product length]) {
-    DEBUGLOG(stderr,
+    DEBUGLOG(stderr, "Missing required product key.\n");
-             "Missing required product or version subdirectory keys\n");
+    return false;
+  }
+
+  if (![version length]) {
+    DEBUGLOG(stderr, "Missing required version key.\n");
+    return false;
+  }
+
+  if (![urlStr length]) {
+    DEBUGLOG(stderr, "Missing required URL key.\n");
     return false;
   }
 
@@ -555,8 +567,6 @@ bool Breakpad::ExtractParameters(NSDictionary *parameters) {
   dictionary.SetKeyValue(BREAKPAD_DUMP_DIRECTORY,
                          [dumpSubdirectory UTF8String]);
   
-  dictionary.SetKeyValue(BREAKPAD_BUILD_ID,
-                         [buildId UTF8String]);
   struct timeval tv;
   gettimeofday(&tv, NULL);
   char timeStartedString[32];
@@ -579,6 +589,15 @@ bool Breakpad::ExtractParameters(NSDictionary *parameters) {
                            [reportEmail UTF8String]);
   }
 
+  if (serverParameters) {
+    // For each key-value pair, call BreakpadAddUploadParameter()
+    NSEnumerator *keyEnumerator = [serverParameters keyEnumerator];
+    NSString *aParameter;
+    while (aParameter = [keyEnumerator nextObject]) {
+      BreakpadAddUploadParameter(this, aParameter,
+				 [serverParameters objectForKey:aParameter]);
+    }
+  }
   return true;
 }
 
@@ -622,7 +641,8 @@ bool Breakpad::HandleException(int           exception_type,
   if (filter_callback_) {
     bool should_handle = filter_callback_(exception_type,
                                           exception_code,
-                                          crashing_thread);
+                                          crashing_thread,
+                                          filter_callback_context_);
     if (!should_handle) return false;
   }
 
@@ -769,7 +789,7 @@ BreakpadRef BreakpadCreate(NSDictionary *parameters) {
 
     [pool release];
     return (BreakpadRef)breakpad;
-  } catch(...) {    // don't let exception leave this C API
+  } catch(...) {    // don't let exceptions leave this C API
     if (gKeyValueAllocator) {
       gKeyValueAllocator->~ProtectedMemoryAllocator();
       gKeyValueAllocator = NULL;
@@ -817,7 +837,7 @@ void BreakpadRelease(BreakpadRef ref) {
 
       pthread_mutex_destroy(&gDictionaryMutex);
     }
-  } catch(...) {    // don't let exception leave this C API
+  } catch(...) {    // don't let exceptions leave this C API
     fprintf(stderr, "BreakpadRelease() : error\n");
   }
 }
@@ -833,11 +853,51 @@ void BreakpadSetKeyValue(BreakpadRef ref, NSString *key, NSString *value) {
 
       breakpad->SetKeyValue(key, value);
     }
-  } catch(...) {    // don't let exception leave this C API
+  } catch(...) {    // don't let exceptions leave this C API
     fprintf(stderr, "BreakpadSetKeyValue() : error\n");
   }
 }
 
+void BreakpadAddUploadParameter(BreakpadRef ref,
+                                NSString *key,
+                                NSString *value) {
+  // The only difference, internally, between an upload parameter and
+  // a key value one that is set with BreakpadSetKeyValue is that we
+  // prepend the keyname with a special prefix.  This informs the
+  // crash sender that the parameter should be sent along with the
+  // POST of the crash dump upload.
+  try {
+    Breakpad *breakpad = (Breakpad *)ref;
+
+    if (breakpad && key && gKeyValueAllocator) {
+      ProtectedMemoryLocker locker(&gDictionaryMutex, gKeyValueAllocator);
+
+      NSString *prefixedKey = [@BREAKPAD_SERVER_PARAMETER_PREFIX
+				stringByAppendingString:key];
+      breakpad->SetKeyValue(prefixedKey, value);
+    }
+  } catch(...) {    // don't let exceptions leave this C API
+    fprintf(stderr, "BreakpadSetKeyValue() : error\n");
+  }
+}
+
+void BreakpadRemoveUploadParameter(BreakpadRef ref,
+                                   NSString *key) {
+  try {
+    // Not called at exception time
+    Breakpad *breakpad = (Breakpad *)ref;
+
+    if (breakpad && key && gKeyValueAllocator) {
+      ProtectedMemoryLocker locker(&gDictionaryMutex, gKeyValueAllocator);
+
+      NSString *prefixedKey = [NSString stringWithFormat:@"%@%@",
+                                        @BREAKPAD_SERVER_PARAMETER_PREFIX, key];
+      breakpad->RemoveKeyValue(prefixedKey);
+    }
+  } catch(...) {    // don't let exceptions leave this C API
+    fprintf(stderr, "BreakpadRemoveKeyValue() : error\n");
+  }
+}
 //=============================================================================
 NSString *BreakpadKeyValue(BreakpadRef ref, NSString *key) {
   NSString *value = nil;
@@ -852,7 +912,7 @@ NSString *BreakpadKeyValue(BreakpadRef ref, NSString *key) {
     ProtectedMemoryLocker locker(&gDictionaryMutex, gKeyValueAllocator);
 
     value = breakpad->KeyValue(key);
-  } catch(...) {    // don't let exception leave this C API
+  } catch(...) {    // don't let exceptions leave this C API
     fprintf(stderr, "BreakpadKeyValue() : error\n");
   }
 
@@ -870,7 +930,7 @@ void BreakpadRemoveKeyValue(BreakpadRef ref, NSString *key) {
 
       breakpad->RemoveKeyValue(key);
     }
-  } catch(...) {    // don't let exception leave this C API
+  } catch(...) {    // don't let exceptions leave this C API
     fprintf(stderr, "BreakpadRemoveKeyValue() : error\n");
   }
 }
@@ -887,14 +947,15 @@ void BreakpadGenerateAndSendReport(BreakpadRef ref) {
       breakpad->GenerateAndSendReport();
       gBreakpadAllocator->Protect();
     }
-  } catch(...) {    // don't let exception leave this C API
+  } catch(...) {    // don't let exceptions leave this C API
     fprintf(stderr, "BreakpadGenerateAndSendReport() : error\n");
   }
 }
 
 //=============================================================================
 void BreakpadSetFilterCallback(BreakpadRef ref,
-                               BreakpadFilterCallback callback) {
+                               BreakpadFilterCallback callback,
+                               void *context) {
 
   try {
     Breakpad *breakpad = (Breakpad *)ref;
@@ -903,9 +964,9 @@ void BreakpadSetFilterCallback(BreakpadRef ref,
       // share the dictionary mutex here (we really don't need a mutex)
       ProtectedMemoryLocker locker(&gDictionaryMutex, gBreakpadAllocator);
 
-      breakpad->SetFilterCallback(callback);
+      breakpad->SetFilterCallback(callback, context);
     }
-  } catch(...) {    // don't let exception leave this C API
+  } catch(...) {    // don't let exceptions leave this C API
     fprintf(stderr, "BreakpadSetFilterCallback() : error\n");
   }
 }
@@ -933,5 +994,4 @@ void BreakpadAddLogFile(BreakpadRef ref, NSString *logPathname) {
   }
 
   BreakpadSetKeyValue(ref, logFileKey, logPathname);
-
 }

src/client/mac/sender/crash_report_sender.h

@@ -69,15 +69,20 @@ extern NSString *const kDefaultServerType;
   NSMutableDictionary *parameters_;        // Key value pairs of data (STRONG)
   NSData *minidumpContents_;               // The data in the minidump (STRONG)
   NSData *logFileData_;                    // An NSdata for the tar,
-                                           // bz2'd log file
+                                           // bz2'd log file.
   NSMutableDictionary *serverDictionary_;  // The dictionary mapping a
                                            // server type name to a
-                                           // dictionary of URL
+                                           // dictionary of server
-                                           // parameter names
+                                           // parameter names.
   NSMutableDictionary *socorroDictionary_; // The dictionary for
-                                           // Socorro
+                                           // Socorro.
   NSMutableDictionary *googleDictionary_;  // The dictionary for
-                                           // Google
+                                           // Google.
+  NSMutableDictionary *extraServerVars_;   // A dictionary containing
+                                           // extra key/value pairs
+                                           // that are uploaded to the
+                                           // crash server with the
+                                           // minidump.
 }
 
 // Stops the modal panel with an NSAlertDefaultReturn value. This is the action
@@ -95,9 +100,6 @@ extern NSString *const kDefaultServerType;
 - (BOOL)control:(NSControl *)control textView:(NSTextView *)textView
                           doCommandBySelector:(SEL)commandSelector;
 
-// Helper method to set HTTP parameters based on server type
-- (BOOL)setPostParametersFromDictionary:(NSMutableDictionary *)crashParameters;
-
 // Accessors to make bindings work
 - (NSString *)commentsValue;
 - (void)setCommentsValue:(NSString *)value;
@@ -105,7 +107,4 @@ extern NSString *const kDefaultServerType;
 - (NSString *)emailValue;
 - (void)setEmailValue:(NSString *)value;
 
-// Initialization helper to create dictionaries mapping Breakpad
-// parameters to URL parameters
-- (void)createServerParameterDictionaries;
 @end

src/client/mac/sender/crash_report_sender.m

@@ -217,7 +217,24 @@ NSString *const kDefaultServerType = @"google";
 
 // Returns a dictionary that can be used to map Breakpad parameter names to
 // URL parameter names.
-- (NSDictionary *)dictionaryForServerType:(NSString *)serverType;
+- (NSMutableDictionary *)dictionaryForServerType:(NSString *)serverType;
+
+// Helper method to set HTTP parameters based on server type.  This is
+// called right before the upload - crashParameters will contain, on exit,
+// URL parameters that should be sent with the minidump.
+- (BOOL)populateServerDictionary:(NSMutableDictionary *)crashParameters;
+
+// Initialization helper to create dictionaries mapping Breakpad
+// parameters to URL parameters
+- (void)createServerParameterDictionaries;
+
+// Accessor method for the URL parameter dictionary
+- (NSMutableDictionary *)urlParameterDictionary;
+
+// This method adds a key/value pair to the dictionary that
+// will be uploaded to the crash server.
+- (void)addServerParameter:(id)value forKey:(NSString *)key;
+
 @end
 
 @implementation Reporter
@@ -308,7 +325,24 @@ NSString *const kDefaultServerType = @"google";
     id value = [[NSString alloc] initWithData:data
                                      encoding:NSUTF8StringEncoding];
 
-    [parameters_ setObject:value ? value : data forKey:key];
+    // If the keyname is prefixed by BREAKPAD_SERVER_PARAMETER_PREFIX
+    // that indicates that it should be uploaded to the server along
+    // with the minidump, so we treat it specially.
+    if ([key hasPrefix:@BREAKPAD_SERVER_PARAMETER_PREFIX]) {
+      NSString *urlParameterKey =
+        [key substringFromIndex:[@BREAKPAD_SERVER_PARAMETER_PREFIX length]];
+      if ([urlParameterKey length]) {
+        if (value) {
+          [self addServerParameter:value
+                            forKey:urlParameterKey];
+        } else {
+          [self addServerParameter:data
+                            forKey:urlParameterKey];
+        }
+      }
+    } else {
+      [parameters_ setObject:(value ? value : data) forKey:key];
+    }
     [value release];
   }
 
@@ -743,6 +777,7 @@ doCommandBySelector:(SEL)commandSelector {
   serverDictionary_ = [[NSMutableDictionary alloc] init];
   socorroDictionary_ = [[NSMutableDictionary alloc] init];
   googleDictionary_ = [[NSMutableDictionary alloc] init];
+  extraServerVars_ = [[NSMutableDictionary alloc] init];
 
   [serverDictionary_ setObject:socorroDictionary_ forKey:kSocorroServerType];
   [serverDictionary_ setObject:googleDictionary_ forKey:kGoogleServerType];
@@ -752,8 +787,6 @@ doCommandBySelector:(SEL)commandSelector {
   [googleDictionary_ setObject:@"comments" forKey:@BREAKPAD_COMMENTS];
   [googleDictionary_ setObject:@"prod" forKey:@BREAKPAD_PRODUCT];
   [googleDictionary_ setObject:@"ver" forKey:@BREAKPAD_VERSION];
-  // TODO: just for testing, google's server doesn't support it
-  [googleDictionary_ setObject:@"buildid" forKey:@BREAKPAD_BUILD_ID];
 
   [socorroDictionary_ setObject:@"Comments" forKey:@BREAKPAD_COMMENTS];
   [socorroDictionary_ setObject:@"CrashTime"
@@ -766,21 +799,23 @@ doCommandBySelector:(SEL)commandSelector {
                          forKey:@BREAKPAD_PRODUCT];
   [socorroDictionary_ setObject:@"ProductName"
                          forKey:@BREAKPAD_PRODUCT];
-  [socorroDictionary_ setObject:@"BuildID"
-                         forKey:@BREAKPAD_BUILD_ID];
 }
 
-- (NSDictionary *)dictionaryForServerType:(NSString *)serverType {
+- (NSMutableDictionary *)dictionaryForServerType:(NSString *)serverType {
   if (serverType == nil || [serverType length] == 0) {
     return [serverDictionary_ objectForKey:kDefaultServerType];
   }
   return [serverDictionary_ objectForKey:serverType];
 }
 
-// Helper method to set HTTP parameters based on server type
+- (NSMutableDictionary *)urlParameterDictionary {
-- (BOOL)setPostParametersFromDictionary:(NSMutableDictionary *)crashParameters {
   NSString *serverType = [parameters_ objectForKey:@BREAKPAD_SERVER_TYPE];
-  NSDictionary *urlParameterNames = [self dictionaryForServerType:serverType];
+  return [self dictionaryForServerType:serverType];
+
+}
+
+- (BOOL)populateServerDictionary:(NSMutableDictionary *)crashParameters {
+  NSDictionary *urlParameterNames = [self urlParameterDictionary];
 
   id key;
   NSEnumerator *enumerator = [parameters_ keyEnumerator];
@@ -802,16 +837,31 @@ doCommandBySelector:(SEL)commandSelector {
                           forKey:urlParameter];
     }
   }
+
+  // Now, add the parameters that were added by the application.
+  enumerator = [extraServerVars_ keyEnumerator];
+
+  while ((key = [enumerator nextObject])) {
+    NSString *urlParameterName = (NSString *)key;
+    NSString *urlParameterValue =
+      [extraServerVars_ objectForKey:urlParameterName];
+    [crashParameters setObject:urlParameterValue
+                        forKey:urlParameterName];
+  }
   return YES;
 }
 
+- (void)addServerParameter:(id)value forKey:(NSString *)key {
+  [extraServerVars_ setObject:value forKey:key];
+}
+
 //=============================================================================
 - (void)report {
   NSURL *url = [NSURL URLWithString:[parameters_ objectForKey:@BREAKPAD_URL]];
   HTTPMultipartUpload *upload = [[HTTPMultipartUpload alloc] initWithURL:url];
   NSMutableDictionary *uploadParameters = [NSMutableDictionary dictionary];
 
-  if (![self setPostParametersFromDictionary:uploadParameters]) {
+  if (![self populateServerDictionary:uploadParameters]) {
     return;
   }
 
@@ -886,6 +936,7 @@ doCommandBySelector:(SEL)commandSelector {
   [googleDictionary_ release];
   [socorroDictionary_ release];
   [serverDictionary_ release];
+  [extraServerVars_ release];
   [super dealloc];
 }
 

src/client/mac/testapp/Controller.m

@@ -234,6 +234,9 @@
     NSLog(@"Shouldn't find BreakpadKeyValue (key2)");
   }
 
+  BreakpadAddUploadParameter(breakpad_,
+                             @"MeaningOfLife",
+                             @"42");
   [NSThread detachNewThreadSelector:@selector(anotherThread)
                            toTarget:self withObject:nil];
 
@@ -244,8 +247,8 @@
   // automated testing.
   if ([args boolForKey:@"autocrash"]) {
     BreakpadSetKeyValue(breakpad_,
-                              @BREAKPAD_SKIP_CONFIRM,
+                        @BREAKPAD_SKIP_CONFIRM,
-                              @"YES");
+                        @"YES");
     [self causeCrash];
   }
 

src/client/mac/testapp/Info.plist

@@ -42,6 +42,13 @@
 	<string>YES</string>
 	<key>BreakpadVendor</key>
 	<string>Foo Bar Corp, Incorporated, LTD, LLC</string>
+	<key>BreakpadServerParameters</key>
+	<dict>
+	  <key>Param1</key>
+	  <string>Value1</string>
+	  <key>Param2</key>
+	  <string>Value2</string>
+	</dict>
 	<key>LSUIElement</key>
 	<string>1</string>
 </dict>

src/client/mac/tests/BreakpadFramework_Test.mm

@@ -0,0 +1,216 @@
+// Copyright (c) 2009, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+//
+// BreakpadFramework_Test.mm
+// Test case file for Breakpad.h/mm.
+//
+
+#import "GTMSenTestCase.h"
+#import "Breakpad.h"
+
+#include <mach/mach.h>
+
+@interface BreakpadFramework_Test : GTMTestCase {
+ @private
+  int last_exception_code_;
+  int last_exception_type_;
+  mach_port_t last_exception_thread_;
+  // We're not using Obj-C BOOL because we need to interop with
+  // Breakpad's callback.
+  bool shouldHandleException_;
+}
+
+// This method is used by a callback used by test cases to determine
+// whether to return true or false to Breakpad when handling an
+// exception.
+- (bool)shouldHandleException;
+// This method returns a minimal dictionary that has what
+// Breakpad needs to initialize.
+- (NSMutableDictionary *)breakpadInitializationDictionary;
+// This method is used by the exception handling callback
+// to communicate to test cases the properites of the last
+// exception.
+- (void)setLastExceptionType:(int)type andCode:(int)code
+                   andThread:(mach_port_t)thread;
+@end
+
+// Callback for Breakpad exceptions
+bool myBreakpadCallback(int exception_type,
+                        int exception_code,
+                        mach_port_t crashing_thread,
+                        void *context);
+
+bool myBreakpadCallback(int exception_type,
+                        int exception_code,
+                        mach_port_t crashing_thread,
+                        void *context) {
+  BreakpadFramework_Test *testCaseClass =
+    (BreakpadFramework_Test *)context;
+  [testCaseClass setLastExceptionType:exception_type
+                              andCode:exception_code
+                            andThread:crashing_thread];
+  bool shouldHandleException =
+    [testCaseClass shouldHandleException];
+  NSLog(@"Callback returning %d", shouldHandleException);
+  return shouldHandleException;
+}
+const int kNoLastExceptionCode = -1;
+const int kNoLastExceptionType = -1;
+const mach_port_t kNoLastExceptionThread = MACH_PORT_NULL;
+
+@implementation BreakpadFramework_Test
+- (void) initializeExceptionStateVariables {
+  last_exception_code_ = kNoLastExceptionCode;
+  last_exception_type_ = kNoLastExceptionType;
+  last_exception_thread_ = kNoLastExceptionThread;
+}
+
+- (NSMutableDictionary *)breakpadInitializationDictionary {
+  NSMutableDictionary *breakpadParams =
+    [NSMutableDictionary dictionaryWithCapacity:3];
+
+  [breakpadParams setObject:@"UnitTests" forKey:@BREAKPAD_PRODUCT];
+  [breakpadParams setObject:@"1.0" forKey:@BREAKPAD_VERSION];
+  [breakpadParams setObject:@"http://staging" forKey:@BREAKPAD_URL];
+  return breakpadParams;
+}
+
+- (bool)shouldHandleException {
+  return shouldHandleException_;
+}
+
+- (void)setLastExceptionType:(int)type 
+		     andCode:(int)code
+                   andThread:(mach_port_t)thread {
+  last_exception_type_ = type;
+  last_exception_code_ = code;
+  last_exception_thread_ = thread;
+}
+
+// Test that the parameters mark required actually enable Breakpad to
+// be initialized.
+- (void)testBreakpadInstantiationWithRequiredParameters {
+  BreakpadRef b = BreakpadCreate([self breakpadInitializationDictionary]);
+  STAssertNotNULL(b, @"BreakpadCreate failed with required parameters");
+  BreakpadRelease(b);
+}
+
+// Test that Breakpad fails to initialize cleanly when required
+// parameters are not present.
+- (void)testBreakpadInstantiationWithoutRequiredParameters {
+  NSMutableDictionary *breakpadDictionary =
+    [self breakpadInitializationDictionary];
+
+  // Skip setting version, so that BreakpadCreate fails.
+  [breakpadDictionary removeObjectForKey:@BREAKPAD_VERSION];
+  BreakpadRef b = BreakpadCreate(breakpadDictionary);
+  STAssertNULL(b, @"BreakpadCreate did not fail when missing a required"
+               " parameter!");
+
+  breakpadDictionary = [self breakpadInitializationDictionary];
+  // Now test with no product
+  [breakpadDictionary removeObjectForKey:@BREAKPAD_PRODUCT];
+  b = BreakpadCreate(breakpadDictionary);
+  STAssertNULL(b, @"BreakpadCreate did not fail when missing a required"
+               " parameter!");
+
+  breakpadDictionary = [self breakpadInitializationDictionary];
+  // Now test with no URL
+  [breakpadDictionary removeObjectForKey:@BREAKPAD_URL];
+  b = BreakpadCreate(breakpadDictionary);
+  STAssertNULL(b, @"BreakpadCreate did not fail when missing a required"
+               " parameter!");
+  BreakpadRelease(b);
+}
+
+// Test to ensure that when we call BreakpadAddUploadParameter,
+// it's added to the dictionary correctly(this test depends on
+// some internal details of Breakpad, namely, the special prefix
+// that it uses to figure out which key/value pairs to upload).
+- (void)testAddingBreakpadServerVariable {
+  NSMutableDictionary *breakpadDictionary =
+    [self breakpadInitializationDictionary];
+
+  BreakpadRef b = BreakpadCreate(breakpadDictionary);
+  STAssertNotNULL(b, @"BreakpadCreate failed with valid dictionary!");
+
+  BreakpadAddUploadParameter(b,
+                             @"key",
+                             @"value");
+
+  // Test that it did not add the key/value directly, e.g. without
+  // prepending the key with the prefix.
+  STAssertNil(BreakpadKeyValue(b, @"key"),
+              @"AddUploadParameter added key directly to dictionary"
+              " instead of prepending it!");
+
+  NSString *prependedKeyname =
+    [@BREAKPAD_SERVER_PARAMETER_PREFIX stringByAppendingString:@"key"];
+    
+  STAssertEqualStrings(BreakpadKeyValue(b, prependedKeyname),
+                       @"value",
+                       @"Calling BreakpadAddUploadParameter did not prepend "
+                       "key name");
+  BreakpadRelease(b);
+}
+
+// Test that when we do on-demand minidump generation,
+// the exception code/type/thread are set properly.
+- (void)testFilterCallbackReturnsFalse {
+  NSMutableDictionary *breakpadDictionary =
+    [self breakpadInitializationDictionary];
+
+  BreakpadRef b = BreakpadCreate(breakpadDictionary);
+  STAssertNotNULL(b, @"BreakpadCreate failed with valid dictionary!");
+  BreakpadSetFilterCallback(b, &myBreakpadCallback, self);
+
+  // This causes the callback to return false, meaning
+  // Breakpad won't take the exception
+  shouldHandleException_ = false;
+
+  [self initializeExceptionStateVariables];
+  STAssertEquals(last_exception_type_, kNoLastExceptionType,
+                 @"Last exception type not initialized correctly.");
+  STAssertEquals(last_exception_code_, kNoLastExceptionCode,
+                 @"Last exception code not initialized correctly.");
+  STAssertEquals(last_exception_thread_, kNoLastExceptionThread,
+                 @"Last exception thread is not initialized correctly.");
+
+  // Cause Breakpad's exception handler to be invoked.
+  BreakpadGenerateAndSendReport(b);
+
+  STAssertEquals(last_exception_type_, 0,
+                 @"Last exception type is not 0 for on demand");
+  STAssertEquals(last_exception_code_, 0,
+                 @"Last exception code is not 0 for on demand");
+  STAssertEquals(last_exception_thread_, (mach_port_t)0,
+                 @"Last exception thread is not 0 for on demand");
+}
+
+@end