[Breakpad iOS] Add a callback on report upload completion.

This CL adds a result callback on report upload completion.
On failure, Breakpad deletes the configuration file and does retry to
upload a report.
Using this callback, the client will be able to log some metrics and to
act on upload failure.

Bug: 954175
Change-Id: I95a3264b65d4c06ba5d8dde8377440d23f1e2081
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/1572661
Reviewed-by: Mark Mentovai <mark@chromium.org>

6 files changed, 88 insertions, 23 deletions

diff --git a/src/client/ios/Breakpad.h b/src/client/ios/Breakpad.h
index 30b13449..fa790f77 100644
--- a/src/client/ios/Breakpad.h
+++ b/src/client/ios/Breakpad.h
@@ -62,6 +62,14 @@ typedef bool (*BreakpadFilterCallback)(int exception_type,
                                        mach_port_t crashing_thread,
                                        void *context);
 
+// Optional user-defined function that will be called after a network upload
+// of a crash report.
+// |report_id| will be the id returned by the server, or "ERR" if an error
+// occurred.
+// |error| will contain the error, or nil if no error occured.
+typedef void (*BreakpadUploadCompletionCallback)(NSString *report_id,
+                                                 NSError *error);
+
 // Create a new BreakpadRef object and install it as an exception
 // handler.  The |parameters| will typically be the contents of your
 // bundle's Info.plist.
@@ -210,8 +218,10 @@ void BreakpadUploadNextReport(BreakpadRef ref);
 
 // Upload next report to the server.
 // |server_parameters| is additional server parameters to send.
-void BreakpadUploadNextReportWithParameters(BreakpadRef ref,
-                                            NSDictionary *server_parameters);
+void BreakpadUploadNextReportWithParameters(
+    BreakpadRef ref,
+    NSDictionary *server_parameters,
+    BreakpadUploadCompletionCallback callback);
 
 // Upload a report to the server.
 // |server_parameters| is additional server parameters to send.
@@ -219,7 +229,8 @@ void BreakpadUploadNextReportWithParameters(BreakpadRef ref,
 void BreakpadUploadReportWithParametersAndConfiguration(
     BreakpadRef ref,
     NSDictionary *server_parameters,
-    NSDictionary *configuration);
+    NSDictionary *configuration,
+    BreakpadUploadCompletionCallback callback);
 
 // Handles the network response of a breakpad upload. This function is needed if
 // the actual upload is done by the Breakpad client.
