Make base::TerminateBecauseOutOfMemory call RaiseException on Windows.

Make base::TerminateBecauseOutOfMemory call RaiseException on Windows.

This is a follow-on CL to https://codereview.chromium.org/2130293003 which added the RaiseException call in memory_win.cc but missed memory.cc

BUG=614440
Committed: https://crrev.com/3850a4d9df7a7e6f1c2d48715362de6bac01523a
Cr-Commit-Position: refs/heads/master@{#407315}

[Will Harris, 2016-07-21 18:00:44]

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

[commit-bot: I haz the power, 2016-07-21 18:01:57]

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, 2016-07-21 18:08:42]

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

[commit-bot: I haz the power, 2016-07-21 18:08:43]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

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

Description was changed from

==========
Make base::TerminateBecauseOutOfMemory raise exception on Windows.

BUG=614440
==========

to

==========
Make base::TerminateBecauseOutOfMemory raise exception on Windows.

BUG=614440
==========

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

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

[Will Harris, 2016-07-21 21:10:27]

wfh@chromium.org changed reviewers:
+ mtklein@chromium.org

[Will Harris, 2016-07-21 21:11:14]

Description was changed from

==========
Make base::TerminateBecauseOutOfMemory raise exception on Windows.

BUG=614440
==========

to

==========
Make base::TerminateBecauseOutOfMemory call RaiseException on Windows.

This is a follow-on CL to https://codereview.chromium.org/2130293003 which added
the RaiseException call in memory_win.cc but missed memory.cc

BUG=614440
==========

[Will Harris, 2016-07-21 21:11:27]

PTAL - missed an OOM handler.

[mtklein_C, 2016-07-21 21:13:47]

the Skia bit (very) lgtm

[Mark Mentovai, 2016-07-21 21:15:12]

LGTM

https://codereview.chromium.org/2173463002/diff/20001/base/process/memory_win.cc
File base/process/memory_win.cc (right):

https://codereview.chromium.org/2173463002/diff/20001/base/process/memory_win...
base/process/memory_win.cc:41: const DWORD kOomExceptionCode = 0xe0000008;
Now you’ve got to update the comment in Breakpad to point to this file. :)

[Will Harris, 2016-07-21 23:02:45]

actually I need this code to be exposed out of this file, as it's needed by
base/process/kill_win.cc to decode the right termination status, so I'm going to
move it back out into a header. I'll re-upload a CL soon! Kill two birds with
one stone!

[Will Harris, 2016-07-21 23:32:05]

mark - this is enough different it probably needs another quick review. I made
the OOM exception code not wrapped in OS_WIN because it would mean anything that
wanted to use it would have to be wrapped in OS_WIN too and it gets ugly really
quick... otherwise the only functional change is that OnNoMemory is NOINLINE
just to make sure it appears in its own stack, which it does already but just
extra safety - crash/12b34f8900000000

[Mark Mentovai, 2016-07-22 01:51:02]

This is LGTM, except

Will Harris wrote:
> mark - this is enough different it probably needs another quick review. I made
> the OOM exception code not wrapped in OS_WIN because it would mean anything
that
> wanted to use it would have to be wrapped in OS_WIN too and it gets ugly
really
> quick...

I don’t agree with this. I think it should be in an OS_WIN #ifdef, and it looks
like everything that uses it here is also already Windows-only, so that
shouldn’t be a problem.

[Will Harris, 2016-07-22 02:01:08]

On 2016/07/22 01:51:02, Mark Mentovai wrote:
> This is LGTM, except
> 
> Will Harris wrote:
> > mark - this is enough different it probably needs another quick review. I
made
> > the OOM exception code not wrapped in OS_WIN because it would mean anything
> that
> > wanted to use it would have to be wrapped in OS_WIN too and it gets ugly
> really
> > quick...
> 
> I don’t agree with this. I think it should be in an OS_WIN #ifdef, and it
looks
> like everything that uses it here is also already Windows-only, so that
> shouldn’t be a problem.

understood -  and you're owner, but future code
https://codereview.chromium.org/2172013002/ uses this and will end up being
massively OS_WIN-deffed including other stuff in base and grit resources. So let
me know either way and I can do what you want.

[Will Harris, 2016-07-22 02:04:25]

