chromeos: Initial support for crash reporting for chrome --mash

chromeos: Initial support for crash reporting for chrome --mash

This enables crash dumping with breakpad for chrome --mash.
* Create a MashCrashReporterClient for breakpad
* Initialize breakpad inside MashRunner
* Move EnableInProcessStackDumping calls so they don't overwrite signal
handlers installed by breakpad

Crash reporting is limited to chromeos only because MashCrashReporterClient
duplicates some code in ChromeCrashReporterClient. The latter has content and
installer dependencies that will require more work to refactor before the
code can be shared with mash.

Crash reporting doesn't yet work on device with mash because LibCrosService
network proxy resolution isn't wired up yet.

BUG=585218
TEST=run chrome --mash --enable-crash-reporter-for-testing --ash-debug-shortcuts
then hit Ctrl-Alt-Shift-K and see crash dumped and system crash reporter run.

Review-Url: https://codereview.chromium.org/2643853005
Cr-Commit-Position: refs/heads/master@{#444856}
Committed: https://chromium.googlesource.com/chromium/src/+/56508cd35b92846ad5f61bc8115c5186cd1cb152

[James Cook, 2017-01-19 02:38:04]

The CQ bit was checked by jamescook@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 02:38:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[James Cook, 2017-01-19 02:39:50]

jamescook@chromium.org changed reviewers:
+ rockot@chromium.org, sadrul@chromium.org

[James Cook, 2017-01-19 02:39:50]

rockot, PTAL at chrome/app/mash and services/service_manager

sadrul, PTAL at services/ui/gpu

This code is a bit heavy on TODOs, but I need something preliminary working with
--mash before I start moving around existing crash code. This also allows us to
potentially work on the LibCrosService dbus stuff in parallel.

[commit-bot: I haz the power, 2017-01-19 02:43:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 02:43:05]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[James Cook, 2017-01-19 02:50:17]

https://codereview.chromium.org/2643853005/diff/20001/chrome/app/mash/BUILD.gn
File chrome/app/mash/BUILD.gn (right):

https://codereview.chromium.org/2643853005/diff/20001/chrome/app/mash/BUILD.g...
chrome/app/mash/BUILD.gn:46: "//services/ui/gpu",
Also, this is unfortunate. Any thoughts on how to handle crash keys, which need
to be used inside service code, but registered in a central location near main()
for crash reporting?

[James Cook, 2017-01-19 02:50:53]

The CQ bit was checked by jamescook@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 02:51:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-19 03:12:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 03:12:11]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sadrul, 2017-01-19 04:22:11]

https://codereview.chromium.org/2643853005/diff/20001/chrome/app/mash/BUILD.gn
File chrome/app/mash/BUILD.gn (right):

https://codereview.chromium.org/2643853005/diff/20001/chrome/app/mash/BUILD.g...
chrome/app/mash/BUILD.gn:46: "//services/ui/gpu",
On 2017/01/19 02:50:17, James Cook wrote:
> Also, this is unfortunate. Any thoughts on how to handle crash keys, which
need
> to be used inside service code, but registered in a central location near
main()
> for crash reporting?

Maybe use crash_keys::kActiveURL in chrome code instead?

