Crashpad!

Crashpad!: Use the Crashpad client instead of Breakpad on Mac OS X.

Crashpad is always compiled in to Chrome and its handler is always
enabled. It is only possible to enable uploads in official builds.

Crashpad talks to the existing Breakpad server. There should be no
noticeable changes to crash reporting on the server side, except the
client IDs will all change to a new ID and will no longer be
synchronized with UMA client IDs. This is a one-time change. After this,
the client ID will remain stable within a single --user-data-dir.

BUG=386208, 390217, 415547, 427611, crashpad:12
R=rsesek@chromium.org
TBR=cpu@chromium.org,jochen@chromium.org

Committed: https://crrev.com/d413b2dcb54d523811d386f1ff4084f677a6d089
Cr-Commit-Position: refs/heads/master@{#320466}

[Mark Mentovai, 2015-03-12 23:14:14]

Patchset #3 (id:40001) has been deleted

[Mark Mentovai, 2015-03-12 23:20:19]

Patchset #3 (id:60001) has been deleted

[Mark Mentovai, 2015-03-12 23:20:30]

Patchset #3 (id:60001) has been deleted

[Mark Mentovai, 2015-03-12 23:20:54]

mark@chromium.org changed reviewers:
+ cpu@chromium.org, rsesek@chromium.org

[Mark Mentovai, 2015-03-12 23:21:12]

rsesek for review of most things. cpu as a root-level OWNER.

[Robert Sesek, 2015-03-12 23:51:34]

✨🌟✨🌟 Wooohoooo! 🌟✨🌟✨

https://codereview.chromium.org/1001103002/diff/80001/chrome/app/chrome_main_...
File chrome/app/chrome_main_delegate.cc (right):

https://codereview.chromium.org/1001103002/diff/80001/chrome/app/chrome_main_...
chrome/app/chrome_main_delegate.cc:560: // CommandLine::Init() and
chrome::RegisterPathProvider().  Ideally, Breakpad
Breakpad -> Crashpad

https://codereview.chromium.org/1001103002/diff/80001/chrome/browser/crash_up...
File chrome/browser/crash_upload_list_mac.h (right):

https://codereview.chromium.org/1001103002/diff/80001/chrome/browser/crash_up...
chrome/browser/crash_upload_list_mac.h:13: class CrashUploadListMac : public
CrashUploadList {
Short comment?

// A CrashUploadList that retrieves the list of uploaded reports from
// the Crashpad database.

https://codereview.chromium.org/1001103002/diff/80001/chrome/browser/crash_up...
chrome/browser/crash_upload_list_mac.h:20: void LoadUploadList() override;
// Called on the FILE thread.

https://codereview.chromium.org/1001103002/diff/80001/components/crash/app/cr...
File components/crash/app/crashpad_mac.mm (right):

https://codereview.chromium.org/1001103002/diff/80001/components/crash/app/cr...
components/crash/app/crashpad_mac.mm:86: CRASHPAD_SIMULATE_CRASH();
Should we set a crashkey here?

[Mark Mentovai, 2015-03-13 02:39:27]

Thanks for the review! Updated.

I also fixed the component=shared_library build and replaced the Breakpad
shippables with crashpad_handler in the dump_product_syms script, so we’ll save
its .dSYM and dumped symbols.

https://codereview.chromium.org/1001103002/diff/80001/components/crash/app/cr...
File components/crash/app/crashpad_mac.mm (right):

https://codereview.chromium.org/1001103002/diff/80001/components/crash/app/cr...
components/crash/app/crashpad_mac.mm:86: CRASHPAD_SIMULATE_CRASH();
Robert Sesek wrote:
> Should we set a crashkey here?

Breakpad didn’t. I don’t know what we could save here that would be useful
that’s not already information we’ve got.

It’ll be obvious on the server side that these came from DumpWithoutCrashing()
because the exception value is kMachExceptionSimulated ('CPsx', 0x43507378).
Also, at least for now (see the comment at the SetDumpWithoutCrashingFunction()
call below), they’ll all have DumpWithoutCrashing() at the very top of the stack
too.

[Robert Sesek, 2015-03-13 02:54:08]

The only other thing I can think of is the GN build, but I think since the GYP
changes are largely packing-related and GN doesn't support that yet, there's no
way to make corresponding ones.

There's also a link failure because of duplicate catch_exception_raise (grr
breakpad).

https://codereview.chromium.org/1001103002/diff/80001/components/crash/app/cr...
File components/crash/app/crashpad_mac.mm (right):

https://codereview.chromium.org/1001103002/diff/80001/components/crash/app/cr...
components/crash/app/crashpad_mac.mm:86: CRASHPAD_SIMULATE_CRASH();
On 2015/03/13 02:39:27, Mark Mentovai wrote:
> Robert Sesek wrote:
> > Should we set a crashkey here?
> 
> Breakpad didn’t. I don’t know what we could save here that would be useful
> that’s not already information we’ve got.

Ah, okay. I couldn't remember if Breakpad did or not.

> It’ll be obvious on the server side that these came from DumpWithoutCrashing()
> because the exception value is kMachExceptionSimulated ('CPsx', 0x43507378).

Perfect.

[Mark Mentovai, 2015-03-13 02:57:06]

Yeah, I’m looking at that now. It’s happening in the component build, Content
Shell Framework. Maybe I have to convert Content Shell to use Crashpad after
all, and get Breakpad out of there entirely.

[Mark Mentovai, 2015-03-13 04:13:05]

Fixed content_shell by allowing it to continue using breakpad_mac while Chrome
uses crashpad_mac. Filed bug 466890 to clean up the situation.

[Robert Sesek, 2015-03-13 04:26:32]

https://codereview.chromium.org/1001103002/diff/140001/components/crash.gypi
File components/crash.gypi (right):

https://codereview.chromium.org/1001103002/diff/140001/components/crash.gypi#...
components/crash.gypi:37: 'target_name': 'crash_component_base',
This could also be crash_component_non_mac, and the bikeshed is #002104.

[Mark Mentovai, 2015-03-13 04:40:12]

Updated. I also had to pull out an extra dependency hiding in chrome_main_dll,
and remove filter callback support because Crashpad doesn’t do any work
in-process at exception time.

[Robert Sesek, 2015-03-13 04:51:29]

__         ______     ______   __    __    
/\ \       /\  ___\   /\__  _\ /\ "-./  \   
\ \ \____  \ \ \__ \  \/_/\ \/ \ \ \-./\ \  
 \ \_____\  \ \_____\    \ \_\  \ \_\ \ \_\ 
  \/_____/   \/_____/     \/_/   \/_/  \/_/                                     
       

LGTM 👍

[Mark Mentovai, 2015-03-13 04:57:05]

mark@chromium.org changed reviewers:
+ jochen@chromium.org

[Mark Mentovai, 2015-03-13 04:58:22]

cpu or jochen for OWNERS review.

TBR because this actually isn’t much code (it’s really all DEPS) and the TPMs
are eager to see this in a canary so we can get a merge to Chrome 42, and the
clock is ticking on that.

[Mark Mentovai, 2015-03-13 05:10:26]

The CQ bit was checked by mark@chromium.org

[commit-bot: I haz the power, 2015-03-13 05:10:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1001103002/160001

[Mark Mentovai, 2015-03-13 05:35:55]

The CQ bit was checked by mark@chromium.org

[Mark Mentovai, 2015-03-13 05:35:56]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org
Link to the patchset: https://codereview.chromium.org/1001103002/#ps180001
(title: "Sign crashpad_handler")

[commit-bot: I haz the power, 2015-03-13 05:36:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1001103002/180001

[commit-bot: I haz the power, 2015-03-13 07:45:48]

Committed patchset #8 (id:180001)

[commit-bot: I haz the power, 2015-03-13 07:46:21]

Patchset 8 (id:??) landed as
https://crrev.com/d413b2dcb54d523811d386f1ff4084f677a6d089
Cr-Commit-Position: refs/heads/master@{#320466}