diff options
19 files changed, 481 insertions, 30 deletions
diff --git a/components/components_tests.gyp b/components/components_tests.gyp index 3820e93..b380c35 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..5e5f992 --- /dev/null +++ b/components/metrics/histogram_manager_unittest.cc @@ -0,0 +1,58 @@ +// 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. +// TODO(rtenneti): enable flaky HistogramBucketFields unit test. +TEST(HistogramManager, DISABLED_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( diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index af43383..f83bac4 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -58839,6 +58839,13 @@ To add a new entry, add it with any value and run test to compute valid value. <affected-histogram name="PLT.PT_StartToFinish_WithPreview"/> </histogram_suffixes> +<histogram_suffixes name="HistogramInconsistencies" separator="."> + <suffix name="Cronet" label="Cronet histograms."/> + <affected-histogram name="Histogram.InconsistenciesBrowser"/> + <affected-histogram name="Histogram.InconsistenciesBrowserUnique"/> + <affected-histogram name="Histogram.InconsistentSnapshotBrowser"/> +</histogram_suffixes> + <histogram_suffixes name="HttpPipeliningCompatibility"> <suffix name="disable_test" label="Do nothing"/> <suffix name="enable_test" label="Test connection for HTTP pipelining"/> |