Support "shared" histograms between processes.

base/metrics/histogram.cc

 // Copyright (c) 2012 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.
 
 // Histogram is an object that aggregates statistics, and can summarize them in
 // various forms, including ASCII graphical, HTML, and numerically (as a
 // vector of numbers corresponding to each of the aggregating buckets).
 // See header file for details and examples.
 
 #include "base/metrics/histogram.h"
 
 #include <math.h>
 
 #include <algorithm>
 #include <string>
 
 #include "base/compiler_specific.h"
 #include "base/debug/alias.h"
 #include "base/logging.h"
+#include "base/memory/persistent_memory_allocator.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/metrics/histogram_persistence.h"
 #include "base/metrics/metrics_hashes.h"
 #include "base/metrics/sample_vector.h"
 #include "base/metrics/statistics_recorder.h"
 #include "base/pickle.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/synchronization/lock.h"
 #include "base/values.h"
 
 namespace base {
@@ skipping 39 matching lines @@
 bool ValidateRangeChecksum(const HistogramBase& histogram,
                            uint32 range_checksum) {
   const Histogram& casted_histogram =
       static_cast<const Histogram&>(histogram);
 
   return casted_histogram.bucket_ranges()->checksum() == range_checksum;
 }
 
 }  // namespace
 
+
+class Histogram::Factory {
+ public:
+  Factory(const std::string& name,
+          HistogramBase::Sample minimum,
+          HistogramBase::Sample maximum,
+          size_t bucket_count,
+          int32 flags)
+    : Factory(name, HISTOGRAM, minimum, maximum, bucket_count, flags) {}
+
+  HistogramBase* Build();
+
+ protected:
+  Factory(const std::string& name,
+          HistogramType histogram_type,
+          HistogramBase::Sample minimum,
+          HistogramBase::Sample maximum,
+          size_t bucket_count,
+          int32 flags)
+    : name_(name),
+      histogram_type_(histogram_type),
+      minimum_(minimum),
+      maximum_(maximum),
+      bucket_count_(bucket_count),
+      flags_(flags) {}
+
+  virtual BucketRanges* CreateRanges();
+  virtual HistogramBase* HeapAlloc(const BucketRanges* ranges);
+  virtual void FillHistogram(HistogramBase* histogram);
+
+  const std::string& name_;
+  const HistogramType histogram_type_;
+  HistogramBase::Sample minimum_;
+  HistogramBase::Sample maximum_;
+  size_t bucket_count_;
+  int32 flags_;

[Alexei Svitkine (slow), 2015/12/15 23:03:37]

Do these need to be members or could they be params to Build() instead of the
ctor so that they can be used directly?

I know that for e.g. BooleanHistogram, Build() doesn't need that many arguments.
But it could just call the base Build() supplying the defaults there, as is done
in its ctor.

[bcwhite, 2015/12/16 13:36:34]

Whether the defaults are set in the ctor or as paremeters doesn't change much
for the caller.

However, the base Build and some of the virtual methods do use them all even if
some are derived or constant based on the histogram type.  If they were
parameters, they would have to be passed to the virtual functions even if not
used, complicating things there.

I think it's also more efficient to set everything in the ctor where the
compiler can easily inline the storing of constants into memory locations as
opposed to parameters passed around which would involve multiple copies even
when they're not going to be used.

[Alexei Svitkine (slow), 2015/12/16 16:33:49]

Sorry, I didn't mention it, but one of the reason I suggested it is because
protected member variables are discouraged by the style guide. ("Note that data
members should be private.")

So to keep the current structure, you'd need to add accessors for them that will
be called from sub classes, which adds a lot of boilerplate. :\ So I was
thinking passing them as params would be more concise.

Anyway, thinking about this a bit more, I have a third alternate suggestion -
see my comment below.

[bcwhite, 2015/12/17 15:55:48]

I think given that the class is a utility class completely embedded within the
implementing .cc file, we're not worried about implementation changes.  I'm
assuming that the reason for not using protected members but rather protected
accessors is so that the implementation can change without necessarily breaking
all derived classes.

+};
+
+HistogramBase* Histogram::Factory::Build() {
+  ImportPersistentHistograms();
+
+  HistogramBase* histogram = StatisticsRecorder::FindHistogram(name_);
+  if (!histogram) {
+    // To avoid racy destruction at shutdown, the following will be leaked.
+    const BucketRanges* created_ranges = CreateRanges();
+    const BucketRanges* registered_ranges =
+        StatisticsRecorder::RegisterOrDeleteDuplicateRanges(created_ranges);
+
+    // Sometimes the bucket info is dynamically determined in create_ranges().
+    if (bucket_count_ == 0) {
+      bucket_count_ = registered_ranges->bucket_count();
+      minimum_ = registered_ranges->range(1);
+      maximum_ = registered_ranges->range(bucket_count_ - 1);
+    }
+
+    PersistentMemoryAllocator::Reference histogram_ref = 0;
+    HistogramBase* tentative_histogram = nullptr;
+    PersistentMemoryAllocator* allocator =
+        GetPersistentHistogramMemoryAllocator();
+    if (allocator) {
+      flags_ |= HistogramBase::kIsPersistent;
+      tentative_histogram = AllocatePersistentHistogram(
+          allocator,
+          histogram_type_,
+          name_,
+          minimum_,
+          maximum_,
+          registered_ranges,
+          flags_,
+          &histogram_ref);
+    }
+    if (!tentative_histogram) {
+      flags_ &= ~HistogramBase::kIsPersistent;
+      tentative_histogram = HeapAlloc(registered_ranges);
+    }
+
+    FillHistogram(tentative_histogram);
+
+    tentative_histogram->SetFlags(flags_);
+    histogram =
+        StatisticsRecorder::RegisterOrDeleteDuplicate(tentative_histogram);
+
+    // Persistent histograms need some follow-up processing.
+    if (histogram_ref) {
+      // If the created persistent histogram is canonical then it needs to be
+      // marked as "iterable" in order to be found by other processes.
+      if (histogram == tentative_histogram)
+        GetPersistentHistogramMemoryAllocator()->MakeIterable(histogram_ref);
+      // If it's not the canonical one then a race condition must have caused
+      // two to be created. The allocator does not support releasing the
+      // acquired memory so just change the type to be empty.
+      else
+        GetPersistentHistogramMemoryAllocator()->SetType(histogram_ref, 0);
+    }
+  }
+
+  DCHECK_EQ(histogram_type_, histogram->GetHistogramType());
+  if (bucket_count_ != 0 &&
+      !histogram->HasConstructionArguments(minimum_, maximum_, bucket_count_)) {
+    // The construction arguments do not match the existing histogram.  This can
+    // come about if an extension updates in the middle of a chrome run and has
+    // changed one of them, or simply by bad code within Chrome itself.  We
+    // return NULL here with the expectation that bad code in Chrome will crash
+    // on dereference, but extension/Pepper APIs will guard against NULL and not
+    // crash.
+    DLOG(ERROR) << "Histogram " << name_ << " has bad construction arguments";
+    return NULL;

[Alexei Svitkine (slow), 2015/12/16 16:33:49]

Nit: nullptr

[bcwhite, 2015/12/17 15:55:48]

Old habits die hard.  Done.

+  }
+  return histogram;
+}
+
+BucketRanges* Histogram::Factory::CreateRanges() {

[Alexei Svitkine (slow), 2015/12/16 16:33:49]

So, thinking about this more.... given that these type-specific functions are
short, how about just having one factory class and having a switch on
histogram_type in the places that matter? That way, there's no issue with the
member variables being protected per the comment above (they can be private) and
the whole class can be in the anon namespace.

[bcwhite, 2015/12/17 15:55:48]

I wouldn't even need a class for that.  It would just be the original
FactoryGet() function but with multiple switch statements within it.  It can't
be anonymous, though, because it would have to be a friend of every class to
access the non-public constructors of the various Histogram classes.

This would also require complete knowledge of all the various classes within
that one class/method.  Right now, the LinearHistogram::Factory class has extra
knowledge that indicates how to set descriptions and is used by the overridden
"datafill" method.  With a single class/method, every caller would have to pass
a "descriptions" pointer (null or otherwise) even though most have no knowledge
about what that is.

I think the use of virtual functions is a much cleaner (and more extensible)
method than numerous embedded switch statements.

+  BucketRanges* ranges = new BucketRanges(bucket_count_ + 1);
+  Histogram::InitializeBucketRanges(minimum_, maximum_, ranges);
+  return ranges;
+}
+
+HistogramBase* Histogram::Factory::HeapAlloc(const BucketRanges* ranges) {
+  return new Histogram(name_, minimum_, maximum_, ranges);
+}
+
+void Histogram::Factory::FillHistogram(HistogramBase* histogram) {
+}
+
+
+class LinearHistogram::Factory : public Histogram::Factory {
+ public:
+  Factory(const std::string& name,
+          HistogramBase::Sample minimum,
+          HistogramBase::Sample maximum,
+          size_t bucket_count,
+          int32 flags,
+          const DescriptionPair* descriptions)
+    : Histogram::Factory(name, LINEAR_HISTOGRAM, minimum, maximum,
+                         bucket_count, flags) {
+    descriptions_ = descriptions;
+  }
+
+ protected:
+  BucketRanges* CreateRanges();
+  HistogramBase* HeapAlloc(const BucketRanges* ranges) override;
+  void FillHistogram(HistogramBase* histogram) override;
+
+  const DescriptionPair* descriptions_;
+};
+
+BucketRanges* LinearHistogram::Factory::CreateRanges() {
+  BucketRanges* ranges = new BucketRanges(bucket_count_ + 1);
+  LinearHistogram::InitializeBucketRanges(minimum_, maximum_, ranges);
+  return ranges;
+}
+
+HistogramBase* LinearHistogram::Factory::HeapAlloc(const BucketRanges* ranges) {
+  return new LinearHistogram(name_, minimum_, maximum_, ranges);
+}
+
+void LinearHistogram::Factory::FillHistogram(HistogramBase* base_histogram) {
+  LinearHistogram* histogram = static_cast<LinearHistogram*>(base_histogram);
+  // Set range descriptions.
+  if (descriptions_) {
+    for (int i = 0; descriptions_[i].description; ++i) {
+      histogram->bucket_description_[descriptions_[i].sample] =
+          descriptions_[i].description;
+    }
+  }
+}
+
+
+class BooleanHistogram::Factory : public Histogram::Factory {
+ public:
+  Factory(const std::string& name, int32 flags)
+    : Histogram::Factory(name, BOOLEAN_HISTOGRAM, 1, 2, 3, flags) {}
+
+ protected:
+  BucketRanges* CreateRanges();
+  HistogramBase* HeapAlloc(const BucketRanges* ranges) override;
+};
+
+BucketRanges* BooleanHistogram::Factory::CreateRanges() {
+  BucketRanges* ranges = new BucketRanges(3 + 1);
+  LinearHistogram::InitializeBucketRanges(1, 2, ranges);
+  return ranges;
+}
+
+HistogramBase* BooleanHistogram::Factory::HeapAlloc(
+    const BucketRanges* ranges) {
+  return new BooleanHistogram(name_, ranges);
+}
+
+
+class CustomHistogram::Factory : public Histogram::Factory {
+ public:
+  Factory(const std::string& name,
+          const std::vector<Sample>* custom_ranges,
+          int32 flags)
+    : Histogram::Factory(name, CUSTOM_HISTOGRAM, 0, 0, 0, flags) {
+    custom_ranges_ = custom_ranges;
+  }
+
+ protected:
+  BucketRanges* CreateRanges();
+  HistogramBase* HeapAlloc(const BucketRanges* ranges) override;
+
+  const std::vector<Sample>* custom_ranges_;
+};
+
+BucketRanges* CustomHistogram::Factory::CreateRanges() {
+  // Remove the duplicates in the custom ranges array.
+  std::vector<int> ranges = *custom_ranges_;
+  ranges.push_back(0);  // Ensure we have a zero value.
+  ranges.push_back(HistogramBase::kSampleType_MAX);
+  std::sort(ranges.begin(), ranges.end());
+  ranges.erase(std::unique(ranges.begin(), ranges.end()), ranges.end());
+
+  BucketRanges* bucket_ranges = new BucketRanges(ranges.size());
+  for (size_t i = 0; i < ranges.size(); i++) {
+    bucket_ranges->set_range(i, ranges[i]);
+  }
+  bucket_ranges->ResetChecksum();
+  return bucket_ranges;
+}
+
+HistogramBase* CustomHistogram::Factory::HeapAlloc(const BucketRanges* ranges) {
+  return new CustomHistogram(name_, ranges);
+}
+
+
 typedef HistogramBase::Count Count;
 typedef HistogramBase::Sample Sample;
 
 // static
 const size_t Histogram::kBucketCount_MAX = 16384u;
 
 HistogramBase* Histogram::FactoryGet(const std::string& name,
                                      Sample minimum,
                                      Sample maximum,
                                      size_t bucket_count,
                                      int32 flags) {
   bool valid_arguments =
       InspectConstructionArguments(name, &minimum, &maximum, &bucket_count);
   DCHECK(valid_arguments);
 
-  HistogramBase* histogram = StatisticsRecorder::FindHistogram(name);
-  if (!histogram) {
-    // To avoid racy destruction at shutdown, the following will be leaked.
-    BucketRanges* ranges = new BucketRanges(bucket_count + 1);
-    InitializeBucketRanges(minimum, maximum, ranges);
-    const BucketRanges* registered_ranges =
-        StatisticsRecorder::RegisterOrDeleteDuplicateRanges(ranges);
-
-    Histogram* tentative_histogram =
-        new Histogram(name, minimum, maximum, registered_ranges);
-
-    tentative_histogram->SetFlags(flags);
-    histogram =
-        StatisticsRecorder::RegisterOrDeleteDuplicate(tentative_histogram);
-  }
-
-  DCHECK_EQ(HISTOGRAM, histogram->GetHistogramType());
-  if (!histogram->HasConstructionArguments(minimum, maximum, bucket_count)) {
-    // The construction arguments do not match the existing histogram.  This can
-    // come about if an extension updates in the middle of a chrome run and has
-    // changed one of them, or simply by bad code within Chrome itself.  We
-    // return NULL here with the expectation that bad code in Chrome will crash
-    // on dereference, but extension/Pepper APIs will guard against NULL and not
-    // crash.
-    DLOG(ERROR) << "Histogram " << name << " has bad construction arguments";
-    return NULL;
-  }
-  return histogram;
+  return Factory(name, minimum, maximum, bucket_count, flags).Build();
 }
 
 HistogramBase* Histogram::FactoryTimeGet(const std::string& name,
                                          TimeDelta minimum,
                                          TimeDelta maximum,
                                          size_t bucket_count,
                                          int32 flags) {
   return FactoryGet(name, static_cast<Sample>(minimum.InMilliseconds()),
                     static_cast<Sample>(maximum.InMilliseconds()), bucket_count,
                     flags);
@@ skipping 203 matching lines @@
                      Sample maximum,
                      const BucketRanges* ranges)
   : HistogramBase(name),
     bucket_ranges_(ranges),
     declared_min_(minimum),
     declared_max_(maximum) {
   if (ranges)
     samples_.reset(new SampleVector(HashMetricName(name), ranges));
 }
 
+Histogram::Histogram(const std::string& name,
+                     Sample minimum,
+                     Sample maximum,
+                     const BucketRanges* ranges,
+                     HistogramBase::AtomicCount* counts,
+                     size_t counts_size,
+                     HistogramSamples::Metadata* meta)
+  : HistogramBase(name),
+    bucket_ranges_(ranges),
+    declared_min_(minimum),
+    declared_max_(maximum) {
+  if (ranges)
+    samples_.reset(new SampleVector(HashMetricName(name),
+                                    counts, counts_size, meta, ranges));
+}
+
 Histogram::~Histogram() {
 }
 
 bool Histogram::PrintEmptyBucket(size_t index) const {
   return true;
 }
 
 // Use the actual bucket widths (like a linear histogram) until the widths get
 // over some transition value, and then use that transition width.  Exponentials
 // get so big so fast (and we don't expect to see a lot of entries in the large
@@ skipping 230 matching lines @@
       const std::string& name,
       Sample minimum,
       Sample maximum,
       size_t bucket_count,
       int32 flags,
       const DescriptionPair descriptions[]) {
   bool valid_arguments = Histogram::InspectConstructionArguments(
       name, &minimum, &maximum, &bucket_count);
   DCHECK(valid_arguments);
 
-  HistogramBase* histogram = StatisticsRecorder::FindHistogram(name);
-  if (!histogram) {
-    // To avoid racy destruction at shutdown, the following will be leaked.
-    BucketRanges* ranges = new BucketRanges(bucket_count + 1);
-    InitializeBucketRanges(minimum, maximum, ranges);
-    const BucketRanges* registered_ranges =
-        StatisticsRecorder::RegisterOrDeleteDuplicateRanges(ranges);
-
-    LinearHistogram* tentative_histogram =
-        new LinearHistogram(name, minimum, maximum, registered_ranges);
-
-    // Set range descriptions.
-    if (descriptions) {
-      for (int i = 0; descriptions[i].description; ++i) {
-        tentative_histogram->bucket_description_[descriptions[i].sample] =
-            descriptions[i].description;
-      }
-    }
-
-    tentative_histogram->SetFlags(flags);
-    histogram =
-        StatisticsRecorder::RegisterOrDeleteDuplicate(tentative_histogram);
-  }
-
-  DCHECK_EQ(LINEAR_HISTOGRAM, histogram->GetHistogramType());
-  if (!histogram->HasConstructionArguments(minimum, maximum, bucket_count)) {
-    // The construction arguments do not match the existing histogram.  This can
-    // come about if an extension updates in the middle of a chrome run and has
-    // changed one of them, or simply by bad code within Chrome itself.  We
-    // return NULL here with the expectation that bad code in Chrome will crash
-    // on dereference, but extension/Pepper APIs will guard against NULL and not
-    // crash.
-    DLOG(ERROR) << "Histogram " << name << " has bad construction arguments";
-    return NULL;
-  }
-  return histogram;
+  return Factory(name, minimum, maximum, bucket_count, flags, descriptions)
+      .Build();
 }
 
 HistogramType LinearHistogram::GetHistogramType() const {
   return LINEAR_HISTOGRAM;
 }
 
 LinearHistogram::LinearHistogram(const std::string& name,
                                  Sample minimum,
                                  Sample maximum,
                                  const BucketRanges* ranges)
     : Histogram(name, minimum, maximum, ranges) {
 }
 
