Support negative sample values in PersistentHistogramMap.

base/metrics/persistent_sample_map.cc

 // 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.
 
 #include "base/metrics/persistent_sample_map.h"
 
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/persistent_histogram_allocator.h"
 #include "base/stl_util.h"
@@ skipping 186 matching lines @@
     if (min + 1 != max)
       return false;  // SparseHistogram only supports bucket with size 1.
 
     *GetOrCreateSampleCountStorage(min) +=
         (op == HistogramSamples::ADD) ? count : -count;
   }
   return true;
 }
 
 Count* PersistentSampleMap::GetSampleCountStorage(Sample value) {
-  DCHECK_LE(0, value);
-
   // If |value| is already in the map, just return that.
   auto it = sample_counts_.find(value);
   if (it != sample_counts_.end())
     return it->second;
 
   // Import any new samples from persistent memory looking for the value.
   return ImportSamples(value);
 }
 
 Count* PersistentSampleMap::GetOrCreateSampleCountStorage(Sample value) {
@@ skipping 22 matching lines @@
   // will always use the same record, whichever was first made iterable.
   // Thread-safety within a process where multiple threads use the same
   // histogram object is delegated to the controlling histogram object which,
   // for sparse histograms, is a lock object.
   count_pointer = ImportSamples(value);
   DCHECK(count_pointer);
   return count_pointer;
 }
 
 Count* PersistentSampleMap::ImportSamples(Sample until_value) {
+  Count* found_count = nullptr;
   PersistentMemoryAllocator::Reference ref;
   while ((ref = records_->GetNext()) != 0) {
     SampleRecord* record =
         records_->GetAsObject<SampleRecord>(ref, kTypeIdSampleRecord);
     if (!record)
       continue;
 
     DCHECK_EQ(id(), record->id);
 
     // Check if the record's value is already known.
     if (!ContainsKey(sample_counts_, record->value)) {
-      // No: Add it to map of known values if the value is valid.
-      if (record->value >= 0)
-        sample_counts_[record->value] = &record->count;
+      // No: Add it to map of known values.
+      sample_counts_[record->value] = &record->count;
     } else {
       // Yes: Ignore it; it's a duplicate caused by a race condition -- see
       // code & comment in GetOrCreateSampleCountStorage() for details.
       // Check that nothing ever operated on the duplicate record.
       DCHECK_EQ(0, record->count);
     }
 
-    // Stop if it's the value being searched for.
-    if (record->value == until_value)
-      return &record->count;
+    // Check if it's the value being searched for. There is a small chance
+    // that some histogram is actually using the "kAllSamples" value. In
+    // that case, keep the pointer for later but continue loading all the
+    // rest of the samples, too. And because race conditions can cause
+    // multiple records for a single value, be sure to only return the
+    // first one found.
+    if (record->value == until_value) {
+      if (!found_count)
+        found_count = &record->count;
+      if (until_value != kAllSamples)
+        break;
+    }
   }
 
-  return nullptr;
+  return found_count;
 }
 
 }  // namespace base