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

components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.h

 // Copyright 2014 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.
 
 #ifndef COMPONENTS_DATA_REDUCTION_PROXY_BROWSER_DATA_REDUCTION_PROXY_USAGE_STATS_H_
 #define COMPONENTS_DATA_REDUCTION_PROXY_BROWSER_DATA_REDUCTION_PROXY_USAGE_STATS_H_
 
 #include "base/callback.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/prefs/pref_member.h"
 #include "base/threading/thread_checker.h"
 #include "components/data_reduction_proxy/browser/data_reduction_proxy_params.h"
 #include "components/data_reduction_proxy/common/data_reduction_proxy_headers.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/network_change_notifier.h"
 #include "net/url_request/url_request.h"
 
 namespace net {
 class ProxyServer;
+class HttpResponseHeaders;
 }
 
 namespace data_reduction_proxy {
 
 class DataReductionProxyUsageStats
     : public net::NetworkChangeNotifier::NetworkChangeObserver {
  public:
   // Records a data reduction proxy bypass event as a "BlockType" if
   // |bypass_all| is true and as a "BypassType" otherwise. Records the event as
   // "Primary" if |is_primary| is true and "Fallback" otherwise.
   static void RecordDataReductionProxyBypassInfo(
       bool is_primary,
       bool bypass_all,
       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,
                                base::MessageLoopProxy* ui_thread_proxy);
   virtual ~DataReductionProxyUsageStats();
 
   // Sets the callback to be called on the UI thread when the unavailability
   // status has changed.
   void set_unavailable_callback(
       const base::Callback<void(bool)>& unavailable_callback) {
     unavailable_callback_ = unavailable_callback;
   }
 
   // Callback intended to be called from |ChromeNetworkDelegate| when a
   // request completes. This method is used to gather usage stats.
   void OnUrlRequestCompleted(const net::URLRequest* request, bool started);
 
   // Records the last bypass reason to |bypass_type_| and sets
   // |triggering_request_| to true. A triggering request is the first request to
   // 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,
       const net::ProxyConfig& data_reduction_proxy_config);
 
   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. */
     SSL,                      /* Bypass due to SSL. */
     LOCAL_BYPASS_RULES,       /* Bypass due to client-side bypass rules. */
     MANAGED_PROXY_CONFIG,     /* Bypass due to managed config. */
     AUDIO_VIDEO,              /* Audio/Video bypass. */
     TRIGGERING_REQUEST,       /* Triggering request bypass. */
     NETWORK_ERROR,            /* Network error. */
     BYPASSED_BYTES_TYPE_MAX   /* This must always be last.*/
@@ skipping 51 matching lines @@
 
   // Called when the unavailability status has changed. Runs on the UI thread.
   base::Callback<void(bool)> unavailable_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyUsageStats);
 };
 
 }  // namespace data_reduction_proxy
 
 #endif  // COMPONENTS_DATA_REDUCTION_PROXY_BROWSER_DATA_REDUCTION_PROXY_USAGE_STATS_H_