SampleSet -> HistogramSamples (will be reused by SparseHistogram)

chrome/browser/net/http_pipelining_compatibility_client_unittest.cc

 // Copyright (c) 2012 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 "chrome/browser/net/http_pipelining_compatibility_client.h"
 
 #include <map>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/metrics/histogram.h"
+#include "base/metrics/histogram_samples.h"
 #include "base/metrics/statistics_recorder.h"
 #include "base/stl_util.h"
 #include "base/stringprintf.h"
 #include "content/public/test/test_browser_thread.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_completion_callback.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_test_util.h"
 #include "net/test/test_server.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using base::Histogram;
+using base::HistogramSamples;
+
 namespace chrome_browser_net {
 
 namespace {
 
 static const char* const kHistogramNames[] = {
   "NetConnectivity.Pipeline.CanarySuccess",
   "NetConnectivity.Pipeline.Depth",
   "NetConnectivity.Pipeline.AllHTTP11",
   "NetConnectivity.Pipeline.Success",
   "NetConnectivity.Pipeline.0.NetworkError",
@@ skipping 52 matching lines @@
     // Start up a histogram recorder.
     // TODO(rtenneti): Leaks StatisticsRecorder and will update suppressions.
     base::StatisticsRecorder::Initialize();
     ASSERT_TRUE(test_server_.Start());
     context_ = new TestURLRequestContextGetter(
         BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO));
     context_->AddRef();
 
     for (size_t i = 0; i < arraysize(kHistogramNames); ++i) {
       const char* name = kHistogramNames[i];
-      base::Histogram::SampleSet sample = GetHistogram(name);
-      if (sample.TotalCount() > 0) {
-        original_samples_[name] = sample;
+      scoped_ptr<HistogramSamples> samples = GetHistogram(name);
+      if (samples.get() && samples->TotalCount() > 0) {
+        original_samples_[name] = samples.release();
       }
     }
   }
 
   virtual void TearDown() OVERRIDE {
     BrowserThread::ReleaseSoon(BrowserThread::IO, FROM_HERE, context_);
     message_loop_.RunAllPending();
+    STLDeleteValues(&original_samples_);
   }
 
   void RunTest(
       std::vector<RequestInfo> requests,
       HttpPipeliningCompatibilityClient::Options options) {
     HttpPipeliningCompatibilityClient client(NULL);
     net::TestCompletionCallback callback;
     client.Start(test_server_.GetURL("").spec(),
                  requests, options, callback.callback(),
                  context_->GetURLRequestContext());
     callback.WaitForResult();
   }
 
-  void ExpectHistogramCount(int expected_count, int expected_value,
+  void ExpectHistogramCount(int expected_count,
+                            int expected_value,
                             HistogramField field) {
     const char* name;
 
     switch (field) {
       case FIELD_CANARY:
         name = "NetConnectivity.Pipeline.CanarySuccess";
         break;
 
       case FIELD_DEPTH:
         name = "NetConnectivity.Pipeline.Depth";
         break;
 
       case FIELD_HTTP_1_1:
         name = "NetConnectivity.Pipeline.AllHTTP11";
         break;
 
       case FIELD_SUCCESS:
         name = "NetConnectivity.Pipeline.Success";
         break;
 
       default:
         FAIL() << "Unexpected field: " << field;
     }
 
-    base::Histogram::SampleSet sample = GetHistogram(name);
+    scoped_ptr<HistogramSamples> samples = GetHistogram(name);
+    if (!samples.get())

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

Should this be an ASSERT_TRUE(samples.get()), i.e. should this trigger a test
failure?  (Same comment applies to other cases below.)

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

The old GetHistogram function can return an empty SampleSet. But new
implementation will return an empty scoped_ptr because we don't initiate
concrete class SampleVector here.

According to the unittest logic, the following lines are no-op when samples is
empty. So it's safe to early return

+      return;
+
     if (ContainsKey(original_samples_, name)) {
-      sample.Subtract(original_samples_[name]);
+      samples->Subtract((*original_samples_[name]));
     }
 
-    EXPECT_EQ(expected_count, sample.TotalCount()) << name;
+    EXPECT_EQ(expected_count, samples->TotalCount()) << name;
     if (expected_count > 0) {
-      EXPECT_EQ(expected_count, sample.counts(expected_value)) << name;
+      EXPECT_EQ(expected_count, samples->GetCount(expected_value)) << name;
     }
   }
 
-  void ExpectRequestHistogramCount(int expected_count, int expected_value,
-                                   int request_id, HistogramField field) {
+  void ExpectRequestHistogramCount(int expected_count,
+                                   int expected_value,
+                                   int request_id,
+                                   HistogramField field) {
     const char* field_str = "";
     switch (field) {
       case FIELD_STATUS:
         field_str = "Status";
         break;
 
       case FIELD_NETWORK_ERROR:
         field_str = "NetworkError";
         break;
 
       case FIELD_RESPONSE_CODE:
         field_str = "ResponseCode";
         break;
 
       default:
         FAIL() << "Unexpected field: " << field;
     }
 
     std::string name = base::StringPrintf("NetConnectivity.Pipeline.%d.%s",
                                           request_id, field_str);
-    base::Histogram::SampleSet sample = GetHistogram(name.c_str());
+    scoped_ptr<HistogramSamples> samples = GetHistogram(name.c_str());
+    if (!samples.get())
+      return;
+
     if (ContainsKey(original_samples_, name)) {
-      sample.Subtract(original_samples_[name]);
+      samples->Subtract(*(original_samples_[name]));
     }
 
-    EXPECT_EQ(expected_count, sample.TotalCount()) << name;
+    EXPECT_EQ(expected_count, samples->TotalCount()) << name;
     if (expected_count > 0) {
-      EXPECT_EQ(expected_count, sample.counts(expected_value)) << name;
+      EXPECT_EQ(expected_count, samples->GetCount(expected_value)) << name;
     }
   }
 
   MessageLoopForIO message_loop_;
   net::TestServer test_server_;
   TestURLRequestContextGetter* context_;
   content::TestBrowserThread io_thread_;
 
  private:
-  base::Histogram::SampleSet GetHistogram(const char* name) {
-    base::Histogram::SampleSet sample;
-    base::Histogram* cached_histogram = NULL;
-    base::Histogram* current_histogram =
+  scoped_ptr<HistogramSamples> GetHistogram(const char* name) {
+    scoped_ptr<HistogramSamples> samples;
+    Histogram* cached_histogram = NULL;
+    Histogram* current_histogram =
         base::StatisticsRecorder::FindHistogram(name);
     if (ContainsKey(histograms_, name)) {
       cached_histogram = histograms_[name];
     }
 
     // This is to work around the CACHE_HISTOGRAM_* macros caching the last used
     // histogram by name. So, even though we throw out the StatisticsRecorder
     // between tests, the CACHE_HISTOGRAM_* might still write into the old
     // Histogram if it has the same name as the last run. We keep a cache of the
     // last used Histogram and then update the cache if it's different than the
     // current Histogram.
     if (cached_histogram && current_histogram) {
-      cached_histogram->SnapshotSample(&sample);
+      samples = cached_histogram->SnapshotSamples();
       if (cached_histogram != current_histogram) {
-        base::Histogram::SampleSet current_sample;
-        current_histogram->SnapshotSample(&current_sample);
-        sample.Add(current_sample);
+        samples->Add(*(current_histogram->SnapshotSamples()));
         histograms_[name] = current_histogram;
       }
     } else if (current_histogram) {
-      current_histogram->SnapshotSample(&sample);
+      samples = current_histogram->SnapshotSamples();
       histograms_[name] = current_histogram;
     } else if (cached_histogram) {
-      cached_histogram->SnapshotSample(&sample);
+      samples = cached_histogram->SnapshotSamples();
     }
-    return sample;
+    return samples.Pass();
   }
 
-  static std::map<std::string, base::Histogram*> histograms_;
-  std::map<std::string, base::Histogram::SampleSet> samples_;
-  std::map<std::string, base::Histogram::SampleSet> original_samples_;
+  static std::map<std::string, Histogram*> histograms_;
+  std::map<std::string, HistogramSamples*> original_samples_;
 };
 
 // static
-std::map<std::string, base::Histogram*>
+std::map<std::string, Histogram*>
     HttpPipeliningCompatibilityClientTest::histograms_;
 
 TEST_F(HttpPipeliningCompatibilityClientTest, Success) {
   RequestInfo info;
   info.filename = "files/alphabet.txt";
   info.expected_response = "abcdefghijklmnopqrstuvwxyz";
   std::vector<RequestInfo> requests;
   requests.push_back(info);
 
   RunTest(requests, HttpPipeliningCompatibilityClient::PIPE_TEST_DEFAULTS);
@@ skipping 311 matching lines @@
   callback.WaitForResult();
 
   ExpectHistogramCount(1, false, FIELD_CANARY);
   ExpectHistogramCount(0, false, FIELD_SUCCESS);
   ExpectHistogramCount(0, true, FIELD_SUCCESS);
 }
 
 }  // anonymous namespace
 
 }  // namespace chrome_browser_net