Chromium Code Reviews| Index: chrome/test/nacl/nacl_browsertest_uma.cc | 
| =================================================================== | 
| --- chrome/test/nacl/nacl_browsertest_uma.cc (revision 155400) | 
| +++ chrome/test/nacl/nacl_browsertest_uma.cc (working copy) | 
| @@ -3,7 +3,9 @@ | 
| // found in the LICENSE file. | 
| #include "base/bind.h" | 
| +#include "base/memory/scoped_ptr.h" | 
| #include "base/metrics/histogram.h" | 
| +#include "base/metrics/histogram_samples.h" | 
| #include "base/metrics/statistics_recorder.h" | 
| #include "chrome/test/base/ui_test_utils.h" | 
| #include "chrome/test/nacl/nacl_browsertest_util.h" | 
| @@ -23,7 +25,8 @@ | 
| // We know the exact number of samples in a bucket, and that no other bucket | 
| // should have samples. | 
| - void ExpectUniqueSample(const std::string& name, size_t bucket_id, | 
| + void ExpectUniqueSample(const std::string& name, | 
| + base::Histogram::Sample sample, | 
| base::Histogram::Count expected_count); | 
| // We don't know the values of the samples, but we know how many there are. | 
| @@ -32,13 +35,14 @@ | 
| private: | 
| void FetchCallback(); | 
| - void CheckBucketCount(const std::string& name, size_t bucket_id, | 
| + void CheckBucketCount(const std::string& name, | 
| + base::Histogram::Sample sample, | 
| base::Histogram::Count expected_count, | 
| - base::Histogram::SampleSet& samples); | 
| + const base::HistogramSamples& samples); | 
| void CheckTotalCount(const std::string& name, | 
| base::Histogram::Count expected_count, | 
| - base::Histogram::SampleSet& samples); | 
| + const base::HistogramSamples& samples); | 
| }; | 
| HistogramHelper::HistogramHelper() { | 
| @@ -60,16 +64,15 @@ | 
| void HistogramHelper::ExpectUniqueSample( | 
| const std::string& name, | 
| - size_t bucket_id, | 
| + base::Histogram::Sample sample, | 
| base::Histogram::Count expected_count) { | 
| base::Histogram* histogram = base::StatisticsRecorder::FindHistogram(name); | 
| ASSERT_NE(static_cast<base::Histogram*>(NULL), histogram) << | 
| "Histogram \"" << name << "\" does not exist."; | 
| - base::Histogram::SampleSet samples; | 
| - histogram->SnapshotSample(&samples); | 
| - CheckBucketCount(name, bucket_id, expected_count, samples); | 
| - CheckTotalCount(name, expected_count, samples); | 
| + scoped_ptr<base::HistogramSamples> samples(histogram->SnapshotSamples()); | 
| + CheckBucketCount(name, sample, expected_count, *samples); | 
| + CheckTotalCount(name, expected_count, *samples); | 
| } | 
| void HistogramHelper::ExpectTotalCount(const std::string& name, | 
| @@ -78,9 +81,8 @@ | 
| ASSERT_NE((base::Histogram*)NULL, histogram) << "Histogram \"" << name << | 
| "\" does not exist."; | 
| - base::Histogram::SampleSet samples; | 
| - histogram->SnapshotSample(&samples); | 
| - CheckTotalCount(name, count, samples); | 
| + scoped_ptr<base::HistogramSamples> samples(histogram->SnapshotSamples()); | 
| + CheckTotalCount(name, count, *samples); | 
| } | 
| void HistogramHelper::FetchCallback() { | 
| @@ -88,20 +90,22 @@ | 
| } | 
| void HistogramHelper::CheckBucketCount(const std::string& name, | 
| - size_t bucket_id, | 
| + base::Histogram::Sample sample, | 
| base::Histogram::Count expected_count, | 
| - base::Histogram::SampleSet& samples) { | 
| - EXPECT_EQ(expected_count, samples.counts(bucket_id)) << "Histogram \"" << | 
| - name << "\" does not have the right number of samples (" << | 
| - expected_count << ") in the expected bucket (" << bucket_id << ")."; | 
| + const base::HistogramSamples& samples) { | 
| + EXPECT_EQ(expected_count, samples.GetCount(sample)) | 
| + << "Histogram \"" << name | 
| + << "\" does not have the right number of samples (" << expected_count | 
| + << ") in the expected bucket (" << sample << ")."; | 
| } | 
| void HistogramHelper::CheckTotalCount(const std::string& name, | 
| base::Histogram::Count expected_count, | 
| - base::Histogram::SampleSet& samples) { | 
| - EXPECT_EQ(expected_count, samples.TotalCount()) << "Histogram \"" << name << | 
| - "\" does not have the right total number of samples (" << | 
| - expected_count << ")."; | 
| + const base::HistogramSamples& samples) { | 
| + EXPECT_EQ(expected_count, samples.TotalCount()) | 
| + << "Histogram \"" << name | 
| + << "\" does not have the right total number of samples (" | 
| + << expected_count << ")."; | 
| } | 
| NACL_BROWSER_TEST_F(NaClBrowserTest, SuccessfulLoadUMA, { | 
| @@ -114,12 +118,12 @@ | 
| histograms.Fetch(); | 
| // Did the plugin report success? | 
| - histograms.ExpectUniqueSample("NaCl.LoadStatus.Plugin", | 
| - plugin::ERROR_LOAD_SUCCESS, 1); | 
| + histograms.ExpectUniqueSample( | 
| + "NaCl.LoadStatus.Plugin", plugin::ERROR_LOAD_SUCCESS, 1); | 
| 
 
Ilya Sherman
2012/09/12 03:20:58
nit: No need to re-wrap this line; the previous wr
 
 | 
| // Did the sel_ldr report success? | 
| - histograms.ExpectUniqueSample("NaCl.LoadStatus.SelLdr", | 
| - LOAD_OK, 1); | 
| + histograms.ExpectUniqueSample( | 
| + "NaCl.LoadStatus.SelLdr", LOAD_OK, 1); | 
| 
 
Ilya Sherman
2012/09/12 03:20:58
nit: No need to re-wrap this line; the previous wr
 
kaiwang
2012/09/20 22:54:59
they are more readable, in previous wrapping, it's
 
 | 
| // Make sure we have other important histograms. | 
| histograms.ExpectTotalCount("NaCl.Perf.StartupTime.LoadModule", 1); |