Signal chrome_elf to initialize crash reporting, rather than doing it in DllMain

Signal chrome_elf to initialize crash reporting, rather than doing it in DllMain

Currently, initialization crash reporting indirectly calls
CreateProcess() from chrome_elf's DllMain(), which causes deadlock in
some configurations (showing up in M54).

Instead, signal to chrome_elf that it should initialize crash reporting
later in WinMain(). This means we lose a little coverage that we were
hoping to gain from the move to chrome_elf, but a localized fix is
required for merge to stable.

Follow up bug is https://crbug.com/656800.

BUG=655788, 656800
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
TEST=enforce certificate rules per https://bugs.chromium.org/p/chromium/issues/detail?id=655788#c57 and then try to launch chrome.

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

[scottmg, 2016-10-17 20:57:53]

Description was changed from

==========
Signal chrome_elf to initialize crash reporting, rather than doing it in DllMain

Currently, initialization crash reporting indirectly calls
CreateProcess() from chrome_elf's DllMain(), which causes deadlock in
some configurations (showing up in M54).

Instead, signal to chrome_elf that it should initialize crash reporting
later in WinMain(). This means we lose a little coverage that we were
hoping to gain from the move to chrome_elf, but a localized fix is
required for merge to stable.

BUG=655788
==========

to

==========
Signal chrome_elf to initialize crash reporting, rather than doing it in DllMain

Currently, initialization crash reporting indirectly calls
CreateProcess() from chrome_elf's DllMain(), which causes deadlock in
some configurations (showing up in M54).

Instead, signal to chrome_elf that it should initialize crash reporting
later in WinMain(). This means we lose a little coverage that we were
hoping to gain from the move to chrome_elf, but a localized fix is
required for merge to stable.

BUG=655788
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[Will Harris, 2016-10-17 21:07:45]

On 2016/10/17 20:57:53, scottmg wrote:
> Description was changed from
> 
> ==========
> Signal chrome_elf to initialize crash reporting, rather than doing it in
DllMain
> 
> Currently, initialization crash reporting indirectly calls
> CreateProcess() from chrome_elf's DllMain(), which causes deadlock in
> some configurations (showing up in M54).
> 
> Instead, signal to chrome_elf that it should initialize crash reporting
> later in WinMain(). This means we lose a little coverage that we were
> hoping to gain from the move to chrome_elf, but a localized fix is
> required for merge to stable.
> 
> BUG=655788
> ==========
> 
> to
> 
> ==========
> Signal chrome_elf to initialize crash reporting, rather than doing it in
DllMain
> 
> Currently, initialization crash reporting indirectly calls
> CreateProcess() from chrome_elf's DllMain(), which causes deadlock in
> some configurations (showing up in M54).
> 
> Instead, signal to chrome_elf that it should initialize crash reporting
> later in WinMain(). This means we lose a little coverage that we were
> hoping to gain from the move to chrome_elf, but a localized fix is
> required for merge to stable.
> 
> BUG=655788
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
> ==========

[Will Harris, 2016-10-17 21:07:57]

Seems like this should work to me. Can you add a manual TEST= line for TE?

[scottmg, 2016-10-17 21:08:51]

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

[ananta, 2016-10-17 21:09:13]

ananta@chromium.org changed reviewers:
+ ananta@chromium.org

[ananta, 2016-10-17 21:09:13]

lgtm

[commit-bot: I haz the power, 2016-10-17 21:09:25]

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

[scottmg, 2016-10-17 21:09:47]

Description was changed from

==========
Signal chrome_elf to initialize crash reporting, rather than doing it in DllMain

Currently, initialization crash reporting indirectly calls
CreateProcess() from chrome_elf's DllMain(), which causes deadlock in
some configurations (showing up in M54).

Instead, signal to chrome_elf that it should initialize crash reporting
later in WinMain(). This means we lose a little coverage that we were
hoping to gain from the move to chrome_elf, but a localized fix is
required for merge to stable.

BUG=655788
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Signal chrome_elf to initialize crash reporting, rather than doing it in DllMain

