[tracing] Dump child processes' memory metrics in browser

components/tracing/process_metrics_memory_dump_provider.cc

+// Copyright 2015 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 "components/tracing/process_metrics_memory_dump_provider.h"
+
+#include <map>
+
+#include "base/lazy_instance.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/process/process_metrics.h"
+#include "base/trace_event/memory_dump_manager.h"
+#include "base/trace_event/process_memory_dump.h"
+#include "base/trace_event/process_memory_totals.h"
+#include "build/build_config.h"
+
+namespace tracing {
+
+namespace {
+
+base::LazyInstance<std::map<base::ProcessHandle,
+                            scoped_ptr<ProcessMetricsMemoryDumpProvider>>>::
+    Leaky g_dump_providers_map = LAZY_INSTANCE_INITIALIZER;
+
+scoped_ptr<base::ProcessMetrics> CreateProcessMetrics(
+    base::ProcessHandle process) {
+  if (process == base::kNullProcessHandle)
+    return make_scoped_ptr(base::ProcessMetrics::CreateCurrentProcessMetrics());
+#if !defined(OS_MACOSX) || defined(OS_IOS)
+  return make_scoped_ptr(base::ProcessMetrics::CreateProcessMetrics(process));
+#else
+  // Creating process metrics for child processes in mac requires PortProvider.

[Primiano Tucci (use gerrit), 2016/01/05 11:55:17]

I'd add "and is a non needed use case" to the comment

[ssid, 2016/01/06 20:26:33]

Done.

+  NOTREACHED();
+#endif
+}
+
+}  // namespace
+
+// static
+uint64_t ProcessMetricsMemoryDumpProvider::rss_bytes_for_testing = 0;
+
+// static
+void ProcessMetricsMemoryDumpProvider::RegisterForProcess(
+    base::ProcessHandle process) {
+  scoped_ptr<ProcessMetricsMemoryDumpProvider> metrics_provider(
+      new ProcessMetricsMemoryDumpProvider(process));
+  base::trace_event::MemoryDumpProvider::Options options(
+      base::GetProcId(process));

[Primiano Tucci (use gerrit), 2016/01/05 11:55:17]

If you need a process_id at the end, doesn't it make more sense that you use
ProcessId instead of ProcessHandle here (and in all the rest?).
So you don't have to make this conversion.
Also, the code for GetProcID says:
"DEPRECATED. New code should be using Process::Pid() instead"

[ssid, 2016/01/06 20:26:33]

Done.

+  base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
+      metrics_provider.get(), "ProcessMemoryMetrics", nullptr, options);
+  g_dump_providers_map.Get().insert(

[Primiano Tucci (use gerrit), 2016/01/05 11:55:17]

maybe make this a 

bool did_insert = g_dump_providers_map.Get().insert(...).second
DCHECK(did_insert) << "ProcessMetricsMemoryDumpProvider already registered for
process " << pid

[ssid, 2016/01/06 20:26:33]

hm, there are nacl processes in chromeos builds which do not give the correct
pid to browser process. These processes are all with pid "1" and registers
multiple times. It seems complicated to understand why the nacl helper processes
are not giving the correct pids and I don't think its worth the time to fix
that. So, making this a warning message.

[Primiano Tucci (use gerrit), 2016/01/07 11:34:34]

Ahh I see. Agree not worth wasting time for those. 

+      std::make_pair(process, std::move(metrics_provider)));
+}
+
+// static
+void ProcessMetricsMemoryDumpProvider::UnregisterForProcess(
+    base::ProcessHandle process) {
+  auto iter = g_dump_providers_map.Get().find(process);
+  DCHECK(iter != g_dump_providers_map.Get().end());

[Primiano Tucci (use gerrit), 2016/01/05 11:55:17]

I'd make this a bit safer and do:

if (iter == g_dump_providers_map.Get().end()) {
 NOTREACHED();
 return;
}

[ssid, 2016/01/06 20:26:33]

Done.

+  base::trace_event::MemoryDumpManager::GetInstance()
+      ->UnregisterAndDeleteDumpProviderAsync(std::move(iter->second));
+  g_dump_providers_map.Get().erase(iter);
+}
+
+ProcessMetricsMemoryDumpProvider::ProcessMetricsMemoryDumpProvider(
+    base::ProcessHandle process)
+    : process_(process), process_metrics_(CreateProcessMetrics(process)) {}
+
+ProcessMetricsMemoryDumpProvider::~ProcessMetricsMemoryDumpProvider() {}
+
+// Called at trace dump point time. Creates a snapshot of the memory maps for
+// the current process.
+bool ProcessMetricsMemoryDumpProvider::OnMemoryDump(
+    const base::trace_event::MemoryDumpArgs& args,
+    base::trace_event::ProcessMemoryDump* pmd) {
+  bool res = DumpProcessTotals(args, pmd);
+
+#if defined(OS_LINUX) || defined(OS_ANDROID)
+  res &= DumpProcessMemoryMaps(args, pmd);

[Primiano Tucci (use gerrit), 2016/01/05 11:55:17]

shouldn't this check for heavy vs light dump here?

[ssid, 2016/01/06 20:26:33]

Moved it here. It was inside the function.

+#endif
+  return res;
+}
+
+// Linux and Android have specialized function in

[Primiano Tucci (use gerrit), 2016/01/05 11:55:17]

I'd reword this as:

// See process_metrics_memory_dump_provider_linux.cc for the
Linux/Android-specific implementation.

[ssid, 2016/01/06 20:26:33]

removed.

+// process_metrics_memory_dump_provider_linux.cc because kernel supports rss
+// peak resetting.
+#if !defined(OS_LINUX) && !defined(OS_ANDROID)
+bool ProcessMetricsMemoryDumpProvider::DumpProcessTotals(
+    const base::trace_event::MemoryDumpArgs& args,
+    base::trace_event::ProcessMemoryDump* pmd) {
+  const uint64_t rss_bytes = rss_bytes_for_testing
+                                 ? rss_bytes_for_testing
+                                 : process_metrics_->GetWorkingSetSize();
+
+  // rss_bytes will be 0 if the process ended while dumping.
+  if (!rss_bytes)
+    return false;
+
+  uint64_t peak_rss_bytes = 0;
+
+#if !defined(OS_IOS)
+  peak_rss_bytes = process_metrics_->GetPeakWorkingSetSize();
+#if defined(MACOSX)
+  size_t private_bytes;
+  bool res = process_metrics_->GetMemoryBytes(&private_bytes,
+                                              nullptr /* shared_bytes */);
+  if (res) {

[Primiano Tucci (use gerrit), 2016/01/05 11:55:17]

nit: you can remove these braces here

[ssid, 2016/01/06 20:26:33]

Done.

+    pmd->process_totals()->SetExtraFieldInBytes("private_bytes", private_bytes);
+  }
+#endif  // defined(MACOSX)
+#endif  // !defined(OS_IOS)
+
+  pmd->process_totals()->set_resident_set_bytes(rss_bytes);
+  pmd->set_has_process_totals();
+  pmd->process_totals()->set_peak_resident_set_bytes(peak_rss_bytes);
+
+  // Returns true even if other metrics failed, since rss is reported.

[Primiano Tucci (use gerrit), 2016/01/05 11:55:17]

/reported/always reported/

[ssid, 2016/01/06 20:26:33]

The rss is not reported, and function fails if child process ended.

+  return true;
+}
+#endif  // !defined(OS_LINUX) && !defined(OS_ANDROID)
+
+}  // namespace tracing