Set a "metrics_client_id" crash key instead of "guid" on Mac OS X

Set a "metrics_client_id" crash key instead of "guid" on Mac OS X.

Crashpad maintains the client ID on its own and is responsible for
sending this form parameter to the Breakpad server during report upload.
When using Crashpad as its crash-reporting implementation, Chrome does
not need to set this crash key.

When Chrome does attempt to set this crash key on its own,
crashpad_handler produces these harmless log messages:

[mmdd/hhmmss:WARNING:crash_report_upload_thread.cc(44)] duplicate key
guid, discarding value 0123456789ABCDEF0123456789ABCDEF

There are valid reasons to provide the metrics client ID along with
the crash report, so this is placed in a distinct crash key,
"metrics_client_id".

BUG=466964
R=cpu@chromium.org, isherman@chromium.org, rsesek@chromium.org

Committed: https://chromium.googlesource.com/chromium/src/+/c67fa64ff088022c1149f50ca69a1b173167afe5

[Mark Mentovai, 2015-03-13 20:36:02]

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

[Mark Mentovai, 2015-03-13 20:36:13]

How convenient, you OWN these files.

[Robert Sesek, 2015-03-13 20:47:58]

I think it'd be best to #if the function definition out too, rather than
silently doing nothing. It looks like it's only called in one place in OS X
(chrome_metrics_service_client.cc).

[Mark Mentovai, 2015-03-13 21:00:28]

It’s called from chrome_crash_reporter_client.cc too, and this is what you get
when you pull on that string until it comes all the way out.

[Robert Sesek, 2015-03-13 21:07:54]

LGTM

[Mark Mentovai, 2015-03-13 21:18:52]

Updated to pull the call out of breakpad_mac.mm (unused now that Mac Chrome
isn’t using Breakpad, and content_shell doesn’t have client IDs).

[Mark Mentovai, 2015-03-13 21:44:28]

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

[Mark Mentovai, 2015-03-13 21:44:29]

cpu for OWNERS

[cpu_(ooo_6.6-7.5), 2015-03-14 02:19:10]

dang I and going to lgtm this but if it was me I would have only done the change
in chrome_metrics_service_client.cc

just saying. good luck.

[Mark Mentovai, 2015-03-17 13:29:48]

I’m waiting to land this to see what the outcome of the UMA/crash reporting
client ID discussion is.

[Mark Mentovai, 2015-03-23 18:01:50]

Patchset #4 (id:60001) has been deleted

[Mark Mentovai, 2015-03-23 18:12:08]

Patchset #4 (id:80001) has been deleted

[Mark Mentovai, 2015-03-23 19:26:01]

Patchset #4 (id:100001) has been deleted

[Mark Mentovai, 2015-03-23 19:31:50]

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

[Mark Mentovai, 2015-03-23 19:31:50]

Let’s do a re-review of this now that we have a direction.

The new strategy is to provide a metrics_client_id crash key when Crashpad is in
use. This will contain the client ID used for metrics reporting. Crashpad still
owns the “main” client ID automatically transmitted as the guid key.

+isherman for components/metrics

[Ilya Sherman, 2015-03-24 00:22:23]

Thanks, Mark.  Metrics changes LGTM % a couple of nits:

https://codereview.chromium.org/1000203007/diff/160001/chrome/browser/metrics...
File chrome/browser/metrics/chrome_metrics_service_client.cc (right):

https://codereview.chromium.org/1000203007/diff/160001/chrome/browser/metrics...
chrome/browser/metrics/chrome_metrics_service_client.cc:193: #endif
Optional: IMO, it would be nicer if the crash_keys API was platform-neutral, and
the implementation details handled all the OS-checks.

https://codereview.chromium.org/1000203007/diff/160001/components/metrics/met...
File components/metrics/metrics_service_client.h (right):

https://codereview.chromium.org/1000203007/diff/160001/components/metrics/met...
components/metrics/metrics_service_client.h:34: virtual void RecordingDisabled()
= 0;
nit: Let's name this "OnRecordingDisabled", to emphasize that it's an event
listener.

[Mark Mentovai, 2015-03-24 01:27:29]

Patchset #7 (id:180001) has been deleted

[Mark Mentovai, 2015-03-24 01:30:24]

Thank you, Ilya. I took the optional suggestion, too. Updated.

[Ilya Sherman, 2015-03-24 03:52:50]

LGTM, thanks :)

[Robert Sesek, 2015-03-24 13:34:03]

LGTM

https://codereview.chromium.org/1000203007/diff/200001/chrome/common/crash_ke...
File chrome/common/crash_keys.h (right):

https://codereview.chromium.org/1000203007/diff/200001/chrome/common/crash_ke...
chrome/common/crash_keys.h:26: // dashes before setting the crash key) by which
this crash reporting client can
"by which this crash reporting client" isn't quite accurate anymore.

[Mark Mentovai, 2015-03-24 13:59:27]

Patchset #8 (id:220001) has been deleted

[commit-bot: I haz the power, 2015-03-24 14:02:07]

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

[Mark Mentovai, 2015-03-24 14:02:20]

Committed patchset #8 (id:240001) manually as
c67fa64ff088022c1149f50ca69a1b173167afe5 (presubmit successful).