Allow exception forwarding to the system’s native crash reporter to be disabled

Allow exception forwarding to the system’s native crash reporter to be
disabled.

ClientInfo::set_system_crash_reporter_forwarding() can be used to
disable forwarding. The first module that is found with a non-default
value in this field will dictate whether forwarding is enabled or
disabled. It is possible to enable or disable reporting with this call,
as well as reset it to default, which will allow later modules a chance
to influence the behavior.

ClientInfo::set_crashpad_handler_behavior() is also provided, which can
be used to disable Crashpad’s handling of the exception. Most users
should not call this, but should use Settings::SetUploadsEnabled()
instead.

TEST=crashpad_snapshot_test \
         CrashpadInfoClientOptions.*:MachOImageReader.Self_DyldImages; \
     run_with_crashpad --handler crashpad_handler \
         -a --database=/tmp/crashpad_db \
         -a --url=https://clients2.google.com/cr/staging_report \
         -a --annotation=prod=crashpad \
         -a --annotation=ver=0.7.0 \
         crashy_program

R=rsesek@chromium.org

Committed: https://chromium.googlesource.com/crashpad/crashpad/+/9b7ff0ea5a329dbff41f2435f06f308fce3696f1

[Mark Mentovai, 2015-03-10 22:42:11]

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

[Robert Sesek, 2015-03-10 23:03:04]

https://codereview.chromium.org/997713002/diff/1/client/crashpad_info.cc
File client/crashpad_info.cc (right):

https://codereview.chromium.org/997713002/diff/1/client/crashpad_info.cc#newc...
client/crashpad_info.cc:95: options_ = (options_ & ~0x3) | state;
This is a bit magic-y, especially across crashpad source directories.

https://codereview.chromium.org/997713002/diff/1/snapshot/mac/module_snapshot...
File snapshot/mac/module_snapshot_mac.cc (right):

https://codereview.chromium.org/997713002/diff/1/snapshot/mac/module_snapshot...
snapshot/mac/module_snapshot_mac.cc:72: int value = crashpad_info.options & 0x3;
Yeah, this should be an enum with constants.

[Robert Sesek, 2015-03-10 23:44:23]

Also, I think you should be able to test the process_snapshot search logic,
since we have the ability to create test modules already.

[Mark Mentovai, 2015-03-11 17:57:53]

Updated. I addressed all feedback and added tests.

[Robert Sesek, 2015-03-11 19:25:30]

https://codereview.chromium.org/997713002/diff/20001/snapshot/mac/crashpad_in...
File snapshot/mac/crashpad_info_client_options.h (right):

https://codereview.chromium.org/997713002/diff/20001/snapshot/mac/crashpad_in...
snapshot/mac/crashpad_info_client_options.h:48: enum class TriState : uint8_t {
This could be moved into util/ and then it wouldn't have to be duplicated.
Thoughts?

(The converter function below could also be on the enum class itself).

https://codereview.chromium.org/997713002/diff/20001/snapshot/mac/mach_o_imag...
File snapshot/mac/mach_o_image_reader_test.cc (right):

https://codereview.chromium.org/997713002/diff/20001/snapshot/mac/mach_o_imag...
snapshot/mac/mach_o_image_reader_test.cc:573:
EXPECT_GE(modules_with_crashpad_info, 1u);
1u first

[Mark Mentovai, 2015-03-11 19:45:53]

https://codereview.chromium.org/997713002/diff/20001/snapshot/mac/crashpad_in...
File snapshot/mac/crashpad_info_client_options.h (right):

https://codereview.chromium.org/997713002/diff/20001/snapshot/mac/crashpad_in...
snapshot/mac/crashpad_info_client_options.h:48: enum class TriState : uint8_t {
Robert Sesek wrote:
> This could be moved into util/ and then it wouldn't have to be duplicated.
> Thoughts?
> 
> (The converter function below could also be on the enum class itself).

util doesn’t really feel right. What I really think we need is a layer where we
could just stick definitions. Headers, no code. I would want to put
minidump_extensions.h in there too, so that I can resolve the cycle that
resulted from snapshot depending on it.

I don’t really know what to name a new layer like that. What do you think?

https://codereview.chromium.org/997713002/diff/20001/snapshot/mac/mach_o_imag...
File snapshot/mac/mach_o_image_reader_test.cc (right):

https://codereview.chromium.org/997713002/diff/20001/snapshot/mac/mach_o_imag...
snapshot/mac/mach_o_image_reader_test.cc:573:
EXPECT_GE(modules_with_crashpad_info, 1u);
Robert Sesek wrote:
> 1u first

This is EXPECT_GE, that would change the meaning to 1u >=
modules_with_crashpad_info.

[Mark Mentovai, 2015-03-11 20:20:46]

As discussed, I moved TriState into util/misc where everyone can see and share
it.

[Robert Sesek, 2015-03-11 20:26:59]

https://codereview.chromium.org/997713002/diff/40001/util/misc/tri_state.h
File util/misc/tri_state.h (right):

https://codereview.chromium.org/997713002/diff/40001/util/misc/tri_state.h#ne...
util/misc/tri_state.h:39: TriState ConvertToTriState(uint32_t value);
Missing a file that implements this? And unused?

[Mark Mentovai, 2015-03-11 21:02:11]

Patchset #4 (id:60001) has been deleted

[Mark Mentovai, 2015-03-11 21:02:26]

https://codereview.chromium.org/997713002/diff/40001/util/misc/tri_state.h
File util/misc/tri_state.h (right):

https://codereview.chromium.org/997713002/diff/40001/util/misc/tri_state.h#ne...
util/misc/tri_state.h:39: TriState ConvertToTriState(uint32_t value);
Robert Sesek wrote:
> Missing a file that implements this? And unused?

Removed. I was going to move the converter here, but left it alone since it’s
only used in snapshot.

[Robert Sesek, 2015-03-11 21:03:20]

LGTM

[Mark Mentovai, 2015-03-11 21:07:15]

Committed patchset #4 (id:80001) manually as
9b7ff0ea5a329dbff41f2435f06f308fce3696f1 (presubmit successful).