Refactor Mac crash key reporting - move to base/mac/crash_logging.{h,mm}
Breakpad support currently resides in chrome/app/breakpad_mac.* this makes it inaccessible to lower level code that would also like to set crash keys e.g. The sandboxing infrastructure.
This CL refactors the crash key reporting code to reside in base/mac.
Logging is also added to the Sandbox code to try to track down the cause of crbug.com/94758.
BUG=95272, 94758
TEST=On official builds crash logs should contain crash keys e.g. OS version.
Review URL: http://codereview.chromium.org/7849011
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@100626 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/app/breakpad_mac.h b/chrome/app/breakpad_mac.h
index b5eb9e0fa..b737f0d3 100644
--- a/chrome/app/breakpad_mac.h
+++ b/chrome/app/breakpad_mac.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -22,28 +22,4 @@
// Call on clean process shutdown.
void DestructCrashReporter();
-#ifdef __OBJC__
-
-#include "base/memory/scoped_nsobject.h"
-
-@class NSString;
-
-// Set and clear meta information for Minidump.
-// IMPORTANT: On OS X, the key/value pairs are sent to the crash server
-// out of bounds and not recorded on disk in the minidump, this means
-// that if you look at the minidump file locally you won't see them!
-void SetCrashKeyValue(NSString* key, NSString* value);
-void ClearCrashKeyValue(NSString* key);
-
-class ScopedCrashKey {
- public:
- ScopedCrashKey(NSString* key, NSString* value);
- ~ScopedCrashKey();
-
- private:
- scoped_nsobject<NSString> crash_key_;
-};
-
-#endif // __OBJC__
-
#endif // CHROME_APP_BREAKPAD_MAC_H_
diff --git a/chrome/app/breakpad_mac.mm b/chrome/app/breakpad_mac.mm
index 840ac47..4f4d11c 100644
--- a/chrome/app/breakpad_mac.mm
+++ b/chrome/app/breakpad_mac.mm
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -30,6 +30,26 @@
BreakpadRef gBreakpadRef = NULL;
+void SetCrashKeyValue(NSString* key, NSString* value) {
+ // Comment repeated from header to prevent confusion:
+ // IMPORTANT: On OS X, the key/value pairs are sent to the crash server
+ // out of bounds and not recorded on disk in the minidump, this means
+ // that if you look at the minidump file locally you won't see them!
+ if (gBreakpadRef == NULL) {
+ return;
+ }
+
+ BreakpadAddUploadParameter(gBreakpadRef, key, value);
+}
+
+void ClearCrashKeyValue(NSString* key) {
+ if (gBreakpadRef == NULL) {
+ return;
+ }
+
+ BreakpadRemoveUploadParameter(gBreakpadRef, key);
+}
+
} // namespace
bool IsCrashReporterEnabled() {
@@ -155,8 +175,8 @@
// Enable child process crashes to include the page URL.
// TODO: Should this only be done for certain process types?
- child_process_logging::SetCrashKeyFunctions(SetCrashKeyValue,
- ClearCrashKeyValue);
+ base::mac::SetCrashKeyFunctions(SetCrashKeyValue,
+ ClearCrashKeyValue);
if (!is_browser) {
// Get the guid from the command line switch.
@@ -183,33 +203,3 @@
// Store process type in crash dump.
SetCrashKeyValue(@"ptype", process_type);
}
-
-void SetCrashKeyValue(NSString* key, NSString* value) {
- // Comment repeated from header to prevent confusion:
- // IMPORTANT: On OS X, the key/value pairs are sent to the crash server
- // out of bounds and not recorded on disk in the minidump, this means
- // that if you look at the minidump file locally you won't see them!
- if (gBreakpadRef == NULL) {
- return;
- }
-
- BreakpadAddUploadParameter(gBreakpadRef, key, value);
-}
-
-void ClearCrashKeyValue(NSString* key) {
- if (gBreakpadRef == NULL) {
- return;
- }
-
- BreakpadRemoveUploadParameter(gBreakpadRef, key);
-}
-
-// NOTE(shess): These also exist in breakpad_mac_stubs.mm.
-ScopedCrashKey::ScopedCrashKey(NSString* key, NSString* value)
- : crash_key_([key retain]) {
- SetCrashKeyValue(crash_key_.get(), value);
-}
-
-ScopedCrashKey::~ScopedCrashKey() {
- ClearCrashKeyValue(crash_key_.get());
-}
diff --git a/chrome/app/breakpad_mac_stubs.mm b/chrome/app/breakpad_mac_stubs.mm
index b07ded6..db2db6b 100644
--- a/chrome/app/breakpad_mac_stubs.mm
+++ b/chrome/app/breakpad_mac_stubs.mm
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -21,22 +21,3 @@
void InitCrashReporter() {
}
-
-void SetCrashKeyValue(NSString* key, NSString* value) {
-}
-
-void ClearCrashKeyValue(NSString* key) {
-}
-
-// NOTE(shess): These functions could clearly be replaced by stubs,
-// but since they are seldom-used helpers, it seemed more reasonable
-// to duplicate them from the primary implementation in
-// breakpad_mac.mm.
-ScopedCrashKey::ScopedCrashKey(NSString* key, NSString* value)
- : crash_key_([key retain]) {
- SetCrashKeyValue(crash_key_.get(), value);
-}
-
-ScopedCrashKey::~ScopedCrashKey() {
- ClearCrashKeyValue(crash_key_.get());
-}
diff --git a/chrome/browser/chrome_browser_application_mac.mm b/chrome/browser/chrome_browser_application_mac.mm
index 852d2ba..c7706f2 100644
--- a/chrome/browser/chrome_browser_application_mac.mm
+++ b/chrome/browser/chrome_browser_application_mac.mm
@@ -5,11 +5,11 @@
#import "chrome/browser/chrome_browser_application_mac.h"
#import "base/logging.h"
+#include "base/mac/crash_logging.h"
#import "base/mac/scoped_nsexception_enabler.h"
#import "base/metrics/histogram.h"
#import "base/memory/scoped_nsobject.h"
#import "base/sys_string_conversions.h"
-#import "chrome/app/breakpad_mac.h"
#import "chrome/browser/app_controller_mac.h"
#include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
@@ -63,7 +63,7 @@
static NSString* const kNSExceptionKey = @"nsexception";
NSString* value =
[NSString stringWithFormat:@"%@ reason %@", aName, aReason];
- SetCrashKeyValue(kNSExceptionKey, value);
+ base::mac::SetCrashKeyValue(kNSExceptionKey, value);
// Force crash for selected exceptions to generate crash dumps.
BOOL fatal = NO;
@@ -338,7 +338,7 @@
[NSString stringWithFormat:@"%@ tag %d sending %@ to %p",
[sender className], tag, actionString, aTarget];
- ScopedCrashKey key(kActionKey, value);
+ base::mac::ScopedCrashKey key(kActionKey, value);
// Certain third-party code, such as print drivers, can still throw
// exceptions and Chromium cannot fix them. This provides a way to
@@ -406,10 +406,10 @@
NSString* value = [NSString stringWithFormat:@"%@ reason %@",
[anException name], [anException reason]];
if (!trackedFirstException) {
- SetCrashKeyValue(kFirstExceptionKey, value);
+ base::mac::SetCrashKeyValue(kFirstExceptionKey, value);
trackedFirstException = YES;
} else {
- SetCrashKeyValue(kLastExceptionKey, value);
+ base::mac::SetCrashKeyValue(kLastExceptionKey, value);
}
reportingException = NO;
diff --git a/chrome/browser/ui/cocoa/objc_zombie.mm b/chrome/browser/ui/cocoa/objc_zombie.mm
index ef6d458..0918fd7e 100644
--- a/chrome/browser/ui/cocoa/objc_zombie.mm
+++ b/chrome/browser/ui/cocoa/objc_zombie.mm
@@ -16,10 +16,10 @@
#include "base/debug/stack_trace.h"
#include "base/logging.h"
+#include "base/mac/crash_logging.h"
#include "base/mac/mac_util.h"
#include "base/metrics/histogram.h"
#include "base/synchronization/lock.h"
-#import "chrome/app/breakpad_mac.h"
#import "chrome/common/mac/objc_method_swizzle.h"
// Deallocated objects are re-classed as |CrZombie|. No superclass
@@ -284,7 +284,7 @@
}
// Set a value for breakpad to report.
- SetCrashKeyValue(@"zombie", aString);
+ base::mac::SetCrashKeyValue(@"zombie", aString);
// Hex-encode the backtrace and tuck it into a breakpad key.
NSString* deallocTrace = @"<unknown>";
@@ -300,7 +300,7 @@
// Warn someone if this exceeds the breakpad limits.
DCHECK_LE(strlen([deallocTrace UTF8String]), 255U);
}
- SetCrashKeyValue(@"zombie_dealloc_bt", deallocTrace);
+ base::mac::SetCrashKeyValue(@"zombie_dealloc_bt", deallocTrace);
// Log -dealloc backtrace in debug builds then crash with a useful
// stack trace.
diff --git a/chrome/browser/ui/cocoa/tab_contents/web_drag_source.mm b/chrome/browser/ui/cocoa/tab_contents/web_drag_source.mm
index b9624893..395526f1 100644
--- a/chrome/browser/ui/cocoa/tab_contents/web_drag_source.mm
+++ b/chrome/browser/ui/cocoa/tab_contents/web_drag_source.mm
@@ -7,6 +7,7 @@
#include <sys/param.h>
#include "base/file_path.h"
+#include "base/mac/crash_logging.h"
#include "base/string_util.h"
#include "base/sys_string_conversions.h"
#include "base/task.h"
@@ -60,7 +61,7 @@
// http://crbug.com/78782
static NSString* const kUrlKey = @"drop_data_url";
NSString* value = SysUTF8ToNSString(drop_data.url.spec());
- ScopedCrashKey key(kUrlKey, value);
+ base::mac::ScopedCrashKey key(kUrlKey, value);
// Images without ALT text will only have a file extension so we need to
// synthesize one from the provided extension and URL.
diff --git a/chrome/common/child_process_logging.h b/chrome/common/child_process_logging.h
index 7b80c82..0daabb8 100644
--- a/chrome/common/child_process_logging.h
+++ b/chrome/common/child_process_logging.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -10,6 +10,7 @@
#include <string>
#include "base/basictypes.h"
+#include "base/mac/crash_logging.h"
#include "googleurl/src/gurl.h"
struct GPUInfo;
@@ -78,25 +79,20 @@
} // namespace child_process_logging
-#if defined(OS_MACOSX) && __OBJC__
-
-@class NSString;
-
-typedef void (*SetCrashKeyValueFuncPtr)(NSString*, NSString*);
-typedef void (*ClearCrashKeyValueFuncPtr)(NSString*);
+#if defined(OS_MACOSX)
namespace child_process_logging {
-void SetCrashKeyFunctions(SetCrashKeyValueFuncPtr set_key_func,
- ClearCrashKeyValueFuncPtr clear_key_func);
+
void SetActiveURLImpl(const GURL& url,
- SetCrashKeyValueFuncPtr set_key_func,
- ClearCrashKeyValueFuncPtr clear_key_func);
+ base::mac::SetCrashKeyValueFuncPtr set_key_func,
+ base::mac::ClearCrashKeyValueFuncPtr clear_key_func);
extern const int kMaxNumCrashURLChunks;
extern const int kMaxNumURLChunkValueLength;
extern const char *kUrlChunkFormatStr;
+
} // namespace child_process_logging
-#endif // defined(OS_MACOSX) && __OBJC__
+#endif // defined(OS_MACOSX)
#endif // CHROME_COMMON_CHILD_PROCESS_LOGGING_H_
diff --git a/chrome/common/child_process_logging_mac.mm b/chrome/common/child_process_logging_mac.mm
index ab3d021..70ee2a7 100644
--- a/chrome/common/child_process_logging_mac.mm
+++ b/chrome/common/child_process_logging_mac.mm
@@ -16,6 +16,11 @@
namespace child_process_logging {
+using base::mac::SetCrashKeyValueFuncPtr;
+using base::mac::ClearCrashKeyValueFuncPtr;
+using base::mac::SetCrashKeyValue;
+using base::mac::ClearCrashKey;
+
const int kMaxNumCrashURLChunks = 8;
const int kMaxNumURLChunkValueLength = 255;
const char *kUrlChunkFormatStr = "url-chunk-%d";
@@ -30,19 +35,10 @@
NSString* const kNumExtensionsName = @"num-extensions";
NSString* const kExtensionNameFormat = @"extension-%d";
-static SetCrashKeyValueFuncPtr g_set_key_func;
-static ClearCrashKeyValueFuncPtr g_clear_key_func;
-
// Account for the terminating null character.
static const size_t kClientIdSize = 32 + 1;
static char g_client_id[kClientIdSize];
-void SetCrashKeyFunctions(SetCrashKeyValueFuncPtr set_key_func,
- ClearCrashKeyValueFuncPtr clear_key_func) {
- g_set_key_func = set_key_func;
- g_clear_key_func = clear_key_func;
-}
-
void SetActiveURLImpl(const GURL& url,
SetCrashKeyValueFuncPtr set_key_func,
ClearCrashKeyValueFuncPtr clear_key_func) {
@@ -94,8 +90,7 @@
}
void SetActiveURL(const GURL& url) {
- if (g_set_key_func && g_clear_key_func)
- SetActiveURLImpl(url, g_set_key_func, g_clear_key_func);
+ SetActiveURLImpl(url, SetCrashKeyValue, ClearCrashKey);
}
void SetClientId(const std::string& client_id) {
@@ -103,8 +98,7 @@
ReplaceSubstringsAfterOffset(&str, 0, "-", "");
base::strlcpy(g_client_id, str.c_str(), kClientIdSize);
- if (g_set_key_func)
- SetClientIdImpl(str, g_set_key_func);
+ SetClientIdImpl(str, SetCrashKeyValue);
std::wstring wstr = ASCIIToWide(str);
GoogleUpdateSettings::SetMetricsId(wstr);
@@ -115,12 +109,10 @@
}
void SetActiveExtensions(const std::set<std::string>& extension_ids) {
- if (!g_set_key_func)
- return;
-
// Log the count separately to track heavy users.
const int count = static_cast<int>(extension_ids.size());
- g_set_key_func(kNumExtensionsName, [NSString stringWithFormat:@"%i", count]);
+ SetCrashKeyValue(kNumExtensionsName,
+ [NSString stringWithFormat:@"%i", count]);
// Record up to |kMaxReportedActiveExtensions| extensions, clearing
// keys if there aren't that many.
@@ -128,10 +120,10 @@
for (int i = 0; i < kMaxReportedActiveExtensions; ++i) {
NSString* key = [NSString stringWithFormat:kExtensionNameFormat, i];
if (iter != extension_ids.end()) {
- g_set_key_func(key, [NSString stringWithUTF8String:iter->c_str()]);
+ SetCrashKeyValue(key, [NSString stringWithUTF8String:iter->c_str()]);
++iter;
} else {
- g_clear_key_func(key);
+ ClearCrashKey(key);
}
}
}
@@ -166,8 +158,7 @@
}
void SetGpuInfo(const GPUInfo& gpu_info) {
- if (g_set_key_func)
- SetGpuInfoImpl(gpu_info, g_set_key_func);
+ SetGpuInfoImpl(gpu_info, SetCrashKeyValue);
}
@@ -179,8 +170,7 @@
}
void SetNumberOfViews(int number_of_views) {
- if (g_set_key_func)
- SetNumberOfViewsImpl(number_of_views, g_set_key_func);
+ SetNumberOfViewsImpl(number_of_views, SetCrashKeyValue);
}
} // namespace child_process_logging
diff --git a/chrome/common/child_process_logging_mac_unittest.mm b/chrome/common/child_process_logging_mac_unittest.mm
index a751287..feedcbe 100644
--- a/chrome/common/child_process_logging_mac_unittest.mm
+++ b/chrome/common/child_process_logging_mac_unittest.mm
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -7,6 +7,7 @@
#import <Foundation/Foundation.h>
#include "base/logging.h"
+#include "base/mac/crash_logging.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "testing/platform_test.h"
@@ -88,8 +89,9 @@
void SetActiveURLWithMock(const GURL& url) {
using child_process_logging::SetActiveURLImpl;
- SetCrashKeyValueFuncPtr setFunc = MockBreakpadKeyValueStore::SetKeyValue;
- ClearCrashKeyValueFuncPtr clearFunc =
+ base::mac::SetCrashKeyValueFuncPtr setFunc =
+ MockBreakpadKeyValueStore::SetKeyValue;
+ base::mac::ClearCrashKeyValueFuncPtr clearFunc =
MockBreakpadKeyValueStore::ClearKeyValue;
SetActiveURLImpl(url, setFunc, clearFunc);
diff --git a/chrome/common/mac/DEPS b/chrome/common/mac/DEPS
index b0c96ed2..77a551b 100644
--- a/chrome/common/mac/DEPS
+++ b/chrome/common/mac/DEPS
@@ -1,8 +1,3 @@
include_rules = [
- # TODO(shess): http://crbug.com/95272
- # This dependency needed by objc_zombie.mm. Unfortunately, putting
- # objc_zombie.mm in chrome/app calls into existence unspeakable horrors.
- "+chrome/app",
-
"+third_party/mach_override",
]
diff --git a/chrome/common/mac/objc_zombie.mm b/chrome/common/mac/objc_zombie.mm
index c60981b..3e9ce8a 100644
--- a/chrome/common/mac/objc_zombie.mm
+++ b/chrome/common/mac/objc_zombie.mm
@@ -16,10 +16,10 @@
#include "base/debug/stack_trace.h"
#include "base/logging.h"
+#include "base/mac/crash_logging.h"
#include "base/mac/mac_util.h"
#include "base/metrics/histogram.h"
#include "base/synchronization/lock.h"
-#import "chrome/app/breakpad_mac.h"
#import "chrome/common/mac/objc_method_swizzle.h"
// Deallocated objects are re-classed as |CrZombie|. No superclass
@@ -284,7 +284,7 @@
}
// Set a value for breakpad to report.
- SetCrashKeyValue(@"zombie", aString);
+ base::mac::SetCrashKeyValue(@"zombie", aString);
// Hex-encode the backtrace and tuck it into a breakpad key.
NSString* deallocTrace = @"<unknown>";
@@ -300,7 +300,7 @@
// Warn someone if this exceeds the breakpad limits.
DCHECK_LE(strlen([deallocTrace UTF8String]), 255U);
}
- SetCrashKeyValue(@"zombie_dealloc_bt", deallocTrace);
+ base::mac::SetCrashKeyValue(@"zombie_dealloc_bt", deallocTrace);
// Log -dealloc backtrace in debug builds then crash with a useful
// stack trace.
