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

components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.h

Index: components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.h
diff --git a/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.h b/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.h
index 33fd29e560fa2f533fa044f1c3be60a83537c599..10c1fbc956888ee0dafb93866ca2855d65977a98 100644
--- a/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.h
+++ b/components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.h
@@ -17,6 +17,7 @@
 
 namespace net {
 class ProxyServer;
+class HttpResponseHeaders;
 }
 
 namespace data_reduction_proxy {
@@ -33,6 +34,12 @@ class DataReductionProxyUsageStats
       const net::ProxyServer& proxy_server,
       DataReductionProxyBypassType bypass_type);
 
+  // For the given response |headers| that are expected to include the data
+  // reduction proxy via header, records response code UMA if the data reduction
+  // proxy via header is not present.
+  static void DetectAndRecordMissingViaHeaderResponseCode(
+      bool is_primary, const net::HttpResponseHeaders* headers);
+
   // MessageLoopProxy instance is owned by io_thread. |params| outlives
   // this class instance.
   DataReductionProxyUsageStats(DataReductionProxyParams* params,
@@ -55,10 +62,19 @@ class DataReductionProxyUsageStats
   // cause the current bypass.
   void SetBypassType(DataReductionProxyBypassType type);
 
+  // Record all the data reduction proxy bytes-related histograms for the

[bengr, 2014/09/19 21:42:56]

"Records"

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

Done.

+  // completed URLRequest |request|.
+  void RecordBytesHistograms(
+      net::URLRequest& request,
+      const BooleanPrefMember& data_reduction_proxy_enabled,
+      const net::ProxyConfig& data_reduction_proxy_config);

[bengr, 2014/09/19 21:42:56]

Forward declare ProxyConfig

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

Done.

+
   // Given |data_reduction_proxy_enabled|, a |request|, and the
   // |data_reduction_proxy_config| records the number of bypassed bytes for that
   // |request| into UMAs based on bypass type. |data_reduction_proxy_enabled|
   // tells us the state of the kDataReductionProxyEnabled preference.
+  // This method is publicly visible for testing purposes; use the method
+  // RecordBytesHistograms instead of calling this method directly.

[bengr, 2014/09/19 21:42:56]

What test uses it? Can't you friend the test or make this protected?

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

This particular method isn't actually yet used by any tests; I hope that this is
soon rectified and that tests are added for it like the tests for
RecordMissingViaHeaderBytes. The issue here is that we want to test
RecordBypassedBytesHistograms separately from RecordMissingViaHeaderBytes
because it significantly simplifies the tests, and to allow this the tests must
be able to directly call one or the other.

This is a question of coding style; the main options I see here are:
(1) Make RecordBypassedBytes and RecordMissingViaHeaderBytes public like they
are here, so that the tests can call them directly
(2) Make RecordBypassedBytes and RecordMissingViaHeaderBytes private, friend the
test, and add "proxy" methods in the DataReductionProxyUsageStatsTest class that
forward calls and arguments on to the real RecordBypassedBytes and
RecordMissingViaHeaderBytes methods.

I originally chose (1) because it was simpler and easier, and adding "proxy"
methods in the test class that forward arguments as described in (2) seemed ugly
to me. Do you think otherwise or see another option?

   void RecordBypassedBytesHistograms(
       net::URLRequest& request,
       const BooleanPrefMember& data_reduction_proxy_enabled,
@@ -67,6 +83,13 @@ class DataReductionProxyUsageStats
   void RecordBypassEventHistograms(const net::ProxyServer& bypassed_proxy,
                                    int net_error) const;
 
+  // Records UMA of the number of response bytes of responses that are expected
+  // to have the data reduction proxy via header, but where the data reduction
+  // proxy via header is not present.
+  // This method is publicly visible for testing purposes; use the method
+  // RecordBytesHistograms instead of calling this method directly.
+  void RecordMissingViaHeaderBytes(net::URLRequest& request);
+
  private:
   enum BypassedBytesType {
     NOT_BYPASSED = 0,         /* Not bypassed. */