Fix EnableTerminationOnOutOfMemory for malloc

Fix EnableTerminationOnOutOfMemory for malloc

Currently only crashes on new and new[]. With the change it will also
crash when there is no memory for malloc as well.

BUG=434397
TEST=none.

Committed: https://crrev.com/22d64abdc792516fd0f4895b26ae3a73c85239e0
Cr-Commit-Position: refs/heads/master@{#310320}

Committed: https://crrev.com/3e11a840e89d6dce0084fc4631366b88117b5eea
Cr-Commit-Position: refs/heads/master@{#313307}

[cpu_(ooo_6.6-7.5), 2015-01-07 03:20:17]

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

[cpu_(ooo_6.6-7.5), 2015-01-07 03:20:27]

ptal.

[grt (UTC plus 2), 2015-01-07 15:46:23]

lgtm. does anything need to change in skia/ext/SkMemory_new_handler.cpp? it also
diddles the new handler.

[cpu_(ooo_6.6-7.5), 2015-01-07 17:14:10]

The CQ bit was checked by cpu@chromium.org

[commit-bot: I haz the power, 2015-01-07 17:14:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/836733006/1

[cpu_(ooo_6.6-7.5), 2015-01-07 17:15:17]

nice find. I'll check if that code is hit.

This is process wide so it should only be touched by the house owner (chrome)
not by the guest (skia).

[commit-bot: I haz the power, 2015-01-07 18:06:42]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-01-07 18:07:44]

Patchset 1 (id:??) landed as
https://crrev.com/22d64abdc792516fd0f4895b26ae3a73c85239e0
Cr-Commit-Position: refs/heads/master@{#310320}

[Marijn Kruisselbrink, 2015-01-07 18:29:18]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/833923003/ by mek@chromium.org.

The reason for reverting is: Fails to compile on win8 gn dbg builder
(http://build.chromium.org/p/chromium.win/builders/Win8%20GN%20%28dbg%29/build...):

c:\b\build\slave\win8_gn__dbg_\build\src\base\process\memory_win.cc(22) :error
C2220: warning treated as error - no 'object' file generated
c:\b\build\slave\win8_gn__dbg_\build\src\base\process\memory_win.cc(22) :
warning C4702: unreachable code.

[cpu_(ooo_6.6-7.5), 2015-01-10 05:17:32]

cpu@chromium.org changed reviewers:
+ b.kelemen@chromium.org

[cpu_(ooo_6.6-7.5), 2015-01-10 05:37:28]

ok, back to this patch, why can't anything be simple anymore in this codebase?

grt: please look at the whole patch
b.kelemen : skia part

https://codereview.chromium.org/836733006/diff/40001/content/app/content_main...
File content/app/content_main_runner.cc (right):

https://codereview.chromium.org/836733006/diff/40001/content/app/content_main...
content/app/content_main_runner.cc:490: #if defined(OS_WIN)
moving it up here so we have both crt "OMG handlers" close to each other, the
ones in RegisterInvalidParamHandler() and the ones from
EnableTerminationOnOutOfMemory()

There is one left in line 605 but I can tell that is a can of worms, so for
another time.

https://codereview.chromium.org/836733006/diff/40001/content/app/startup_help...
File content/app/startup_helper_win.cc (left):

https://codereview.chromium.org/836733006/diff/40001/content/app/startup_help...
content/app/startup_helper_win.cc:55: // Also enable the new handler for
malloc() based failures.
this was mixing concerns, set_new_mode has no relevance here.

https://codereview.chromium.org/836733006/diff/40001/skia/ext/SkMemory_new_ha...
File skia/ext/SkMemory_new_handler.cpp (left):

https://codereview.chromium.org/836733006/diff/40001/skia/ext/SkMemory_new_ha...
skia/ext/SkMemory_new_handler.cpp:63: // This is not really thread safe.  It
only won't collide with itself, but we're totally
this code is wrong. libraries shall not to touch global crt state, which btw the
std::set_new_handler and the _set_new_handler() are not the same so this code
was not doing anything: old_handler is 0 even if _set_new_handler is active
which means that OOM still crashes.

I mean other than grabbing/release 3 or 4 locks for each allocation.

[cpu_(ooo_6.6-7.5), 2015-01-10 05:39:38]

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

https://codereview.chromium.org/836733006/diff/40001/base/process/memory_win....
base/process/memory_win.cc:18: #pragma warning(disable: 4702)
funny story, this only broke the gn build because the gyp build had the
optimizations wrong.

That is why the patch was reverted.

[grt (UTC plus 2), 2015-01-12 14:29:14]

https://codereview.chromium.org/836733006/diff/40001/skia/ext/SkMemory_new_ha...
File skia/ext/SkMemory_new_handler.cpp (left):

https://codereview.chromium.org/836733006/diff/40001/skia/ext/SkMemory_new_ha...
skia/ext/SkMemory_new_handler.cpp:63: // This is not really thread safe.  It
only won't collide with itself, but we're totally
On 2015/01/10 05:37:28, cpu wrote:
> this code is wrong. libraries shall not to touch global crt state, which btw
the
> std::set_new_handler and the _set_new_handler() are not the same so this code
> was not doing anything: old_handler is 0 even if _set_new_handler is active
> which means that OOM still crashes.
> 
> I mean other than grabbing/release 3 or 4 locks for each allocation.

Should this use base::UncheckedMalloc on Win as well (ignoring for the moment
that it just calls through to malloc, and hence is really a checked malloc)
since that seems to be the intent here? That way this code will automatically do
what is intended if someone makes base::UncheckedMalloc work on Win.

[cpu_(ooo_6.6-7.5), 2015-01-24 01:09:35]

Patchset #6 (id:100001) has been deleted

[cpu_(ooo_6.6-7.5), 2015-01-24 01:59:52]

victory!!

After doing a few painful iterations I can do what grt wants (except on IOS it
seems).  

Surrender to my AWESOMENESS .   

Or

please look at the patch again.

[grt (UTC plus 2), 2015-01-27 14:56:44]

lgtm. apologies for the delay.

[cpu_(ooo_6.6-7.5), 2015-01-27 17:23:26]

The CQ bit was checked by cpu@chromium.org

[commit-bot: I haz the power, 2015-01-27 17:23:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/836733006/120001

[commit-bot: I haz the power, 2015-01-27 18:14:15]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2015-01-27 18:15:04]

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