Currently, initialization crash reporting indirectly calls
CreateProcess() from chrome_elf's DllMain(), which causes deadlock in
some configurations (showing up in M54).

Instead, signal to chrome_elf that it should initialize crash reporting
later in WinMain(). This means we lose a little coverage that we were
hoping to gain from the move to chrome_elf, but a localized fix is
required for merge to stable.

BUG=655788
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
TEST=enforce certificate rules per
https://bugs.chromium.org/p/chromium/issues/detail?id=655788#c57 and then try to
launch chrome.
==========

[scottmg, 2016-10-17 21:09:47]

scottmg@chromium.org changed reviewers:
- ananta@chromium.org

[scottmg, 2016-10-17 21:10:05]

On 2016/10/17 21:07:57, Will Harris wrote:
> Seems like this should work to me. Can you add a manual TEST= line for TE?

Done.

[scottmg, 2016-10-17 21:19:30]

scottmg@chromium.org changed reviewers:
+ brettw@chromium.org, pennymac@chromium.org

[scottmg, 2016-10-17 21:21:13]

scottmg@chromium.org changed reviewers:
+ thestig@chromium.org
- brettw@chromium.org

[scottmg, 2016-10-17 21:21:14]

pennymac: chrome_elf/ OWNERS
thestig: for small change in chrome_exe_main_win.cc (sorry for going to
chrome/OWNERS, my other options are grt who's out for the week, and cpu who's
... just out).

[scottmg, 2016-10-17 21:21:44]

Description was changed from

==========
Signal chrome_elf to initialize crash reporting, rather than doing it in DllMain

Currently, initialization crash reporting indirectly calls
CreateProcess() from chrome_elf's DllMain(), which causes deadlock in
some configurations (showing up in M54).

Instead, signal to chrome_elf that it should initialize crash reporting
later in WinMain(). This means we lose a little coverage that we were
hoping to gain from the move to chrome_elf, but a localized fix is
required for merge to stable.

BUG=655788
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
TEST=enforce certificate rules per
https://bugs.chromium.org/p/chromium/issues/detail?id=655788#c57 and then try to
launch chrome.
==========

to

==========
Signal chrome_elf to initialize crash reporting, rather than doing it in DllMain

Currently, initialization crash reporting indirectly calls
CreateProcess() from chrome_elf's DllMain(), which causes deadlock in
some configurations (showing up in M54).

Instead, signal to chrome_elf that it should initialize crash reporting
later in WinMain(). This means we lose a little coverage that we were
hoping to gain from the move to chrome_elf, but a localized fix is
required for merge to stable.

Follow up bug is https://crbug.com/656800.

BUG=655788, 656800
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
TEST=enforce certificate rules per
https://bugs.chromium.org/p/chromium/issues/detail?id=655788#c57 and then try to
launch chrome.
==========

[scottmg, 2016-10-17 21:30:01]

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

[commit-bot: I haz the power, 2016-10-17 21:30:06]

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

[penny, 2016-10-17 21:32:41]

On 2016/10/17 21:21:14, scottmg wrote:
> pennymac: chrome_elf/ OWNERS
> thestig: for small change in chrome_exe_main_win.cc (sorry for going to
> chrome/OWNERS, my other options are grt who's out for the week, and cpu who's
> ... just out)

Thanks Scott.  Quick sanity check though:

We're not disabling/hooking SetUnhandledExceptionFilter() in the ELF with this
CL.... even later when we do initialize crashpad.  And it's no help to do so
later, because it's too late.  

And if we do let the hook happen despite cp not being initialized... what
happens if there's an early crash and there's an unhandled exception (and the
CRT handler is not in place either)?

[penny, 2016-10-17 21:36:40]

On 2016/10/17 21:32:41, penny wrote:
> On 2016/10/17 21:21:14, scottmg wrote:
> > pennymac: chrome_elf/ OWNERS
> > thestig: for small change in chrome_exe_main_win.cc (sorry for going to
> > chrome/OWNERS, my other options are grt who's out for the week, and cpu
who's
> > ... just out)
> 
> Thanks Scott.  Quick sanity check though:
> 
> We're not disabling/hooking SetUnhandledExceptionFilter() in the ELF with this
> CL.... even later when we do initialize crashpad.  And it's no help to do so
> later, because it's too late.  
> 
> And if we do let the hook happen despite cp not being initialized... what
> happens if there's an early crash and there's an unhandled exception (and the
> CRT handler is not in place either)?

