Support negative sample values in PersistentHistogramMap.

base/metrics/persistent_sample_map.h

 // Copyright 2016 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.
 
 // PersistentSampleMap implements HistogramSamples interface. It is used
 // by the SparseHistogram class to store samples in persistent memory which
 // allows it to be shared between processes or live across restarts.
 
 #ifndef BASE_METRICS_PERSISTENT_SAMPLE_MAP_H_
 #define BASE_METRICS_PERSISTENT_SAMPLE_MAP_H_
 
 #include <stdint.h>
 
 #include <map>
 #include <memory>
 
 #include "base/compiler_specific.h"
+#include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_base.h"
 #include "base/metrics/histogram_samples.h"
 #include "base/metrics/persistent_memory_allocator.h"
 
 namespace base {
 
 class PersistentHistogramAllocator;
 class PersistentSampleMapRecords;
 class PersistentSparseHistogramDataManager;
@@ skipping 46 matching lines @@
   // Gets a pointer to a "count" corresponding to a given |value|. Returns NULL
   // if sample does not exist.
   HistogramBase::Count* GetSampleCountStorage(HistogramBase::Sample value);
 
   // Gets a pointer to a "count" corresponding to a given |value|, creating
   // the sample (initialized to zero) if it does not already exists.
   HistogramBase::Count* GetOrCreateSampleCountStorage(
       HistogramBase::Sample value);
 
  private:
-  enum : HistogramBase::Sample { kAllSamples = -1 };
+  FRIEND_TEST_ALL_PREFIXES(PersistentSampleMapTest, PersistenceTest);
+
+  // Since all values are possible in a SparseHistogram, the value of
+  // kAllSamples is chosen to be something unlikely. In the case that this
+  // value is used within the sample map, it is still safe.
+  enum : HistogramBase::Sample { kAllSamples = -10000 };

[Alexei Svitkine (slow), 2016/04/20 21:51:18]

Can we avoid having this constant completely? For example, having a different
function to return all samples?

[Alexei Svitkine (slow), 2016/04/20 21:51:55]

Or a separate boolean parameter?

[bcwhite, 2016/04/21 10:33:15]

Sure, though the two parameters would generally be mutually exclusive.  I
suppose I could pass a pointer to a Sample and allow "nullptr" to mean "all". 
Nah...  Boolean works better.

Done.

Two functions would be pretty much complete duplicates of each other so I don't
think it's a good idea.

 
   // Imports samples from persistent memory by iterating over all sample
   // records found therein, adding them to the sample_counts_ map. If a
   // count for the sample |until_value| is found, stop the import and return
   // a pointer to that counter. If that value is not found, null will be
   // returned after all currently available samples have been loaded. Pass
   // kAllSamples to force the importing of all available samples.
   HistogramBase::Count* ImportSamples(HistogramBase::Sample until_value);
 
   // All created/loaded sample values and their associated counts. The storage
   // for the actual Count numbers is owned by the |records_| object and its
   // underlying allocator.
   std::map<HistogramBase::Sample, HistogramBase::Count*> sample_counts_;
 
   // The object that manages records inside persistent memory. This is owned
   // externally (typically by a PersistentHistogramAllocator) and is expected
   // to live beyond the life of this object.
   PersistentSampleMapRecords* records_;
 
   DISALLOW_COPY_AND_ASSIGN(PersistentSampleMap);
 };
 
 }  // namespace base
 
 #endif  // BASE_METRICS_PERSISTENT_SAMPLE_MAP_H_