Enable crash dump collection on builders

Enable crash dump collection on builders

Toolchain crashes on build machines are an ongoing problem, particularly
with the VC++ 2015 migration. Setting this registry key tells Windows
Error Reporting to record minidumps (a few MB typically) to a local
directory, up to a maximum of ten. This should help with investigations.

This change also suppresses Windows Error Reporting dialogs with the
DontShowUI registry value, to avoid builder hangs on crashes.

See also crrev.com/1816333002

BUG=440500
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299426

[brucedawson, 2016-03-22 18:47:41]

brucedawson@chromium.org changed reviewers:
+ scottmg@chromium.org

[brucedawson, 2016-03-22 18:47:41]

This is a companion change to crrev.com/1816333002. It should be safe/benign.

It won't affect developers unless then do "gclient runhooks" as administrator.
If they do that then error reporting will be enabled but any existing settings
will be unchanged, which seems reasonable and appropriate.

The code fails silently if not elevated.

[scottmg, 2016-03-22 19:27:45]

lgtm

https://codereview.chromium.org/1825163003/diff/1/win_toolchain/get_toolchain...
File win_toolchain/get_toolchain_if_necessary.py (right):

https://codereview.chromium.org/1825163003/diff/1/win_toolchain/get_toolchain...
win_toolchain/get_toolchain_if_necessary.py:363:
https://msdn.microsoft.com/en-us/library/windows/desktop/bb787181(v=vs.85).aspx
Remove "(v=vs.85)" to make it fix in 80 col

https://codereview.chromium.org/1825163003/diff/1/win_toolchain/get_toolchain...
win_toolchain/get_toolchain_if_necessary.py:365: key_name =
r'SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps'
if not sys.platform.startswith('win'):
  return

before this line for --force on Linux.

https://codereview.chromium.org/1825163003/diff/1/win_toolchain/get_toolchain...
win_toolchain/get_toolchain_if_necessary.py:376: return
Normally just "pass" instead of "return" to indicate it's being ignored.

[scottmg, 2016-03-22 19:32:02]

Actually, if we want it for builders, we should probably key it off
os.environ.get('CHROME_HEADLESS') or whatever it is, rather than assuming "other
things" will be non-elevated.

[brucedawson, 2016-03-22 20:39:18]

CR comments addressed, and DontShowUI added!

PTAL?

https://codereview.chromium.org/1825163003/diff/1/win_toolchain/get_toolchain...
File win_toolchain/get_toolchain_if_necessary.py (right):

https://codereview.chromium.org/1825163003/diff/1/win_toolchain/get_toolchain...
win_toolchain/get_toolchain_if_necessary.py:363:
https://msdn.microsoft.com/en-us/library/windows/desktop/bb787181(v=vs.85).aspx
On 2016/03/22 19:27:44, scottmg wrote:
> Remove "(v=vs.85)" to make it fix in 80 col

Done.

https://codereview.chromium.org/1825163003/diff/1/win_toolchain/get_toolchain...
win_toolchain/get_toolchain_if_necessary.py:365: key_name =
r'SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps'
On 2016/03/22 19:27:44, scottmg wrote:
> if not sys.platform.startswith('win'):
>   return
> 
> before this line for --force on Linux.

sys.platform == 'win32' seems to be standard in this file, so using that.

https://codereview.chromium.org/1825163003/diff/1/win_toolchain/get_toolchain...
win_toolchain/get_toolchain_if_necessary.py:376: return
On 2016/03/22 19:27:44, scottmg wrote:
> Normally just "pass" instead of "return" to indicate it's being ignored.

Yep. I previously had the except elsewhere where 'return' was needed, but now it
isn't.

Done.

[brucedawson, 2016-03-22 20:40:43]

Description was changed from

==========
Enable crash dump collection on builders

Toolchain crashes on build machines are an ongoing problem, particularly
with the VC++ 2015 migration. Setting this registry key tells Windows
Error Reporting to record minidumps (a few MB typically) to a local
directory, up to a maximum of ten. This should help with investigations.

See also crrev.com/1816333002

BUG=440500
==========

to

==========
Enable crash dump collection on builders

Toolchain crashes on build machines are an ongoing problem, particularly
with the VC++ 2015 migration. Setting this registry key tells Windows
Error Reporting to record minidumps (a few MB typically) to a local
directory, up to a maximum of ten. This should help with investigations.

This change also suppresses Windows Error Reporting dialogs with the
DontShowUI registry value, to avoid builder hangs on crashes.

See also crrev.com/1816333002

BUG=440500
==========

[scottmg, 2016-03-22 20:46:13]

lgtm

[brucedawson, 2016-03-22 20:48:08]

The CQ bit was checked by brucedawson@chromium.org

[commit-bot: I haz the power, 2016-03-22 20:48:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1825163003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1825163003/60001

[commit-bot: I haz the power, 2016-03-22 20:50:49]

Description was changed from

==========
Enable crash dump collection on builders

Toolchain crashes on build machines are an ongoing problem, particularly
with the VC++ 2015 migration. Setting this registry key tells Windows
Error Reporting to record minidumps (a few MB typically) to a local
directory, up to a maximum of ten. This should help with investigations.

This change also suppresses Windows Error Reporting dialogs with the
DontShowUI registry value, to avoid builder hangs on crashes.

See also crrev.com/1816333002

BUG=440500
==========

to

==========
Enable crash dump collection on builders

Toolchain crashes on build machines are an ongoing problem, particularly
with the VC++ 2015 migration. Setting this registry key tells Windows
Error Reporting to record minidumps (a few MB typically) to a local
directory, up to a maximum of ten. This should help with investigations.

This change also suppresses Windows Error Reporting dialogs with the
DontShowUI registry value, to avoid builder hangs on crashes.

See also crrev.com/1816333002

BUG=440500

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299426
==========

[commit-bot: I haz the power, 2016-03-22 20:50:50]

Committed patchset #4 (id:60001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=299426

[brucedawson, 2017-04-08 00:00:58]

On 2016/03/22 20:50:50, commit-bot: I haz the power wrote:
> Committed patchset #4 (id:60001) as
> http://src.chromium.org/viewvc/chrome?view=rev&revision=299426

This doesn't appear to be working. We are getting compiler crashes but no crash
dumps saved. There are many possible reasons but my theory is that we are
running 32-bit Python, so instead of creating: 
    HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\Windows Error
Reporting\LocalDumps
we are creating:
    HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\Windows\Windows Error
Reporting\LocalDumps

Sigh...

I'll put together a patch to fix this.

[brucedawson, 2017-04-10 18:29:34]

> I'll put together a patch to fix this.

https://chromium-review.googlesource.com/c/473567/ should fix this.