+LinearHistogram::LinearHistogram(const std::string& name,
+                                 Sample minimum,
+                                 Sample maximum,
+                                 const BucketRanges* ranges,
+                                 HistogramBase::AtomicCount* counts,
+                                 size_t counts_size,
+                                 HistogramSamples::Metadata* meta)
+    : Histogram(name,
+                minimum,
+                maximum,
+                ranges,
+                counts,
+                counts_size,
+                meta) {}
+
 double LinearHistogram::GetBucketSize(Count current, size_t i) const {
   DCHECK_GT(ranges(i + 1), ranges(i));
   // Adjacent buckets with different widths would have "surprisingly" many (few)
   // samples in a histogram if we didn't normalize this way.
   double denominator = ranges(i + 1) - ranges(i);
   return current/denominator;
 }
 
 const std::string LinearHistogram::GetAsciiBucketRange(size_t i) const {
   int range = ranges(i);
@@ skipping 45 matching lines @@
   }
   return histogram;
 }
 
 //------------------------------------------------------------------------------
 // This section provides implementation for BooleanHistogram.
 //------------------------------------------------------------------------------
 
 HistogramBase* BooleanHistogram::FactoryGet(const std::string& name,
                                             int32 flags) {
-  HistogramBase* histogram = StatisticsRecorder::FindHistogram(name);
-  if (!histogram) {
-    // To avoid racy destruction at shutdown, the following will be leaked.
-    BucketRanges* ranges = new BucketRanges(4);
-    LinearHistogram::InitializeBucketRanges(1, 2, ranges);
-    const BucketRanges* registered_ranges =
-        StatisticsRecorder::RegisterOrDeleteDuplicateRanges(ranges);
-
-    BooleanHistogram* tentative_histogram =
-        new BooleanHistogram(name, registered_ranges);
-
-    tentative_histogram->SetFlags(flags);
-    histogram =
-        StatisticsRecorder::RegisterOrDeleteDuplicate(tentative_histogram);
-  }
-
-  DCHECK_EQ(BOOLEAN_HISTOGRAM, histogram->GetHistogramType());
-  return histogram;
+  return Factory(name, flags).Build();
 }
 
 HistogramBase* BooleanHistogram::FactoryGet(const char* name, int32 flags) {
   return FactoryGet(std::string(name), flags);
 }
 
 HistogramType BooleanHistogram::GetHistogramType() const {
   return BOOLEAN_HISTOGRAM;
 }
 
 BooleanHistogram::BooleanHistogram(const std::string& name,
                                    const BucketRanges* ranges)
     : LinearHistogram(name, 1, 2, ranges) {}
 