diff --git a/src/client/ios/Breakpad.mm b/src/client/ios/Breakpad.mm
index c99ee4fd..2d8ba61e 100644
--- a/src/client/ios/Breakpad.mm
+++ b/src/client/ios/Breakpad.mm
@@ -164,7 +164,8 @@ class Breakpad {
   NSDate *DateOfMostRecentCrashReport();
   void UploadNextReport(NSDictionary *server_parameters);
   void UploadReportWithConfiguration(NSDictionary *configuration,
-                                     NSDictionary *server_parameters);
+                                     NSDictionary *server_parameters,
+                                     BreakpadUploadCompletionCallback callback);
   void UploadData(NSData *data, NSString *name,
                   NSDictionary *server_parameters);
   void HandleNetworkResponse(NSDictionary *configuration,
@@ -502,8 +503,10 @@ void Breakpad::HandleNetworkResponse(NSDictionary *configuration,
 }
 
 //=============================================================================
-void Breakpad::UploadReportWithConfiguration(NSDictionary *configuration,
-                                             NSDictionary *server_parameters) {
+void Breakpad::UploadReportWithConfiguration(
+    NSDictionary *configuration,
+    NSDictionary *server_parameters,
+    BreakpadUploadCompletionCallback callback) {
   Uploader *uploader = [[[Uploader alloc]
       initWithConfig:configuration] autorelease];
   if (!uploader)
@@ -512,6 +515,13 @@ void Breakpad::UploadReportWithConfiguration(NSDictionary *configuration,
     [uploader addServerParameter:[server_parameters objectForKey:key]
                           forKey:key];
   }
+  if (callback) {
+    [uploader setUploadCompletionBlock:^(NSString *report_id, NSError *error) {
+      dispatch_async(dispatch_get_main_queue(), ^{
+        callback(report_id, error);
+      });
+    }];
+  }
   [uploader report];
 }
 
@@ -519,7 +529,8 @@ void Breakpad::UploadReportWithConfiguration(NSDictionary *configuration,
 void Breakpad::UploadNextReport(NSDictionary *server_parameters) {
   NSDictionary *configuration = NextCrashReportConfiguration();
   if (configuration) {
-    return UploadReportWithConfiguration(configuration, server_parameters);
+    return UploadReportWithConfiguration(configuration, server_parameters,
+                                         nullptr);
   }
 }
 
@@ -850,7 +861,7 @@ int BreakpadGetCrashReportCount(BreakpadRef ref) {
 
 //=============================================================================
 void BreakpadUploadNextReport(BreakpadRef ref) {
-  BreakpadUploadNextReportWithParameters(ref, nil);
+  BreakpadUploadNextReportWithParameters(ref, nil, nullptr);
 }
 
 //=============================================================================
@@ -882,22 +893,25 @@ NSDate *BreakpadGetDateOfMostRecentCrashReport(BreakpadRef ref) {
 void BreakpadUploadReportWithParametersAndConfiguration(
     BreakpadRef ref,
     NSDictionary *server_parameters,
-    NSDictionary *configuration) {
+    NSDictionary *configuration,
+    BreakpadUploadCompletionCallback callback) {
   try {
     Breakpad *breakpad = (Breakpad *)ref;
     if (!breakpad || !configuration)
       return;
-    breakpad->UploadReportWithConfiguration(configuration, server_parameters);
+    breakpad->UploadReportWithConfiguration(configuration, server_parameters,
+                                            callback);
   } catch(...) {    // don't let exceptions leave this C API
     fprintf(stderr,
         "BreakpadUploadReportWithParametersAndConfiguration() : error\n");
   }
-
 }
 
 //=============================================================================
-void BreakpadUploadNextReportWithParameters(BreakpadRef ref,
-                                            NSDictionary *server_parameters) {
+void BreakpadUploadNextReportWithParameters(
+    BreakpadRef ref,
+    NSDictionary *server_parameters,
+    BreakpadUploadCompletionCallback callback) {
   try {
     Breakpad *breakpad = (Breakpad *)ref;
     if (!breakpad)
@@ -905,9 +919,8 @@ void BreakpadUploadNextReportWithParameters(BreakpadRef ref,
     NSDictionary *configuration = breakpad->NextCrashReportConfiguration();
     if (!configuration)
       return;
-    return BreakpadUploadReportWithParametersAndConfiguration(ref,
-                                                              server_parameters,
-                                                              configuration);
+    return BreakpadUploadReportWithParametersAndConfiguration(
+        ref, server_parameters, configuration, callback);
   } catch(...) {    // don't let exceptions leave this C API
     fprintf(stderr, "BreakpadUploadNextReportWithParameters() : error\n");
   }
diff --git a/src/client/ios/BreakpadController.h b/src/client/ios/BreakpadController.h
index 20868473..6c70c202 100644
--- a/src/client/ios/BreakpadController.h
+++ b/src/client/ios/BreakpadController.h
@@ -66,6 +66,9 @@
   // The dictionary that contains additional server parameters to send when
   // uploading crash reports.
   NSDictionary* uploadTimeParameters_;
+
+  // The callback to call on report upload completion.
+  BreakpadUploadCompletionCallback uploadCompleteCallback_;
 }
 
 // Singleton.
@@ -95,6 +98,10 @@
 // crash report is generated. See |BreakpadAddUploadParameter|.
 - (void)addUploadParameter:(NSString*)value forKey:(NSString*)key;
 
+// Sets the callback to be called after uploading a crash report to the server.
+// Only the latest callback registered will be called.
+- (void)setUploadCallback:(BreakpadUploadCompletionCallback)callback;
+
 // Remove a previously-added parameter from the upload parameter set. See
 // |BreakpadRemoveUploadParameter|.
 - (void)removeUploadParameterForKey:(NSString*)key;
diff --git a/src/client/ios/BreakpadController.mm b/src/client/ios/BreakpadController.mm
index 8d499c27..01fb5f13 100644
--- a/src/client/ios/BreakpadController.mm
+++ b/src/client/ios/BreakpadController.mm
@@ -166,9 +166,9 @@ NSString* GetPlatform() {
   NSAssert(started_, @"The controller must be started before "
                      "threadUnsafeSendReportWithConfiguration is called");
   if (breakpadRef_) {
-    BreakpadUploadReportWithParametersAndConfiguration(breakpadRef_,
-                                                       uploadTimeParameters_,
-                                                       configuration);
+    BreakpadUploadReportWithParametersAndConfiguration(
+        breakpadRef_, uploadTimeParameters_, configuration,
+        uploadCompleteCallback_);
   }
 }
 
@@ -195,7 +195,7 @@ NSString* GetPlatform() {
   NSAssert(!started_,
       @"The controller must not be started when updateConfiguration is called");
   [configuration_ addEntriesFromDictionary:configuration];
-  NSString* uploadInterval =
+  NSString *uploadInterval =
       [configuration_ valueForKey:@BREAKPAD_REPORT_INTERVAL];
   if (uploadInterval)
     [self setUploadInterval:[uploadInterval intValue]];
@@ -206,7 +206,7 @@ NSString* GetPlatform() {
       @"The controller must not be started when resetConfiguration is called");
   [configuration_ autorelease];
   configuration_ = [[[NSBundle mainBundle] infoDictionary] mutableCopy];
-  NSString* uploadInterval =
+  NSString *uploadInterval =
       [configuration_ valueForKey:@BREAKPAD_REPORT_INTERVAL];
   [self setUploadInterval:[uploadInterval intValue]];
   [self setParametersToAddAtUploadTime:nil];
@@ -243,6 +243,15 @@ NSString* GetPlatform() {
   });
 }
 
+- (void)setUploadCallback:(BreakpadUploadCompletionCallback)callback {
+  NSAssert(started_,
+           @"The controller must not be started before setUploadCallback is "
+            "called");
+  dispatch_async(queue_, ^{
+    uploadCompleteCallback_ = callback;
+  });
+}
+
 - (void)removeUploadParameterForKey:(NSString*)key {
   NSAssert(started_, @"The controller must be started before "
                      "removeUploadParameterForKey is called");
@@ -345,8 +354,8 @@ NSString* GetPlatform() {
   // A report can be sent now.
   if (timeToWait == 0) {
     [self reportWillBeSent];
-    BreakpadUploadNextReportWithParameters(breakpadRef_,
-                                           uploadTimeParameters_);
+    BreakpadUploadNextReportWithParameters(breakpadRef_, uploadTimeParameters_,
+                                           uploadCompleteCallback_);
 
     // If more reports must be sent, make sure this method is called again.
     if (BreakpadGetCrashReportCount(breakpadRef_) > 0)
diff --git a/src/client/mac/sender/uploader.h b/src/client/mac/sender/uploader.h
index 5f6aa464..0897dade 100644
--- a/src/client/mac/sender/uploader.h
+++ b/src/client/mac/sender/uploader.h
@@ -42,6 +42,13 @@ extern NSString *const kGoogleServerType;
 extern NSString *const kSocorroServerType;
 extern NSString *const kDefaultServerType;
 
+// Optional user-defined function that will be called after a network upload
+// of a crash report.
+// |report_id| will be the id returned by the server, or "ERR" if an error
+// occurred.
+// |error| will contain the error, or nil if no error occured.
+typedef void (^UploadCompletionBlock)(NSString *reportId, NSError *error);
+
 @interface Uploader : NSObject {
  @private
   NSMutableDictionary *parameters_;        // Key value pairs of data (STRONG)
@@ -61,6 +68,12 @@ extern NSString *const kDefaultServerType;
                                            // that are uploaded to the
                                            // crash server with the
                                            // minidump.
+  UploadCompletionBlock uploadCompletion_;  // A block called on network upload
+                                            // completion. Parameters are:
+                                            // The report ID returned by the
+                                            // server,
+                                            // the NSError triggered during
+                                            // upload.
 }
 
 - (id)initWithConfigFile:(const char *)configFile;
@@ -86,4 +99,8 @@ extern NSString *const kDefaultServerType;
 // new ID.
 - (void)handleNetworkResponse:(NSData *)data withError:(NSError *)error;
 
+// Sets the callback to be called after uploading a crash report to the server.
+// Only the latest callback registered will be called.
+- (void)setUploadCompletionBlock:(UploadCompletionBlock)uploadCompletion;
+
 @end
diff --git a/src/client/mac/sender/uploader.mm b/src/client/mac/sender/uploader.mm
index c74c0583..08aecf76 100644
--- a/src/client/mac/sender/uploader.mm
+++ b/src/client/mac/sender/uploader.mm
@@ -511,6 +511,9 @@ NSDictionary *readConfigurationData(const char *configFile) {
     reportID = [[result stringByTrimmingCharactersInSet:trimSet] UTF8String];
     [self logUploadWithID:reportID];
   }
+  if (uploadCompletion_) {
+    uploadCompletion_([NSString stringWithUTF8String:reportID], error);
+  }
 
   // rename the minidump file according to the id returned from the server
   NSString *minidumpDir =
@@ -548,6 +551,11 @@ NSDictionary *readConfigurationData(const char *configFile) {
 }
 
 //=============================================================================
+- (void)setUploadCompletionBlock:(UploadCompletionBlock)uploadCompletion {
+  uploadCompletion_ = uploadCompletion;
+}
+
+//=============================================================================
 - (void)report {
   NSURL *url = [NSURL URLWithString:[parameters_ objectForKey:@BREAKPAD_URL]];