Add a Java API for UMA histograms.

base/android/record_histogram.cc

+// Copyright 2014 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 "base/android/jni_android.h"
+#include "base/android/jni_string.h"
+#include "base/android/record_histogram.h"
+#include "base/metrics/histogram.h"
+#include "base/metrics/statistics_recorder.h"
+#include "jni/RecordHistogram_jni.h"
+
+namespace base {
+namespace android {
+
+void RecordBooleanHistogram(JNIEnv* env,
+                            jclass,
+                            jstring j_histogram_name,
+                            jboolean j_sample) {
+  std::string histogram_name = ConvertJavaStringToUTF8(env, j_histogram_name);
+  int sample = static_cast<int>(j_sample);

[Ilya Sherman, 2014/12/16 20:14:08]

Why int rather than bool?

[ppi, 2014/12/17 13:15:31]

Done.

+
+  BooleanHistogram::FactoryGet(histogram_name,
+                               base::HistogramBase::kUmaTargetedHistogramFlag)
+      ->Add(sample);

[Ilya Sherman, 2014/12/16 20:14:08]

nit: AddBoolean

[ppi, 2014/12/17 13:15:31]

Done.

+}
+
+void RecordEnumeratedHistogram(JNIEnv* env,
+                               jclass,
+                               jstring j_histogram_name,
+                               jint j_sample,
+                               jint j_boundary) {
+  std::string histogram_name = ConvertJavaStringToUTF8(env, j_histogram_name);
+  int sample = static_cast<int>(j_sample);
+  int boundary = static_cast<int>(j_boundary);
+
+  base::LinearHistogram::FactoryGet(
+      histogram_name, 1, boundary, boundary + 1,
+      base::HistogramBase::kUmaTargetedHistogramFlag)->Add(sample);
+}
+
+jint GetHistogramValueCount(JNIEnv* env,
+                            jclass,
+                            jstring histogram_name,
+                            jint sample) {
+  base::HistogramBase* histogram = base::StatisticsRecorder::FindHistogram(
+      base::android::ConvertJavaStringToUTF8(env, histogram_name).c_str());

[Ilya Sherman, 2014/12/16 20:14:08]

nit: Do you need the .c_str() call?

[ppi, 2014/12/17 13:15:31]

Done.

+  if (histogram == NULL) {

[Ilya Sherman, 2014/12/16 20:14:08]

nit: Either "if (!histogram)" or "if (histogram == nullptr)"

[ppi, 2014/12/17 13:15:31]

Done.

+    // No samples have been recorded for this histogram (yet?).
+    return 0;
+  }
+
+  scoped_ptr<base::HistogramSamples> samples = histogram->SnapshotSamples();
+  return samples->GetCount(static_cast<int>(sample));
+}

[Ilya Sherman, 2014/12/16 20:14:08]

What is this used for?  If it's for tests only, then please move it to a
test-specific file.  In C++, most people use //base/test/histogram_tester

[ppi, 2014/12/17 13:15:31]

Yes, this is needed for the Java-side test util being upstreamed in this same CL
( org.chromium.base.test.util.MetricsUtils.HistogramDelta ), and inspired by the
predecessor of histogram_tester :).

Unfortunately, we can't have test-specific native code backing Java
test-specific utils - the context and more information is at
http://crbug.com/415945 .

In Chrome for Android, where this was originally added, we concluded that the
gains from being able to test histograms in Java beat the cons of having a
test-only method in the native code.

Wdyt?

[Ilya Sherman, 2014/12/17 22:58:51]

Thanks for the explanation.  I think that with the "ForTesting" suffix that
you've added, this is now clear enough.

+
+void Initialize(JNIEnv* env, jclass) {
+  StatisticsRecorder::Initialize();
+}
+
+bool RegisterRecordHistogram(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
+}  // namespace android
+
+}  // namespace base