Display histograms from subprocesses in chrome://histograms

chrome/browser/metrics/subprocess_metrics_provider.h

Index: chrome/browser/metrics/subprocess_metrics_provider.h
diff --git a/chrome/browser/metrics/subprocess_metrics_provider.h b/chrome/browser/metrics/subprocess_metrics_provider.h
index 684ce195acf186da09ba648ad7eb7b2fe02dc862..e706c213d0f2ffebd8fcdc3a56e9232530567879 100644
--- a/chrome/browser/metrics/subprocess_metrics_provider.h
+++ b/chrome/browser/metrics/subprocess_metrics_provider.h
@@ -5,13 +5,14 @@
 #ifndef CHROME_BROWSER_METRICS_SUBPROCESS_METRICS_PROVIDER_H_
 #define CHROME_BROWSER_METRICS_SUBPROCESS_METRICS_PROVIDER_H_
 
+#include <map>
 #include <memory>
-#include <set>
+#include <vector>
 
 #include "base/gtest_prod_util.h"
-#include "base/id_map.h"
+#include "base/metrics/statistics_recorder.h"
 #include "base/scoped_observer.h"
-#include "base/threading/thread_checker.h"
+#include "base/synchronization/lock.h"
 #include "components/metrics/metrics_provider.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
@@ -24,9 +25,11 @@ class SharedPersistentMemoryAllocator;
 
 // SubprocessMetricsProvider gathers and logs histograms stored in shared
 // memory segments between processes.
-class SubprocessMetricsProvider : public metrics::MetricsProvider,
-                                  public content::NotificationObserver,
-                                  public content::RenderProcessHostObserver {
+class SubprocessMetricsProvider
+    : public metrics::MetricsProvider,
+      public base::StatisticsRecorder::MetricsDisplay,
+      public content::NotificationObserver,
+      public content::RenderProcessHostObserver {
  public:
   SubprocessMetricsProvider();
   ~SubprocessMetricsProvider() override;
@@ -44,9 +47,15 @@ class SubprocessMetricsProvider : public metrics::MetricsProvider,
   // allocator it was using.
   void DeregisterSubprocessAllocator(int id);
 
-  // Report all histograms of a given allocator to the snapshot-manager.
+  // Report histograms of a given allocator to the snapshot-manager. If
+  // |query| is not empty, only histograms that contain that string will
+  // be recorded. If |absolute| is true, no delta from the previous snapshot
+  // will be done. Only one caller, the caller that uploads to UMA, is
+  // allowed to calcuate deltas because they are not independent.
   void RecordHistogramSnapshotsFromAllocator(
       base::HistogramSnapshotManager* snapshot_manager,
+      const std::string& query,
+      bool absolute,
       int id,
       base::PersistentHistogramAllocator* allocator);
 
@@ -56,6 +65,12 @@ class SubprocessMetricsProvider : public metrics::MetricsProvider,
   void RecordHistogramSnapshots(
       base::HistogramSnapshotManager* snapshot_manager) override;
 
+  // base::StatisticsRecorder::MetricsDisplay:
+  void WriteTitleString(std::string* output) override;
+  void WriteGraphs(const std::string& query,
+                   bool html,
+                   std::string* output) override;
+
   // content::NotificationObserver:
   void Observe(int type,
                const content::NotificationSource& source,
@@ -68,14 +83,16 @@ class SubprocessMetricsProvider : public metrics::MetricsProvider,
                            int exit_code) override;
   void RenderProcessHostDestroyed(content::RenderProcessHost* host) override;
 
-  base::ThreadChecker thread_checker_;
+  // Lock for accessing objects. This is required because MetricsDisplay is
+  // typically called by a background thread.

[Ilya Sherman, 2016/04/13 00:29:32]

Hmm, why so?  I would have expected it to be called from the main thread.  Could
the messages from background threads instead be bounced to the main thread?

[bcwhite, 2016/04/13 11:58:18]

The previous code broke because it was called on a different thread.  I didn't
investigate.  I assume that it's merely being collected in the background for
sending to the renderer process for display.

I considered bouncing but it seemed an overly complex solution.  Acquiring the
lock is something that happens relatively infrequently.  Also, generating all
these histogram graphs could take significant time so probably best if it's not
on the UI thread.

I can reduce the amount of time for which the lock is held, however.

+  base::Lock lock_;
 
   // Object for registing notification requests.
   content::NotificationRegistrar registrar_;
 
   // All of the shared-persistent-allocators for known sub-processes.
   using AllocatorByIdMap =
-      IDMap<base::PersistentHistogramAllocator, IDMapOwnPointer, int>;
+      std::map<int, std::unique_ptr<base::PersistentHistogramAllocator>>;
   AllocatorByIdMap allocators_by_id_;
 
   // Allocators that are no longer attached to a subprocess, to be released