The best option here would be to let CRT register, and then when we do
initialize crashpad in the elf later, make sure we become the top level
exception handler.  But that's easier said than done:

https://cs.chromium.org/chromium/src/chrome_elf/crash/crash_helper.cc?q=g_set...

[scottmg, 2016-10-17 21:37:43]

On 2016/10/17 21:32:41, penny wrote:
> On 2016/10/17 21:21:14, scottmg wrote:
> > pennymac: chrome_elf/ OWNERS
> > thestig: for small change in chrome_exe_main_win.cc (sorry for going to
> > chrome/OWNERS, my other options are grt who's out for the week, and cpu
who's
> > ... just out)
> 
> Thanks Scott.  Quick sanity check though:
> 
> We're not disabling/hooking SetUnhandledExceptionFilter() in the ELF with this
> CL.... even later when we do initialize crashpad.  And it's no help to do so
> later, because it's too late.  
> 
> And if we do let the hook happen despite cp not being initialized... what
> happens if there's an early crash and there's an unhandled exception (and the
> CRT handler is not in place either)?

Good catch. Duh, that's not going to work at all. :(

I guess I have to move the whole thing to the signal function to at least work
correctly.

We're going to just get WerFault until WinMain() then.

[scottmg, 2016-10-17 21:38:00]

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

[commit-bot: I haz the power, 2016-10-17 21:38:34]

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

[penny, 2016-10-17 21:46:56]

On 2016/10/17 21:37:43, scottmg wrote:
> On 2016/10/17 21:32:41, penny wrote:
> > On 2016/10/17 21:21:14, scottmg wrote:
> > > pennymac: chrome_elf/ OWNERS
> > > thestig: for small change in chrome_exe_main_win.cc (sorry for going to
> > > chrome/OWNERS, my other options are grt who's out for the week, and cpu
> who's
> > > ... just out)
> > 
> > Thanks Scott.  Quick sanity check though:
> > 
> > We're not disabling/hooking SetUnhandledExceptionFilter() in the ELF with
this
> > CL.... even later when we do initialize crashpad.  And it's no help to do so
> > later, because it's too late.  
> > 
> > And if we do let the hook happen despite cp not being initialized... what
> > happens if there's an early crash and there's an unhandled exception (and
the
> > CRT handler is not in place either)?
> 
> Good catch. Duh, that's not going to work at all. :(
> 
> I guess I have to move the whole thing to the signal function to at least work
> correctly.
> 
> We're going to just get WerFault until WinMain() then.

I'd like to see a test for what happens if we let the hook happen early
(essentially disabling the CRT unhandled exception handler), and then cause an
exception before CP is initialized.  What's the OS behaviour.  We might be able
to get away with that... and then we'll still be top-level exception handler
later once CP initializes.

[penny, 2016-10-17 22:44:31]

Patchset #4 (id:60001) has been deleted

[penny, 2016-10-17 22:46:56]

On 2016/10/17 21:46:56, penny wrote:
> On 2016/10/17 21:37:43, scottmg wrote:
> > On 2016/10/17 21:32:41, penny wrote:
> > > On 2016/10/17 21:21:14, scottmg wrote:
> > > > pennymac: chrome_elf/ OWNERS
> > > > thestig: for small change in chrome_exe_main_win.cc (sorry for going to
> > > > chrome/OWNERS, my other options are grt who's out for the week, and cpu
> > who's
> > > > ... just out)
> > > 
> > > Thanks Scott.  Quick sanity check though:
> > > 
> > > We're not disabling/hooking SetUnhandledExceptionFilter() in the ELF with
> this
> > > CL.... even later when we do initialize crashpad.  And it's no help to do
so
> > > later, because it's too late.  
> > > 
> > > And if we do let the hook happen despite cp not being initialized... what
> > > happens if there's an early crash and there's an unhandled exception (and
> the
> > > CRT handler is not in place either)?
> > 
> > Good catch. Duh, that's not going to work at all. :(
> > 
> > I guess I have to move the whole thing to the signal function to at least
work
> > correctly.
> > 
> > We're going to just get WerFault until WinMain() then.
> 
> I'd like to see a test for what happens if we let the hook happen early
> (essentially disabling the CRT unhandled exception handler), and then cause an
> exception before CP is initialized.  What's the OS behaviour.  We might be
able
> to get away with that... and then we'll still be top-level exception handler
> later once CP initializes.

