Commit f5116ada authored by Eugene But's avatar Eugene But Committed by Commit Bot

[Breadcrumbs] Use 6 product data keys for Breadcrumbs

Most crashes with known steps to repro require 4 product data keys to
fit all steps. Users will likely do unnecessary steps between required
steps, so using 2 extra keys should allow to accommodate for all steps.

Bug: 1046223
Change-Id: Ia6b97453822a2ab7e6b80cc11488f9f0f91fc949
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2089720
Commit-Queue: Eugene But <eugenebut@chromium.org>
Auto-Submit: Eugene But <eugenebut@chromium.org>
Reviewed-by: default avatarMike Dougherty <michaeldo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748911}
parent 192b7988
...@@ -371,7 +371,8 @@ void RemoveGridToVisibleTabAnimation() { ...@@ -371,7 +371,8 @@ void RemoveGridToVisibleTabAnimation() {
} }
void SetBreadcrumbEvents(NSArray* breadcrumbs) { void SetBreadcrumbEvents(NSArray* breadcrumbs) {
DCHECK_LE(breadcrumbs.count, 4U); DCHECK_GT(breadcrumbs.count, 0U);
DCHECK_LE(breadcrumbs.count, 6U);
for (NSUInteger i = 0; i < breadcrumbs.count; i++) { for (NSUInteger i = 0; i < breadcrumbs.count; i++) {
NSString* key = [NSString stringWithFormat:kBreadcrumbs, i]; NSString* key = [NSString stringWithFormat:kBreadcrumbs, i];
AddReportParameter(key, breadcrumbs[i], /*async=*/true); AddReportParameter(key, breadcrumbs[i], /*async=*/true);
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
// found in the LICENSE file. // found in the LICENSE file.
#import "ios/chrome/browser/crash_report/breakpad_helper.h" #import "ios/chrome/browser/crash_report/breakpad_helper.h"
#include "ios/chrome/browser/crash_report/crash_report_helper.h"
#include "ios/chrome/browser/crash_report/main_thread_freeze_detector.h" #include "ios/chrome/browser/crash_report/main_thread_freeze_detector.h"
#import "ios/chrome/test/ocmock/OCMockObject+BreakpadControllerTesting.h" #import "ios/chrome/test/ocmock/OCMockObject+BreakpadControllerTesting.h"
#import "ios/testing/scoped_block_swizzler.h" #import "ios/testing/scoped_block_swizzler.h"
...@@ -82,7 +84,12 @@ TEST_F(BreakpadHelperTest, CrashReportUserApplicationStateAllKeys) { ...@@ -82,7 +84,12 @@ TEST_F(BreakpadHelperTest, CrashReportUserApplicationStateAllKeys) {
while (breadcrumbs.length < 255) { while (breadcrumbs.length < 255) {
[breadcrumbs appendString:@"12:01 Fake Breadcrumb Event/n"]; [breadcrumbs appendString:@"12:01 Fake Breadcrumb Event/n"];
} }
breakpad_helper::SetBreadcrumbEvents(@[ breadcrumbs, breadcrumbs ]);
NSMutableArray* events = [[NSMutableArray alloc] init];
for (int i = 0; i < breakpad::kBreadcrumbsKeyCount; i++) {
[events addObject:breadcrumbs];
}
breakpad_helper::SetBreadcrumbEvents(events);
} }
TEST_F(BreakpadHelperTest, GetCrashReportCount) { TEST_F(BreakpadHelperTest, GetCrashReportCount) {
......
...@@ -18,6 +18,12 @@ class WebStateList; ...@@ -18,6 +18,12 @@ class WebStateList;
namespace breakpad { 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 // Monitors the urls loaded by |web_state| to allow crash reports to contain the
// current loading url. // current loading url.
void MonitorURLsForWebState(web::WebState* web_state); void MonitorURLsForWebState(web::WebState* web_state);
......
...@@ -37,6 +37,12 @@ ...@@ -37,6 +37,12 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
namespace breakpad {
// IMPORTANT: be careful if ever increasing this value, Breakpad reports have an
// overall size limit
const int kBreadcrumbsKeyCount = 6;
}
// WebStateListObserver that allows loaded urls to be sent to the crash server. // WebStateListObserver that allows loaded urls to be sent to the crash server.
@interface CrashReporterURLObserver @interface CrashReporterURLObserver
: NSObject <WebStateListObserving, CRWWebStateObserver> { : NSObject <WebStateListObserving, CRWWebStateObserver> {
...@@ -442,6 +448,8 @@ void ClearStateForWebStateList(WebStateList* web_state_list) { ...@@ -442,6 +448,8 @@ void ClearStateForWebStateList(WebStateList* web_state_list) {
void MonitorBreadcrumbManager(BreadcrumbManager* breadcrumb_manager) { void MonitorBreadcrumbManager(BreadcrumbManager* breadcrumb_manager) {
[[CrashReporterBreadcrumbObserver uniqueInstance] [[CrashReporterBreadcrumbObserver uniqueInstance]
observeBreadcrumbManager:breadcrumb_manager]; observeBreadcrumbManager:breadcrumb_manager];
[CrashReporterBreadcrumbObserver uniqueInstance].breadcrumbsKeyCount =
kBreadcrumbsKeyCount;
} }
void StopMonitoringBreadcrumbManager(BreadcrumbManager* breadcrumb_manager) { void StopMonitoringBreadcrumbManager(BreadcrumbManager* breadcrumb_manager) {
......
...@@ -24,6 +24,9 @@ extern const int kMaxProductDataLength; ...@@ -24,6 +24,9 @@ extern const int kMaxProductDataLength;
// Creates a singleton instance. // Creates a singleton instance.
+ (CrashReporterBreadcrumbObserver*)uniqueInstance; + (CrashReporterBreadcrumbObserver*)uniqueInstance;
// Number of product data keys to use for breadcrumbs.
@property(nonatomic) NSUInteger breadcrumbsKeyCount;
// Starts collecting breadcrumb events logged to |breadcrumbManager|. // Starts collecting breadcrumb events logged to |breadcrumbManager|.
- (void)observeBreadcrumbManager:(BreadcrumbManager*)breadcrumbManager; - (void)observeBreadcrumbManager:(BreadcrumbManager*)breadcrumbManager;
......
...@@ -12,15 +12,6 @@ ...@@ -12,15 +12,6 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
namespace {
// IMPORTANT: the value of this constant should not exceed 4.
// 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.
const int kBreadcrumbsKeyCount = 2;
}
const int kMaxProductDataLength = 255; const int kMaxProductDataLength = 255;
@interface CrashReporterBreadcrumbObserver () { @interface CrashReporterBreadcrumbObserver () {
...@@ -91,7 +82,7 @@ const int kMaxProductDataLength = 255; ...@@ -91,7 +82,7 @@ const int kMaxProductDataLength = 255;
[_breadcrumbs insertString:eventWithSeperator atIndex:0]; [_breadcrumbs insertString:eventWithSeperator atIndex:0];
NSUInteger maxBreadcrumbsLength = NSUInteger maxBreadcrumbsLength =
kBreadcrumbsKeyCount * kMaxProductDataLength; self.breadcrumbsKeyCount * kMaxProductDataLength;
if (_breadcrumbs.length > maxBreadcrumbsLength) { if (_breadcrumbs.length > maxBreadcrumbsLength) {
NSRange trimRange = NSMakeRange(maxBreadcrumbsLength, NSRange trimRange = NSMakeRange(maxBreadcrumbsLength,
_breadcrumbs.length - maxBreadcrumbsLength); _breadcrumbs.length - maxBreadcrumbsLength);
...@@ -100,8 +91,8 @@ const int kMaxProductDataLength = 255; ...@@ -100,8 +91,8 @@ const int kMaxProductDataLength = 255;
// Cut breadcrumbs strings into multiple pieces and upload with separate keys. // Cut breadcrumbs strings into multiple pieces and upload with separate keys.
NSMutableArray* breadcrumbs = NSMutableArray* breadcrumbs =
[[NSMutableArray alloc] initWithCapacity:kBreadcrumbsKeyCount]; [[NSMutableArray alloc] initWithCapacity:self.breadcrumbsKeyCount];
for (NSUInteger i = 0; i < kBreadcrumbsKeyCount && for (NSUInteger i = 0; i < self.breadcrumbsKeyCount &&
(i * kMaxProductDataLength) < _breadcrumbs.length; (i * kMaxProductDataLength) < _breadcrumbs.length;
i++) { i++) {
NSUInteger location = i * kMaxProductDataLength; NSUInteger location = i * kMaxProductDataLength;
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service.h" #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service.h"
#include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service_factory.h" #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service_factory.h"
#import "ios/chrome/browser/crash_report/breakpad_helper.h" #import "ios/chrome/browser/crash_report/breakpad_helper.h"
#include "ios/chrome/browser/crash_report/crash_report_helper.h"
#import "ios/chrome/test/ocmock/OCMockObject+BreakpadControllerTesting.h" #import "ios/chrome/test/ocmock/OCMockObject+BreakpadControllerTesting.h"
#import "ios/testing/scoped_block_swizzler.h" #import "ios/testing/scoped_block_swizzler.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -100,6 +101,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest, EventsAttachedToCrashReport) { ...@@ -100,6 +101,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest, EventsAttachedToCrashReport) {
chrome_browser_state_.get()); chrome_browser_state_.get());
CrashReporterBreadcrumbObserver* crash_reporter_breadcrumb_observer = CrashReporterBreadcrumbObserver* crash_reporter_breadcrumb_observer =
[[CrashReporterBreadcrumbObserver alloc] init]; [[CrashReporterBreadcrumbObserver alloc] init];
crash_reporter_breadcrumb_observer.breadcrumbsKeyCount =
breakpad::kBreadcrumbsKeyCount;
[crash_reporter_breadcrumb_observer [crash_reporter_breadcrumb_observer
observeBreadcrumbManagerService:breadcrumb_service]; observeBreadcrumbManagerService:breadcrumb_service];
...@@ -134,6 +137,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest, MultipleKeysAttachedToCrashReport) { ...@@ -134,6 +137,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest, MultipleKeysAttachedToCrashReport) {
chrome_browser_state_.get()); chrome_browser_state_.get());
CrashReporterBreadcrumbObserver* crash_reporter_breadcrumb_observer = CrashReporterBreadcrumbObserver* crash_reporter_breadcrumb_observer =
[[CrashReporterBreadcrumbObserver alloc] init]; [[CrashReporterBreadcrumbObserver alloc] init];
crash_reporter_breadcrumb_observer.breadcrumbsKeyCount =
breakpad::kBreadcrumbsKeyCount;
[crash_reporter_breadcrumb_observer [crash_reporter_breadcrumb_observer
observeBreadcrumbManagerService:breadcrumb_service]; observeBreadcrumbManagerService:breadcrumb_service];
...@@ -179,6 +184,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest, ProductDataOverflow) { ...@@ -179,6 +184,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest, ProductDataOverflow) {
chrome_browser_state_.get()); chrome_browser_state_.get());
CrashReporterBreadcrumbObserver* crash_reporter_breadcrumb_observer = CrashReporterBreadcrumbObserver* crash_reporter_breadcrumb_observer =
[[CrashReporterBreadcrumbObserver alloc] init]; [[CrashReporterBreadcrumbObserver alloc] init];
// Testing with 2 keys requires less code and complexity.
crash_reporter_breadcrumb_observer.breadcrumbsKeyCount = 2;
[crash_reporter_breadcrumb_observer [crash_reporter_breadcrumb_observer
observeBreadcrumbManagerService:breadcrumb_service]; observeBreadcrumbManagerService:breadcrumb_service];
...@@ -244,6 +251,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest, ...@@ -244,6 +251,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest,
chrome_browser_state_.get()); chrome_browser_state_.get());
CrashReporterBreadcrumbObserver* crash_reporter_breadcrumb_observer = CrashReporterBreadcrumbObserver* crash_reporter_breadcrumb_observer =
[[CrashReporterBreadcrumbObserver alloc] init]; [[CrashReporterBreadcrumbObserver alloc] init];
crash_reporter_breadcrumb_observer.breadcrumbsKeyCount =
breakpad::kBreadcrumbsKeyCount;
[crash_reporter_breadcrumb_observer [crash_reporter_breadcrumb_observer
observeBreadcrumbManagerService:breadcrumb_service]; observeBreadcrumbManagerService:breadcrumb_service];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment