Use persistent memory for receiving metrics from sub-processes.

chrome/browser/metrics/subprocess_metrics_provider.cc

Index: chrome/browser/metrics/subprocess_metrics_provider.cc
diff --git a/chrome/browser/metrics/subprocess_metrics_provider.cc b/chrome/browser/metrics/subprocess_metrics_provider.cc
index dbc5774479477a6f2161192d80e498bfad3287bd..79d2cda987643d8d797dc75d7ca751e81630e16a 100644
--- a/chrome/browser/metrics/subprocess_metrics_provider.cc
+++ b/chrome/browser/metrics/subprocess_metrics_provider.cc
@@ -11,18 +11,27 @@
 #include "base/metrics/persistent_histogram_allocator.h"
 #include "base/metrics/persistent_memory_allocator.h"
 #include "components/metrics/metrics_service.h"
+#include "content/public/browser/browser_child_process_host.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/child_process_data.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 
 SubprocessMetricsProvider::SubprocessMetricsProvider()
-    : scoped_observer_(this) {
+    : scoped_observer_(this), weak_ptr_factory_(this) {}
+
+SubprocessMetricsProvider::~SubprocessMetricsProvider() {
+  // Safe even if this object has never been added as an observer.
+  content::BrowserChildProcessObserver::Remove(this);
+}
+
+void SubprocessMetricsProvider::EnableSubprocessTracking() {
+  content::BrowserChildProcessObserver::Add(this);
   registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CREATED,
                  content::NotificationService::AllBrowserContextsAndSources());
 }
 
-SubprocessMetricsProvider::~SubprocessMetricsProvider() {}
-
 void SubprocessMetricsProvider::RegisterSubprocessAllocator(
     int id,
     std::unique_ptr<base::PersistentHistogramAllocator> allocator) {
@@ -83,6 +92,41 @@ void SubprocessMetricsProvider::MergeHistogramDeltas() {
       allocators_by_id_.size());
 }
 
+void SubprocessMetricsProvider::BrowserChildProcessHostConnected(
+    const content::ChildProcessData& data) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  // It's necessary to access the BrowserChildProcessHost object that is
+  // managing the child in order to extract the metrics memory from it.
+  // Unfortunately, the required lookup can only be performed on the IO
+  // thread so do the necessary dance.
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&SubprocessMetricsProvider::
+                     RegisterBrowserChildProcessAllocatorFromIOThread,
+                 weak_ptr_factory_.GetWeakPtr(), data.id));
+}
+
+void SubprocessMetricsProvider::BrowserChildProcessHostDisconnected(
+    const content::ChildProcessData& data) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DeregisterSubprocessAllocator(data.id);
+}
+
+void SubprocessMetricsProvider::BrowserChildProcessCrashed(
+    const content::ChildProcessData& data,
+    int exit_code) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DeregisterSubprocessAllocator(data.id);
+}
+
+void SubprocessMetricsProvider::BrowserChildProcessKilled(
+    const content::ChildProcessData& data,
+    int exit_code) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DeregisterSubprocessAllocator(data.id);
+}
+
 void SubprocessMetricsProvider::Observe(
     int type,
     const content::NotificationSource& source,
@@ -135,3 +179,34 @@ void SubprocessMetricsProvider::RenderProcessHostDestroyed(
   DeregisterSubprocessAllocator(host->GetID());
   scoped_observer_.Remove(host);
 }
+
+// static
+void SubprocessMetricsProvider::
+    RegisterBrowserChildProcessAllocatorFromIOThread(
+        base::WeakPtr<SubprocessMetricsProvider> provider,
+        int id) {
+  // See if the new process has a memory allocator and take control of it if so.
+  // This call can only be made on the browser's IO thread.
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  content::BrowserChildProcessHost* host =
+      content::BrowserChildProcessHost::FromID(id);
+  if (!host)
+    return;
+  std::unique_ptr<base::SharedPersistentMemoryAllocator> allocator =
+      host->TakeMetricsAllocator();
+  if (!allocator)
+    return;
+
+  // Register the new allocator so metrics will be extracted from it. This has
+  // to be done on the UI thread.
+  content::BrowserThread::PostTask(

[Alexei Svitkine (slow), 2016/08/12 23:14:36]

This seems overly complicated.

I think you can simplify this via making
unique_ptr<base::PersistentHistogramAllocator> the return value of this function
and doing PostTaskAndReply() from the call site.

I guess the downside is that early returns from lines 194 and 198 will still
result in a reply with an empty unique ptr, but I think that's OK.

[bcwhite, 2016/08/15 19:43:09]

Done.

+      content::BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(
+          &SubprocessMetricsProvider::RegisterSubprocessAllocator,
+          provider,
+          id,
+          base::Passed(
+              WrapUnique(new base::PersistentHistogramAllocator(

[Alexei Svitkine (slow), 2016/08/12 23:14:36]

Use base::MakeUnique.

[bcwhite, 2016/08/15 19:43:10]

Done.

+                  std::move(allocator))))));
+}