libchrome: Several upstreamable fixes from libchrome

base/metrics/histogram.cc

Index: base/metrics/histogram.cc
diff --git a/base/metrics/histogram.cc b/base/metrics/histogram.cc
index 3571657513f6482c5724f721c17e8a2c354ab79a..0e363e3f5199ef68a2ddb2d79696fc28f43cea6e 100644
--- a/base/metrics/histogram.cc
+++ b/base/metrics/histogram.cc
@@ -95,6 +95,7 @@ class Histogram::Factory {
           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.
@@ -130,7 +131,7 @@ class Histogram::Factory {
   // 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
@@ -533,7 +534,7 @@ Histogram::Histogram(const std::string& name,
 Histogram::~Histogram() {
 }
 
-bool Histogram::PrintEmptyBucket(uint32_t index) const {
+bool Histogram::PrintEmptyBucket(uint32_t /*index*/) const {
   return true;
 }
 
@@ -743,6 +744,7 @@ class LinearHistogram::Factory : public Histogram::Factory {
                          bucket_count, flags) {
     descriptions_ = descriptions;
   }
+  ~Factory() override = default;
 
  protected:
   BucketRanges* CreateRanges() override {
@@ -935,6 +937,7 @@ 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 {
@@ -1023,6 +1026,7 @@ class CustomHistogram::Factory : public Histogram::Factory {
     : Histogram::Factory(name, CUSTOM_HISTOGRAM, 0, 0, 0, flags) {
     custom_ranges_ = custom_ranges;
   }
+  ~Factory() override = default;
 
  protected:
   BucketRanges* CreateRanges() override {
@@ -1136,7 +1140,7 @@ bool CustomHistogram::SerializeInfoImpl(Pickle* pickle) const {
   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;
 }