On 2016/07/22 02:01:08, Will Harris wrote:
> On 2016/07/22 01:51:02, Mark Mentovai wrote:
> > This is LGTM, except
> > 
> > Will Harris wrote:
> > > mark - this is enough different it probably needs another quick review. I
> made
> > > the OOM exception code not wrapped in OS_WIN because it would mean
anything
> > that
> > > wanted to use it would have to be wrapped in OS_WIN too and it gets ugly
> > really
> > > quick...
> > 
> > I don’t agree with this. I think it should be in an OS_WIN #ifdef, and it
> looks
> > like everything that uses it here is also already Windows-only, so that
> > shouldn’t be a problem.
> 
> understood -  and you're owner, but future code
> https://codereview.chromium.org/2172013002/ uses this and will end up being
> massively OS_WIN-deffed including other stuff in base and grit resources. So
let
> me know either way and I can do what you want.

actually in support of your argument, I originally thought other variables e.g.
kMaxOomScore were not wrapped in #if defined but they actually are, so it does
make sense to wrap this one too... I can grudgingly do this and change the other
CL. :)

[Will Harris, 2016-07-22 15:35:34]

Patchset #5 (id:80001) has been deleted

[Mark Mentovai, 2016-07-22 18:30:12]

LGTM here now

[Will Harris, 2016-07-22 18:39:44]

The CQ bit was checked by wfh@chromium.org

[Will Harris, 2016-07-22 18:39:45]

The patchset sent to the CQ was uploaded after l-g-t-m from mtklein@chromium.org
Link to the patchset: https://codereview.chromium.org/2173463002/#ps100001
(title: "make win only")

[commit-bot: I haz the power, 2016-07-22 18:40:21]

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, 2016-07-22 18:47:46]

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

[commit-bot: I haz the power, 2016-07-22 18:47:47]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[Will Harris, 2016-07-22 19:07:05]

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

[commit-bot: I haz the power, 2016-07-22 19:08:01]

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, 2016-07-22 19:22:29]

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

[commit-bot: I haz the power, 2016-07-22 19:22:30]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[Will Harris, 2016-07-22 21:50:05]

Patchset #6 (id:120001) has been deleted

[Will Harris, 2016-07-22 21:55:39]

The CQ bit was checked by wfh@chromium.org

[Will Harris, 2016-07-22 21:55:39]

The patchset sent to the CQ was uploaded after l-g-t-m from mark@chromium.org,
mtklein@chromium.org
Link to the patchset: https://codereview.chromium.org/2173463002/#ps140001
(title: "implement in memory_stubs")

[commit-bot: I haz the power, 2016-07-22 21:56:26]

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, 2016-07-22 22:19:05]

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

[commit-bot: I haz the power, 2016-07-22 22:19:06]

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

[Will Harris, 2016-07-22 22:22:09]

On 2016/07/22 22:19:06, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

not my CL! :)

[Will Harris, 2016-07-22 22:28:50]

The CQ bit was checked by wfh@chromium.org

[commit-bot: I haz the power, 2016-07-22 22:29:12]

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, 2016-07-23 00:24:08]

Description was changed from

==========
Make base::TerminateBecauseOutOfMemory call RaiseException on Windows.

This is a follow-on CL to https://codereview.chromium.org/2130293003 which added
the RaiseException call in memory_win.cc but missed memory.cc

BUG=614440
==========

to

==========
Make base::TerminateBecauseOutOfMemory call RaiseException on Windows.

This is a follow-on CL to https://codereview.chromium.org/2130293003 which added
the RaiseException call in memory_win.cc but missed memory.cc

BUG=614440
==========

[commit-bot: I haz the power, 2016-07-23 00:24:09]

Committed patchset #6 (id:140001)

[commit-bot: I haz the power, 2016-07-23 00:26:42]

Description was changed from

==========
Make base::TerminateBecauseOutOfMemory call RaiseException on Windows.

This is a follow-on CL to https://codereview.chromium.org/2130293003 which added
the RaiseException call in memory_win.cc but missed memory.cc

BUG=614440
==========

to

==========
Make base::TerminateBecauseOutOfMemory call RaiseException on Windows.

This is a follow-on CL to https://codereview.chromium.org/2130293003 which added
the RaiseException call in memory_win.cc but missed memory.cc

BUG=614440

Committed: https://crrev.com/3850a4d9df7a7e6f1c2d48715362de6bac01523a
Cr-Commit-Position: refs/heads/master@{#407315}
==========

[commit-bot: I haz the power, 2016-07-23 00:26:43]

Patchset 6 (id:??) landed as
https://crrev.com/3850a4d9df7a7e6f1c2d48715362de6bac01523a
Cr-Commit-Position: refs/heads/master@{#407315}