Add base::TerminateBecauseOutOfMemory

Add base::TerminateBecauseOutOfMemory

base::TerminateBecauseOutOfMemory will call base::::OnNoMemory which is classified by
breakpad server as OOM crash.
We have a lot of variations of crashes which are expected OOM crashes.
Combining them into one class of crashes helps to discard them triaging crashes.

BUG=429763, 460995
Committed: https://crrev.com/06c8318ea6fe05cdbc7a1f75e136c6af32b5a494
Cr-Commit-Position: refs/heads/master@{#322621}

[Vitaly Buka (NO REVIEWS), 2015-03-26 23:13:17]

vitalybuka@chromium.org changed reviewers:
+ jbauman@chromium.org, reveman@chromium.org

[Vitaly Buka (NO REVIEWS), 2015-03-26 23:27:56]

Patchset #3 (id:30001) has been deleted

[Vitaly Buka (NO REVIEWS), 2015-03-26 23:29:20]

ptal

[Vitaly Buka (NO REVIEWS), 2015-03-26 23:30:22]

vitalybuka@chromium.org changed reviewers:
+ jam@chromium.org

[jbauman, 2015-03-26 23:33:45]

lgtm

[reveman, 2015-03-27 05:05:20]

https://codereview.chromium.org/1039043002/diff/2/base/process/memory.cc
File base/process/memory.cc (right):

https://codereview.chromium.org/1039043002/diff/2/base/process/memory.cc#newc...
base/process/memory.cc:14: NOINLINE void OnNoMemory() {
We seem to have different implementations of this for each platform. Should
CrashWithOutOfMemory() be implemented in each platform specific file and use the
existing OnNoMemory() functions instead?

https://codereview.chromium.org/1039043002/diff/2/base/process/memory.h
File base/process/memory.h (right):

https://codereview.chromium.org/1039043002/diff/2/base/process/memory.h#newco...
base/process/memory.h:44: BASE_EXPORT void CrashWithOutOfMemory();
Maybe TerminateBecauseOutOfMemory() would make it a bit more clear what this
function does. Just a suggestion.

Also, should we add a size argument so it's easy to see how large the allocation
that caused an OOM error was?

https://codereview.chromium.org/1039043002/diff/2/content/child/child_discard...
File content/child/child_discardable_shared_memory_manager.cc (right):

https://codereview.chromium.org/1039043002/diff/2/content/child/child_discard...
content/child/child_discardable_shared_memory_manager.cc:268:
CHECK(base::SharedMemory::IsHandleValid(handle));
This is an OOM error too. You can just remove this line as Map will fail in
these cases.

[Vitaly Buka (NO REVIEWS), 2015-03-27 06:29:17]

https://codereview.chromium.org/1039043002/diff/2/base/process/memory.cc
File base/process/memory.cc (right):

https://codereview.chromium.org/1039043002/diff/2/base/process/memory.cc#newc...
base/process/memory.cc:14: NOINLINE void OnNoMemory() {
On 2015/03/27 05:05:20, reveman wrote:
> We seem to have different implementations of this for each platform. Should
> CrashWithOutOfMemory() be implemented in each platform specific file and use
the
> existing OnNoMemory() functions instead?

I guess it's just historical, I see no good reason to have this as platform
specific function. Later we replace platform specific with this one.

I don't want to do this right now to avoid changing crash stacks for stuff that
already classified, just in case. If this one classified as expected we can
consider removing platform specific code.

https://codereview.chromium.org/1039043002/diff/2/base/process/memory.h
File base/process/memory.h (right):

https://codereview.chromium.org/1039043002/diff/2/base/process/memory.h#newco...
base/process/memory.h:44: BASE_EXPORT void CrashWithOutOfMemory();
On 2015/03/27 05:05:20, reveman wrote:
> Maybe TerminateBecauseOutOfMemory() would make it a bit more clear what this
> function does. Just a suggestion.
> 
> Also, should we add a size argument so it's easy to see how large the
allocation
> that caused an OOM error was?

Done.

https://codereview.chromium.org/1039043002/diff/2/content/child/child_discard...
File content/child/child_discardable_shared_memory_manager.cc (right):

https://codereview.chromium.org/1039043002/diff/2/content/child/child_discard...
content/child/child_discardable_shared_memory_manager.cc:268:
CHECK(base::SharedMemory::IsHandleValid(handle));
On 2015/03/27 05:05:20, reveman wrote:
> This is an OOM error too. You can just remove this line as Map will fail in
> these cases.

Done.

[reveman, 2015-03-27 13:04:51]

lgtm

https://codereview.chromium.org/1039043002/diff/2/base/process/memory.cc
File base/process/memory.cc (right):

https://codereview.chromium.org/1039043002/diff/2/base/process/memory.cc#newc...
base/process/memory.cc:14: NOINLINE void OnNoMemory() {
On 2015/03/27 06:29:13, Vitaly Buka wrote:
> On 2015/03/27 05:05:20, reveman wrote:
> > We seem to have different implementations of this for each platform. Should
> > CrashWithOutOfMemory() be implemented in each platform specific file and use
> the
> > existing OnNoMemory() functions instead?
> 
> I guess it's just historical, I see no good reason to have this as platform
> specific function.
> Later we replace platform specific with this one.

Looks like the return type of OnNoMemory is different depending on the platform.
However, if there's a clean way to avoid platform specific versions of this
function than that sgtm.

> 
> I don't want to do this right now to avoid changing crash stacks for stuff
that
> already classified, just in case. If this one classified as expected we can
> consider removing platform specific code.

[jam, 2015-03-27 16:00:33]

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

[jam, 2015-03-27 16:00:33]

+mark for base ownership

content lgtm if mark is fine with base.

So to make sure I understand correctly, the motivation here is that for other
OOM, we already know that. but these two scenarios don't go through the same
code paths hence the need to explicitly mark this as OOM?

[Vitaly Buka (NO REVIEWS), 2015-03-27 16:23:13]

On 2015/03/27 16:00:33, jam wrote:
> +mark for base ownership
> 
> content lgtm if mark is fine with base.
> 
> So to make sure I understand correctly, the motivation here is that for other
> OOM, we already know that. but these two scenarios don't go through the same
> code paths hence the need to explicitly mark this as OOM?

Yes. Also you can search Google3 for "`anonymous namespace'::OnNoMemory"
These two cases had no OnNoMemory in stack.

[Mark Mentovai, 2015-03-27 17:33:35]

LGTM

https://codereview.chromium.org/1039043002/diff/70001/base/process/memory.cc
File base/process/memory.cc (right):

https://codereview.chromium.org/1039043002/diff/70001/base/process/memory.cc#...
base/process/memory.cc:13: // Breakpad server already classifies
base::`anonymous namespace'::OnNoMemory as
“already” makes sense for a commit message but has no place in a code comment.

[Vitaly Buka (NO REVIEWS), 2015-03-27 17:36:52]

https://codereview.chromium.org/1039043002/diff/70001/base/process/memory.cc
File base/process/memory.cc (right):

https://codereview.chromium.org/1039043002/diff/70001/base/process/memory.cc#...
base/process/memory.cc:13: // Breakpad server already classifies
base::`anonymous namespace'::OnNoMemory as
On 2015/03/27 17:33:35, Mark Mentovai wrote:
> “already” makes sense for a commit message but has no place in a code comment.

Done.

[Vitaly Buka (NO REVIEWS), 2015-03-27 17:36:56]

The CQ bit was checked by vitalybuka@chromium.org

[Vitaly Buka (NO REVIEWS), 2015-03-27 17:36:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbauman@chromium.org, mark@chromium.org, reveman@chromium.org, jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1039043002/#ps90001
(title: "Fri Mar 27 10:36:24 PDT 2015")

[commit-bot: I haz the power, 2015-03-27 17:37:38]

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

[commit-bot: I haz the power, 2015-03-27 19:06:51]

Committed patchset #5 (id:90001)

[commit-bot: I haz the power, 2015-03-27 19:08:10]

Patchset 5 (id:??) landed as
https://crrev.com/06c8318ea6fe05cdbc7a1f75e136c6af32b5a494
Cr-Commit-Position: refs/heads/master@{#322621}

[Vitaly Buka (NO REVIEWS), 2015-03-29 04:46:56]

Patchset #6 (id:110001) has been deleted