LGTM, patch set #3.

We don't need the hook to happen if we're initializing "later", because we'll be
at the top of the handler chain.  All good.

[Lei Zhang, 2016-10-17 22:58:12]

lgtm

[scottmg, 2016-10-17 23:12:18]

The CQ bit was unchecked by scottmg@chromium.org

[scottmg, 2016-10-17 23:14:39]

On 2016/10/17 22:46:56, penny wrote:
> On 2016/10/17 21:46:56, penny wrote:
> > On 2016/10/17 21:37:43, scottmg wrote:
> > > On 2016/10/17 21:32:41, penny wrote:
> > > > On 2016/10/17 21:21:14, scottmg wrote:
> > > > > pennymac: chrome_elf/ OWNERS
> > > > > thestig: for small change in chrome_exe_main_win.cc (sorry for going
to
> > > > > chrome/OWNERS, my other options are grt who's out for the week, and
cpu
> > > who's
> > > > > ... just out)
> > > > 
> > > > Thanks Scott.  Quick sanity check though:
> > > > 
> > > > We're not disabling/hooking SetUnhandledExceptionFilter() in the ELF
with
> > this
> > > > CL.... even later when we do initialize crashpad.  And it's no help to
do
> so
> > > > later, because it's too late.  
> > > > 
> > > > And if we do let the hook happen despite cp not being initialized...
what
> > > > happens if there's an early crash and there's an unhandled exception
(and
> > the
> > > > CRT handler is not in place either)?
> > > 
> > > Good catch. Duh, that's not going to work at all. :(
> > > 
> > > I guess I have to move the whole thing to the signal function to at least
> work
> > > correctly.
> > > 
> > > We're going to just get WerFault until WinMain() then.
> > 
> > I'd like to see a test for what happens if we let the hook happen early
> > (essentially disabling the CRT unhandled exception handler), and then cause
an
> > exception before CP is initialized.  What's the OS behaviour.  We might be
> able
> > to get away with that... and then we'll still be top-level exception handler
> > later once CP initializes.
> 
> LGTM, patch set #3.
> 
> We don't need the hook to happen if we're initializing "later", because we'll
be
> at the top of the handler chain.  All good.

I don't think ps#3 is correct. The disabling of SetUnhandledExceptionFilter in
DllMain means that the later signalling and registration won't do anything, so
Crashpad won't handle any crashes. I thought that's what was being pointed out
here https://codereview.chromium.org/2428703002/#msg17.

So I think I'll resurrect ps#4?

[scottmg, 2016-10-17 23:27:46]

