Commit 52937dcb authored by Justin Cohen's avatar Justin Cohen Committed by Chromium LUCI CQ

ios: Migrate from breakpad report parameter to CrashKeyString.

CrashKeyString wraps AddReportParameter/RemoveReportParameter when using
breakpad.

Bug: 1061431
Change-Id: Ia651045b8efe2ae79b769a099d49d5d85e00688b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2574024Reviewed-by: default avatarMark Mentovai <mark@chromium.org>
Reviewed-by: default avatarOlivier Robin <olivierrobin@chromium.org>
Reviewed-by: default avatarRohit Rao <rohitrao@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Auto-Submit: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843961}
parent fac99e7b
...@@ -99,7 +99,10 @@ target(crash_key_target_type, "crash_key_lib") { ...@@ -99,7 +99,10 @@ target(crash_key_target_type, "crash_key_lib") {
if (is_ios) { if (is_ios) {
sources += [ "crash_key_breakpad_ios.mm" ] sources += [ "crash_key_breakpad_ios.mm" ]
deps += [ "//components/previous_session_info" ] deps += [
":breakpad_running_ios",
"//components/previous_session_info",
]
configs += [ "//build/config/compiler:enable_arc" ] configs += [ "//build/config/compiler:enable_arc" ]
} else { } else {
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <dispatch/dispatch.h> #include <dispatch/dispatch.h>
#include "base/strings/sys_string_conversions.h" #include "base/strings/sys_string_conversions.h"
#include "components/crash/core/common/breakpad_running_ios.h"
#include "components/crash/core/common/crash_key_base_support.h" #include "components/crash/core/common/crash_key_base_support.h"
#import "components/previous_session_info/previous_session_info.h" #import "components/previous_session_info/previous_session_info.h"
#import "third_party/breakpad/breakpad/src/client/ios/Breakpad.h" #import "third_party/breakpad/breakpad/src/client/ios/Breakpad.h"
...@@ -40,37 +41,34 @@ void WithBreakpadRefSync(void (^block)(BreakpadRef ref)) { ...@@ -40,37 +41,34 @@ void WithBreakpadRefSync(void (^block)(BreakpadRef ref)) {
dispatch_semaphore_wait(semaphore, DISPATCH_TIME_FOREVER); dispatch_semaphore_wait(semaphore, DISPATCH_TIME_FOREVER);
} }
// When setting a value, avoid to block the current thread.
void WithBreakpadRefAsync(void (^block)(BreakpadRef ref)) {
[[BreakpadController sharedInstance] withBreakpadRef:^(BreakpadRef ref) {
block(ref);
}];
}
} // namespace } // namespace
void CrashKeyStringImpl::Set(base::StringPiece value) { void CrashKeyStringImpl::Set(base::StringPiece value) {
if (!crash_reporter::IsBreakpadRunning())
return;
NSString* key = base::SysUTF8ToNSString(name_); NSString* key = base::SysUTF8ToNSString(name_);
NSString* value_ns = base::SysUTF8ToNSString(value.as_string()); NSString* value_ns = base::SysUTF8ToNSString(value.as_string());
WithBreakpadRefAsync(^(BreakpadRef ref) { [[BreakpadController sharedInstance] addUploadParameter:value_ns forKey:key];
BreakpadAddUploadParameter(ref, key, value_ns);
});
[[PreviousSessionInfo sharedInstance] setReportParameterValue:value_ns [[PreviousSessionInfo sharedInstance] setReportParameterValue:value_ns
forKey:key]; forKey:key];
} }
void CrashKeyStringImpl::Clear() { void CrashKeyStringImpl::Clear() {
if (!crash_reporter::IsBreakpadRunning())
return;
NSString* key = base::SysUTF8ToNSString(name_); NSString* key = base::SysUTF8ToNSString(name_);
WithBreakpadRefAsync(^(BreakpadRef ref) { [[BreakpadController sharedInstance] removeUploadParameterForKey:key];
BreakpadRemoveUploadParameter(ref, key);
});
[[PreviousSessionInfo sharedInstance] removeReportParameterForKey:key]; [[PreviousSessionInfo sharedInstance] removeReportParameterForKey:key];
} }
bool CrashKeyStringImpl::is_set() const { bool CrashKeyStringImpl::is_set() const {
if (!crash_reporter::IsBreakpadRunning())
return false;
__block bool is_set = false; __block bool is_set = false;
NSString* key = base::SysUTF8ToNSString( NSString* key = base::SysUTF8ToNSString(
std::string(BREAKPAD_SERVER_PARAMETER_PREFIX) + name_); std::string(BREAKPAD_SERVER_PARAMETER_PREFIX) + name_);
...@@ -105,11 +103,13 @@ void InitializeCrashKeysForTesting() { ...@@ -105,11 +103,13 @@ void InitializeCrashKeysForTesting() {
@BREAKPAD_URL : @"http://breakpad.test" @BREAKPAD_URL : @"http://breakpad.test"
}]; }];
[[BreakpadController sharedInstance] start:YES]; [[BreakpadController sharedInstance] start:YES];
crash_reporter::SetBreakpadRunning(true);
InitializeCrashKeys(); InitializeCrashKeys();
} }
void ResetCrashKeysForTesting() { void ResetCrashKeysForTesting() {
[[BreakpadController sharedInstance] stop]; [[BreakpadController sharedInstance] stop];
crash_reporter::SetBreakpadRunning(false);
} }
} // namespace crash_reporter } // namespace crash_reporter
...@@ -25,7 +25,6 @@ ...@@ -25,7 +25,6 @@
#import "ios/web/public/test/web_task_environment.h" #import "ios/web/public/test/web_task_environment.h"
#include "net/base/network_change_notifier.h" #include "net/base/network_change_notifier.h"
#include "testing/platform_test.h" #include "testing/platform_test.h"
#import "third_party/breakpad/breakpad/src/client/ios/BreakpadController.h"
#import "third_party/ocmock/OCMock/OCMock.h" #import "third_party/ocmock/OCMock/OCMock.h"
#include "third_party/ocmock/gtest_support.h" #include "third_party/ocmock/gtest_support.h"
......
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
#include "base/at_exit.h" #include "base/at_exit.h"
#include "base/debug/crash_logging.h" #include "base/debug/crash_logging.h"
#include "base/strings/sys_string_conversions.h" #include "base/strings/sys_string_conversions.h"
#include "components/crash/core/common/crash_keys.h"
#include "ios/chrome/app/startup/ios_chrome_main.h" #include "ios/chrome/app/startup/ios_chrome_main.h"
#include "ios/chrome/app/startup/ios_enable_sandbox_dump_buildflags.h" #include "ios/chrome/app/startup/ios_enable_sandbox_dump_buildflags.h"
#include "ios/chrome/browser/crash_report/breakpad_helper.h" #include "ios/chrome/browser/crash_report/breakpad_helper.h"
...@@ -28,10 +27,7 @@ NSString* const kUIApplicationDelegateInfoKey = @"UIApplicationDelegate"; ...@@ -28,10 +27,7 @@ NSString* const kUIApplicationDelegateInfoKey = @"UIApplicationDelegate";
void StartCrashController() { void StartCrashController() {
@autoreleasepool { @autoreleasepool {
std::string channel_string = GetChannelString(); breakpad_helper::Start(GetChannelString());
breakpad_helper::AddReportParameter(
@"channel", base::SysUTF8ToNSString(channel_string), false);
breakpad_helper::Start(channel_string);
} }
} }
......
...@@ -61,6 +61,7 @@ source_set("crash_report_internal") { ...@@ -61,6 +61,7 @@ source_set("crash_report_internal") {
deps = [ deps = [
":crash_report", ":crash_report",
"//base", "//base",
"//components/crash/core/common:crash_key_lib",
"//components/infobars/core", "//components/infobars/core",
"//components/previous_session_info", "//components/previous_session_info",
"//components/sessions", "//components/sessions",
......
...@@ -32,13 +32,6 @@ bool UserEnabledUploading(); ...@@ -32,13 +32,6 @@ bool UserEnabledUploading();
// Cleans up all stored crash reports. // Cleans up all stored crash reports.
void CleanupCrashReports(); void CleanupCrashReports();
// Add a key/value pair the next crash report. If async is false, this function
// will wait until the key is registered before returning.
void AddReportParameter(NSString* key, NSString* value, bool async);
// Remove the key/value pair associated to key from the next crash report.
void RemoveReportParameter(NSString* key);
// Returns the number of crash reports waiting to send to the server. This // Returns the number of crash reports waiting to send to the server. This
// function will wait for an operation to complete on a background thread. // function will wait for an operation to complete on a background thread.
int GetCrashReportCount(); int GetCrashReportCount();
......
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include "base/task/post_task.h" #include "base/task/post_task.h"
#include "base/task/thread_pool.h" #include "base/task/thread_pool.h"
#include "components/crash/core/common/breakpad_running_ios.h" #include "components/crash/core/common/breakpad_running_ios.h"
#include "components/crash/core/common/crash_key.h"
#include "ios/chrome/browser/chrome_paths.h" #include "ios/chrome/browser/chrome_paths.h"
#import "ios/chrome/browser/crash_report/crash_report_user_application_state.h" #import "ios/chrome/browser/crash_report/crash_report_user_application_state.h"
#import "ios/chrome/browser/crash_report/main_thread_freeze_detector.h" #import "ios/chrome/browser/crash_report/main_thread_freeze_detector.h"
...@@ -83,10 +84,10 @@ bool FatalMessageHandler(int severity, ...@@ -83,10 +84,10 @@ bool FatalMessageHandler(int severity,
file = slash + 1; file = slash + 1;
} }
NSString* fatal_key = @"LOG_FATAL";
NSString* fatal_value = [NSString NSString* fatal_value = [NSString
stringWithFormat:@"%s:%d: %s", file, line, str.c_str() + message_start]; stringWithFormat:@"%s:%d: %s", file, line, str.c_str() + message_start];
AddReportParameter(fatal_key, fatal_value, true); static crash_reporter::CrashKeyString<2550> key("LOG_FATAL");
key.Set(base::SysNSStringToUTF8(fatal_value));
// Rather than including the code to force the crash here, allow the // Rather than including the code to force the crash here, allow the
// caller to do it. // caller to do it.
...@@ -108,7 +109,8 @@ void Start(const std::string& channel_name) { ...@@ -108,7 +109,8 @@ void Start(const std::string& channel_name) {
crash_reporter::SetBreakpadRunning(true); crash_reporter::SetBreakpadRunning(true);
// Register channel information. // Register channel information.
if (channel_name.length()) { if (channel_name.length()) {
AddReportParameter(@"channel", base::SysUTF8ToNSString(channel_name), true); static crash_reporter::CrashKeyString<64> key("channel");
key.Set(channel_name);
} }
// Notifying the PathService on the location of the crashes so that crashes // Notifying the PathService on the location of the crashes so that crashes
// can be displayed to the user on the about:crashes page. // can be displayed to the user on the about:crashes page.
...@@ -185,22 +187,6 @@ void CleanupCrashReports() { ...@@ -185,22 +187,6 @@ void CleanupCrashReports() {
base::BindOnce(&DeleteAllReportsInDirectory, crash_directory)); base::BindOnce(&DeleteAllReportsInDirectory, crash_directory));
} }
void AddReportParameter(NSString* key, NSString* value, bool async) {
if (!crash_reporter::IsBreakpadRunning())
return;
if (async) {
[[BreakpadController sharedInstance] addUploadParameter:value forKey:key];
return;
}
dispatch_semaphore_t semaphore = dispatch_semaphore_create(0);
[[BreakpadController sharedInstance] withBreakpadRef:^(BreakpadRef ref) {
if (ref)
BreakpadAddUploadParameter(ref, key, value);
dispatch_semaphore_signal(semaphore);
}];
dispatch_semaphore_wait(semaphore, DISPATCH_TIME_FOREVER);
}
int GetCrashReportCount() { int GetCrashReportCount() {
dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); dispatch_semaphore_t semaphore = dispatch_semaphore_create(0);
__block int outerCrashReportCount = 0; __block int outerCrashReportCount = 0;
...@@ -220,12 +206,6 @@ bool HasReportToUpload() { ...@@ -220,12 +206,6 @@ bool HasReportToUpload() {
return GetCrashReportCount() > 0; return GetCrashReportCount() > 0;
} }
void RemoveReportParameter(NSString* key) {
if (!crash_reporter::IsBreakpadRunning())
return;
[[BreakpadController sharedInstance] removeUploadParameterForKey:key];
}
// Records the current process uptime in the "uptime_at_restore_in_ms". This // Records the current process uptime in the "uptime_at_restore_in_ms". This
// will allow engineers to dremel crash logs to find crash whose delta between // will allow engineers to dremel crash logs to find crash whose delta between
// process uptime at crash and process uptime at restore is smaller than X // process uptime at crash and process uptime at restore is smaller than X
......
...@@ -13,7 +13,7 @@ ...@@ -13,7 +13,7 @@
namespace crash_keys { namespace crash_keys {
// Key for breadcrumbs attached to crash reports. // Key for breadcrumbs attached to crash reports.
extern NSString* const kBreadcrumbsProductDataKey; extern const char kBreadcrumbsProductDataKey[];
// Sets a key if |background| is true, unset if false. This will allow tracking // Sets a key if |background| is true, unset if false. This will allow tracking
// of crashes that occur when the app is backgrounded. // of crashes that occur when the app is backgrounded.
......
...@@ -5,8 +5,10 @@ ...@@ -5,8 +5,10 @@
#import "ios/chrome/browser/crash_report/crash_keys_helper.h" #import "ios/chrome/browser/crash_report/crash_keys_helper.h"
#include "base/check.h" #include "base/check.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/sys_string_conversions.h"
#include "components/crash/core/common/crash_key.h"
#import "components/previous_session_info/previous_session_info.h" #import "components/previous_session_info/previous_session_info.h"
#import "ios/chrome/browser/crash_report/breakpad_helper.h"
#import "ios/chrome/browser/crash_report/crash_report_user_application_state.h" #import "ios/chrome/browser/crash_report/crash_report_user_application_state.h"
#import "ios/chrome/browser/crash_report/main_thread_freeze_detector.h" #import "ios/chrome/browser/crash_report/main_thread_freeze_detector.h"
...@@ -14,21 +16,20 @@ ...@@ -14,21 +16,20 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
using breakpad_helper::AddReportParameter;
using breakpad_helper::RemoveReportParameter;
namespace crash_keys { namespace crash_keys {
NSString* const kBreadcrumbsProductDataKey = @"breadcrumbs"; const char kBreadcrumbsProductDataKey[] = "breadcrumbs";
namespace { namespace {
NSString* const kCrashedInBackground = @"crashed_in_background"; const char kCrashedInBackground[] = "crashed_in_background";
NSString* const kFreeDiskInKB = @"free_disk_in_kb"; const char kFreeDiskInKB[] = "free_disk_in_kb";
NSString* const kFreeMemoryInKB = @"free_memory_in_kb"; const char kFreeMemoryInKB[] = "free_memory_in_kb";
NSString* const kMemoryWarningInProgress = @"memory_warning_in_progress"; const char kMemoryWarningInProgress[] = "memory_warning_in_progress";
NSString* const kMemoryWarningCount = @"memory_warning_count"; const char kMemoryWarningCount[] = "memory_warning_count";
NSString* const kGridToVisibleTabAnimation = @"grid_to_visible_tab_animation"; const char kGridToVisibleTabAnimation[] = "grid_to_visible_tab_animation";
static crash_reporter::CrashKeyString<1028> kRemoveGridToVisibleTabAnimationKey(
kGridToVisibleTabAnimation);
// Multiple state information are combined into one CrashReportMultiParameter // Multiple state information are combined into one CrashReportMultiParameter
// to save limited and finite number of ReportParameters. // to save limited and finite number of ReportParameters.
...@@ -47,39 +48,41 @@ NSString* const kDestroyingAndRebuildingIncognitoBrowserState = ...@@ -47,39 +48,41 @@ NSString* const kDestroyingAndRebuildingIncognitoBrowserState =
} // namespace } // namespace
void SetCurrentlyInBackground(bool background) { void SetCurrentlyInBackground(bool background) {
static crash_reporter::CrashKeyString<4> key(kCrashedInBackground);
if (background) { if (background) {
AddReportParameter(kCrashedInBackground, @"yes", true); key.Set("yes");
[[MainThreadFreezeDetector sharedInstance] stop]; [[MainThreadFreezeDetector sharedInstance] stop];
} else { } else {
RemoveReportParameter(kCrashedInBackground); key.Clear();
[[MainThreadFreezeDetector sharedInstance] start]; [[MainThreadFreezeDetector sharedInstance] start];
} }
} }
void SetMemoryWarningCount(int count) { void SetMemoryWarningCount(int count) {
static crash_reporter::CrashKeyString<16> key(kMemoryWarningCount);
if (count) { if (count) {
AddReportParameter(kMemoryWarningCount, key.Set(base::NumberToString(count));
[NSString stringWithFormat:@"%d", count], true);
} else { } else {
RemoveReportParameter(kMemoryWarningCount); key.Clear();
} }
} }
void SetMemoryWarningInProgress(bool value) { void SetMemoryWarningInProgress(bool value) {
static crash_reporter::CrashKeyString<4> key(kMemoryWarningInProgress);
if (value) if (value)
AddReportParameter(kMemoryWarningInProgress, @"yes", true); key.Set("yes");
else else
RemoveReportParameter(kMemoryWarningInProgress); key.Clear();
} }
void SetCurrentFreeMemoryInKB(int value) { void SetCurrentFreeMemoryInKB(int value) {
AddReportParameter(kFreeMemoryInKB, [NSString stringWithFormat:@"%d", value], static crash_reporter::CrashKeyString<16> key(kFreeMemoryInKB);
true); key.Set(base::NumberToString(value));
} }
void SetCurrentFreeDiskInKB(int value) { void SetCurrentFreeDiskInKB(int value) {
AddReportParameter(kFreeDiskInKB, [NSString stringWithFormat:@"%d", value], static crash_reporter::CrashKeyString<16> key(kFreeDiskInKB);
true); key.Set(base::NumberToString(value));
} }
void SetCurrentTabIsPDF(bool value) { void SetCurrentTabIsPDF(bool value) {
...@@ -154,15 +157,17 @@ void SetGridToVisibleTabAnimation(NSString* to_view_controller, ...@@ -154,15 +157,17 @@ void SetGridToVisibleTabAnimation(NSString* to_view_controller,
@"{toVC:%@, presentingVC:%@, presentedVC:%@, parentVC:%@}", @"{toVC:%@, presentingVC:%@, presentedVC:%@, parentVC:%@}",
to_view_controller, presenting_view_controller, to_view_controller, presenting_view_controller,
presented_view_controller, parent_view_controller]; presented_view_controller, parent_view_controller];
AddReportParameter(kGridToVisibleTabAnimation, formatted_value, true); kRemoveGridToVisibleTabAnimationKey.Set(
base::SysNSStringToUTF8(formatted_value));
} }
void RemoveGridToVisibleTabAnimation() { void RemoveGridToVisibleTabAnimation() {
RemoveReportParameter(kGridToVisibleTabAnimation); kRemoveGridToVisibleTabAnimationKey.Clear();
} }
void SetBreadcrumbEvents(NSString* breadcrumbs) { void SetBreadcrumbEvents(NSString* breadcrumbs) {
AddReportParameter(kBreadcrumbsProductDataKey, breadcrumbs, /*async=*/true); static crash_reporter::CrashKeyString<2550> key(kBreadcrumbsProductDataKey);
key.Set(base::SysNSStringToUTF8(breadcrumbs));
} }
void MediaStreamPlaybackDidStart() { void MediaStreamPlaybackDidStart() {
...@@ -175,4 +180,4 @@ void MediaStreamPlaybackDidStop() { ...@@ -175,4 +180,4 @@ void MediaStreamPlaybackDidStop() {
decrementValue:kVideoPlaying]; decrementValue:kVideoPlaying];
} }
} // namespace breakpad_helper } // namespace crash_keys
...@@ -7,12 +7,15 @@ ...@@ -7,12 +7,15 @@
#import <Foundation/Foundation.h> #import <Foundation/Foundation.h>
#include "components/crash/core/common/crash_key.h"
// CrashReportMultiParameter keeps state of multiple report values that will be // CrashReportMultiParameter keeps state of multiple report values that will be
// grouped in a single breakpad element to save limited number of breakpad // grouped in a single breakpad element to save limited number of breakpad
// values. // values.
@interface CrashReportMultiParameter : NSObject @interface CrashReportMultiParameter : NSObject
// Init with the breakpad parameter key. // Init with the breakpad parameter key.
- (instancetype)initWithKey:(NSString*)key NS_DESIGNATED_INITIALIZER; - (instancetype)initWithKey:(crash_reporter::CrashKeyString<256>&)key
NS_DESIGNATED_INITIALIZER;
- (instancetype)init NS_UNAVAILABLE; - (instancetype)init NS_UNAVAILABLE;
......
...@@ -26,15 +26,14 @@ const int kMaximumBreakpadValueSize = 255; ...@@ -26,15 +26,14 @@ const int kMaximumBreakpadValueSize = 255;
} }
@implementation CrashReportMultiParameter { @implementation CrashReportMultiParameter {
NSString* _crashReportKey; crash_reporter::CrashKeyString<256>* _key;
std::unique_ptr<base::DictionaryValue> _dictionary; std::unique_ptr<base::DictionaryValue> _dictionary;
} }
- (instancetype)initWithKey:(NSString*)key { - (instancetype)initWithKey:(crash_reporter::CrashKeyString<256>&)key {
if ((self = [super init])) { if ((self = [super init])) {
DCHECK([key length] && ([key length] <= kMaximumBreakpadValueSize));
_dictionary.reset(new base::DictionaryValue()); _dictionary.reset(new base::DictionaryValue());
_crashReportKey = [key copy]; _key = &key;
} }
return self; return self;
} }
...@@ -80,11 +79,7 @@ const int kMaximumBreakpadValueSize = 255; ...@@ -80,11 +79,7 @@ const int kMaximumBreakpadValueSize = 255;
NOTREACHED(); NOTREACHED();
return; return;
} }
NSString* value = base::SysUTF8ToNSString(stateAsJson); _key->Set(stateAsJson);
breakpad_helper::AddReportParameter(_crashReportKey, value, /*async=*/true);
[[PreviousSessionInfo sharedInstance]
setReportParameterValue:value
forKey:_crashReportKey];
} }
@end @end
...@@ -4,19 +4,18 @@ ...@@ -4,19 +4,18 @@
#import "ios/chrome/browser/crash_report/crash_report_user_application_state.h" #import "ios/chrome/browser/crash_report/crash_report_user_application_state.h"
#include "components/crash/core/common/crash_key.h"
#if !defined(__has_feature) || !__has_feature(objc_arc) #if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
namespace {
NSString* const kBrowserState = @"user_application_state";
}
@implementation CrashReportUserApplicationState @implementation CrashReportUserApplicationState
+ (CrashReportUserApplicationState*)sharedInstance { + (CrashReportUserApplicationState*)sharedInstance {
static crash_reporter::CrashKeyString<256> key("user_application_state");
static CrashReportUserApplicationState* instance = static CrashReportUserApplicationState* instance =
[[CrashReportUserApplicationState alloc] initWithKey:kBrowserState]; [[CrashReportUserApplicationState alloc] initWithKey:key];
return instance; return instance;
} }
......
...@@ -119,7 +119,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest, EventsAttachedToCrashReport) { ...@@ -119,7 +119,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest, EventsAttachedToCrashReport) {
}]; }];
[[mock_breakpad_controller_ expect] [[mock_breakpad_controller_ expect]
addUploadParameter:breadcrumbs_param_vaidation_block addUploadParameter:breadcrumbs_param_vaidation_block
forKey:crash_keys::kBreadcrumbsProductDataKey]; forKey:base::SysUTF8ToNSString(
crash_keys::kBreadcrumbsProductDataKey)];
breadcrumb_service->AddEvent(std::string("Breadcrumb Event")); breadcrumb_service->AddEvent(std::string("Breadcrumb Event"));
EXPECT_OCMOCK_VERIFY(mock_breakpad_controller_); EXPECT_OCMOCK_VERIFY(mock_breakpad_controller_);
...@@ -152,7 +153,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest, ProductDataOverflow) { ...@@ -152,7 +153,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest, ProductDataOverflow) {
}]; }];
[[mock_breakpad_controller_ expect] [[mock_breakpad_controller_ expect]
addUploadParameter:validation_block addUploadParameter:validation_block
forKey:crash_keys::kBreadcrumbsProductDataKey]; forKey:base::SysUTF8ToNSString(
crash_keys::kBreadcrumbsProductDataKey)];
breadcrumb_service->AddEvent(base::SysNSStringToUTF8(breadcrumbs)); breadcrumb_service->AddEvent(base::SysNSStringToUTF8(breadcrumbs));
EXPECT_OCMOCK_VERIFY(mock_breakpad_controller_); EXPECT_OCMOCK_VERIFY(mock_breakpad_controller_);
} }
...@@ -179,7 +181,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest, ...@@ -179,7 +181,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest,
[[mock_breakpad_controller_ expect] [[mock_breakpad_controller_ expect]
addUploadParameter:StringParameterValidatorWithCountOfSubstring( addUploadParameter:StringParameterValidatorWithCountOfSubstring(
1, event_nsstring) 1, event_nsstring)
forKey:crash_keys::kBreadcrumbsProductDataKey]; forKey:base::SysUTF8ToNSString(
crash_keys::kBreadcrumbsProductDataKey)];
breadcrumb_service->AddEvent(event); breadcrumb_service->AddEvent(event);
ChromeBrowserState* otr_browser_state = ChromeBrowserState* otr_browser_state =
...@@ -193,7 +196,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest, ...@@ -193,7 +196,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest,
[[mock_breakpad_controller_ expect] [[mock_breakpad_controller_ expect]
addUploadParameter:StringParameterValidatorWithCountOfSubstring( addUploadParameter:StringParameterValidatorWithCountOfSubstring(
2, event_nsstring) 2, event_nsstring)
forKey:crash_keys::kBreadcrumbsProductDataKey]; forKey:base::SysUTF8ToNSString(
crash_keys::kBreadcrumbsProductDataKey)];
otr_breadcrumb_service->AddEvent(event); otr_breadcrumb_service->AddEvent(event);
TestChromeBrowserState::Builder test_cbs_builder; TestChromeBrowserState::Builder test_cbs_builder;
...@@ -208,7 +212,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest, ...@@ -208,7 +212,8 @@ TEST_F(CrashReporterBreadcrumbObserverTest,
[[mock_breakpad_controller_ expect] [[mock_breakpad_controller_ expect]
addUploadParameter:StringParameterValidatorWithCountOfSubstring( addUploadParameter:StringParameterValidatorWithCountOfSubstring(
3, event_nsstring) 3, event_nsstring)
forKey:crash_keys::kBreadcrumbsProductDataKey]; forKey:base::SysUTF8ToNSString(
crash_keys::kBreadcrumbsProductDataKey)];
breadcrumb_service_2->AddEvent(event); breadcrumb_service_2->AddEvent(event);
EXPECT_OCMOCK_VERIFY(mock_breakpad_controller_); EXPECT_OCMOCK_VERIFY(mock_breakpad_controller_);
......
...@@ -21,11 +21,13 @@ class AllWebStateObservationForwarder; ...@@ -21,11 +21,13 @@ class AllWebStateObservationForwarder;
// Provides method to set and remove parameter values. // Provides method to set and remove parameter values.
@protocol CrashReporterParameterSetter @protocol CrashReporterParameterSetter
// Sets a parameter named |key| with value |value|. // Sets a parameter named |key||pending| with value |value|.
- (void)setReportParameterURL:(const GURL&)URL forKey:(NSString*)key; - (void)setReportParameterURL:(const GURL&)URL
forKey:(NSNumber*)key
pending:(BOOL)pending;
// Deletes the parameter |key|. // Deletes the parameter |key||pending|.
- (void)removeReportParameter:(NSString*)key; - (void)removeReportParameter:(NSNumber*)key pending:(BOOL)pending;
@end @end
// WebStateListObserver that allows loaded urls to be sent to the crash server. // WebStateListObserver that allows loaded urls to be sent to the crash server.
...@@ -90,7 +92,7 @@ class CrashReporterURLObserver : public WebStateListObserver, ...@@ -90,7 +92,7 @@ class CrashReporterURLObserver : public WebStateListObserver,
void RemoveGroup(const std::string& group); void RemoveGroup(const std::string& group);
// Map associating each Breakpad key with the group it is currently reporting // Map associating each Breakpad key with the group it is currently reporting
// URLs for. // URLs for.
NSMutableDictionary<NSString*, NSString*>* breakpad_key_by_group_; NSMutableDictionary<NSString*, NSNumber*>* breakpad_key_by_group_;
// Map associating each WebState to its group. // Map associating each WebState to its group.
std::map<web::WebState*, std::string> web_state_to_group_; std::map<web::WebState*, std::string> web_state_to_group_;
// Map associating each group to the WebState currently reported in crash // Map associating each group to the WebState currently reported in crash
...@@ -98,7 +100,7 @@ class CrashReporterURLObserver : public WebStateListObserver, ...@@ -98,7 +100,7 @@ class CrashReporterURLObserver : public WebStateListObserver,
std::map<std::string, web::WebState*> current_web_states_; std::map<std::string, web::WebState*> current_web_states_;
// List of keys to use for recording URLs. This list is sorted such that a new // List of keys to use for recording URLs. This list is sorted such that a new
// tab must use the first key in this list to record its URLs. // tab must use the first key in this list to record its URLs.
NSMutableArray<NSString*>* breakpad_keys_; NSMutableArray<NSNumber*>* breakpad_keys_;
// Forwards observer methods for all WebStates in the WebStateList monitored // Forwards observer methods for all WebStates in the WebStateList monitored
// by the CrashReporterURLObserver. // by the CrashReporterURLObserver.
std::map<WebStateList*, std::unique_ptr<AllWebStateObservationForwarder>> std::map<WebStateList*, std::unique_ptr<AllWebStateObservationForwarder>>
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/check.h" #include "base/check.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/strings/sys_string_conversions.h" #include "base/strings/sys_string_conversions.h"
#include "components/crash/core/common/crash_key.h"
#import "components/previous_session_info/previous_session_info.h" #import "components/previous_session_info/previous_session_info.h"
#include "ios/chrome/browser/crash_report/breakpad_helper.h" #include "ios/chrome/browser/crash_report/breakpad_helper.h"
#import "ios/chrome/browser/web_state_list/all_web_state_observation_forwarder.h" #import "ios/chrome/browser/web_state_list/all_web_state_observation_forwarder.h"
...@@ -26,17 +27,25 @@ ...@@ -26,17 +27,25 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
namespace { using crash_reporter::CrashKeyString;
// Returns the breakpad key to use for a pending URL corresponding to the namespace {
// same tab that is using |key|.
NSString* PendingURLKeyForKey(NSString* key) {
return [key stringByAppendingString:@"-pending"];
}
// Max number of urls to send. This represent 1 URL per WebState group. // Max number of urls to send. This represent 1 URL per WebState group.
const int kNumberOfURLsToSend = 3; const int kNumberOfURLsToSend = 3;
// Keep the following two CrashKey arrays in sync with |kNumberOfURLsToSend|.
static crash_reporter::CrashKeyString<1024> url_crash_keys[] = {
{"url0", CrashKeyString<1024>::Tag::kArray},
{"url1", CrashKeyString<1024>::Tag::kArray},
{"url2", CrashKeyString<1024>::Tag::kArray},
};
static CrashKeyString<1024> pending_url_crash_keys[] = {
{"url0-pending", CrashKeyString<1024>::Tag::kArray},
{"url1-pending", CrashKeyString<1024>::Tag::kArray},
{"url2-pending", CrashKeyString<1024>::Tag::kArray},
};
// The group for preload WebStates. // The group for preload WebStates.
const char kPreloadWebStateGroup[] = "PreloadGroup"; const char kPreloadWebStateGroup[] = "PreloadGroup";
...@@ -49,14 +58,23 @@ const char kPreloadWebStateGroup[] = "PreloadGroup"; ...@@ -49,14 +58,23 @@ const char kPreloadWebStateGroup[] = "PreloadGroup";
@end @end
@implementation CrashReporterParameterSetter @implementation CrashReporterParameterSetter
- (void)removeReportParameter:(NSString*)key { - (void)removeReportParameter:(NSNumber*)key pending:(BOOL)pending {
breakpad_helper::RemoveReportParameter(key); int index = key.intValue;
[[PreviousSessionInfo sharedInstance] removeReportParameterForKey:key]; DCHECK(index < kNumberOfURLsToSend);
if (pending)
pending_url_crash_keys[index].Clear();
else
url_crash_keys[index].Clear();
} }
- (void)setReportParameterURL:(const GURL&)URL forKey:(NSString*)key { - (void)setReportParameterURL:(const GURL&)URL
breakpad_helper::AddReportParameter(key, base::SysUTF8ToNSString(URL.spec()), forKey:(NSNumber*)key
true); pending:(BOOL)pending {
[[PreviousSessionInfo sharedInstance] setReportParameterURL:URL forKey:key]; int index = key.intValue;
DCHECK(index < kNumberOfURLsToSend);
if (pending)
pending_url_crash_keys[index].Set(URL.spec());
else
url_crash_keys[index].Set(URL.spec());
} }
@end @end
...@@ -77,7 +95,7 @@ CrashReporterURLObserver::CrashReporterURLObserver( ...@@ -77,7 +95,7 @@ CrashReporterURLObserver::CrashReporterURLObserver(
breakpad_keys_ = breakpad_keys_ =
[[NSMutableArray alloc] initWithCapacity:kNumberOfURLsToSend]; [[NSMutableArray alloc] initWithCapacity:kNumberOfURLsToSend];
for (int i = 0; i < kNumberOfURLsToSend; ++i) for (int i = 0; i < kNumberOfURLsToSend; ++i)
[breakpad_keys_ addObject:[NSString stringWithFormat:@"url%d", i]]; [breakpad_keys_ addObject:[NSNumber numberWithInt:i]];
} }
CrashReporterURLObserver::~CrashReporterURLObserver() {} CrashReporterURLObserver::~CrashReporterURLObserver() {}
...@@ -91,11 +109,11 @@ std::string CrashReporterURLObserver::GroupForWebStateList( ...@@ -91,11 +109,11 @@ std::string CrashReporterURLObserver::GroupForWebStateList(
void CrashReporterURLObserver::RemoveGroup(const std::string& group) { void CrashReporterURLObserver::RemoveGroup(const std::string& group) {
NSString* ns_group = base::SysUTF8ToNSString(group); NSString* ns_group = base::SysUTF8ToNSString(group);
NSString* key = [breakpad_key_by_group_ objectForKey:ns_group]; NSNumber* key = [breakpad_key_by_group_ objectForKey:ns_group];
if (!key) if (!key)
return; return;
[params_setter_ removeReportParameter:key]; [params_setter_ removeReportParameter:key pending:NO];
[params_setter_ removeReportParameter:PendingURLKeyForKey(key)]; [params_setter_ removeReportParameter:key pending:YES];
[breakpad_key_by_group_ removeObjectForKey:ns_group]; [breakpad_key_by_group_ removeObjectForKey:ns_group];
[breakpad_keys_ removeObject:key]; [breakpad_keys_ removeObject:key];
[breakpad_keys_ insertObject:key atIndex:0]; [breakpad_keys_ insertObject:key atIndex:0];
...@@ -116,7 +134,7 @@ void CrashReporterURLObserver::RecordURL(const GURL& url, ...@@ -116,7 +134,7 @@ void CrashReporterURLObserver::RecordURL(const GURL& url,
std::string group = web_state_to_group_[web_state]; std::string group = web_state_to_group_[web_state];
DCHECK(group.size()); DCHECK(group.size());
NSString* ns_group = base::SysUTF8ToNSString(group); NSString* ns_group = base::SysUTF8ToNSString(group);
NSString* breakpad_key = [breakpad_key_by_group_ objectForKey:ns_group]; NSNumber* breakpad_key = [breakpad_key_by_group_ objectForKey:ns_group];
BOOL reusing_key = NO; BOOL reusing_key = NO;
if (!breakpad_key) { if (!breakpad_key) {
// Get the first breakpad key and push it back at the end of the keys. // Get the first breakpad key and push it back at the end of the keys.
...@@ -136,14 +154,13 @@ void CrashReporterURLObserver::RecordURL(const GURL& url, ...@@ -136,14 +154,13 @@ void CrashReporterURLObserver::RecordURL(const GURL& url,
[breakpad_keys_ addObject:breakpad_key]; [breakpad_keys_ addObject:breakpad_key];
current_web_states_[group] = web_state; current_web_states_[group] = web_state;
NSString* pending_key = PendingURLKeyForKey(breakpad_key);
if (pending) { if (pending) {
if (reusing_key) if (reusing_key)
[params_setter_ removeReportParameter:breakpad_key]; [params_setter_ removeReportParameter:breakpad_key pending:NO];
[params_setter_ setReportParameterURL:url forKey:pending_key]; [params_setter_ setReportParameterURL:url forKey:breakpad_key pending:YES];
} else { } else {
[params_setter_ setReportParameterURL:url forKey:breakpad_key]; [params_setter_ setReportParameterURL:url forKey:breakpad_key pending:NO];
[params_setter_ removeReportParameter:pending_key]; [params_setter_ removeReportParameter:breakpad_key pending:YES];
} }
} }
......
...@@ -56,6 +56,11 @@ class FakeWebState : public web::FakeWebState { ...@@ -56,6 +56,11 @@ class FakeWebState : public web::FakeWebState {
std::unique_ptr<web::NavigationItem> pending_item_; std::unique_ptr<web::NavigationItem> pending_item_;
}; };
NSString* NumberToKey(NSNumber* number, bool pending) {
return [NSString stringWithFormat:@"url%d%@", number.intValue,
pending ? @"-pending" : @""];
}
} // namespace } // namespace
@interface DictionaryParameterSetter : NSObject <CrashReporterParameterSetter> @interface DictionaryParameterSetter : NSObject <CrashReporterParameterSetter>
...@@ -72,11 +77,15 @@ class FakeWebState : public web::FakeWebState { ...@@ -72,11 +77,15 @@ class FakeWebState : public web::FakeWebState {
return self; return self;
} }
- (void)removeReportParameter:(NSString*)key { - (void)removeReportParameter:(NSNumber*)number pending:(BOOL)pending {
NSString* key = NumberToKey(number, pending);
[_params removeObjectForKey:key]; [_params removeObjectForKey:key];
} }
- (void)setReportParameterURL:(const GURL&)URL forKey:(NSString*)key { - (void)setReportParameterURL:(const GURL&)URL
forKey:(NSNumber*)number
pending:(BOOL)pending {
NSString* key = NumberToKey(number, pending);
[_params setObject:base::SysUTF8ToNSString(URL.spec()) forKey:key]; [_params setObject:base::SysUTF8ToNSString(URL.spec()) forKey:key];
} }
......
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