Add UMA Histogram Manager to Cronet.

Part of change to collect, upload and expose histogram data
for Cronet.

BUG=441466

Review URL: https://codereview.chromium.org/794493002

Cr-Commit-Position: refs/heads/master@{#308239}

18 files changed, 473 insertions, 30 deletions

diff --git a/components/components_tests.gyp b/components/components_tests.gyp
index e9f0c6c..e602fae 100644
--- a/components/components_tests.gyp
+++ b/components/components_tests.gyp
@@ -159,6 +159,8 @@
             'login/screens/screen_context_unittest.cc',
             'metrics/compression_utils_unittest.cc',
             'metrics/daily_event_unittest.cc',
+            'metrics/histogram_encoder_unittest.cc',
+            'metrics/histogram_manager_unittest.cc',
             'metrics/machine_id_provider_win_unittest.cc',
             'metrics/metrics_hashes_unittest.cc',
             'metrics/metrics_log_manager_unittest.cc',
diff --git a/components/cronet.gypi b/components/cronet.gypi
index e2a6f08..4f18ec3 100644
--- a/components/cronet.gypi
+++ b/components/cronet.gypi
@@ -15,6 +15,7 @@
             'cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java',
             'cronet/android/java/src/org/chromium/net/ChromiumUrlRequest.java',
             'cronet/android/java/src/org/chromium/net/ChromiumUrlRequestContext.java',
+            'cronet/android/java/src/org/chromium/net/HistogramManager.java',
           ],
           'variables': {
             'jni_gen_package': 'cronet',
@@ -101,12 +102,14 @@
           'dependencies': [
             '../base/base.gyp:base',
             '../base/base.gyp:base_i18n',
+            '../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
             '../third_party/icu/icu.gyp:icui18n',
             '../third_party/icu/icu.gyp:icuuc',
             'cronet_jni_headers',
             'cronet_url_request_context_config_list',
             'cronet_url_request_java',
             'cronet_version',
+            'metrics',
             '../net/net.gyp:net',
           ],
           'sources': [
@@ -129,6 +132,8 @@
             'cronet/android/cronet_url_request_context.h',
             'cronet/android/cronet_url_request_context_adapter.cc',
             'cronet/android/cronet_url_request_context_adapter.h',
+            'cronet/android/histogram_manager.cc',
+            'cronet/android/histogram_manager.h',
             'cronet/android/url_request_adapter.cc',
             'cronet/android/url_request_adapter.h',
             'cronet/android/url_request_context_adapter.cc',
@@ -229,6 +234,7 @@
               '**/CronetUrlRequest.java',
               '**/CronetUrlRequestContext.java',
               '**/CronetUrlRequestFactory.java',
+              '**/HistogramManager.java',
               '**/RequestPriority.java',
               '**/urlconnection/CronetInputStream.java',
               '**/urlconnection/CronetHttpURLConnection.java',
diff --git a/components/cronet/android/DEPS b/components/cronet/android/DEPS
index c80012b5..d7c1e71 100644
--- a/components/cronet/android/DEPS
+++ b/components/cronet/android/DEPS
@@ -1,3 +1,4 @@
 include_rules = [
+  "+components/metrics",
   "+jni",
 ]
diff --git a/components/cronet/android/cronet_loader.cc b/components/cronet/android/cronet_loader.cc
index 9af4795..3929b6c 100644
--- a/components/cronet/android/cronet_loader.cc
+++ b/components/cronet/android/cronet_loader.cc
@@ -10,6 +10,7 @@
 #include "components/cronet/android/chromium_url_request_context.h"
 #include "components/cronet/android/cronet_url_request.h"
 #include "components/cronet/android/cronet_url_request_context.h"
+#include "components/cronet/android/histogram_manager.h"
 #include "net/android/net_jni_registrar.h"
 #include "url/android/url_jni_registrar.h"
 #include "url/url_util.h"
@@ -27,6 +28,7 @@ const base::android::RegistrationMethod kCronetRegisteredMethods[] = {
     {"ChromiumUrlRequestContext", cronet::ChromiumUrlRequestContextRegisterJni},
     {"CronetUrlRequest", cronet::CronetUrlRequestRegisterJni},
     {"CronetUrlRequestContext", cronet::CronetUrlRequestContextRegisterJni},
+    {"HistogramManager", cronet::HistogramManagerRegisterJni},
     {"NetAndroid", net::android::RegisterJni},
     {"UrlAndroid", url::android::RegisterJni},
 };
diff --git a/components/cronet/android/histogram_manager.cc b/components/cronet/android/histogram_manager.cc
new file mode 100644
index 0000000..5ad7bb8
--- /dev/null
+++ b/components/cronet/android/histogram_manager.cc
@@ -0,0 +1,29 @@
+// 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/android/jni_array.h"
+#include "components/metrics/histogram_manager.h"
+
+#include "jni/HistogramManager_jni.h"
+
+namespace cronet {
+
+// Explicitly register static JNI functions.
+bool HistogramManagerRegisterJni(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
+static jbyteArray GetHistogramDeltas(JNIEnv* env, jobject jcaller) {
+  std::vector<uint8> data;
+  if (!metrics::HistogramManager::GetInstance()->GetDeltas(&data))
+    return NULL;
+  return base::android::ToJavaByteArray(env, &data[0], data.size()).Release();
+}
+
+}  // namespace cronet
diff --git a/components/cronet/android/histogram_manager.h b/components/cronet/android/histogram_manager.h
new file mode 100644
index 0000000..94001a48
--- /dev/null
+++ b/components/cronet/android/histogram_manager.h
@@ -0,0 +1,16 @@
+// 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.
+
+#ifndef COMPONENTS_CRONET_ANDROID_CRONET_HISTOGRAM_MANAGER_H_
+#define COMPONENTS_CRONET_ANDROID_CRONET_HISTOGRAM_MANAGER_H_
+
+#include <jni.h>
+
+namespace cronet {
+
+bool HistogramManagerRegisterJni(JNIEnv* env);
+
+}  // namespace cronet
+
+#endif  // COMPONENTS_CRONET_ANDROID_CRONET_HISTOGRAM_MANAGER_H_
diff --git a/components/cronet/android/java/src/org/chromium/net/HistogramManager.java b/components/cronet/android/java/src/org/chromium/net/HistogramManager.java
new file mode 100644
index 0000000..4995129
--- /dev/null
+++ b/components/cronet/android/java/src/org/chromium/net/HistogramManager.java
@@ -0,0 +1,25 @@
+// 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.
+
+package org.chromium.net;
+
+import org.chromium.base.JNINamespace;
+
+/**
+ * Controls UMA histograms.
+ */
+@JNINamespace("cronet")
+public final class HistogramManager {
+    public HistogramManager() {
+    }
+
+    /**
+     * Get histogram deltas serialized as protobuf.
+     */
+    public byte[] getHistogramDeltas() {
+        return nativeGetHistogramDeltas();
+    }
+
+    private native byte[] nativeGetHistogramDeltas();
+}
diff --git a/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/HistogramManagerTest.java b/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/HistogramManagerTest.java
new file mode 100644
index 0000000..4a20c6a
--- /dev/null
+++ b/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/HistogramManagerTest.java
@@ -0,0 +1,34 @@
+// 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.
+
+package org.chromium.cronet_test_apk;
+
+import android.test.suitebuilder.annotation.SmallTest;
+
+import org.chromium.base.test.util.Feature;
+import org.chromium.net.UrlRequest;
+
+/**
+ * Test HistogramManager.
+ */
+public class HistogramManagerTest extends CronetTestBase {
+    // URLs used for tests.
+    private static final String TEST_URL = "http://127.0.0.1:8000";
+
+    CronetTestActivity mActivity;
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testHistogramManager() throws Exception {
+        mActivity = launchCronetTestApp();
+        byte delta1[] = mActivity.mHistogramManager.getHistogramDeltas();
+
+        TestUrlRequestListener listener = new TestUrlRequestListener();
+        UrlRequest urlRequest = mActivity.mUrlRequestContext.createRequest(
+                TEST_URL, listener, listener.getExecutor());
+        urlRequest.start();
+        listener.blockForDone();
+        byte delta2[] = mActivity.mHistogramManager.getHistogramDeltas();
+    }
+}
diff --git a/components/cronet/android/test/src/org/chromium/cronet_test_apk/CronetTestActivity.java b/components/cronet/android/test/src/org/chromium/cronet_test_apk/CronetTestActivity.java
index 0934cf7..094b388 100644
--- a/components/cronet/android/test/src/org/chromium/cronet_test_apk/CronetTestActivity.java
+++ b/components/cronet/android/test/src/org/chromium/cronet_test_apk/CronetTestActivity.java
@@ -10,6 +10,7 @@ import android.os.Bundle;
 import android.os.Environment;
 import android.util.Log;
 
+import org.chromium.net.HistogramManager;
 import org.chromium.net.HttpUrlRequest;
 import org.chromium.net.HttpUrlRequestFactory;
 import org.chromium.net.HttpUrlRequestListener;
@@ -39,6 +40,8 @@ public class CronetTestActivity extends Activity {
 
     HttpUrlRequestFactory mRequestFactory;
     UrlRequestContext mUrlRequestContext;
+    HistogramManager mHistogramManager = new HistogramManager();
+
     String mUrl;
 
     boolean mLoading = false;
diff --git a/components/metrics.gypi b/components/metrics.gypi
index ea027b6..f144881 100644
--- a/components/metrics.gypi
+++ b/components/metrics.gypi
@@ -33,6 +33,10 @@
         'metrics/compression_utils.h',
         'metrics/daily_event.cc',
         'metrics/daily_event.h',
+        'metrics/histogram_encoder.cc',
+        'metrics/histogram_encoder.h',
+        'metrics/histogram_manager.cc',
+        'metrics/histogram_manager.h',
         'metrics/machine_id_provider.h',
         'metrics/machine_id_provider_stub.cc',
         'metrics/machine_id_provider_win.cc',
diff --git a/components/metrics/BUILD.gn b/components/metrics/BUILD.gn
index 48a6a01..964bf94 100644
--- a/components/metrics/BUILD.gn
+++ b/components/metrics/BUILD.gn
@@ -15,6 +15,10 @@ source_set("metrics") {
     "compression_utils.h",
     "daily_event.cc",
     "daily_event.h",
+    "histogram_encoder.cc",
+    "histogram_encoder.h",
+    "histogram_manager.cc",
+    "histogram_manager.h",
     "machine_id_provider.h",
     "machine_id_provider_stub.cc",
     "machine_id_provider_win.cc",
@@ -159,6 +163,8 @@ source_set("unit_tests") {
   sources = [
     "compression_utils_unittest.cc",
     "daily_event_unittest.cc",
+    "histogram_encoder_unittest.cc",
+    "histogram_manager_unittest.cc",
     "machine_id_provider_win_unittest.cc",
     "metrics_hashes_unittest.cc",
     "metrics_log_manager_unittest.cc",
diff --git a/components/metrics/histogram_encoder.cc b/components/metrics/histogram_encoder.cc
new file mode 100644
index 0000000..efca70b
--- /dev/null
+++ b/components/metrics/histogram_encoder.cc
@@ -0,0 +1,55 @@
+// 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/metrics/histogram_encoder.h"
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/metrics/histogram.h"
+#include "base/metrics/histogram_samples.h"
+#include "components/metrics/metrics_hashes.h"
+
+using base::SampleCountIterator;
+
+namespace metrics {
+
+void EncodeHistogramDelta(const std::string& histogram_name,
+                          const base::HistogramSamples& snapshot,
+                          ChromeUserMetricsExtension* uma_proto) {
+  DCHECK_NE(0, snapshot.TotalCount());
+  DCHECK(uma_proto);
+
+  // We will ignore the MAX_INT/infinite value in the last element of range[].
+
+  HistogramEventProto* histogram_proto = uma_proto->add_histogram_event();
+  histogram_proto->set_name_hash(HashMetricName(histogram_name));
+  histogram_proto->set_sum(snapshot.sum());
+
+  for (scoped_ptr<SampleCountIterator> it = snapshot.Iterator(); !it->Done();
+       it->Next()) {
+    base::Histogram::Sample min;
+    base::Histogram::Sample max;
+    base::Histogram::Count count;
+    it->Get(&min, &max, &count);
+    HistogramEventProto::Bucket* bucket = histogram_proto->add_bucket();
+    bucket->set_min(min);
+    bucket->set_max(max);
+    bucket->set_count(count);
+  }
+
+  // Omit fields to save space (see rules in histogram_event.proto comments).
+  for (int i = 0; i < histogram_proto->bucket_size(); ++i) {
+    HistogramEventProto::Bucket* bucket = histogram_proto->mutable_bucket(i);
+    if (i + 1 < histogram_proto->bucket_size() &&
+        bucket->max() == histogram_proto->bucket(i + 1).min()) {
+      bucket->clear_max();
+    } else if (bucket->max() == bucket->min() + 1) {
+      bucket->clear_min();
+    }
+  }
+}
+
+}  // namespace metrics
diff --git a/components/metrics/histogram_encoder.h b/components/metrics/histogram_encoder.h
new file mode 100644
index 0000000..0151194
--- /dev/null
+++ b/components/metrics/histogram_encoder.h
@@ -0,0 +1,30 @@
+// 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.
+
+// This file defines an utility function that records any changes in a given
+// histogram for transmission.
+
+#ifndef COMPONENTS_METRICS_HISTOGRAM_ENCODER_H_
+#define COMPONENTS_METRICS_HISTOGRAM_ENCODER_H_
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "components/metrics/proto/chrome_user_metrics_extension.pb.h"
+
+namespace base {
+class HistogramSamples;
+}
+
+namespace metrics {
+
+// Record any changes (histogram deltas of counts from |snapshot|) into
+// |uma_proto| for the given histogram (|histogram_name|).
+void EncodeHistogramDelta(const std::string& histogram_name,
+                          const base::HistogramSamples& snapshot,
+                          ChromeUserMetricsExtension* uma_proto);
+
+}  // namespace metrics
+
+#endif  // COMPONENTS_METRICS_HISTOGRAM_ENCODER_H_
diff --git a/components/metrics/histogram_encoder_unittest.cc b/components/metrics/histogram_encoder_unittest.cc
new file mode 100644
index 0000000..fb1d47d
--- /dev/null
+++ b/components/metrics/histogram_encoder_unittest.cc
@@ -0,0 +1,72 @@
+// 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/metrics/histogram_encoder.h"
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/metrics/bucket_ranges.h"
+#include "base/metrics/sample_vector.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace metrics {
+
+TEST(HistogramEncoder, HistogramBucketFields) {
+  // Create buckets: 1-5, 5-7, 7-8, 8-9, 9-10, 10-11, 11-12.
+  base::BucketRanges ranges(8);
+  ranges.set_range(0, 1);
+  ranges.set_range(1, 5);
+  ranges.set_range(2, 7);
+  ranges.set_range(3, 8);
+  ranges.set_range(4, 9);
+  ranges.set_range(5, 10);
+  ranges.set_range(6, 11);
+  ranges.set_range(7, 12);
+
+  base::SampleVector samples(&ranges);
+  samples.Accumulate(3, 1);   // Bucket 1-5.
+  samples.Accumulate(6, 1);   // Bucket 5-7.
+  samples.Accumulate(8, 1);   // Bucket 8-9. (7-8 skipped)
+  samples.Accumulate(10, 1);  // Bucket 10-11. (9-10 skipped)
+  samples.Accumulate(11, 1);  // Bucket 11-12.
+
+  ChromeUserMetricsExtension uma_proto;
+  EncodeHistogramDelta("Test", samples, &uma_proto);
+
+  const HistogramEventProto& histogram_proto =
+      uma_proto.histogram_event(uma_proto.histogram_event_size() - 1);
+
+  // Buckets with samples: 1-5, 5-7, 8-9, 10-11, 11-12.
+  // Should become: 1-/, 5-7, /-9, 10-/, /-12.
+  ASSERT_EQ(5, histogram_proto.bucket_size());
+
+  // 1-5 becomes 1-/ (max is same as next min).
+  EXPECT_TRUE(histogram_proto.bucket(0).has_min());
+  EXPECT_FALSE(histogram_proto.bucket(0).has_max());
+  EXPECT_EQ(1, histogram_proto.bucket(0).min());
+
+  // 5-7 stays 5-7 (no optimization possible).
+  EXPECT_TRUE(histogram_proto.bucket(1).has_min());
+  EXPECT_TRUE(histogram_proto.bucket(1).has_max());
+  EXPECT_EQ(5, histogram_proto.bucket(1).min());
+  EXPECT_EQ(7, histogram_proto.bucket(1).max());
+
+  // 8-9 becomes /-9 (min is same as max - 1).
+  EXPECT_FALSE(histogram_proto.bucket(2).has_min());
+  EXPECT_TRUE(histogram_proto.bucket(2).has_max());
+  EXPECT_EQ(9, histogram_proto.bucket(2).max());
+
+  // 10-11 becomes 10-/ (both optimizations apply, omit max is prioritized).
+  EXPECT_TRUE(histogram_proto.bucket(3).has_min());
+  EXPECT_FALSE(histogram_proto.bucket(3).has_max());
+  EXPECT_EQ(10, histogram_proto.bucket(3).min());
+
+  // 11-12 becomes /-12 (last record must keep max, min is same as max - 1).
+  EXPECT_FALSE(histogram_proto.bucket(4).has_min());
+  EXPECT_TRUE(histogram_proto.bucket(4).has_max());
+  EXPECT_EQ(12, histogram_proto.bucket(4).max());
+}
+
+}  // namespace metrics
diff --git a/components/metrics/histogram_manager.cc b/components/metrics/histogram_manager.cc
new file mode 100644
index 0000000..dfa865b
--- /dev/null
+++ b/components/metrics/histogram_manager.cc
@@ -0,0 +1,67 @@
+// 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/metrics/histogram_manager.h"
+
+#include <string>
+#include <vector>
+
+#include "base/lazy_instance.h"
+#include "base/metrics/histogram.h"
+#include "base/metrics/histogram_samples.h"
+#include "components/metrics/histogram_encoder.h"
+
+namespace metrics {
+
+// TODO(rtenneti): move g_histogram_manager into java code.
+static base::LazyInstance<HistogramManager>::Leaky
+    g_histogram_manager = LAZY_INSTANCE_INITIALIZER;
+
+HistogramManager::HistogramManager() : histogram_snapshot_manager_(this) {
+}
+
+HistogramManager::~HistogramManager() {
+}
+
+// static
+HistogramManager* HistogramManager::GetInstance() {
+  return g_histogram_manager.Pointer();
+}
+
+void HistogramManager::RecordDelta(const base::HistogramBase& histogram,
+                                   const base::HistogramSamples& snapshot) {
+  EncodeHistogramDelta(histogram.histogram_name(), snapshot, &uma_proto_);
+}
+
+void HistogramManager::InconsistencyDetected(
+    base::HistogramBase::Inconsistency problem) {
+  UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesBrowser.Cronet",
+                            problem, base::HistogramBase::NEVER_EXCEEDED_VALUE);
+}
+
+void HistogramManager::UniqueInconsistencyDetected(
+    base::HistogramBase::Inconsistency problem) {
+  UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesBrowserUnique.Cronet",
+                            problem, base::HistogramBase::NEVER_EXCEEDED_VALUE);
+}
+
+void HistogramManager::InconsistencyDetectedInLoggedCount(int amount) {
+  UMA_HISTOGRAM_COUNTS("Histogram.InconsistentSnapshotBrowser.Cronet",
+                       std::abs(amount));
+}
+
+bool HistogramManager::GetDeltas(std::vector<uint8>* data) {
+  // Clear the protobuf between calls.
+  uma_proto_.Clear();
+  histogram_snapshot_manager_.PrepareDeltas(
+      base::Histogram::kNoFlags, base::Histogram::kUmaTargetedHistogramFlag);
+  int32_t data_size = uma_proto_.ByteSize();
+  data->resize(data_size);
+  if (uma_proto_.SerializeToArray(&(*data)[0], data_size))
+    return true;
+  data->clear();
+  return false;
+}
+
+}  // namespace metrics
diff --git a/components/metrics/histogram_manager.h b/components/metrics/histogram_manager.h
new file mode 100644
index 0000000..a1f988a
--- /dev/null
+++ b/components/metrics/histogram_manager.h
@@ -0,0 +1,62 @@
+// 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.
+
+#ifndef COMPONENTS_METRICS_HISTOGRAM_MANAGER_H_
+#define COMPONENTS_METRICS_HISTOGRAM_MANAGER_H_
+
+#include <string>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/lazy_instance.h"
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/metrics/histogram_flattener.h"
+#include "base/metrics/histogram_snapshot_manager.h"
+#include "components/metrics/proto/chrome_user_metrics_extension.pb.h"
+
+namespace metrics {
+
+// TODO(mef): crbug.com/441441. Move components/metrics/histogram_manager.*
+// files into components/android/cronet.
+//
+// A HistogramManager instance is created by Android.  It is the central
+// controller for the acquisition of log data, and the automatic transmission of
+// that log data to an external server.
+class HistogramManager : public base::HistogramFlattener {
+ public:
+  HistogramManager();
+  ~HistogramManager();
+
+  // Snapshot all histograms to record the delta into |uma_proto_| and then
+  // returns the serialized protobuf representation of the record in |data|.
+  // Returns true if it was successfully serialized.
+  bool GetDeltas(std::vector<uint8>* data);
+
+  // TODO(mef): Hang Histogram Manager off java object instead of singleton.
+  static HistogramManager* GetInstance();
+
+ private:
+  friend struct base::DefaultLazyInstanceTraits<HistogramManager>;
+
+  // base::HistogramFlattener:
+  void RecordDelta(const base::HistogramBase& histogram,
+                   const base::HistogramSamples& snapshot) override;
+  void InconsistencyDetected(
+      base::HistogramBase::Inconsistency problem) override;
+  void UniqueInconsistencyDetected(
+      base::HistogramBase::Inconsistency problem) override;
+  void InconsistencyDetectedInLoggedCount(int amount) override;
+
+  base::HistogramSnapshotManager histogram_snapshot_manager_;
+
+  // Stores the protocol buffer representation for this log.
+  metrics::ChromeUserMetricsExtension uma_proto_;
+
+  DISALLOW_COPY_AND_ASSIGN(HistogramManager);
+};
+
+}  // namespace metrics
+
+#endif  // COMPONENTS_METRICS_HISTOGRAM_MANAGER_H_
diff --git a/components/metrics/histogram_manager_unittest.cc b/components/metrics/histogram_manager_unittest.cc
new file mode 100644
index 0000000..b0934d3
--- /dev/null
+++ b/components/metrics/histogram_manager_unittest.cc
@@ -0,0 +1,57 @@
+// 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/metrics/histogram_manager.h"
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace metrics {
+
+// TODO(mef): crbug.com/441441. Move components/metrics/histogram_manager.*
+// files into components/android/cronet.
+TEST(HistogramManager, HistogramBucketFields) {
+  // Capture histograms at the start of the test to avoid later GetDeltas()
+  // calls picking them up.
+  std::vector<uint8> data_init;
+  HistogramManager::GetInstance()->GetDeltas(&data_init);
+
+  // kNoFlags filter should record all histograms.
+  UMA_HISTOGRAM_ENUMERATION("UmaHistogramManager", 1, 2);
+
+  std::vector<uint8> data;
+  EXPECT_TRUE(HistogramManager::GetInstance()->GetDeltas(&data));
+  EXPECT_FALSE(data.empty());
+  ChromeUserMetricsExtension uma_proto;
+  EXPECT_TRUE(uma_proto.ParseFromArray(
+      reinterpret_cast<const char*>(&data[0]), data.size()));
+  EXPECT_FALSE(data.empty());
+
+  const HistogramEventProto& histogram_proto =
+      uma_proto.histogram_event(uma_proto.histogram_event_size() - 1);
+  ASSERT_EQ(1, histogram_proto.bucket_size());
+  EXPECT_LE(0, histogram_proto.bucket(0).min());
+  EXPECT_LE(2, histogram_proto.bucket(0).max());
+  EXPECT_EQ(1, histogram_proto.bucket(0).count());
+
+  UMA_HISTOGRAM_ENUMERATION("UmaHistogramManager2", 2, 3);
+  std::vector<uint8> data2;
+  EXPECT_TRUE(HistogramManager::GetInstance()->GetDeltas(&data2));
+  EXPECT_FALSE(data2.empty());
+  ChromeUserMetricsExtension uma_proto2;
+  EXPECT_TRUE(uma_proto2.ParseFromArray(
+      reinterpret_cast<const char*>(&data2[0]), data2.size()));
+  EXPECT_FALSE(data2.empty());
+
+  const HistogramEventProto& histogram_proto2 =
+      uma_proto2.histogram_event(uma_proto2.histogram_event_size() - 1);
+  ASSERT_EQ(1, histogram_proto2.bucket_size());
+  EXPECT_LE(0, histogram_proto2.bucket(0).min());
+  EXPECT_LE(3, histogram_proto2.bucket(0).max());
+  EXPECT_EQ(1, histogram_proto2.bucket(0).count());
+}
+
+}  // namespace metrics
diff --git a/components/metrics/metrics_log.cc b/components/metrics/metrics_log.cc
index 2ea6f89..fdfe305 100644
--- a/components/metrics/metrics_log.cc
+++ b/components/metrics/metrics_log.cc
@@ -23,6 +23,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/sys_info.h"
 #include "base/time/time.h"
+#include "components/metrics/histogram_encoder.h"
 #include "components/metrics/metrics_hashes.h"
 #include "components/metrics/metrics_pref_names.h"
 #include "components/metrics/metrics_provider.h"
@@ -178,36 +179,7 @@ void MetricsLog::RecordUserAction(const std::string& key) {
 void MetricsLog::RecordHistogramDelta(const std::string& histogram_name,
                                       const base::HistogramSamples& snapshot) {
   DCHECK(!closed_);
-  DCHECK_NE(0, snapshot.TotalCount());
-
-  // We will ignore the MAX_INT/infinite value in the last element of range[].
-
-  HistogramEventProto* histogram_proto = uma_proto_.add_histogram_event();
-  histogram_proto->set_name_hash(Hash(histogram_name));
-  histogram_proto->set_sum(snapshot.sum());
-
-  for (scoped_ptr<SampleCountIterator> it = snapshot.Iterator(); !it->Done();
-       it->Next()) {
-    base::Histogram::Sample min;
-    base::Histogram::Sample max;
-    base::Histogram::Count count;
-    it->Get(&min, &max, &count);
-    HistogramEventProto::Bucket* bucket = histogram_proto->add_bucket();
-    bucket->set_min(min);
-    bucket->set_max(max);
-    bucket->set_count(count);
-  }
-
-  // Omit fields to save space (see rules in histogram_event.proto comments).
-  for (int i = 0; i < histogram_proto->bucket_size(); ++i) {
-    HistogramEventProto::Bucket* bucket = histogram_proto->mutable_bucket(i);
-    if (i + 1 < histogram_proto->bucket_size() &&
-        bucket->max() == histogram_proto->bucket(i + 1).min()) {
-      bucket->clear_max();
-    } else if (bucket->max() == bucket->min() + 1) {
-      bucket->clear_min();
-    }
-  }
+  EncodeHistogramDelta(histogram_name, snapshot, &uma_proto_);
 }
 
 void MetricsLog::RecordStabilityMetrics(