+BooleanHistogram::BooleanHistogram(const std::string& name,
+                                   const BucketRanges* ranges,
+                                   HistogramBase::AtomicCount* counts,
+                                   HistogramSamples::Metadata* meta)
+    : LinearHistogram(name, 1, 2, ranges, counts, 2, meta) {}
+
 HistogramBase* BooleanHistogram::DeserializeInfoImpl(PickleIterator* iter) {
   std::string histogram_name;
   int flags;
   int declared_min;
   int declared_max;
   size_t bucket_count;
   uint32 range_checksum;
 
   if (!ReadHistogramArguments(iter, &histogram_name, &flags, &declared_min,
                               &declared_max, &bucket_count, &range_checksum)) {
@@ skipping 12 matching lines @@
 //------------------------------------------------------------------------------
 // CustomHistogram:
 //------------------------------------------------------------------------------
 
 HistogramBase* CustomHistogram::FactoryGet(
     const std::string& name,
     const std::vector<Sample>& custom_ranges,
     int32 flags) {
   CHECK(ValidateCustomRanges(custom_ranges));
 
-  HistogramBase* histogram = StatisticsRecorder::FindHistogram(name);
-  if (!histogram) {
-    BucketRanges* ranges = CreateBucketRangesFromCustomRanges(custom_ranges);
-    const BucketRanges* registered_ranges =
-        StatisticsRecorder::RegisterOrDeleteDuplicateRanges(ranges);
-
-    // To avoid racy destruction at shutdown, the following will be leaked.
-    CustomHistogram* tentative_histogram =
-        new CustomHistogram(name, registered_ranges);
-
-    tentative_histogram->SetFlags(flags);
-
-    histogram =
-        StatisticsRecorder::RegisterOrDeleteDuplicate(tentative_histogram);
-  }
-
-  DCHECK_EQ(histogram->GetHistogramType(), CUSTOM_HISTOGRAM);
-  return histogram;
+  return Factory(name, &custom_ranges, flags).Build();
 }
 
 HistogramBase* CustomHistogram::FactoryGet(
     const char* name,
     const std::vector<Sample>& custom_ranges,
     int32 flags) {
   return FactoryGet(std::string(name), custom_ranges, flags);
 }
 
 HistogramType CustomHistogram::GetHistogramType() const {
@@ skipping 15 matching lines @@
   return all_values;
 }
 
 CustomHistogram::CustomHistogram(const std::string& name,
                                  const BucketRanges* ranges)
     : Histogram(name,
                 ranges->range(1),
                 ranges->range(ranges->bucket_count() - 1),
                 ranges) {}
 
+CustomHistogram::CustomHistogram(const std::string& name,
+                                 const BucketRanges* ranges,
+                                 HistogramBase::AtomicCount* counts,
+                                 size_t counts_size,
+                                 HistogramSamples::Metadata* meta)
+    : Histogram(name,
+                ranges->range(1),
+                ranges->range(ranges->bucket_count() - 1),
+                ranges,
+                counts,
+                counts_size,
+                meta) {}
+
 bool CustomHistogram::SerializeInfoImpl(Pickle* pickle) const {
   if (!Histogram::SerializeInfoImpl(pickle))
     return false;
 
   // Serialize ranges. First and last ranges are alwasy 0 and INT_MAX, so don't
   // write them.
   for (size_t i = 1; i < bucket_ranges()->bucket_count(); ++i) {
     if (!pickle->WriteInt(bucket_ranges()->range(i)))
       return false;
   }
@@ skipping 42 matching lines @@
   for (size_t i = 0; i < custom_ranges.size(); i++) {
     Sample sample = custom_ranges[i];
     if (sample < 0 || sample > HistogramBase::kSampleType_MAX - 1)
       return false;
     if (sample != 0)
       has_valid_range = true;
   }
   return has_valid_range;
 }
 
-// static
-BucketRanges* CustomHistogram::CreateBucketRangesFromCustomRanges(
-      const std::vector<Sample>& custom_ranges) {
-  // Remove the duplicates in the custom ranges array.
-  std::vector<int> ranges = custom_ranges;
-  ranges.push_back(0);  // Ensure we have a zero value.
-  ranges.push_back(HistogramBase::kSampleType_MAX);
-  std::sort(ranges.begin(), ranges.end());
-  ranges.erase(std::unique(ranges.begin(), ranges.end()), ranges.end());
-
-  BucketRanges* bucket_ranges = new BucketRanges(ranges.size());
-  for (size_t i = 0; i < ranges.size(); i++) {
-    bucket_ranges->set_range(i, ranges[i]);
-  }
-  bucket_ranges->ResetChecksum();
-  return bucket_ranges;
-}
-
 }  // namespace base