1. Add test for https://src.chromium.org/viewvc/chrome?view=rev&revision=149541

2. Fixed another bug in CustomHistogram ranges counting

Review URL: https://chromiumcodereview.appspot.com/10823137

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@150020 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 50 insertions, 5 deletions

diff --git a/base/metrics/histogram.cc b/base/metrics/histogram.cc
index 4087ad7..5836896 100644
--- a/base/metrics/histogram.cc
+++ b/base/metrics/histogram.cc
@@ -883,14 +883,15 @@ double CustomHistogram::GetBucketSize(Count current, size_t i) const {
 // static
 bool CustomHistogram::ValidateCustomRanges(
     const vector<Sample>& custom_ranges) {
-  if (custom_ranges.size() < 1)
-    return false;
+  bool has_valid_range = false;
   for (size_t i = 0; i < custom_ranges.size(); i++) {
-    Sample s = custom_ranges[i];
-    if (s < 0 || s > HistogramBase::kSampleType_MAX - 1)
+    Sample sample = custom_ranges[i];
+    if (sample < 0 || sample > HistogramBase::kSampleType_MAX - 1)
       return false;
+    if (sample != 0)
+      has_valid_range = true;
   }
-  return true;
+  return has_valid_range;
 }
 
 // static
diff --git a/base/metrics/histogram_unittest.cc b/base/metrics/histogram_unittest.cc
index 7257fa5..7f23ede 100644
--- a/base/metrics/histogram_unittest.cc
+++ b/base/metrics/histogram_unittest.cc
@@ -319,4 +319,48 @@ TEST(HistogramTest, CorruptBucketBounds) {
   bucket_ranges->set_range(4, bucket_ranges->range(4) + 1);
 }
 
+#if GTEST_HAS_DEATH_TEST
+// For Histogram, LinearHistogram and CustomHistogram, the minimum for a
+// declared range is 1, while the maximum is (HistogramBase::kSampleType_MAX -
+// 1). But we accept ranges exceeding those limits, and silently clamped to
+// those limits. This is for backwards compatibility.
+TEST(HistogramDeathTest, BadRangesTest) {
+  Histogram* histogram = Histogram::FactoryGet(
+      "BadRanges", 0, HistogramBase::kSampleType_MAX, 8, Histogram::kNoFlags);
+  EXPECT_EQ(1, histogram->declared_min());
+  EXPECT_EQ(HistogramBase::kSampleType_MAX - 1, histogram->declared_max());
+
+  Histogram* linear_histogram = LinearHistogram::FactoryGet(
+      "BadRangesLinear", 0, HistogramBase::kSampleType_MAX, 8,
+      Histogram::kNoFlags);
+  EXPECT_EQ(1, linear_histogram->declared_min());
+  EXPECT_EQ(HistogramBase::kSampleType_MAX - 1,
+            linear_histogram->declared_max());
+
+  vector<int> custom_ranges;
+  custom_ranges.push_back(0);
+  custom_ranges.push_back(5);
+  Histogram* custom_histogram1 = CustomHistogram::FactoryGet(
+      "BadRangesCustom", custom_ranges, Histogram::kNoFlags);
+  const BucketRanges* ranges = custom_histogram1->bucket_ranges();
+  ASSERT_EQ(3u, ranges->size());
+  EXPECT_EQ(0, ranges->range(0));
+  EXPECT_EQ(5, ranges->range(1));
+  EXPECT_EQ(HistogramBase::kSampleType_MAX, ranges->range(2));
+
+  // CustomHistogram does not accepts kSampleType_MAX as range.
+  custom_ranges.push_back(HistogramBase::kSampleType_MAX);
+  EXPECT_DEATH(CustomHistogram::FactoryGet("BadRangesCustom2", custom_ranges,
+                                           Histogram::kNoFlags),
+               "");
+
+  // CustomHistogram needs at least 1 valid range.
+  custom_ranges.clear();
+  custom_ranges.push_back(0);
+  EXPECT_DEATH(CustomHistogram::FactoryGet("BadRangesCustom3", custom_ranges,
+                                           Histogram::kNoFlags),
+               "");
+}
+#endif
+
 }  // namespace base