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

components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.cc

Index: components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.cc
diff --git a/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.cc b/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.cc
index b24094763c503b3a30ef59cf9ebad7ba84a9127a..a4da0881648a2ae78b0e22bfba5256704458e51a 100644
--- a/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.cc
+++ b/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.cc
@@ -11,6 +11,9 @@
 #include "base/metrics/sparse_histogram.h"
 #include "components/data_reduction_proxy/common/data_reduction_proxy_headers.h"
 #include "net/base/net_errors.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_status_code.h"
+#include "net/http/http_util.h"
 #include "net/proxy/proxy_retry_info.h"
 #include "net/proxy/proxy_server.h"
 #include "net/proxy/proxy_service.h"
@@ -72,6 +75,29 @@ void DataReductionProxyUsageStats::RecordDataReductionProxyBypassInfo(
   }
 }
 
+// static
+void DataReductionProxyUsageStats::DetectAndRecordMissingViaHeaderResponseCode(
+      bool is_primary, const net::HttpResponseHeaders* headers) {
+  if (HasDataReductionProxyViaHeader(headers, NULL)) {
+    // The data reduction proxy via header is present, so don't record anything.
+    return;
+  }
+
+  // Record the response code in the same way that net::HttpResponseHeaders
+  // records the response code for the histogram Net.HttpResponseCode.
+  if (is_primary) {
+    UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+        "DataReductionProxy.MissingViaHeaderResponseCodePrimary",
+        net::HttpUtil::MapStatusCodeForHistogram(headers->response_code()),
+        net::HttpUtil::GetStatusCodesForHistogram());
+  } else {
+    UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+        "DataReductionProxy.MissingViaHeaderResponseCodeFallback",
+        net::HttpUtil::MapStatusCodeForHistogram(headers->response_code()),
+        net::HttpUtil::GetStatusCodesForHistogram());
+  }
+}
+
 DataReductionProxyUsageStats::DataReductionProxyUsageStats(
     DataReductionProxyParams* params,
     MessageLoopProxy* ui_thread_proxy)
@@ -159,6 +185,15 @@ void DataReductionProxyUsageStats::SetBypassType(
   triggering_request_ = true;
 }
 
+void DataReductionProxyUsageStats::RecordBytesHistograms(
+    net::URLRequest& request,
+    const BooleanPrefMember& data_reduction_proxy_enabled,

[bengr, 2014/09/22 19:36:16]

Forward declare?

[sclittle, 2014/09/22 20:54:25]

BooleanPrefMember is a typedef, so we can't forward declare it.

[bengr, 2014/09/22 22:01:26]

Can you include it then?

[sclittle, 2014/09/22 22:38:19]

Done.

+    const net::ProxyConfig& data_reduction_proxy_config) {
+  RecordBypassedBytesHistograms(request, data_reduction_proxy_enabled,
+                                data_reduction_proxy_config);
+  RecordMissingViaHeaderBytes(request);
+}
+
 void DataReductionProxyUsageStats::RecordBypassedBytesHistograms(
     net::URLRequest& request,
     const BooleanPrefMember& data_reduction_proxy_enabled,
@@ -373,6 +408,33 @@ void DataReductionProxyUsageStats::RecordBypassedBytes(
   }
 }
 
+void DataReductionProxyUsageStats::RecordMissingViaHeaderBytes(
+    URLRequest& request) {

[bengr, 2014/09/22 19:36:16]

Can these methods all be const?

[sclittle, 2014/09/22 20:54:25]

Unfortunately, no. It calls request->received_response_content_length(), which
is a non-const method for some reason.

[bengr, 2014/09/22 22:01:26]

Would you mind filing a bug if you believe received_response_content_length()
should be const?

[sclittle, 2014/09/22 22:38:19]

Sure.

+  // Responses that were served from cache should have been filtered out
+  // already.
+  DCHECK(!request.was_cached());
+
+  if (!data_reduction_proxy_params_->WasDataReductionProxyUsed(&request,

[bengr, 2014/09/22 19:36:16]

nit: I'd move &request, down to the next line with "NULL" and indent 4 from
!data_reduction...

[sclittle, 2014/09/22 20:54:25]

Since the "&" is gone now that |request| is a pointer, this all fits on one line
now.

+                                                               NULL) ||
+      HasDataReductionProxyViaHeader(request.response_headers(), NULL)) {
+    // Only track requests that used the data reduction proxy and had responses
+    // that were missing the data reduction proxy via header.
+    return;
+  }
+
+  if (request.GetResponseCode() >= net::HTTP_BAD_REQUEST &&
+      request.GetResponseCode() < net::HTTP_INTERNAL_SERVER_ERROR) {
+    // Track 4xx responses that are missing via headers separately.
+    // separately.
+    UMA_HISTOGRAM_COUNTS("DataReductionProxy.MissingViaHeader4xxResponseBytes",
+                         request.received_response_content_length());
+  } else {
+    UMA_HISTOGRAM_COUNTS(
+        "DataReductionProxy.MissingViaHeaderOtherResponseBytes",
+        request.received_response_content_length());
+  }
+}
+
 }  // namespace data_reduction_proxy