On 2016/10/17 23:14:39, scottmg wrote:
> On 2016/10/17 22:46:56, penny wrote:
> > On 2016/10/17 21:46:56, penny wrote:
> > > On 2016/10/17 21:37:43, scottmg wrote:
> > > > On 2016/10/17 21:32:41, penny wrote:
> > > > > On 2016/10/17 21:21:14, scottmg wrote:
> > > > > > pennymac: chrome_elf/ OWNERS
> > > > > > thestig: for small change in chrome_exe_main_win.cc (sorry for going
> to
> > > > > > chrome/OWNERS, my other options are grt who's out for the week, and
> cpu
> > > > who's
> > > > > > ... just out)
> > > > > 
> > > > > Thanks Scott.  Quick sanity check though:
> > > > > 
> > > > > We're not disabling/hooking SetUnhandledExceptionFilter() in the ELF
> with
> > > this
> > > > > CL.... even later when we do initialize crashpad.  And it's no help to
> do
> > so
> > > > > later, because it's too late.  
> > > > > 
> > > > > And if we do let the hook happen despite cp not being initialized...
> what
> > > > > happens if there's an early crash and there's an unhandled exception
> (and
> > > the
> > > > > CRT handler is not in place either)?
> > > > 
> > > > Good catch. Duh, that's not going to work at all. :(
> > > > 
> > > > I guess I have to move the whole thing to the signal function to at
least
> > work
> > > > correctly.
> > > > 
> > > > We're going to just get WerFault until WinMain() then.
> > > 
> > > I'd like to see a test for what happens if we let the hook happen early
> > > (essentially disabling the CRT unhandled exception handler), and then
cause
> an
> > > exception before CP is initialized.  What's the OS behaviour.  We might be
> > able
> > > to get away with that... and then we'll still be top-level exception
handler
> > > later once CP initializes.
> > 
> > LGTM, patch set #3.
> > 
> > We don't need the hook to happen if we're initializing "later", because
we'll
> be
> > at the top of the handler chain.  All good.
> 
> I don't think ps#3 is correct. The disabling of SetUnhandledExceptionFilter in
> DllMain means that the later signalling and registration won't do anything, so
> Crashpad won't handle any crashes. I thought that's what was being pointed out
> here https://codereview.chromium.org/2428703002/#msg17.
> 
> So I think I'll resurrect ps#4?

Sorry, I was mistaken, set straight over IM. The scary looking
elf_crash::DisableSetUnhandledExceptionFilter() in DllMain() doesn't do
anything.

I have confirmed with ps3 that 1) chrome://inducebrowsercrashforrealz and
chrome://crash both result in Crashpad handled-dumps.

I'll confirm for posterity what happens with an early crash in chrome_elf and
then we can land.

[scottmg, 2016-10-17 23:44:09]

On 2016/10/17 23:27:46, scottmg wrote:
> On 2016/10/17 23:14:39, scottmg wrote:
> > On 2016/10/17 22:46:56, penny wrote:
> > > On 2016/10/17 21:46:56, penny wrote:
> > > > On 2016/10/17 21:37:43, scottmg wrote:
> > > > > On 2016/10/17 21:32:41, penny wrote:
> > > > > > On 2016/10/17 21:21:14, scottmg wrote:
> > > > > > > pennymac: chrome_elf/ OWNERS
> > > > > > > thestig: for small change in chrome_exe_main_win.cc (sorry for
going
> > to
> > > > > > > chrome/OWNERS, my other options are grt who's out for the week,
and
> > cpu
> > > > > who's
> > > > > > > ... just out)
> > > > > > 
> > > > > > Thanks Scott.  Quick sanity check though:
> > > > > > 
> > > > > > We're not disabling/hooking SetUnhandledExceptionFilter() in the ELF
> > with
> > > > this
> > > > > > CL.... even later when we do initialize crashpad.  And it's no help
to
> > do
> > > so
> > > > > > later, because it's too late.  
> > > > > > 
> > > > > > And if we do let the hook happen despite cp not being initialized...
> > what
> > > > > > happens if there's an early crash and there's an unhandled exception
> > (and
> > > > the
> > > > > > CRT handler is not in place either)?
> > > > > 
> > > > > Good catch. Duh, that's not going to work at all. :(
> > > > > 
> > > > > I guess I have to move the whole thing to the signal function to at
> least
> > > work
> > > > > correctly.
> > > > > 
> > > > > We're going to just get WerFault until WinMain() then.
> > > > 
> > > > I'd like to see a test for what happens if we let the hook happen early
> > > > (essentially disabling the CRT unhandled exception handler), and then
> cause
> > an
> > > > exception before CP is initialized.  What's the OS behaviour.  We might
be
> > > able
> > > > to get away with that... and then we'll still be top-level exception
> handler
> > > > later once CP initializes.
> > > 
> > > LGTM, patch set #3.
> > > 
> > > We don't need the hook to happen if we're initializing "later", because
> we'll
> > be
> > > at the top of the handler chain.  All good.
> > 
> > I don't think ps#3 is correct. The disabling of SetUnhandledExceptionFilter
in
> > DllMain means that the later signalling and registration won't do anything,
so
> > Crashpad won't handle any crashes. I thought that's what was being pointed
out
> > here https://codereview.chromium.org/2428703002/#msg17.
> > 
> > So I think I'll resurrect ps#4?
> 
> Sorry, I was mistaken, set straight over IM. The scary looking
> elf_crash::DisableSetUnhandledExceptionFilter() in DllMain() doesn't do
> anything.
> 
> I have confirmed with ps3 that 1) chrome://inducebrowsercrashforrealz and
> chrome://crash both result in Crashpad handled-dumps.
> 
> I'll confirm for posterity what happens with an early crash in chrome_elf and
> then we can land.

