metrics: Connect leak detector to allocator

base/allocator/allocator_extension.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef BASE_ALLOCATOR_ALLOCATOR_EXTENSION_H_
 #define BASE_ALLOCATOR_ALLOCATOR_EXTENSION_H_
 
 #include <stddef.h> // for size_t
 
+#include <type_traits>  // for std::add_pointer
+
 #include "base/base_export.h"
 #include "build/build_config.h"
 
 namespace base {
 namespace allocator {
 
+// Callback types for alloc and free.
+using AllocHookFunc = std::add_pointer<void(const void*, size_t)>::type;

[Primiano Tucci (use gerrit), 2016/03/01 18:54:37]

What is the benefit of std::add_pointer as opposite to just a function pointer
(see below). I don't see any precedence in the codebase nor it seems whitelisted
in https://chromium-cpp.appspot.com/.

e.g. using AllocHookFunc = void (*)(const void*, size_t);

+using FreeHookFunc = std::add_pointer<void(const void*)>::type;
+
 // Request that the allocator release any free memory it knows about to the
 // system.
 BASE_EXPORT void ReleaseFreeMemory();
 
 // Get the named property's |value|. Returns true if the property is known.
 // Returns false if the property is not a valid property name for the current
 // allocator implementation.
 // |name| or |value| cannot be NULL
 BASE_EXPORT bool GetNumericProperty(const char* name, size_t* value);
 
 BASE_EXPORT bool IsHeapProfilerRunning();
 
+// Register callbacks for alloc and free. Can only store one callback at a time
+// for each of alloc and free. Returns the previously stored hook function, or
+// nullptr if none was stored.
+BASE_EXPORT AllocHookFunc SetSingleAllocHook(AllocHookFunc func);

[Primiano Tucci (use gerrit), 2016/03/01 18:54:37]

I'd drop Single from the name. The fact that you have just one should be clear
from the "Set" (as opposite to Add) in the name.
Single is a bit misleading, for a moment I thought you were using this to hook
only one allocation and then self-uninstall.

Also I think is saner to make the CHECK you have in components here in base, and
make this return void to keep it simple, unless you really need the old hook for
reasons other than just checking. More details in the comment in the .cc file

Also, very likely you want to set both these hook, not just one of them, so why
not just making this a single function which takes both argument, ie.

SetAllocatorHooks(AllocHookFunc, FreeHookFunct) ?

+BASE_EXPORT FreeHookFunc SetSingleFreeHook(FreeHookFunc func);
+
+// Returns the currently stored alloc/free callbacks that were registered by
+// calling SetSingle{Alloc,Free}Hook().
+BASE_EXPORT AllocHookFunc GetSingleAllocHook();
+BASE_EXPORT FreeHookFunc GetSingleFreeHook();
+
+// Attempts to unwind the call stack from the current location where this
+// function is being called from. Must be called from a hook function registered
+// by calling SetSingle{Alloc,Free}Hook, directly or indirectly.
+//
+// Arguments:
+//   stack:          pointer to a pre-allocated array of void*'s.
+//   max_stack_size: indicates the size of the array in |stack|.
+//   skip_count:     Indicates how many initial call stack levels to skip before

[Primiano Tucci (use gerrit), 2016/03/01 18:54:36]

It looks like this is really not used in the .cc file. What's the point of
adding a dead parameter?

+//                   storing the caller of alloc/free. This value might not be
+//                   used, depending on the specific implementation.
+//
+// Returns the number of call stack frames stored in |stack|, or 0 if no call
+// stack information is available.
+BASE_EXPORT int GetCallStack(void** stack, int max_stack_size, int skip_count);
+
 }  // namespace allocator
 }  // namespace base
 
 #endif  // BASE_ALLOCATOR_ALLOCATOR_EXTENSION_H_