Add UMA Histogram Manager to Cronet.

components/cronet/android/histogram_manager_unittest.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 "components/cronet/android/histogram_manager.h"
+
+#include <string>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/metrics/histogram.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace cronet {
+
+namespace {
+
+class HistogramManagerTest : public testing::Test {

[Alexei Svitkine (slow), 2014/12/11 15:38:58]

If this class doesn't have anything in it, you can actually just remove it
entirely and change TEST_F() to TEST() below.

[ramant (doing other things), 2014/12/11 20:23:53]

Done.

+ public:
+  HistogramManagerTest() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(HistogramManagerTest);
+};
+
+}  // namespace
+
+using metrics::ChromeUserMetricsExtension;
+using metrics::HistogramEventProto;

[Alexei Svitkine (slow), 2014/12/11 15:38:58]

Nit: I prefer not having using statements and just prefix these in the code.

[ramant (doing other things), 2014/12/11 20:23:53]

Done.

+
+TEST_F(HistogramManagerTest, HistogramBucketFields) {

[ramant (doing other things), 2014/12/11 02:21:53]

Tested this code by moving all HistogramManager.* into components/metrics area
and running unit tests there.

Wondering if we should most of the histogram_managet.* code
into components/metrics and just leave Java wrapper code in 
cronet/android area.

Alexei/Misha: wdyt?

[mef, 2014/12/11 15:35:40]

sgtm. I like making Java wrapper as thin as possible.

[Alexei Svitkine (slow), 2014/12/11 15:38:58]

My preference is to keep this in the cronet component. But if it's really
infeasible to set up the tests here, I am ok with this being inside the metrics
component for now with a TODO to fix later.

[ramant (doing other things), 2014/12/11 20:23:53]

Acknowledged.

[ramant (doing other things), 2014/12/11 20:23:53]

Done.

+  // kNoFlags filter should record all histograms.
+  UMA_HISTOGRAM_ENUMERATION("UmaHistogram", 1, 2);
+
+  ChromeUserMetricsExtension uma_proto;
+
+  std::vector<uint8> data = HistogramManager::GetInstance()->GetDeltas();
+  uma_proto.ParseFromArray(data.data(), data.size());
+  EXPECT_LT(0u, data.size());

[Alexei Svitkine (slow), 2014/12/11 15:38:58]

Shouldn't this expect be before ParseFromArray()? Also, if you're just testing
that data is not empty, I'd just check .empty() is false.

Also, please check the return value of ParseFromArray.

[ramant (doing other things), 2014/12/11 20:23:53]

Done.

+
+  const HistogramEventProto& histogram_proto =
+      uma_proto.histogram_event(uma_proto.histogram_event_size() - 1);
+  ASSERT_EQ(1, histogram_proto.bucket_size());
+  EXPECT_FALSE(histogram_proto.bucket(0).has_min());
+  EXPECT_TRUE(histogram_proto.bucket(0).has_max());
+  EXPECT_EQ(0, histogram_proto.bucket(0).min());
+  EXPECT_EQ(2, histogram_proto.bucket(0).max());
+  EXPECT_EQ(1, histogram_proto.bucket(0).count());
+
+  UMA_STABILITY_HISTOGRAM_ENUMERATION("UmaStabilityHistogram", 2, 3);
+  std::vector<uint8> data2 = HistogramManager::GetInstance()->GetDeltas();
+  ChromeUserMetricsExtension uma_proto2;
+  uma_proto2.ParseFromArray(data2.data(), data2.size());
+  EXPECT_LT(0u, data2.size());
+
+  const HistogramEventProto& histogram_proto2 =
+      uma_proto2.histogram_event(uma_proto2.histogram_event_size() - 1);
+  ASSERT_EQ(1, histogram_proto2.bucket_size());
+  EXPECT_FALSE(histogram_proto2.bucket(0).has_min());
+  EXPECT_TRUE(histogram_proto2.bucket(0).has_max());
+  EXPECT_EQ(0, histogram_proto2.bucket(0).min());
+  EXPECT_EQ(3, histogram_proto2.bucket(0).max());
+  EXPECT_EQ(1, histogram_proto2.bucket(0).count());
+}
+
+}  // namespace cronet