A__debugbreak() chrome_elf DllMain() results in "failed to initialize
application" as if a dll was missing. that's wermgr.exe, not a full WerFault.

A crash in chrome.exe before signaling chrome_elf to initialize crash reporting
(i.e. in a global constructor or something) results in a WerFault dialog crash
report.

Crashes after that go to Crashpad.

So we have some follow up work to do for sure, but I will land this as-is for
54.

[scottmg, 2016-10-17 23:44:16]

The CQ bit was checked by scottmg@chromium.org

[scottmg, 2016-10-17 23:44:17]

The patchset sent to the CQ was uploaded after l-g-t-m from ananta@chromium.org
Link to the patchset: https://codereview.chromium.org/2428703002/#ps40001
(title: ".")

[commit-bot: I haz the power, 2016-10-17 23:45:12]

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

[scottmg, 2016-10-18 00:55:04]

It looks like some the test binaries are going to fail on
win10_chromium_x64_rel_ng for a second time (though a different set than last
time, browser_tests succeeded this time). Should I be "NOTRY=true"ing to get
past this bot then?

[scottmg, 2016-10-18 00:56:47]

On 2016/10/18 00:55:04, scottmg wrote:
> It looks like some the test binaries are going to fail on
> win10_chromium_x64_rel_ng for a second time (though a different set than last
> time, browser_tests succeeded this time). Should I be "NOTRY=true"ing to get
> past this bot then?

Oops, just too impatient. Looks like it might be green after all.

[commit-bot: I haz the power, 2016-10-18 01:05:30]

Description was changed from

==========
Signal chrome_elf to initialize crash reporting, rather than doing it in DllMain

Currently, initialization crash reporting indirectly calls
CreateProcess() from chrome_elf's DllMain(), which causes deadlock in
some configurations (showing up in M54).

Instead, signal to chrome_elf that it should initialize crash reporting
later in WinMain(). This means we lose a little coverage that we were
hoping to gain from the move to chrome_elf, but a localized fix is
required for merge to stable.

Follow up bug is https://crbug.com/656800.

BUG=655788, 656800
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
TEST=enforce certificate rules per
https://bugs.chromium.org/p/chromium/issues/detail?id=655788#c57 and then try to
launch chrome.
==========

to

==========
Signal chrome_elf to initialize crash reporting, rather than doing it in DllMain

Currently, initialization crash reporting indirectly calls
CreateProcess() from chrome_elf's DllMain(), which causes deadlock in
some configurations (showing up in M54).

Instead, signal to chrome_elf that it should initialize crash reporting
later in WinMain(). This means we lose a little coverage that we were
hoping to gain from the move to chrome_elf, but a localized fix is
required for merge to stable.

Follow up bug is https://crbug.com/656800.

BUG=655788, 656800
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
TEST=enforce certificate rules per
https://bugs.chromium.org/p/chromium/issues/detail?id=655788#c57 and then try to
launch chrome.

Committed: https://crrev.com/f3a5670dd8d42b045d31625dde4a2561b471138f
Cr-Commit-Position: refs/heads/master@{#425840}
==========

[commit-bot: I haz the power, 2016-10-18 01:05:31]

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