Localize counter class member functions.

src/counters.h

Index: src/counters.h
diff --git a/src/counters.h b/src/counters.h
index 26b756624b7303163a1d9450da05c102b87568fa..70d734ef90ac01515a869459d7e282d0c0fb0238 100644
--- a/src/counters.h
+++ b/src/counters.h
@@ -36,9 +36,7 @@ class StatsTable {
 
   // Register an application-defined function to create
   // a histogram for passing to the AddHistogramSample function
-  void SetCreateHistogramFunction(CreateHistogramCallback f) {
-    create_histogram_function_ = f;
-  }
+  void SetCreateHistogramFunction(CreateHistogramCallback f);
 
   // Register an application-defined function to add a sample
   // to a histogram created with CreateHistogram function
@@ -121,13 +119,10 @@ class StatsCounterBase {
 // Internally, a counter represents a value in a row of a StatsTable.
 // The row has a 32bit value for each process/thread in the table and also
 // a name (stored in the table metadata).  Since the storage location can be
-// thread-specific, this class cannot be shared across threads.
+// thread-specific, this class cannot be shared across threads. Note: This
+// class is not thread safe.
 class StatsCounter : public StatsCounterBase {
  public:
-  StatsCounter() { }
-  StatsCounter(Counters* counters, const char* name)
-      : StatsCounterBase(counters, name), lookup_done_(false) {}
-
   // Sets the counter to a specific value.
   void Set(int value) {
     if (int* loc = GetPtr()) SetLoc(loc, value);
@@ -166,10 +161,13 @@ class StatsCounter : public StatsCounterBase {
     return loc;
   }
 
+ private:
+  StatsCounter() {}
+  StatsCounter(Counters* counters, const char* name)
+      : StatsCounterBase(counters, name), lookup_done_(false) {}
   // Reset the cached internal pointer.
   void Reset() { lookup_done_ = false; }
 
- private:
   // Returns the cached address of this counter location.
   int* GetPtr() {
     if (lookup_done_) return ptr_;
@@ -179,16 +177,13 @@ class StatsCounter : public StatsCounterBase {
   }
 
   bool lookup_done_;
+
+  friend class Counters;
 };
 
-// Thread safe version of StatsCounter. WARNING: Unlike StatsCounter,
-// StatsCounterThreadSafe's constructor and method Reset() actually do
-// the table lookup, and should be called from the main thread
-// (i.e. not workers).
+// Thread safe version of StatsCounter.
 class StatsCounterThreadSafe : public StatsCounterBase {
  public:
-  StatsCounterThreadSafe(Counters* counters, const char* name);
-
   void Set(int Value);
   void Increment();
   void Increment(int value);
@@ -199,59 +194,48 @@ class StatsCounterThreadSafe : public StatsCounterBase {
     DCHECK(ptr_ != NULL);
     return ptr_;
   }
-  void Reset() { GetPtr(); }
 
  private:
+  StatsCounterThreadSafe(Counters* counters, const char* name);
   int* GetPtr();

[Mircea Trofin, 2017/06/05 15:57:49]

Why not delete GetPtr(), since it's only used in Reset()?

[kschimpf, 2017/06/05 17:38:52]

Your right. It used to have multiple uses, but is no longer needed. Fixing.

+  void Reset() { GetPtr(); }
 
   base::Mutex mutex_;
 
  private:
+  friend class Counters;
   DISALLOW_IMPLICIT_CONSTRUCTORS(StatsCounterThreadSafe);
 };
 
-// A Histogram represents a dynamically created histogram in the StatsTable.
-// It will be registered with the histogram system on first use.
+// A Histogram represents a dynamically created histogram in the
+// StatsTable.  Note: This class is thread safe.
 class Histogram {
  public:
-  Histogram() { }
-  Histogram(const char* name, int min, int max, int num_buckets,
-            Counters* counters)
-      : name_(name),
-        min_(min),
-        max_(max),
-        num_buckets_(num_buckets),
-        histogram_(NULL),
-        lookup_done_(false),
-        counters_(counters) {}
-
   // Add a single sample to this histogram.
   void AddSample(int sample);
 
   // Returns true if this histogram is enabled.
-  bool Enabled() {
-    return GetHistogram() != NULL;
-  }
-
-  // Reset the cached internal pointer.
-  void Reset() {
-    lookup_done_ = false;
-  }
+  bool Enabled() { return histogram_ != nullptr; }
 
   const char* name() { return name_; }
 
  protected:
-  // Returns the handle to the histogram.
-  void* GetHistogram() {
-    if (!lookup_done_) {
-      lookup_done_ = true;
-      histogram_ = CreateHistogram();
-    }
-    return histogram_;
-  }
-
   Counters* counters() const { return counters_; }
 
+ protected:
+  Histogram() {}
+  Histogram(const char* name, int min, int max, int num_buckets,
+            Counters* counters)
+      : name_(name),
+        min_(min),
+        max_(max),
+        num_buckets_(num_buckets),
+        histogram_(nullptr),
+        counters_(counters) {}
+
+  // Reset the cached internal pointer.
+  void Reset() { histogram_ = CreateHistogram(); }
+
  private:
   void* CreateHistogram() const;
 
@@ -260,8 +244,9 @@ class Histogram {
   int max_;
   int num_buckets_;
   void* histogram_;
-  bool lookup_done_;
   Counters* counters_;
+
+  friend class Counters;
 };
 
 // A HistogramTimer allows distributions of results to be created.
@@ -272,7 +257,7 @@ class HistogramTimer : public Histogram {
     MICROSECOND
   };
 
-  HistogramTimer() {}
+  // Note: public for testing purposes only.
   HistogramTimer(const char* name, int min, int max, Resolution resolution,
                  int num_buckets, Counters* counters)
       : Histogram(name, min, max, num_buckets, counters),
@@ -295,8 +280,12 @@ class HistogramTimer : public Histogram {
 #endif
 
  private:
+  HistogramTimer() {}
+
   base::ElapsedTimer timer_;
   Resolution resolution_;
+
+  friend class Counters;
 };
 
 // Helper class for scoping a HistogramTimer.
@@ -355,11 +344,6 @@ class HistogramTimerScope BASE_EMBEDDED {
 //     events to be timed.
 class AggregatableHistogramTimer : public Histogram {
  public:
-  AggregatableHistogramTimer() {}
-  AggregatableHistogramTimer(const char* name, int min, int max,
-                             int num_buckets, Counters* counters)
-      : Histogram(name, min, max, num_buckets, counters) {}
-
   // Start/stop the "outer" scope.
   void Start() { time_ = base::TimeDelta(); }
   void Stop() { AddSample(static_cast<int>(time_.InMicroseconds())); }
@@ -368,7 +352,13 @@ class AggregatableHistogramTimer : public Histogram {
   void Add(base::TimeDelta other) { time_ += other; }
 
  private:
+  AggregatableHistogramTimer() {}
+  AggregatableHistogramTimer(const char* name, int min, int max,
+                             int num_buckets, Counters* counters)
+      : Histogram(name, min, max, num_buckets, counters) {}
+
   base::TimeDelta time_;
+  friend class Counters;
 };
 
 // A helper class for use with AggregatableHistogramTimer. This is the
@@ -414,14 +404,7 @@ class AggregatedHistogramTimerScope {
 template <typename Histogram>
 class AggregatedMemoryHistogram {
  public:
-  AggregatedMemoryHistogram()
-      : is_initialized_(false),
-        start_ms_(0.0),
-        last_ms_(0.0),
-        aggregate_value_(0.0),
-        last_value_(0.0),
-        backing_histogram_(NULL) {}
-
+  // Note: public for testing purposes only.
   explicit AggregatedMemoryHistogram(Histogram* backing_histogram)
       : AggregatedMemoryHistogram() {
     backing_histogram_ = backing_histogram;
@@ -439,6 +422,14 @@ class AggregatedMemoryHistogram {
   void AddSample(double current_ms, double current_value);
 
  private:
+  AggregatedMemoryHistogram()
+      : is_initialized_(false),
+        start_ms_(0.0),
+        last_ms_(0.0),
+        aggregate_value_(0.0),
+        last_value_(0.0),
+        backing_histogram_(NULL) {}
+
   double Aggregate(double current_ms, double current_value);
   bool is_initialized_;
   double start_ms_;
@@ -446,6 +437,8 @@ class AggregatedMemoryHistogram {
   double aggregate_value_;
   double last_value_;
   Histogram* backing_histogram_;
+
+  friend class Counters;
 };
 
 
@@ -1330,10 +1323,6 @@ class Counters : public std::enable_shared_from_this<Counters> {
   };
   // clang-format on
 
-  void ResetCounters();
-  void ResetHistograms();
-  void InitializeHistograms();
-
   RuntimeCallStats* runtime_call_stats() { return &runtime_call_stats_; }
 
   StatsTable* stats_table() { return &stats_table_; }
@@ -1409,6 +1398,10 @@ class Counters : public std::enable_shared_from_this<Counters> {
 
   RuntimeCallStats runtime_call_stats_;
 
+  friend class StatsTable;
+  void ResetCounters();
+  void ResetHistograms();
+
   DISALLOW_IMPLICIT_CONSTRUCTORS(Counters);
 };