Simplified and optimized StatisticsRecorder.

StatisticsRecorder is now an actual container of collections.

There is only one global recorder at any time, referenced by
StatisticsRecorder::top_.

Each StatisticsRecorder also has a pointer to the previous global recorder.

We now have a stack (singly linked list) of StatisticsRecorder objects, where
the top of the stack (StatisticsRecorder::top_) is the current global recorder.

This pattern is clearer and cleaner. It avoids a bunch of static variables,
indirections, unique_ptrs and calls to operator new. It gets rid of the ugly
hacks of UninitializeForTesting. It reduces memory fragmentation by having fewer
dynamically allocated objects.

Removed methods Reset, UninitializeForTesting and DumpHistogramsToVlog. They are
now unnecessary.

Reviewed, clarified and documented the thread safety status of each method.

Added comments.


Change-Id: Ia1a7611905009d0449068c801464ad0df7813c0d
Bug: 
Reviewed-on: https://chromium-review.googlesource.com/830997
Commit-Queue: François Degros <fdegros@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526570}

base/metrics/statistics_recorder_unittest.cc

@@ -30,9 +30,7 @@ class LogStateSaver {
  public:
   LogStateSaver() : old_min_log_level_(logging::GetMinLogLevel()) {}
 
-  ~LogStateSaver() {
-    logging::SetMinLogLevel(old_min_log_level_);
-  }
+  ~LogStateSaver() { logging::SetMinLogLevel(old_min_log_level_); }
 
  private:
   int old_min_log_level_;
@@ -70,8 +68,8 @@ class StatisticsRecorderTest : public testing::TestWithParam<bool> {
 
     // Use persistent memory for histograms if so indicated by test parameter.
     if (use_persistent_histogram_allocator_) {
-      GlobalHistogramAllocator::CreateWithLocalMemory(
-          kAllocatorMemorySize, 0, "StatisticsRecorderTest");
+      GlobalHistogramAllocator::CreateWithLocalMemory(kAllocatorMemorySize, 0,
+                                                      "StatisticsRecorderTest");
     }
   }
 
@@ -82,14 +80,10 @@ class StatisticsRecorderTest : public testing::TestWithParam<bool> {
 
   void InitializeStatisticsRecorder() {
     DCHECK(!statistics_recorder_);
-    StatisticsRecorder::UninitializeForTesting();
     statistics_recorder_ = StatisticsRecorder::CreateTemporaryForTesting();
   }
 
-  void UninitializeStatisticsRecorder() {
-    statistics_recorder_.reset();
-    StatisticsRecorder::UninitializeForTesting();
-  }
+  void UninitializeStatisticsRecorder() { statistics_recorder_.reset(); }
 
   Histogram* CreateHistogram(const char* name,
                              HistogramBase::Sample min,
@@ -102,18 +96,15 @@ class StatisticsRecorderTest : public testing::TestWithParam<bool> {
     return new Histogram(name, min, max, registered_ranges);
   }
 
-  void DeleteHistogram(HistogramBase* histogram) {
-    delete histogram;
-  }
+  void DeleteHistogram(HistogramBase* histogram) { delete histogram; }
 
-  void InitLogOnShutdown() {
-    DCHECK(statistics_recorder_);
-    statistics_recorder_->InitLogOnShutdownWithoutLock();
-  }
+  void InitLogOnShutdown() { StatisticsRecorder::InitLogOnShutdown(); }
 
-  bool VLogInitialized() {
-    DCHECK(statistics_recorder_);
-    return statistics_recorder_->vlog_initialized_;
+  bool IsVLogInitialized() { return StatisticsRecorder::is_vlog_initialized_; }
+
+  void ResetVLogInitialized() {
+    UninitializeStatisticsRecorder();
+    StatisticsRecorder::is_vlog_initialized_ = false;
   }
 
   const bool use_persistent_histogram_allocator_;
@@ -277,8 +268,8 @@ TEST_P(StatisticsRecorderTest, RegisterHistogramWithFactoryGet) {
   ASSERT_EQ(0u, registered_histograms.size());
 
   // Create a histogram.
-  HistogramBase* histogram = Histogram::FactoryGet(
-      "TestHistogram", 1, 1000, 10, HistogramBase::kNoFlags);
+  HistogramBase* histogram = Histogram::FactoryGet("TestHistogram", 1, 1000, 10,
+                                                   HistogramBase::kNoFlags);
   registered_histograms.clear();
   StatisticsRecorder::GetHistograms(&registered_histograms);
   EXPECT_EQ(1u, registered_histograms.size());
@@ -292,15 +283,15 @@ TEST_P(StatisticsRecorderTest, RegisterHistogramWithFactoryGet) {
   EXPECT_EQ(histogram, histogram2);
 
   // Create a LinearHistogram.
-  histogram = LinearHistogram::FactoryGet(
-      "TestLinearHistogram", 1, 1000, 10, HistogramBase::kNoFlags);
+  histogram = LinearHistogram::FactoryGet("TestLinearHistogram", 1, 1000, 10,
+                                          HistogramBase::kNoFlags);
   registered_histograms.clear();
   StatisticsRecorder::GetHistograms(&registered_histograms);
   EXPECT_EQ(2u, registered_histograms.size());
 
   // Create a BooleanHistogram.
-  histogram = BooleanHistogram::FactoryGet(
-      "TestBooleanHistogram", HistogramBase::kNoFlags);
+  histogram = BooleanHistogram::FactoryGet("TestBooleanHistogram",
+                                           HistogramBase::kNoFlags);
   registered_histograms.clear();
   StatisticsRecorder::GetHistograms(&registered_histograms);
   EXPECT_EQ(3u, registered_histograms.size());
@@ -309,8 +300,8 @@ TEST_P(StatisticsRecorderTest, RegisterHistogramWithFactoryGet) {
   std::vector<int> custom_ranges;
   custom_ranges.push_back(1);
   custom_ranges.push_back(5);
-  histogram = CustomHistogram::FactoryGet(
-      "TestCustomHistogram", custom_ranges, HistogramBase::kNoFlags);
+  histogram = CustomHistogram::FactoryGet("TestCustomHistogram", custom_ranges,
+                                          HistogramBase::kNoFlags);
   registered_histograms.clear();
   StatisticsRecorder::GetHistograms(&registered_histograms);
   EXPECT_EQ(4u, registered_histograms.size());
@@ -624,33 +615,33 @@ TEST_P(StatisticsRecorderTest, CallbackUsedBeforeHistogramCreatedTest) {
 }
 
 TEST_P(StatisticsRecorderTest, LogOnShutdownNotInitialized) {
-  UninitializeStatisticsRecorder();
+  ResetVLogInitialized();
   logging::SetMinLogLevel(logging::LOG_WARNING);
   InitializeStatisticsRecorder();
   EXPECT_FALSE(VLOG_IS_ON(1));
-  EXPECT_FALSE(VLogInitialized());
+  EXPECT_FALSE(IsVLogInitialized());
   InitLogOnShutdown();
-  EXPECT_FALSE(VLogInitialized());
+  EXPECT_FALSE(IsVLogInitialized());
 }
 
 TEST_P(StatisticsRecorderTest, LogOnShutdownInitializedExplicitly) {
-  UninitializeStatisticsRecorder();
+  ResetVLogInitialized();
   logging::SetMinLogLevel(logging::LOG_WARNING);
   InitializeStatisticsRecorder();
   EXPECT_FALSE(VLOG_IS_ON(1));
-  EXPECT_FALSE(VLogInitialized());
+  EXPECT_FALSE(IsVLogInitialized());
   logging::SetMinLogLevel(logging::LOG_VERBOSE);
   EXPECT_TRUE(VLOG_IS_ON(1));
   InitLogOnShutdown();
-  EXPECT_TRUE(VLogInitialized());
+  EXPECT_TRUE(IsVLogInitialized());
 }
 
 TEST_P(StatisticsRecorderTest, LogOnShutdownInitialized) {
-  UninitializeStatisticsRecorder();
+  ResetVLogInitialized();
   logging::SetMinLogLevel(logging::LOG_VERBOSE);
   InitializeStatisticsRecorder();
   EXPECT_TRUE(VLOG_IS_ON(1));
-  EXPECT_TRUE(VLogInitialized());
+  EXPECT_TRUE(IsVLogInitialized());
 }
 
 class TestHistogramProvider : public StatisticsRecorder::HistogramProvider {