I would use that key from mus-gpu too, but it brings in more dependencies, so
went with hardcoding the url instead. The crash-key owners were OK with it
(https://codereview.chromium.org/2622073005/#msg4)

https://codereview.chromium.org/2643853005/diff/20001/chrome/app/mash/mash_ru...
File chrome/app/mash/mash_runner.cc (right):

https://codereview.chromium.org/2643853005/diff/20001/chrome/app/mash/mash_ru...
chrome/app/mash/mash_runner.cc:141:
crash_reporter::SetCrashReporterClient(client);
I guess we are deliberately leaking here? Do we need to add some hints to
appease lsan?

[James Cook, 2017-01-19 18:00:21]

Patchset #3 (id:40001) has been deleted

[James Cook, 2017-01-19 18:01:02]

The CQ bit was checked by jamescook@chromium.org to run a CQ dry run

[James Cook, 2017-01-19 18:01:18]

sadrul/rockot - please take another look.

https://codereview.chromium.org/2643853005/diff/20001/chrome/app/mash/BUILD.gn
File chrome/app/mash/BUILD.gn (right):

https://codereview.chromium.org/2643853005/diff/20001/chrome/app/mash/BUILD.g...
chrome/app/mash/BUILD.gn:46: "//services/ui/gpu",
On 2017/01/19 04:22:11, sadrul wrote:
> On 2017/01/19 02:50:17, James Cook wrote:
> > Also, this is unfortunate. Any thoughts on how to handle crash keys, which
> need
> > to be used inside service code, but registered in a central location near
> main()
> > for crash reporting?
> 
> Maybe use crash_keys::kActiveURL in chrome code instead?
> 
> I would use that key from mus-gpu too, but it brings in more dependencies, so
> went with hardcoding the url instead. The crash-key owners were OK with it
> (https://codereview.chromium.org/2622073005/#msg4)

//chrome/app/mash is not supposed to depend on any code in //chrome - see
https://cs.chromium.org/chromium/src/chrome/app/mash/DEPS?type=cs&q=chrome/ap...

I'm going to remove the dep and hardcode the string instead, because crash_keys
owners were OK with that for your CL.

https://codereview.chromium.org/2643853005/diff/20001/chrome/app/mash/mash_ru...
File chrome/app/mash/mash_runner.cc (right):

https://codereview.chromium.org/2643853005/diff/20001/chrome/app/mash/mash_ru...
chrome/app/mash/mash_runner.cc:141:
crash_reporter::SetCrashReporterClient(client);
On 2017/01/19 04:22:11, sadrul wrote:
> I guess we are deliberately leaking here? Do we need to add some hints to
> appease lsan?

Good catch on lsan! Done.

[commit-bot: I haz the power, 2017-01-19 18:01:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ken Rockot(use gerrit already), 2017-01-19 18:03:58]

lgtm

[commit-bot: I haz the power, 2017-01-19 19:10:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 19:10:08]

Dry run: This issue passed the CQ dry run.

[sadrul, 2017-01-19 21:47:40]

lgtm (you can probably update the CL description to remove the bit about
GpuService)

[James Cook, 2017-01-19 21:50:37]

Description was changed from

==========
chromeos: Initial support for crash reporting for chrome --mash

This enables crash dumping with breakpad for chrome --mash.
* Create a MashCrashReporterClient for breakpad
* Initialize breakpad inside MashRunner
* Move EnableInProcessStackDumping calls so they don't overwrite signal
handlers installed by breakpad
* Use a shared constant for the ui::GpuService active URL crash key

Crash reporting is limited to chromeos only because MashCrashReporterClient
duplicates some code in ChromeCrashReporterClient. The latter has content and
installer dependencies that will require more work to refactor before the
code can be shared with mash.

Crash reporting doesn't yet work on device with mash because LibCrosService
network proxy resolution isn't wired up yet.

BUG=585218
TEST=run chrome --mash --enable-crash-reporter-for-testing --ash-debug-shortcuts
then hit Ctrl-Alt-Shift-K and see crash dumped and system crash reporter run.
==========

to

==========
chromeos: Initial support for crash reporting for chrome --mash

This enables crash dumping with breakpad for chrome --mash.
* Create a MashCrashReporterClient for breakpad
* Initialize breakpad inside MashRunner
* Move EnableInProcessStackDumping calls so they don't overwrite signal
handlers installed by breakpad

Crash reporting is limited to chromeos only because MashCrashReporterClient
duplicates some code in ChromeCrashReporterClient. The latter has content and
installer dependencies that will require more work to refactor before the
code can be shared with mash.

Crash reporting doesn't yet work on device with mash because LibCrosService
network proxy resolution isn't wired up yet.

BUG=585218
TEST=run chrome --mash --enable-crash-reporter-for-testing --ash-debug-shortcuts
then hit Ctrl-Alt-Shift-K and see crash dumped and system crash reporter run.
==========

[James Cook, 2017-01-19 21:51:36]

jamescook@chromium.org changed reviewers:
+ sky@chromium.org

[James Cook, 2017-01-19 21:51:36]

sky, can I get OWNERS stamp for chrome/test/base/mash_browser_tests_main.cc ?

(sadrul, I updated the CL description. Thanks for the review.)

[sky, 2017-01-19 21:59:09]

LGTM

[James Cook, 2017-01-19 22:01:58]

The CQ bit was checked by jamescook@chromium.org

[commit-bot: I haz the power, 2017-01-19 22:03:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-19 22:08:53]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484863318595130,
"parent_rev": "9514fbd507c6ffb769209244dc083abfc7879a5b", "commit_rev":
"56508cd35b92846ad5f61bc8115c5186cd1cb152"}

[commit-bot: I haz the power, 2017-01-19 22:09:24]

Description was changed from

==========
chromeos: Initial support for crash reporting for chrome --mash

This enables crash dumping with breakpad for chrome --mash.
* Create a MashCrashReporterClient for breakpad
* Initialize breakpad inside MashRunner
* Move EnableInProcessStackDumping calls so they don't overwrite signal
handlers installed by breakpad

Crash reporting is limited to chromeos only because MashCrashReporterClient
duplicates some code in ChromeCrashReporterClient. The latter has content and
installer dependencies that will require more work to refactor before the
code can be shared with mash.

Crash reporting doesn't yet work on device with mash because LibCrosService
network proxy resolution isn't wired up yet.

BUG=585218
TEST=run chrome --mash --enable-crash-reporter-for-testing --ash-debug-shortcuts
then hit Ctrl-Alt-Shift-K and see crash dumped and system crash reporter run.
==========

to

==========
chromeos: Initial support for crash reporting for chrome --mash

This enables crash dumping with breakpad for chrome --mash.
* Create a MashCrashReporterClient for breakpad
* Initialize breakpad inside MashRunner
* Move EnableInProcessStackDumping calls so they don't overwrite signal
handlers installed by breakpad

Crash reporting is limited to chromeos only because MashCrashReporterClient
duplicates some code in ChromeCrashReporterClient. The latter has content and
installer dependencies that will require more work to refactor before the
code can be shared with mash.

Crash reporting doesn't yet work on device with mash because LibCrosService
network proxy resolution isn't wired up yet.

BUG=585218
TEST=run chrome --mash --enable-crash-reporter-for-testing --ash-debug-shortcuts
then hit Ctrl-Alt-Shift-K and see crash dumped and system crash reporter run.

Review-Url: https://codereview.chromium.org/2643853005
Cr-Commit-Position: refs/heads/master@{#444856}
Committed:
https://chromium.googlesource.com/chromium/src/+/56508cd35b92846ad5f61bc8115c...
==========

[commit-bot: I haz the power, 2017-01-19 22:09:25]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/56508cd35b92846ad5f61bc8115c...