Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(301)

Issue 115493: Fix for breakpad not generating a minidump in certain cases on OSX. (Closed)

Created:
11 years, 7 months ago by jeremy
Modified:
9 years, 6 months ago
Reviewers:
John Grabowski
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Fix for breakpad not generating a minidump in certain cases on OSX. 1)Fix logic inversion in IsCrashReporterEnabled(). 2)Don't intercept SIGPIPE in non-branded builds since it isn't fatal. 3)Roll DEPS to pickup a bunch of Mac Breakpad fixes. When breakpad is disabled, we intercept a bunch of signals so that we can crash fast, without waiting for Apple's crash reporter. The problem was that the function we where using to test whether breakpad was enabled was wrong so we were always installing these signal handlers which where just calling exit(). By fixing the IsCrashReporterEnabled() call, we no longer install these signal handlers if Breakpad is enabled. In any case SIGPIPE is non-fatal so we remove it from the list of signals we intercept. There have been a number of fixes to the OSX version of Breakpad recently, so we pull those in as well. BUG=11929

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -4 lines) Patch
M DEPS View 1 chunk +1 line, -1 line 0 comments Download
M base/debug_util_mac.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/app/breakpad_mac.mm View 1 chunk +1 line, -1 line 1 comment Download

Messages

Total messages: 1 (0 generated)
John Grabowski
11 years, 7 months ago (2009-05-19 00:11:50 UTC) #1
LGTM

(Would a unit test have caught this?)

http://codereview.chromium.org/115493/diff/1/4
File chrome/app/breakpad_mac.mm (right):

http://codereview.chromium.org/115493/diff/1/4#newcode48
Line 48: return gBreakpadRef != NULL;
DOH!!!!!

Powered by Google App Engine
This is Rietveld 408576698