Cleaner fall-back stack capture for --enable-heap-profiling=native.

base/trace_event/memory_dump_manager.cc

Index: base/trace_event/memory_dump_manager.cc
diff --git a/base/trace_event/memory_dump_manager.cc b/base/trace_event/memory_dump_manager.cc
index 45d448425e9afbf7112e9575aefa6f2a5b2a32bc..af66828f1692841d82e421a69f1cd71f7a115d57 100644
--- a/base/trace_event/memory_dump_manager.cc
+++ b/base/trace_event/memory_dump_manager.cc
@@ -33,6 +33,7 @@
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/trace_event_argument.h"
 #include "build/build_config.h"
+#include "build/buildflag.h"
 
 #if defined(OS_ANDROID)
 #include "base/trace_event/java_heap_dump_provider_android.h"
@@ -187,14 +188,11 @@ void MemoryDumpManager::EnableHeapProfilingIfNeeded() {
   if (profiling_mode == "") {
     AllocationContextTracker::SetCaptureMode(
         AllocationContextTracker::CaptureMode::PSEUDO_STACK);
-#if HAVE_TRACE_STACK_FRAME_POINTERS && \
-    (BUILDFLAG(ENABLE_PROFILING) || !defined(NDEBUG))
   } else if (profiling_mode == switches::kEnableHeapProfilingModeNative) {

[Primiano Tucci (use gerrit), 2017/03/22 10:38:35]

should we have an #if defined(COMPONENT_BUILD) here to just die with a "don't
even try, we can't support component builds, too much trouble sorry."?
Not sure what is the state of component build on various platforms. If it works
I am happy. But if it doesn't (or is just too painful to maintain) I am very
happy if we just give a clear signal saying "nope, static build only".

I'd just avoid mixed signals, where we pretend that it works, but then at the
end it doesn't. finding that after some spending some hours of attempts can be
quite frustrating for people.

[DmitrySkiba, 2017/03/23 19:10:24]

Component builds *should* be working on Linux and Android, not sure about other
platforms. I'll double check. 

[Wez, 2017/03/27 03:59:37]

That would equally apply to the pseudo-stack case, above, since its a question
of whether we have working allocator shims, I believe.

Rather than an explicit check for component build here, I'd prefer to test a
macro indicating whether or not we have working allocator shims, and have that
set alongside the shimming code (or in the GN if necessary) appropriately to the
build type. 
Agreed - ideally I'd prefer that we pop up a helpful message as well, rather
than just CHECK()ing to generate a stack trace!

[Wez, 2017/03/27 03:59:37]

Definitely not on Windows, since the shim impl is relies on statically-linked
MSVCRT.

[Primiano Tucci (use gerrit), 2017/03/27 09:42:47]

Ah, I was thinking more to the "can this unwinder cope with multiple .so(s)".
but you make another good point.
+1 I like this. To be clear doesn't  have to happen in this CL, but I'd just
kindly ask that this happens at some point in the near future.
Filed crbug.com/705461 to keep track of this.

-    // We need frame pointers for native tracing to work, and they are
-    // enabled in profiling and debug builds.
+    // If we don't have frame pointers then native tracing falls-back to
+    // using base::debug::StackTrace, which may be slow.
     AllocationContextTracker::SetCaptureMode(
         AllocationContextTracker::CaptureMode::NATIVE_STACK);
-#endif
 #if BUILDFLAG(ENABLE_MEMORY_TASK_PROFILER)
   } else if (profiling_mode == switches::kEnableHeapProfilingTaskProfiler) {
     // Enable heap tracking, which in turn enables capture of heap usage