libchrome: Several upstreamable fixes from libchrome

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"
@@ skipping 77 matching lines @@
 const uint32_t Histogram::kBucketCount_MAX = 16384u;
 
 class Histogram::Factory {
  public:
   Factory(const std::string& name,
           HistogramBase::Sample minimum,
           HistogramBase::Sample maximum,
           uint32_t bucket_count,
           int32_t flags)
     : Factory(name, HISTOGRAM, minimum, maximum, bucket_count, flags) {}
+  virtual ~Factory() = default;
 
   // Create histogram based on construction parameters. Caller takes
   // ownership of the returned object.
   HistogramBase* Build();
 
  protected:
   Factory(const std::string& name,
           HistogramType histogram_type,
           HistogramBase::Sample minimum,
           HistogramBase::Sample maximum,
@@ skipping 15 matching lines @@
 
   // Allocate the correct Histogram object off the heap (in case persistent
   // memory is not available).
   virtual std::unique_ptr<HistogramBase> HeapAlloc(const BucketRanges* ranges) {
     return WrapUnique(new Histogram(name_, minimum_, maximum_, ranges));
   }
 
   // Perform any required datafill on the just-created histogram.  If
   // overridden, be sure to call the "super" version -- this method may not
   // always remain empty.
-  virtual void FillHistogram(HistogramBase* histogram) {}
+  virtual void FillHistogram(HistogramBase* /*histogram*/) {}
 
   // These values are protected (instead of private) because they need to
   // be accessible to methods of sub-classes in order to avoid passing
   // unnecessary parameters everywhere.
   const std::string& name_;
   const HistogramType histogram_type_;
   HistogramBase::Sample minimum_;
   HistogramBase::Sample maximum_;
   uint32_t bucket_count_;
   int32_t flags_;
@@ skipping 382 matching lines @@
     samples_.reset(new SampleVector(HashMetricName(name),
                                     counts, counts_size, meta, ranges));
     logged_samples_.reset(new SampleVector(samples_->id(), logged_counts,
                                            counts_size, logged_meta, ranges));
   }
 }
 
 Histogram::~Histogram() {
 }
 
-bool Histogram::PrintEmptyBucket(uint32_t index) const {
+bool Histogram::PrintEmptyBucket(uint32_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
 // buckets), so we need this to make it possible to see what is going on and
 // not have 0-graphical-height buckets.
 double Histogram::GetBucketSize(Count current, uint32_t i) const {
   DCHECK_GT(ranges(i + 1), ranges(i));
@@ skipping 189 matching lines @@
   Factory(const std::string& name,
           HistogramBase::Sample minimum,
           HistogramBase::Sample maximum,
           uint32_t bucket_count,
           int32_t flags,
           const DescriptionPair* descriptions)
     : Histogram::Factory(name, LINEAR_HISTOGRAM, minimum, maximum,
                          bucket_count, flags) {
     descriptions_ = descriptions;
   }
+  ~Factory() override = default;
 
  protected:
   BucketRanges* CreateRanges() override {
     BucketRanges* ranges = new BucketRanges(bucket_count_ + 1);
     LinearHistogram::InitializeBucketRanges(minimum_, maximum_, ranges);
     return ranges;
   }
 
   std::unique_ptr<HistogramBase> HeapAlloc(
       const BucketRanges* ranges) override {
@@ skipping 172 matching lines @@
 }
 
 //------------------------------------------------------------------------------
 // This section provides implementation for BooleanHistogram.
 //------------------------------------------------------------------------------
 
 class BooleanHistogram::Factory : public Histogram::Factory {
  public:
   Factory(const std::string& name, int32_t flags)
     : Histogram::Factory(name, BOOLEAN_HISTOGRAM, 1, 2, 3, flags) {}
+  ~Factory() override = default;
 
  protected:
   BucketRanges* CreateRanges() override {
     BucketRanges* ranges = new BucketRanges(3 + 1);
     LinearHistogram::InitializeBucketRanges(1, 2, ranges);
     return ranges;
   }
 
   std::unique_ptr<HistogramBase> HeapAlloc(
       const BucketRanges* ranges) override {
@@ skipping 68 matching lines @@
 //------------------------------------------------------------------------------
 
 class CustomHistogram::Factory : public Histogram::Factory {
  public:
   Factory(const std::string& name,
           const std::vector<Sample>* custom_ranges,
           int32_t flags)
     : Histogram::Factory(name, CUSTOM_HISTOGRAM, 0, 0, 0, flags) {
     custom_ranges_ = custom_ranges;
   }
+  ~Factory() override = default;
 
  protected:
   BucketRanges* CreateRanges() override {
     // 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());
 
@@ skipping 93 matching lines @@
 
   // Serialize ranges. First and last ranges are alwasy 0 and INT_MAX, so don't
   // write them.
   for (uint32_t i = 1; i < bucket_ranges()->bucket_count(); ++i) {
     if (!pickle->WriteInt(bucket_ranges()->range(i)))
       return false;
   }
   return true;
 }
 
-double CustomHistogram::GetBucketSize(Count current, uint32_t i) const {
+double CustomHistogram::GetBucketSize(Count /*current*/, uint32_t /*i*/) const {

[Alexei Svitkine (slow), 2016/05/24 17:39:12]

I don't think either commenting out the param or omitting is consistent with the
style of the Chromium codebase. (Not counting Blink which has a style that
evolved from the WebKit style.)

I would like to see this discussion on chromium-dev@ before approving these
changes.

[Luis Héctor Chávez, 2016/05/24 17:45:07]

Chromium follows the Google style, which does allow for omitting param names
when unused if the name is obvious, and comment it if the name is not:
https://google.github.io/styleguide/cppguide.html#Function_Declarations_and_D...

[Alexei Svitkine (slow), 2016/05/24 17:48:54]

Sure, but Chromium also chooses certain patterns for consistency with the rest
of the codebase. For example Google style guide allows both Foo* bar; and Foo
*bar; but Chromium prefers the former.

This is another such case currently in my experience of Chromium code. If we
want to change the preference or allow the inconsistency between different
files, it should be discussed on chromium-dev@.

[danakj, 2016/05/24 18:39:52]

Omitting is not the same as commenting out, and leaving a comment seems to imply
the name is not obvious.

   return 1;
 }
 
 // static
 HistogramBase* CustomHistogram::DeserializeInfoImpl(PickleIterator* iter) {
   std::string histogram_name;
   int flags;
   int declared_min;
   int declared_max;
   uint32_t bucket_count;
@@ skipping 29 matching lines @@
     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;
 }
 
 }  // namespace base