Add Sparse Histogram Support to metricsPrivate

chrome/browser/extensions/api/metrics_private/metrics_private_api.cc

Index: chrome/browser/extensions/api/metrics_private/metrics_private_api.cc
diff --git a/chrome/browser/extensions/api/metrics_private/metrics_private_api.cc b/chrome/browser/extensions/api/metrics_private/metrics_private_api.cc
index 3265bb45674e5576a22b244c86df0f33357aafa3..06ddbfc4a11f01476da1ff879ad2bd1d9ef0b628 100644
--- a/chrome/browser/extensions/api/metrics_private/metrics_private_api.cc
+++ b/chrome/browser/extensions/api/metrics_private/metrics_private_api.cc
@@ -8,6 +8,7 @@
 
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
+#include "base/metrics/sparse_histogram.h"
 #include "chrome/browser/metrics/metrics_service.h"
 #include "chrome/common/extensions/api/metrics_private.h"
 #include "components/variations/variations_associated_data.h"
@@ -22,6 +23,7 @@ namespace GetVariationParams = api::metrics_private::GetVariationParams;
 namespace GetFieldTrial = api::metrics_private::GetFieldTrial;
 namespace RecordUserAction = api::metrics_private::RecordUserAction;
 namespace RecordValue = api::metrics_private::RecordValue;
+namespace RecordSparseValue = api::metrics_private::RecordSparseValue;
 namespace RecordPercentage = api::metrics_private::RecordPercentage;
 namespace RecordCount = api::metrics_private::RecordCount;
 namespace RecordSmallCount = api::metrics_private::RecordSmallCount;
@@ -127,6 +129,14 @@ bool MetricsPrivateRecordValueFunction::RunImpl() {
                      params->metric.buckets, params->value);
 }
 
+bool MetricsPrivateRecordSparseValueFunction::RunImpl() {
+  scoped_ptr<RecordSparseValue::Params> params(
+      RecordSparseValue::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+  UMA_HISTOGRAM_SPARSE_SLOWLY(params->metric_name, params->value);

[Ilya Sherman, 2014/03/26 22:02:03]

This won't work -- you can only use an UMA_HISTOGRAM_ macro if the histogram
name is a runtime constant.  You'll need to instead expand out the macro, as has
been done elsewhere in this file.

[robliao, 2014/03/26 22:30:50]

Why not? The macro seems to expand fine. This is already being used like this
elsewhere.

void BlinkPlatformImpl::histogramSparse(const char* name, int sample) {
  // For sparse histograms, we can use the macro, as it does not incorporate a
  // static.
  UMA_HISTOGRAM_SPARSE_SLOWLY(name, sample);
}


On 2014/03/26 22:02:03, Ilya Sherman wrote:

[Ilya Sherman, 2014/03/26 22:44:14]

Ah, you're right!  Could you please include a similar comment here?  I forgot
that UMA_HISTOGRAM_SPARSE_SLOWLY was different from all of the other
UMA_HISTOGRAM_ macros.  It's nice to document in spots like this where you're
taking advantage of it, so that later maintainers don't get confused.

On 2014/03/26 22:30:50, robliao wrote:

[robliao, 2014/03/26 23:37:54]

Done.

+  return true;
+}
+
 bool MetricsPrivateRecordPercentageFunction::RunImpl() {
   scoped_ptr<RecordPercentage::Params> params(
       RecordPercentage::Params::Create(*args_));