Make Data Reduction Proxy a best effort proxy

components/data_reduction_proxy/core/browser/data_reduction_proxy_usage_stats.cc

Index: components/data_reduction_proxy/core/browser/data_reduction_proxy_usage_stats.cc
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_usage_stats.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_usage_stats.cc
index 907e9cf1259824ceeaef7b7ed8ae1679534a7be7..ddd9b1f56c572b1ee8881e81774aaad269b4990a 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_usage_stats.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_usage_stats.cc
@@ -215,14 +215,15 @@ void DataReductionProxyUsageStats::RecordBypassedBytesHistograms(
     const net::ProxyConfig& data_reduction_proxy_config) {
   int64 content_length = request->received_response_content_length();
 
-  if (data_reduction_proxy_enabled.GetValue() &&
-      !data_reduction_proxy_config.Equals(
-          request->context()->proxy_service()->config())) {
-    RecordBypassedBytes(last_bypass_type_,
-                        DataReductionProxyUsageStats::MANAGED_PROXY_CONFIG,
-                        content_length);
+
+  // Only record histograms when the data reduction proxy is enabled.
+  if (!data_reduction_proxy_enabled.GetValue())
+    return;
+
+  // TODO(bengr): Add histogram(s) for byte counts of unsupported schemes, e.g.,
+  // ws and wss.
+  if (!request->url().SchemeIsHTTPOrHTTPS())
     return;
-  }
 
   DataReductionProxyTypeInfo data_reduction_proxy_type_info;
   if (data_reduction_proxy_params_->WasDataReductionProxyUsed(

[sclittle, 2014/12/30 22:10:41]

Putting NOT_BYPASSED above PROXY_OVERRIDDEN like you did here should fix
https://code.google.com/p/chromium/issues/detail?id=444169.

Maybe add 444169 to BUG= list in CL description?

[bengr, 2014/12/30 23:22:30]

Done.

@@ -241,16 +242,24 @@ void DataReductionProxyUsageStats::RecordBypassedBytesHistograms(
     return;
   }
 
-  if (data_reduction_proxy_enabled.GetValue() &&
-      request->url().SchemeIs(url::kHttpsScheme)) {
+  if (request->url().SchemeIs(url::kHttpsScheme)) {
     RecordBypassedBytes(last_bypass_type_,
                         DataReductionProxyUsageStats::SSL,
                         content_length);
     return;
   }
 
-  if (data_reduction_proxy_enabled.GetValue() &&
-      data_reduction_proxy_params_->IsBypassedByDataReductionProxyLocalRules(
+  // Now that the data reduction proxy is a best effort proxy, if the effective
+  // proxy configuration resolves to anything other than direct:// for a URL,
+  // the data reduction proxy will not be used.
+  if (!request->proxy_server().IsEmpty()) {

[sclittle, 2014/12/30 22:10:41]

Nit: Maybe add a DCHECK or something here to assert that proxy_server() isn't
the DRP?

I know any requests that had the data reduction proxy as proxy_server() would
have been captured above in NOT_BYPASSED, but a DCHECK here would help ensure
that someone moving code around in the future wouldn't accidentally cause
NOT_BYPASSED to be counted as PROXY_OVERRIDDEN. Especially since we don't
currently have test coverage for these BypassedBytes histograms here.

[bengr, 2014/12/30 23:22:30]

Done.

+    RecordBypassedBytes(last_bypass_type_,
+                        DataReductionProxyUsageStats::PROXY_OVERRIDDEN,

[sclittle, 2014/12/30 22:10:41]

Moving PROXY_OVERRIDDEN down below SSL here slightly changes the meaning of
these histograms from before. IIUC, previously if a managed proxy tunneled SSL
traffic, those would be counted as bypassed bytes under ManagedProxyConfig. Now,
those bytes are counted under SSL.

Is this intended?

[bengr, 2014/12/30 23:22:30]

This was intentional. I want to know how much traffic we could have compressed
if there wasn't another proxy in the mix. We don't compress HTTPS, so that
should be removed from consideration. Does that make sense?

[sclittle, 2014/12/30 23:54:26]

OK

+                        content_length);
+    return;
+  }
+
+  if (data_reduction_proxy_params_->IsBypassedByDataReductionProxyLocalRules(
           *request, data_reduction_proxy_config)) {
     RecordBypassedBytes(last_bypass_type_,
                         DataReductionProxyUsageStats::LOCAL_BYPASS_RULES,
@@ -292,8 +301,7 @@ void DataReductionProxyUsageStats::RecordBypassedBytesHistograms(
     return;
   }
 
-  if (data_reduction_proxy_enabled.GetValue() &&
-      data_reduction_proxy_params_->AreDataReductionProxiesBypassed(*request,
+  if (data_reduction_proxy_params_->AreDataReductionProxiesBypassed(*request,
                                                                     NULL)) {
     RecordBypassedBytes(last_bypass_type_,
                         DataReductionProxyUsageStats::NETWORK_ERROR,
@@ -369,9 +377,9 @@ void DataReductionProxyUsageStats::RecordBypassedBytes(
           "DataReductionProxy.BypassedBytes.LocalBypassRules",
           content_length);
       break;
-    case DataReductionProxyUsageStats::MANAGED_PROXY_CONFIG:
+    case DataReductionProxyUsageStats::PROXY_OVERRIDDEN:
       UMA_HISTOGRAM_COUNTS(
-          "DataReductionProxy.BypassedBytes.ManagedProxyConfig",
+          "DataReductionProxy.BypassedBytes.ProxyOverridden",
           content_length);
       break;
     case DataReductionProxyUsageStats::AUDIO_VIDEO: