Use persistent memory for receiving metrics from sub-processes.

chrome/browser/metrics/subprocess_metrics_provider.cc

 // Copyright 2016 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 "chrome/browser/metrics/subprocess_metrics_provider.h"
 
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_base.h"
 #include "base/metrics/histogram_macros.h"
 #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) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!allocators_by_id_.Lookup(id));
 
   // Map is "MapOwnPointer" so transfer ownership to it.
   allocators_by_id_.AddWithID(allocator.release(), id);
 }
 
@@ skipping 40 matching lines @@
        iter.Advance()) {
     MergeHistogramDeltasFromAllocator(iter.GetCurrentKey(),
                                       iter.GetCurrentValue());
   }
 
   UMA_HISTOGRAM_COUNTS_100(
       "UMA.SubprocessMetricsProvider.SubprocessCount",
       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,
     const content::NotificationDetails& details) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_EQ(content::NOTIFICATION_RENDERER_PROCESS_CREATED, type);
 
   content::RenderProcessHost* host =
       content::Source<content::RenderProcessHost>(source).ptr();
 
@@ skipping 32 matching lines @@
     content::RenderProcessHost* host) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // It's possible for a Renderer to terminate without RenderProcessExited
   // (above) being called so it's necessary to de-register also upon the
   // destruction of the host. If both get called, no harm is done.
 
   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))))));
+}