Adds chrome.memoryBenchmarking.heapProfilerDump for the browser process.

content/renderer/memory_benchmarking_extension.cc

 // Copyright (c) 2013 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.
 
 #include "content/renderer/memory_benchmarking_extension.h"
 
 #include "base/string_util.h"
-#include "content/public/renderer/render_thread.h"
+#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.

 
-#if !defined(NO_TCMALLOC) && defined(OS_LINUX)
-#include "third_party/tcmalloc/chromium/src/gperftools/heap-profiler.h"
-#endif  // !defined(NO_TCMALLOC) && defined(OS_LINUX)
+#if defined(USE_TCMALLOC) && (defined(OS_LINUX) || defined(OS_ANDROID))
 
 namespace {
 
 const char kMemoryBenchmarkingExtensionName[] = "v8/MemoryBenchmarking";
 
 }
 
 namespace {
 
 class MemoryBenchmarkingWrapper : public v8::Extension {
  public:
   MemoryBenchmarkingWrapper() :
       v8::Extension(kMemoryBenchmarkingExtensionName,
         "if (typeof(chrome) == 'undefined') {"
         "  chrome = {};"
         "};"
         "if (typeof(chrome.memoryBenchmarking) == 'undefined') {"
         "  chrome.memoryBenchmarking = {};"
         "};"
         "chrome.memoryBenchmarking.isHeapProfilerRunning = function() {"
         "  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.

         "};"
         ) {}
 
   virtual v8::Handle<v8::FunctionTemplate> GetNativeFunction(
       v8::Handle<v8::String> name) OVERRIDE {
     if (name->Equals(v8::String::New("IsHeapProfilerRunning")))
       return v8::FunctionTemplate::New(IsHeapProfilerRunning);
     else if (name->Equals(v8::String::New("HeapProfilerDump")))
       return v8::FunctionTemplate::New(HeapProfilerDump);
 
     return v8::Handle<v8::FunctionTemplate>();
   }
 
   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();
   }
 };
 
 }  // namespace
 
 namespace content {
 
 v8::Extension* MemoryBenchmarkingExtension::Get() {
   return new MemoryBenchmarkingWrapper();
 }
 
 }  // 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. :)