Histograms - Support histograms for Plugins, GPU

content/public/browser/histogram_synchronizer.h

+// Copyright (c) 2012 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.
+
+#ifndef CONTENT_PUBLIC_BROWSER_HISTOGRAM_SYNCHRONIZER_H_
+#define CONTENT_PUBLIC_BROWSER_HISTOGRAM_SYNCHRONIZER_H_
+#pragma once
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/memory/ref_counted.h"
+#include "base/time.h"
+#include "content/common/content_export.h"
+
+class MessageLoop;
+
+namespace content {
+
+// This class is used to fecth histogram data from the various renderer and

[jam, 2012/06/07 03:34:33]

nit: ditto re "renderer and child" -> "child"

[ramant (doing other things), 2012/06/07 23:39:25]

Done.

+// child processes, into the browser process. Such transactions are usually
+// instigated by the browser.
+//
+// There are actually two modes for getting histograms.  One is synchronous (and
+// blocks the UI thread, waiting to populate an about:histograms tab) and the
+// other is asynchronous, and used by the metrics services in preparation for a
+// log upload.
+
+class CONTENT_EXPORT HistogramSynchronizer : public
+    base::RefCountedThreadSafe<HistogramSynchronizer> {

[jam, 2012/06/07 03:34:33]

do you really need to expose that this is ref counted? i.e. if you have a
GetInstance(), it seems that ownership should lie in content, and whoever wants
a pointer to this in chrome can call GetInstance each time like we usually do
with singletons

[ramant (doing other things), 2012/06/07 23:39:25]

Done.

+ public:
+  // Returns the HistogramSynchronizer object for the current process, or NULL
+  // if none.
+  static HistogramSynchronizer* GetInstance();
+
+  HistogramSynchronizer() {}

[jam, 2012/06/07 03:34:33]

nit: not needed so just take out

[ramant (doing other things), 2012/06/07 23:39:25]

Done.

+
+  // Contact all processes, and get them to upload to the browser any/all
+  // changes to histograms. This method is called from about:histograms.
+  static void FetchHistograms();

[jam, 2012/06/07 03:34:33]

this function is only called by code in content, so it shouldn't be in the
public api (i.e. it can just be in the _impl.h)

[ramant (doing other things), 2012/06/07 23:39:25]

Done.

+
+  // Contact all processes, and get them to upload to the browser any/all
+  // changes to histograms.  When all changes have been acquired, or when the
+  // wait time expires (whichever is sooner), post the callback to the
+  // specified message loop. Note the callback is posted exactly once.
+  static void FetchHistogramsAsynchronously(MessageLoop* callback_thread,

[jam, 2012/06/07 03:34:33]

if all these methods are just static, what's the point of having an interface?

[ramant (doing other things), 2012/06/07 23:39:25]

Done.

+                                            const base::Closure& callback,
+                                            base::TimeDelta wait_time);
+
+ protected:
+  friend class base::RefCountedThreadSafe<HistogramSynchronizer>;
+
+  virtual ~HistogramSynchronizer() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(HistogramSynchronizer);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_BROWSER_HISTOGRAM_SYNCHRONIZER_H_