Run SR tests both with and without persistent allocator.

base/metrics/statistics_recorder.cc

Index: base/metrics/statistics_recorder.cc
diff --git a/base/metrics/statistics_recorder.cc b/base/metrics/statistics_recorder.cc
index 9aafe187be23adef5d1fabaf853d109c13fc0033..a75d1f2ebab40fd9202db19bd7121b70be50b500 100644
--- a/base/metrics/statistics_recorder.cc
+++ b/base/metrics/statistics_recorder.cc
@@ -37,6 +37,12 @@ StatisticsRecorder::HistogramIterator::HistogramIterator(
     const HistogramMap::iterator& iter, bool include_persistent)
     : iter_(iter),
       include_persistent_(include_persistent) {
+  // The starting location could point to a persistent histogram when such
+  // is not wanted. If so, skip it.
+  if (!include_persistent_ && iter_ != histograms_->end() &&

[Alexei Svitkine (slow), 2016/04/11 14:51:34]

Shouldn't this be a loop? What if the second entry is also persistent? (If so,
also add a test.)

[bcwhite, 2016/04/12 00:32:02]

operator++ already includes a loop that skips as appropriate.

[Alexei Svitkine (slow), 2016/04/12 21:02:08]

Makes sense. Maybe add a comment so it's clear when reading this code.

[bcwhite, 2016/04/13 12:59:45]

Done.

+      (iter_->second->flags() & HistogramBase::kIsPersistent)) {
+    operator++();
+  }
 }
 
 StatisticsRecorder::HistogramIterator::HistogramIterator(
@@ -287,20 +293,13 @@ void StatisticsRecorder::GetBucketRanges(
 
 // static
 HistogramBase* StatisticsRecorder::FindHistogram(base::StringPiece name) {
-  if (lock_ == NULL)
-    return NULL;
-
-  // Import histograms from known persistent storage. Histograms could have
-  // been added by other processes and they must be fetched and recognized
-  // locally. If the persistent memory segment is not shared between processes,
-  // this call does nothing.
   // This must be called *before* the lock is acquired below because it will
   // call back into this object to register histograms. Those called methods
   // will acquire the lock at that time.
-  GlobalHistogramAllocator* allocator = GlobalHistogramAllocator::Get();
-  if (allocator)
-    allocator->ImportHistogramsToStatisticsRecorder();
+  ImportGlobalPersistentHistograms();
 
+  if (lock_ == NULL)
+    return NULL;
   base::AutoLock auto_lock(*lock_);
   if (histograms_ == NULL)
     return NULL;
@@ -314,6 +313,7 @@ HistogramBase* StatisticsRecorder::FindHistogram(base::StringPiece name) {
 // static
 StatisticsRecorder::HistogramIterator StatisticsRecorder::begin(
     bool include_persistent) {
+  ImportGlobalPersistentHistograms();
   return HistogramIterator(histograms_->begin(), include_persistent);
 }
 
@@ -423,6 +423,20 @@ void StatisticsRecorder::UninitializeForTesting() {
   g_statistics_recorder_.private_instance_ = 0;
 }
 
+// static
+void StatisticsRecorder::ImportGlobalPersistentHistograms() {
+  if (lock_ == NULL)
+    return;
+
+  // Import histograms from known persistent storage. Histograms could have
+  // been added by other processes and they must be fetched and recognized
+  // locally. If the persistent memory segment is not shared between processes,
+  // this call does nothing.
+  GlobalHistogramAllocator* allocator = GlobalHistogramAllocator::Get();
+  if (allocator)
+    allocator->ImportHistogramsToStatisticsRecorder();
+}
+
 // This singleton instance should be started during the single threaded portion
 // of main(), and hence it is not thread safe.  It initializes globals to
 // provide support for all future calls.