Adds UMA to measure when the data reduction proxy via header is missing

components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats_unittest.cc

Index: components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats_unittest.cc
diff --git a/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats_unittest.cc b/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats_unittest.cc
index e522a95f40892f89b50f81391f6a18320a066225..85d82fce7853ad5974c3d9073400281e9f7aad91 100644
--- a/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats_unittest.cc
+++ b/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats_unittest.cc
@@ -4,11 +4,20 @@
 
 #include "components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.h"
 
+#include <string>
+
 #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 "net/base/request_priority.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_util.h"
 #include "net/url_request/url_request.h"
+#include "net/url_request/url_request_job_factory_impl.h"
 #include "net/url_request/url_request_status.h"
+#include "net/url_request/url_request_test_job.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -39,6 +48,50 @@ class DataReductionProxyParamsMock : public DataReductionProxyParams {
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyParamsMock);
 };
 
+// Transform "normal"-looking headers (\n-separated) to the appropriate
+// input format for ParseRawHeaders (\0-separated).
+void HeadersToRaw(std::string* headers) {

[bengr, 2014/09/18 19:55:57]

I think duplicates of this function live elsewhere in the data reduction proxy
component. would you please coalesce these all into one *test_utils file?

[sclittle, 2014/09/19 01:05:01]

Done.

+  std::replace(headers->begin(), headers->end(), '\n', '\0');
+  if (!headers->empty())
+    *headers += '\0';
+}
+
+scoped_ptr<base::HistogramSamples> GetHistogramSamples(
+    const char* histogram_name) {
+  base::HistogramBase* histogram =
+      base::StatisticsRecorder::FindHistogram(histogram_name);
+
+  EXPECT_NE(static_cast<base::HistogramBase*>(NULL), histogram);
+
+  return histogram->SnapshotSamples().Pass();
+}
+
+scoped_ptr<base::HistogramSamples> GetDeltaHistogramSamples(
+    const base::HistogramSamples& initial_samples, const char* histogram_name) {
+  scoped_ptr<base::HistogramSamples> delta_samples(
+      GetHistogramSamples(histogram_name));
+  delta_samples->Subtract(initial_samples);
+  return delta_samples.Pass();
+}
+
+void ExpectNoNewSamples(const base::HistogramSamples& initial_samples,
+                        const char* histogram_name) {
+  scoped_ptr<base::HistogramSamples> delta_samples(
+      GetDeltaHistogramSamples(initial_samples, histogram_name));
+
+  EXPECT_EQ(0, delta_samples->TotalCount());
+}
+
+void ExpectOneNewSample(const base::HistogramSamples& initial_samples,
+                        const char* histogram_name,
+                        base::HistogramBase::Sample sample) {
+  scoped_ptr<base::HistogramSamples> delta_samples(
+      GetDeltaHistogramSamples(initial_samples, histogram_name));
+

[bengr, 2014/09/18 19:55:57]

Here and above, I don't think the blank lines improve readability. Remove.

[sclittle, 2014/09/19 01:05:01]

Done.

+  EXPECT_EQ(1, delta_samples->TotalCount());
+  EXPECT_EQ(1, delta_samples->GetCount(sample));
+}
+
 }  // namespace
 
 namespace data_reduction_proxy {
@@ -48,8 +101,17 @@ class DataReductionProxyUsageStatsTest : public testing::Test {
   DataReductionProxyUsageStatsTest()
       : loop_proxy_(MessageLoopProxy::current().get()),
         context_(true),
-        unavailable_(false) {
+        unavailable_(false){
+    base::StatisticsRecorder::Initialize();
     context_.Init();
+
+    // The |test_job_factory_| takes ownership of the interceptor.
+    test_job_interceptor_ = new net::TestJobInterceptor();
+    DCHECK(
+        test_job_factory_.SetProtocolHandler(url::kHttpScheme,
+                                             test_job_interceptor_));
+    context_.set_job_factory(&test_job_factory_);
+
     mock_url_request_ = context_.CreateRequest(GURL(), net::IDLE, &delegate_,
                                                NULL);
   }
@@ -58,6 +120,28 @@ class DataReductionProxyUsageStatsTest : public testing::Test {
     unavailable_ = unavailable;
   }
 
+  scoped_ptr<URLRequest> CreateURLRequestWithResponseHeaders(
+      const GURL& url, const std::string& raw_response_headers) {
+    scoped_ptr<URLRequest> fake_request = context_.CreateRequest(url, net::IDLE,
+                                                                 &delegate_,
+                                                                 NULL);
+
+    // Create a test job that will fill in the given response headers for the
+    // |fake_request|.
+    scoped_refptr<net::URLRequestTestJob> test_job(
+        new net::URLRequestTestJob(fake_request.get(),
+                                   context_.network_delegate(),
+                                   raw_response_headers, std::string(), true));
+
+    // Configure the interceptor to use the test job to handle the next request.
+    test_job_interceptor_->set_main_intercept_job(test_job.get());
+    fake_request->Start();
+    MessageLoop::current()->RunUntilIdle();
+
+    DCHECK(fake_request->response_headers() != NULL);
+    return fake_request.Pass();
+  }
+
   // Required for MessageLoopProxy::current().
   base::MessageLoopForUI loop_;
   MessageLoopProxy* loop_proxy_;
@@ -67,6 +151,8 @@ class DataReductionProxyUsageStatsTest : public testing::Test {
   TestDelegate delegate_;
   DataReductionProxyParamsMock mock_params_;
   scoped_ptr<URLRequest> mock_url_request_;
+  net::TestJobInterceptor* test_job_interceptor_;
+  net::URLRequestJobFactoryImpl test_job_factory_;
   bool unavailable_;
 };
 
@@ -117,4 +203,238 @@ TEST_F(DataReductionProxyUsageStatsTest, IsDataReductionProxyUnreachable) {
   }
 }
 
+TEST_F(DataReductionProxyUsageStatsTest,
+       DetectAndRecordMissingViaHeaderResponseCode) {
+  const char kPrimaryHistogramName[] =
+      "DataReductionProxy.MissingViaHeaderResponseCodePrimary";
+  const char kFallbackHistogramName[] =
+      "DataReductionProxy.MissingViaHeaderResponseCodeFallback";
+
+  // Log a sample for each histogram, to ensure that they are both created.
+  UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+      kPrimaryHistogramName, net::HttpUtil::MapStatusCodeForHistogram(200),
+      net::HttpUtil::GetStatusCodesForHistogram());
+  UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+      kFallbackHistogramName, net::HttpUtil::MapStatusCodeForHistogram(200),
+      net::HttpUtil::GetStatusCodesForHistogram());
+
+  struct TestCase {
+    bool is_primary;
+    const char* headers;
+    int expected_primary_sample;   // -1 indicates no expected sample.
+    int expected_fallback_sample;  // -1 indicates no expected sample.
+  };
+  const TestCase test_cases[] = {
+    {
+      true,
+      "HTTP/1.1 200 OK\n"
+      "Via: 1.1 Chrome-Compression-Proxy\n",
+      -1,
+      -1
+    },
+    {
+      false,
+      "HTTP/1.1 200 OK\n"
+      "Via: 1.1 Chrome-Compression-Proxy\n",
+      -1,
+      -1
+    },
+    {
+      true,
+      "HTTP/1.1 200 OK\n",
+      200,
+      -1
+    },
+    {
+      false,
+      "HTTP/1.1 200 OK\n",
+      -1,
+      200
+    },
+    {
+      true,
+      "HTTP/1.1 304 Not Modified\n",
+      304,
+      -1
+    },
+    {
+      false,
+      "HTTP/1.1 304 Not Modified\n",
+      -1,
+      304
+    },
+    {
+      true,
+      "HTTP/1.1 404 Not Found\n",
+      404,
+      -1
+    },
+    {
+      false,
+      "HTTP/1.1 404 Not Found\n",
+      -1,
+      404
+    }
+  };
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
+    scoped_ptr<base::HistogramSamples> initial_primary_samples(
+        GetHistogramSamples(
+            kPrimaryHistogramName));

[bengr, 2014/09/18 19:55:57]

move up a with previous line?

[sclittle, 2014/09/19 01:05:01]

Done.

+    scoped_ptr<base::HistogramSamples> initial_fallback_samples(
+        GetHistogramSamples(
+            kFallbackHistogramName));

[bengr, 2014/09/18 19:55:57]

move up a with previous line?

[sclittle, 2014/09/19 01:05:01]

Done.

+
+    std::string raw_headers(test_cases[i].headers);
+    HeadersToRaw(&raw_headers);
+    scoped_refptr<net::HttpResponseHeaders> headers(
+        new net::HttpResponseHeaders(raw_headers));
+
+    DataReductionProxyUsageStats::DetectAndRecordMissingViaHeaderResponseCode(
+        test_cases[i].is_primary, headers.get());
+
+    if (test_cases[i].expected_primary_sample == -1) {
+      ExpectNoNewSamples(*initial_primary_samples, kPrimaryHistogramName);
+    } else {
+      ExpectOneNewSample(*initial_primary_samples, kPrimaryHistogramName,
+                         test_cases[i].expected_primary_sample);
+    }
+
+    if (test_cases[i].expected_fallback_sample == -1) {
+      ExpectNoNewSamples(*initial_fallback_samples, kFallbackHistogramName);
+    } else {
+      ExpectOneNewSample(*initial_fallback_samples, kFallbackHistogramName,
+                         test_cases[i].expected_fallback_sample);
+    }
+  }
+}
+
+TEST_F(DataReductionProxyUsageStatsTest, RecordMissingViaHeaderBytes) {
+  const char k4xxHistogramName[] =
+      "DataReductionProxy.MissingViaHeader4xxResponseBytes";
+  const char kOtherHistogramName[] =
+      "DataReductionProxy.MissingViaHeaderOtherResponseBytes";
+  const int64 kResponseContentLength = 100;
+
+  // Log a sample for each histogram to ensure that they're created.
+  UMA_HISTOGRAM_COUNTS(k4xxHistogramName, 0);
+  UMA_HISTOGRAM_COUNTS(kOtherHistogramName, 0);
+
+  struct TestCase {
+    bool was_proxy_used;
+    const char* headers;
+    bool is_4xx_sample_expected;
+    bool is_other_sample_expected;
+  };
+  const TestCase test_cases[] = {
+    // Nothing should be recorded for requests that don't use the proxy.
+    {
+      false,
+      "HTTP/1.1 404 Not Found\n",
+      false,
+      false
+    },
+    {
+      false,
+      "HTTP/1.1 200 OK\n",
+      false,
+      false
+    },
+    // Nothing should be recorded for responses that have the via header.
+    {
+      true,
+      "HTTP/1.1 404 Not Found\n"
+      "Via: 1.1 Chrome-Compression-Proxy\n",
+      false,
+      false
+    },
+    {
+      true,
+      "HTTP/1.1 200 OK\n"
+      "Via: 1.1 Chrome-Compression-Proxy\n",
+      false,
+      false
+    },
+    // 4xx responses that used the proxy and don't have the via header should be
+    // recorded.
+    {
+      true,
+      "HTTP/1.1 404 Not Found\n",
+      true,
+      false
+    },
+    {
+      true,
+      "HTTP/1.1 400 Bad Request\n",
+      true,
+      false
+    },
+    {
+      true,
+      "HTTP/1.1 499 Big Client Error Response Code\n",
+      true,
+      false
+    },
+    // Non-4xx responses that used the proxy and don't have the via header
+    // should be recorded.
+    {
+      true,
+      "HTTP/1.1 200 OK\n",
+      false,
+      true
+    },
+    {
+      true,
+      "HTTP/1.1 399 Big Redirection Response Code\n",
+      false,
+      true
+    },
+    {
+      true,
+      "HTTP/1.1 500 Internal Server Error\n",
+      false,
+      true
+    }
+  };
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
+    scoped_ptr<base::HistogramSamples> initial_4xx_samples(
+        GetHistogramSamples(k4xxHistogramName));
+    scoped_ptr<base::HistogramSamples> initial_other_samples(
+        GetHistogramSamples(kOtherHistogramName));
+
+    scoped_ptr<DataReductionProxyUsageStats> usage_stats(
+        new DataReductionProxyUsageStats(&mock_params_, loop_proxy_));
+
+    std::string raw_headers(test_cases[i].headers);
+    HeadersToRaw(&raw_headers);
+
+    scoped_ptr<URLRequest> fake_request(
+        CreateURLRequestWithResponseHeaders(GURL("http://www.google.com/"),
+                                            raw_headers));
+    fake_request->set_received_response_content_length(kResponseContentLength);
+
+    EXPECT_CALL(mock_params_,
+                    WasDataReductionProxyUsed(fake_request.get(), NULL))

[bengr, 2014/09/18 19:55:57]

Indentation seems off.

[sclittle, 2014/09/19 01:05:01]

Done.

+            .WillRepeatedly(Return(test_cases[i].was_proxy_used));
+
+    usage_stats->RecordMissingViaHeaderBytes(fake_request.get());
+
+    if (test_cases[i].is_4xx_sample_expected) {
+      ExpectOneNewSample(*initial_4xx_samples, k4xxHistogramName,
+                         kResponseContentLength);
+    } else {
+      ExpectNoNewSamples(*initial_4xx_samples, k4xxHistogramName);
+    }
+
+    if (test_cases[i].is_other_sample_expected) {
+      ExpectOneNewSample(*initial_other_samples, kOtherHistogramName,
+                         kResponseContentLength);
+    } else {
+      ExpectNoNewSamples(*initial_other_samples, kOtherHistogramName);
+    }
+  }
+}
+

[bengr, 2014/09/18 19:55:57]

Remove extra blank line

[sclittle, 2014/09/19 01:05:01]

Done.

+
 }  // namespace data_reduction_proxy