SampleSet -> HistogramSamples (will be reused by SparseHistogram)

net/url_request/url_request_throttler_unittest.cc

Index: net/url_request/url_request_throttler_unittest.cc
===================================================================
--- net/url_request/url_request_throttler_unittest.cc	(revision 155400)
+++ net/url_request/url_request_throttler_unittest.cc	(working copy)
@@ -6,8 +6,10 @@
 
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
+#include "base/metrics/histogram_samples.h"
 #include "base/metrics/statistics_recorder.h"
 #include "base/pickle.h"
+#include "base/stl_util.h"
 #include "base/stringprintf.h"
 #include "base/string_number_conversions.h"
 #include "base/time.h"
@@ -27,6 +29,7 @@
 namespace {
 
 using base::Histogram;
+using base::HistogramSamples;
 using base::StatisticsRecorder;
 
 class MockURLRequestThrottlerEntry : public URLRequestThrottlerEntry {
@@ -172,6 +175,7 @@
   }
 
   virtual void SetUp();
+  virtual void TearDown();
 
   // After calling this function, histogram snapshots in |samples_| contain
   // only the delta caused by the test case currently running.
@@ -181,8 +185,8 @@
   MockURLRequestThrottlerManager manager_;  // Dummy object, not used.
   scoped_refptr<MockURLRequestThrottlerEntry> entry_;
 
-  std::map<std::string, Histogram::SampleSet> original_samples_;
-  std::map<std::string, Histogram::SampleSet> samples_;
+  std::map<std::string, HistogramSamples*> original_samples_;
+  std::map<std::string, HistogramSamples*> samples_;
 
   TestURLRequestContext context_;
   TestURLRequest request_;
@@ -207,33 +211,38 @@
     // Must retrieve original samples for each histogram for comparison
     // as other tests may affect them.
     const char* name = kHistogramNames[i];
-    Histogram::SampleSet& original = original_samples_[name];
     Histogram* histogram = StatisticsRecorder::FindHistogram(name);
     if (histogram) {
-      histogram->SnapshotSample(&original);
+      original_samples_[name] = histogram->SnapshotSamples().release();
+    } else {
+      original_samples_[name] = NULL;
     }
   }
 }
 
+void URLRequestThrottlerEntryTest::TearDown() {
+  STLDeleteValues(&original_samples_);
+  STLDeleteValues(&samples_);
+}
+
 void URLRequestThrottlerEntryTest::CalculateHistogramDeltas() {
   for (size_t i = 0; i < arraysize(kHistogramNames); ++i) {
     const char* name = kHistogramNames[i];
-    Histogram::SampleSet& original = original_samples_[name];
-    Histogram::SampleSet& sample = samples_[name];
+    HistogramSamples* original = original_samples_[name];
 
     Histogram* histogram = StatisticsRecorder::FindHistogram(name);
     if (histogram) {
       ASSERT_EQ(Histogram::kUmaTargetedHistogramFlag, histogram->flags());
 
-      histogram->SnapshotSample(&sample);
-      // Ensure |original| size is same as |sample|, then subtract original
-      // values.
-      original.Resize(histogram->bucket_count());
-      sample.Subtract(original);
+      scoped_ptr<HistogramSamples> samples(histogram->SnapshotSamples());
+      if (original != NULL)

[Ilya Sherman, 2012/09/12 03:20:58]

nit: You can just write this as "if (original)"

[kaiwang, 2012/09/20 22:54:59]

Done.

+        samples->Subtract(*original);
+      samples_[name] = samples.release();

[Ilya Sherman, 2012/09/12 03:20:58]

Is |samples_[name]| guaranteed to be empty prior to this?  If not, this looks
like it will leak memory.

[kaiwang, 2012/09/20 22:54:59]

it's guranteed to be empty

     }
   }
 
   // Ensure we don't accidentally use the originals in our tests.
+  STLDeleteValues(&original_samples_);
   original_samples_.clear();
 }
 
@@ -251,8 +260,8 @@
   EXPECT_FALSE(entry_->ShouldRejectRequest(request_));
 
   CalculateHistogramDeltas();
-  ASSERT_EQ(1, samples_["Throttling.RequestThrottled"].counts(0));
-  ASSERT_EQ(1, samples_["Throttling.RequestThrottled"].counts(1));
+  ASSERT_EQ(1, samples_["Throttling.RequestThrottled"]->GetCount(0));
+  ASSERT_EQ(1, samples_["Throttling.RequestThrottled"]->GetCount(1));
 }
 
 TEST_F(URLRequestThrottlerEntryTest, InterfaceNotDuringExponentialBackoff) {
@@ -263,8 +272,8 @@
   EXPECT_FALSE(entry_->ShouldRejectRequest(request_));
 
   CalculateHistogramDeltas();
-  ASSERT_EQ(2, samples_["Throttling.RequestThrottled"].counts(0));
-  ASSERT_EQ(0, samples_["Throttling.RequestThrottled"].counts(1));
+  ASSERT_EQ(2, samples_["Throttling.RequestThrottled"]->GetCount(0));
+  ASSERT_EQ(0, samples_["Throttling.RequestThrottled"]->GetCount(1));
 }
 
 TEST_F(URLRequestThrottlerEntryTest, InterfaceUpdateFailure) {