[ios] Removes product data splitting code from breadcrumbs.

Breakpad supports product data values up to 2550 bytes long, so there is
no need for breadcrumbs to implement additional splitting internally.
This CL removes the code that splits breadcrumbs data across keys,
simplifies APIs around setting breadcrumbs, and removes any associated
unittests.

BUG=1068802

Change-Id: Ia33320bcbe69c799d62a2ef36cf0e4d11d9f4566
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2146932
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760122}

ios/chrome/browser/crash_report/breakpad_helper.h

@@ -12,10 +12,8 @@
 
 namespace breakpad_helper {
 
-// Key format for breadcrumbs attached to crash reports. Format string contains
-// %lu for index as there are multiple breadcrumbs keys uploaded to crash
-// server.
-extern NSString* const kBreadcrumbs;
+// Key for breadcrumbs attached to crash reports.
+extern NSString* const kBreadcrumbsProductDataKey;
 
 // Starts the crash handlers. This must be run as soon as possible to catch
 // early crashes.
@@ -133,10 +131,8 @@ void SetGridToVisibleTabAnimation(NSString* to_view_controller,
 // tab.
 void RemoveGridToVisibleTabAnimation();
 
-// Sets keys with the given |breadcrumbs|. Each item in |breadcrumbs| will be
-// stored separately with kBreadcrumbs+<index> key (where kBreadcrumbs0 stores
-// the latest breadscrumbs).
-void SetBreadcrumbEvents(NSArray* breadcrumbs);
+// Sets a key with the given |breadcrumbs| events.
+void SetBreadcrumbEvents(NSString* breadcrumbs);
 
 // Sets a key in browser to store the playback state of media player (audio or
 // video). This function records a new start. This function is called for each

ios/chrome/browser/crash_report/breakpad_helper.mm

@@ -35,7 +35,7 @@
 
 namespace breakpad_helper {
 
-NSString* const kBreadcrumbs = @"breadcrumbs%lu";
+NSString* const kBreadcrumbsProductDataKey = @"breadcrumbs";
 
 namespace {
 
@@ -370,13 +370,8 @@ void RemoveGridToVisibleTabAnimation() {
   RemoveReportParameter(kGridToVisibleTabAnimation);
 }
 
-void SetBreadcrumbEvents(NSArray* breadcrumbs) {
-  DCHECK_GT(breadcrumbs.count, 0U);
-  DCHECK_LE(breadcrumbs.count, 6U);
-  for (NSUInteger i = 0; i < breadcrumbs.count; i++) {
-    NSString* key = [NSString stringWithFormat:kBreadcrumbs, i];
-    AddReportParameter(key, breadcrumbs[i], /*async=*/true);
-  }
+void SetBreadcrumbEvents(NSString* breadcrumbs) {
+  AddReportParameter(kBreadcrumbsProductDataKey, breadcrumbs, /*async=*/true);
 }
 
 void MediaStreamPlaybackDidStart() {

ios/chrome/browser/crash_report/breakpad_helper_unittest.mm

@@ -4,7 +4,9 @@
 
 #import "ios/chrome/browser/crash_report/breakpad_helper.h"
 
+#include "base/strings/sys_string_conversions.h"
 #include "ios/chrome/browser/crash_report/crash_report_helper.h"
+#include "ios/chrome/browser/crash_report/crash_reporter_breadcrumb_observer.h"
 #include "ios/chrome/browser/crash_report/main_thread_freeze_detector.h"
 #import "ios/chrome/test/ocmock/OCMockObject+BreakpadControllerTesting.h"
 #import "ios/testing/scoped_block_swizzler.h"
@@ -78,18 +80,9 @@ TEST_F(BreakpadHelperTest, CrashReportUserApplicationStateAllKeys) {
       @"presented_view_controller", @"parent_view_controller");
   breakpad_helper::MediaStreamPlaybackDidStart();
 
-  // Build a sample breadcrumbs string greater than the maximum value size as
-  // defined in Breakpad.h at the BreakpadSetKeyValue function comment.
-  NSMutableString* breadcrumbs = [[NSMutableString alloc] init];
-  while (breadcrumbs.length < 255) {
-    [breadcrumbs appendString:@"12:01 Fake Breadcrumb Event/n"];
-  }
-
-  NSMutableArray* events = [[NSMutableArray alloc] init];
-  for (int i = 0; i < breakpad::kBreadcrumbsKeyCount; i++) {
-    [events addObject:breadcrumbs];
-  }
-  breakpad_helper::SetBreadcrumbEvents(events);
+  // Set a max-length breadcrumbs string.
+  std::string breadcrumbs(kMaxBreadcrumbsDataLength, 'A');
+  breakpad_helper::SetBreadcrumbEvents(base::SysUTF8ToNSString(breadcrumbs));
 }
 
 TEST_F(BreakpadHelperTest, GetCrashReportCount) {

ios/chrome/browser/crash_report/crash_report_helper.h

@@ -18,12 +18,6 @@ class WebStateList;
 
 namespace breakpad {
 
-// Number of product data keys to use for breadcrumbs. Each product key allows
-// to upload limited amount of data (kMaxProductDataLength), so having
-// multiple keys makes breadcrumbs more useful by having access to more
-// breadcrumbs on crash reports.
-extern const int kBreadcrumbsKeyCount;
-
 // Monitors the urls loaded by |web_state| to allow crash reports to contain the
 // current loading url.
 void MonitorURLsForWebState(web::WebState* web_state);

ios/chrome/browser/crash_report/crash_report_helper.mm

@@ -38,12 +38,6 @@
 #error "This file requires ARC support."
 #endif
 
-namespace breakpad {
-// IMPORTANT: be careful if ever increasing this value, Breakpad reports have an
-// overall size limit
-const int kBreadcrumbsKeyCount = 1;
-}
-
 // WebStateListObserver that allows loaded urls to be sent to the crash server.
 @interface CrashReporterURLObserver
     : NSObject <WebStateListObserving, CRWWebStateObserver> {
@@ -454,8 +448,6 @@ void ClearStateForWebStateList(WebStateList* web_state_list) {
 void MonitorBreadcrumbManager(BreadcrumbManager* breadcrumb_manager) {
   [[CrashReporterBreadcrumbObserver uniqueInstance]
       observeBreadcrumbManager:breadcrumb_manager];
-  [CrashReporterBreadcrumbObserver uniqueInstance].breadcrumbsKeyCount =
-      kBreadcrumbsKeyCount;
 }
 
 void StopMonitoringBreadcrumbManager(BreadcrumbManager* breadcrumb_manager) {

ios/chrome/browser/crash_report/crash_reporter_breadcrumb_observer.h

@@ -12,8 +12,8 @@
 
 #import "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_observer_bridge.h"
 
-// The maximum string length for product data supported by Breakpad.
-extern const int kMaxProductDataLength;
+// The maximum string length for breadcrumbs data.
+extern const NSUInteger kMaxBreadcrumbsDataLength;
 
 // Combines breadcrumbs from multiple BreadcrumbManagers and sends the merged
 // breadcrumb events to breakpad for attachment to crash reports.
@@ -24,12 +24,6 @@ extern const int kMaxProductDataLength;
 // Creates a singleton instance.
 + (CrashReporterBreadcrumbObserver*)uniqueInstance;
 
-// Number of product data keys to use for breadcrumbs.
-@property(nonatomic) NSUInteger breadcrumbsKeyCount;
-
-// Maximum allowed length for a single product data value.
-@property(nonatomic) NSUInteger maxProductDataLength;
-
 // Starts collecting breadcrumb events logged to |breadcrumbManager|.
 - (void)observeBreadcrumbManager:(BreadcrumbManager*)breadcrumbManager;
 

ios/chrome/browser/crash_report/crash_reporter_breadcrumb_observer.mm

@@ -12,6 +12,10 @@
 #error "This file requires ARC support."
 #endif
 
+// The breadcrumbs size cannot be larger than the maximum length of a single
+// Breakpad product data value (currently 2550 bytes).
+const NSUInteger kMaxBreadcrumbsDataLength = 1530;
+
 @interface CrashReporterBreadcrumbObserver () {
   // Map associating the observed BreadcrumbManager with the corresponding
   // observer bridge instances.
@@ -42,7 +46,6 @@
 - (instancetype)init {
   if ((self = [super init])) {
     _breadcrumbs = [[NSMutableString alloc] init];
-    _maxProductDataLength = 1530U;  // 6 keys * 255 bytes/key
   }
   return self;
 }
@@ -80,26 +83,14 @@
   NSString* eventWithSeperator = [NSString stringWithFormat:@"%@\n", event];
   [_breadcrumbs insertString:eventWithSeperator atIndex:0];
 
-  NSUInteger maxBreadcrumbsLength =
-      self.breadcrumbsKeyCount * self.maxProductDataLength;
-  if (_breadcrumbs.length > maxBreadcrumbsLength) {
-    NSRange trimRange = NSMakeRange(maxBreadcrumbsLength,
-                                    _breadcrumbs.length - maxBreadcrumbsLength);
+  if (_breadcrumbs.length > kMaxBreadcrumbsDataLength) {
+    NSRange trimRange =
+        NSMakeRange(kMaxBreadcrumbsDataLength,
+                    _breadcrumbs.length - kMaxBreadcrumbsDataLength);
     [_breadcrumbs deleteCharactersInRange:trimRange];
   }
 
-  // Cut breadcrumbs strings into multiple pieces and upload with separate keys.
-  NSMutableArray* breadcrumbs =
-      [[NSMutableArray alloc] initWithCapacity:self.breadcrumbsKeyCount];
-  for (NSUInteger i = 0; i < self.breadcrumbsKeyCount &&
-                         (i * self.maxProductDataLength) < _breadcrumbs.length;
-       i++) {
-    NSUInteger location = i * self.maxProductDataLength;
-    NSRange range = NSMakeRange(location, MIN(self.maxProductDataLength,
-                                              _breadcrumbs.length - location));
-    [breadcrumbs addObject:[_breadcrumbs substringWithRange:range]];
-  }
-  breakpad_helper::SetBreadcrumbEvents(breadcrumbs);
+  breakpad_helper::SetBreadcrumbEvents(_breadcrumbs);
 }
 
 @end

ios/chrome/browser/crash_report/crash_reporter_breadcrumb_observer_unittest.mm

@@ -101,8 +101,6 @@ TEST_F(CrashReporterBreadcrumbObserverTest, EventsAttachedToCrashReport) {
           chrome_browser_state_.get());
   CrashReporterBreadcrumbObserver* crash_reporter_breadcrumb_observer =
       [[CrashReporterBreadcrumbObserver alloc] init];
-  crash_reporter_breadcrumb_observer.breadcrumbsKeyCount =
-      breakpad::kBreadcrumbsKeyCount;
   [crash_reporter_breadcrumb_observer
       observeBreadcrumbManagerService:breadcrumb_service];
 
@@ -118,66 +116,15 @@ TEST_F(CrashReporterBreadcrumbObserverTest, EventsAttachedToCrashReport) {
     return
         [value isEqualToString:base::SysUTF8ToNSString(expected_breadcrumbs)];
   }];
-  NSString* key = [NSString stringWithFormat:breakpad_helper::kBreadcrumbs, 0];
   [[mock_breakpad_controller_ expect]
       addUploadParameter:breadcrumbs_param_vaidation_block
-                  forKey:key];
+                  forKey:breakpad_helper::kBreadcrumbsProductDataKey];
 
   breadcrumb_service->AddEvent(std::string("Breadcrumb Event"));
   EXPECT_OCMOCK_VERIFY(mock_breakpad_controller_);
 }
 
-// Tests that breadcrumb are spit into multiple product data keys.
-TEST_F(CrashReporterBreadcrumbObserverTest, MultipleKeysAttachedToCrashReport) {
-  [[mock_breakpad_controller_ expect] start:NO];
-  breakpad_helper::SetEnabled(true);
-
-  BreadcrumbManagerKeyedService* breadcrumb_service =
-      BreadcrumbManagerKeyedServiceFactory::GetForBrowserState(
-          chrome_browser_state_.get());
-  CrashReporterBreadcrumbObserver* crash_reporter_breadcrumb_observer =
-      [[CrashReporterBreadcrumbObserver alloc] init];
-  crash_reporter_breadcrumb_observer.breadcrumbsKeyCount = 2;
-
-  const int max_product_data_length = 255;
-  crash_reporter_breadcrumb_observer.maxProductDataLength =
-      max_product_data_length;
-  [crash_reporter_breadcrumb_observer
-      observeBreadcrumbManagerService:breadcrumb_service];
-
-  int time_size = strlen("00:00 ");
-  int linebreak_size = strlen("\n");
-  int breadcrumb_size = max_product_data_length - time_size - linebreak_size;
-  std::string value1 = base::StringPrintf("%0*d", breadcrumb_size, 1);
-  id validation_block1 = [OCMArg checkWithBlock:^(id value) {
-    EXPECT_NSEQ(
-        base::SysUTF8ToNSString(value1),
-        [value substringWithRange:NSMakeRange(time_size, breadcrumb_size)]);
-    return YES;
-  }];
-  NSString* key0 = [NSString stringWithFormat:breakpad_helper::kBreadcrumbs, 0];
-  [[mock_breakpad_controller_ expect] addUploadParameter:validation_block1
-                                                  forKey:key0];
-  breadcrumb_service->AddEvent(value1);
-
-  std::string value2 = base::StringPrintf("%0*d", breadcrumb_size, 2);
-  id validation_block2 = [OCMArg checkWithBlock:^(id value) {
-    EXPECT_NSEQ(
-        base::SysUTF8ToNSString(value2),
-        [value substringWithRange:NSMakeRange(time_size, breadcrumb_size)]);
-    return YES;
-  }];
-  NSString* key1 = [NSString stringWithFormat:breakpad_helper::kBreadcrumbs, 1];
-  [[mock_breakpad_controller_ expect] addUploadParameter:validation_block2
-                                                  forKey:key0];
-  [[mock_breakpad_controller_ expect] addUploadParameter:validation_block1
-                                                  forKey:key1];
-  breadcrumb_service->AddEvent(value2);
-  EXPECT_OCMOCK_VERIFY(mock_breakpad_controller_);
-}
-
-// Tests that breadcrumbs string is cut when it does not fit into 2 product data
-// keys.
+// Tests that breadcrumbs string is cut when it exceeds the max allowed length.
 TEST_F(CrashReporterBreadcrumbObserverTest, ProductDataOverflow) {
   [[mock_breakpad_controller_ expect] start:NO];
   breakpad_helper::SetEnabled(true);
@@ -187,58 +134,25 @@ TEST_F(CrashReporterBreadcrumbObserverTest, ProductDataOverflow) {
           chrome_browser_state_.get());
   CrashReporterBreadcrumbObserver* crash_reporter_breadcrumb_observer =
       [[CrashReporterBreadcrumbObserver alloc] init];
-  // Testing with 2 keys requires less code and complexity.
-  crash_reporter_breadcrumb_observer.breadcrumbsKeyCount = 2;
-
-  const int max_product_data_length = 255;
-  crash_reporter_breadcrumb_observer.maxProductDataLength =
-      max_product_data_length;
   [crash_reporter_breadcrumb_observer
       observeBreadcrumbManagerService:breadcrumb_service];
 
-  int time_size = strlen("00:00 ");
-  int linebreak_size = strlen("\n");
-  int breadcrumb_size = max_product_data_length - time_size - linebreak_size;
-  std::string value1 = base::StringPrintf("%0*d", breadcrumb_size, 1);
-  id validation_block1 = [OCMArg checkWithBlock:^(id value) {
-    EXPECT_NSEQ(
-        base::SysUTF8ToNSString(value1),
-        [value substringWithRange:NSMakeRange(time_size, breadcrumb_size)]);
-    return YES;
-  }];
-  NSString* key0 = [NSString stringWithFormat:breakpad_helper::kBreadcrumbs, 0];
-  [[mock_breakpad_controller_ expect] addUploadParameter:validation_block1
-                                                  forKey:key0];
-  breadcrumb_service->AddEvent(value1);
-
-  std::string value2 = base::StringPrintf("%0*d", breadcrumb_size, 2);
-  id validation_block2 = [OCMArg checkWithBlock:^(id value) {
-    EXPECT_NSEQ(
-        base::SysUTF8ToNSString(value2),
-        [value substringWithRange:NSMakeRange(time_size, breadcrumb_size)]);
-    return YES;
-  }];
-  NSString* key1 = [NSString stringWithFormat:breakpad_helper::kBreadcrumbs, 1];
-  [[mock_breakpad_controller_ expect] addUploadParameter:validation_block2
-                                                  forKey:key0];
-  [[mock_breakpad_controller_ expect] addUploadParameter:validation_block1
-                                                  forKey:key1];
-  breadcrumb_service->AddEvent(value2);
+  // Build a sample breadcrumbs string greater than the maximum allowed size.
+  NSMutableString* breadcrumbs = [[NSMutableString alloc] init];
+  while (breadcrumbs.length < kMaxBreadcrumbsDataLength) {
+    [breadcrumbs appendString:@"12:01 Fake Breadcrumb Event/n"];
+  }
+  [breadcrumbs appendString:@"12:01 Fake Breadcrumb Event/n"];
+  ASSERT_GT([breadcrumbs length], kMaxBreadcrumbsDataLength);
 
-  // |value1| will be cut off as overflow.
-  std::string value3 = base::StringPrintf("%0*d", breadcrumb_size, 3);
-  id validation_block3 = [OCMArg checkWithBlock:^(id value) {
-    EXPECT_NSEQ(
-        base::SysUTF8ToNSString(value3),
-        [value substringWithRange:NSMakeRange(time_size, breadcrumb_size)]);
+  id validation_block = [OCMArg checkWithBlock:^(id value) {
+    EXPECT_EQ(kMaxBreadcrumbsDataLength, [value length]);
     return YES;
   }];
-  [[mock_breakpad_controller_ expect] addUploadParameter:validation_block3
-                                                  forKey:key0];
-  [[mock_breakpad_controller_ expect] addUploadParameter:validation_block2
-                                                  forKey:key1];
-  breadcrumb_service->AddEvent(value3);
-
+  [[mock_breakpad_controller_ expect]
+      addUploadParameter:validation_block
+                  forKey:breakpad_helper::kBreadcrumbsProductDataKey];
+  breadcrumb_service->AddEvent(base::SysNSStringToUTF8(breadcrumbs));
   EXPECT_OCMOCK_VERIFY(mock_breakpad_controller_);
 }
 
@@ -258,16 +172,13 @@ TEST_F(CrashReporterBreadcrumbObserverTest,
           chrome_browser_state_.get());
   CrashReporterBreadcrumbObserver* crash_reporter_breadcrumb_observer =
       [[CrashReporterBreadcrumbObserver alloc] init];
-  crash_reporter_breadcrumb_observer.breadcrumbsKeyCount =
-      breakpad::kBreadcrumbsKeyCount;
   [crash_reporter_breadcrumb_observer
       observeBreadcrumbManagerService:breadcrumb_service];
 
-  NSString* key = [NSString stringWithFormat:breakpad_helper::kBreadcrumbs, 0];
   [[mock_breakpad_controller_ expect]
       addUploadParameter:StringParameterValidatorWithCountOfSubstring(
                              1, event_nsstring)
-                  forKey:key];
+                  forKey:breakpad_helper::kBreadcrumbsProductDataKey];
   breadcrumb_service->AddEvent(event);
 
   ChromeBrowserState* otr_browser_state =
@@ -281,7 +192,7 @@ TEST_F(CrashReporterBreadcrumbObserverTest,
   [[mock_breakpad_controller_ expect]
       addUploadParameter:StringParameterValidatorWithCountOfSubstring(
                              2, event_nsstring)
-                  forKey:key];
+                  forKey:breakpad_helper::kBreadcrumbsProductDataKey];
   otr_breadcrumb_service->AddEvent(event);
 
   TestChromeBrowserState::Builder test_cbs_builder;
@@ -296,7 +207,7 @@ TEST_F(CrashReporterBreadcrumbObserverTest,
   [[mock_breakpad_controller_ expect]
       addUploadParameter:StringParameterValidatorWithCountOfSubstring(
                              3, event_nsstring)
-                  forKey:key];
+                  forKey:breakpad_helper::kBreadcrumbsProductDataKey];
   breadcrumb_service_2->AddEvent(event);
 
   EXPECT_OCMOCK_VERIFY(mock_breakpad_controller_);