Adds chrome.memoryBenchmarking.heapProfilerDump for the browser process.

content/renderer/memory_benchmarking_extension.cc

Index: content/renderer/memory_benchmarking_extension.cc
diff --git a/content/renderer/memory_benchmarking_extension.cc b/content/renderer/memory_benchmarking_extension.cc
index 2dbfbda58c2923a096f5bf607516c61975fcd1f6..a1651c16854fadff8efcc9b3ac209ea454337dea 100644
--- a/content/renderer/memory_benchmarking_extension.cc
+++ b/content/renderer/memory_benchmarking_extension.cc
@@ -5,11 +5,11 @@
 #include "content/renderer/memory_benchmarking_extension.h"
 
 #include "base/string_util.h"
-#include "content/public/renderer/render_thread.h"
-
-#if !defined(NO_TCMALLOC) && defined(OS_LINUX)
+#include "content/common/memory_benchmark_messages.h"
+#include "content/renderer/render_thread_impl.h"
 #include "third_party/tcmalloc/chromium/src/gperftools/heap-profiler.h"

[Dai Mikurube (NOT FULLTIME), 2013/05/15 10:24:04]

This "heap-profiler.h" should be included only when using TCMalloc.

[bulach, 2013/05/15 10:58:29]

Done.

-#endif  // !defined(NO_TCMALLOC) && defined(OS_LINUX)
+
+#if defined(USE_TCMALLOC) && (defined(OS_LINUX) || defined(OS_ANDROID))
 
 namespace {
 
@@ -33,9 +33,10 @@ class MemoryBenchmarkingWrapper : public v8::Extension {
         "  native function IsHeapProfilerRunning();"
         "  return IsHeapProfilerRunning();"
         "};"
-        "chrome.memoryBenchmarking.heapProfilerDump = function(reason) {"
+        "chrome.memoryBenchmarking.heapProfilerDump = "
+        "      function(reason, process) {"

[Dai Mikurube (NOT FULLTIME), 2013/05/15 10:24:04]

I wonder if we could order it as (process_type, reason).    It may break
compatibility with the existing V8 HeapProfilerDump(), but I think it's
acceptable since no one else has used it yet.

The reason of this order is to keep consistency with the old PyAuto API
HeapProfilerDump().  For example, Chrome Endure is switching from PyAuto to
Telemetry in https://codereview.chromium.org/14977013/.  They're having a Python
function HeapProfilerDump() which wraps V8's HeapProfilerDump().  If is has the
same interface, it's easy to understand and port.

What do you think?

[bulach, 2013/05/15 10:58:29]

sounds reasonable, done.

         "  native function HeapProfilerDump();"
-        "  return HeapProfilerDump(reason);"
+        "  return HeapProfilerDump(reason, process);"

[Dai Mikurube (NOT FULLTIME), 2013/05/15 10:36:55]

Same argument order for this HeapProfilerDump().
And, we may not need to return a value here.

[bulach, 2013/05/15 10:58:29]

Done.

         "};"
         ) {}
 
@@ -51,24 +52,27 @@ class MemoryBenchmarkingWrapper : public v8::Extension {
 
   static v8::Handle<v8::Value> IsHeapProfilerRunning(
       const v8::Arguments& args) {
-#if !defined(NO_TCMALLOC) && defined(OS_LINUX)
     return v8::Boolean::New(::IsHeapProfilerRunning());
-#else
-    return v8::Boolean::New(false);
-#endif  // !defined(NO_TCMALLOC) && defined(OS_LINUX)
+  }
+
+  static void HeapProfilerDumpBrowser(const std::string& reason) {
+    content::RenderThreadImpl::current()->Send(
+        new MemoryBenchmarkHostMsg_HeapProfilerDump(reason));
   }
 
   static v8::Handle<v8::Value> HeapProfilerDump(const v8::Arguments& args) {
-#if !defined(NO_TCMALLOC) && defined(OS_LINUX)
-    char dumped_filename_buffer[1000];
     std::string reason("benchmarking_extension");
     if (args.Length() && args[0]->IsString())
       reason = *v8::String::AsciiValue(args[0]);
-    ::HeapProfilerDumpWithFileName(reason.c_str(),
-                                   dumped_filename_buffer,
-                                   sizeof(dumped_filename_buffer));
-    return v8::String::New(dumped_filename_buffer);
-#endif  // !defined(NO_TCMALLOC) && defined(OS_LINUX)
+    std::string process;
+    if (args.Length() > 1 && args[1]->IsString())
+      process = *v8::String::AsciiValue(args[1]);
+    if (process == "browser") {
+      content::RenderThreadImpl::current()->Send(
+          new MemoryBenchmarkHostMsg_HeapProfilerDump(reason));
+    } else {
+      ::HeapProfilerDump(reason.c_str());
+    }
     return v8::Undefined();
   }
 };
@@ -82,3 +86,5 @@ v8::Extension* MemoryBenchmarkingExtension::Get() {
 }
 
 }  // namespace content
+
+#endif  // defined(USE_TCMALLOC) && (defined(OS_LINUX) || defined(OS_ANDROID))

[Dai Mikurube (NOT FULLTIME), 2013/05/15 10:24:04]

I think these functions (at least
chrome.memoryBenchmarking.isHeapProfilerRunning, but hopefully both) should be
defined even if TCMalloc is not used.  isHeapProfilerRunning should always
return false in case of no tcmalloc.

[bulach, 2013/05/15 10:58:29]

hmm.. see the comment from palmer, I suppose if we're not going to use, it's
preferred to not have these functions at all (less end-points, etc..).

the very few clients that actually need it can test (chrome &&
chrome.memoryBenchmarking)... wdyt?

[Dai Mikurube (NOT FULLTIME), 2013/05/15 12:04:20]

Hmm, makes sense.  Ok. :)