Change Net.DCL_DRPEnabled_Unknown histogram to not count bypassed bytes.

Previously, the Net.DailyContentLength_DataReductionProxyEnabled_Unknown
histogram counted bytes bypassed due to "Chrome-Proxy: block-once",
bytes bypassed due to local bypass rules (e.g. http://localhost), and
bytes bypassed due to another proxy overriding the DRP.

This change reclassifies these three kinds of bypassed bytes into
Net.DailyContentLength_DataReductionProxyEnabled_ShortBypass instead,
since each of these kinds of bypasses last 0 seconds (i.e. affects only
1 request).

BUG=454972

Review URL: https://codereview.chromium.org/917813003

Cr-Commit-Position: refs/heads/master@{#315877}

2 files changed, 174 insertions, 5 deletions

diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics.cc
index e14c0aa..c6b1234 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics.cc
@@ -11,12 +11,16 @@
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_statistics_prefs.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h"
+#include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.h"
 #include "net/base/host_port_pair.h"
+#include "net/base/load_flags.h"
 #include "net/http/http_response_headers.h"
 #include "net/proxy/proxy_config.h"
 #include "net/proxy/proxy_retry_info.h"
+#include "net/proxy/proxy_server.h"
 #include "net/proxy/proxy_service.h"
+#include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 #include "url/url_constants.h"
 
@@ -302,6 +306,16 @@ DataReductionProxyRequestType GetDataReductionProxyRequestType(
     NOTREACHED();
     return UNKNOWN_TYPE;
   }
+
+  // Check for a Data Reduction Proxy via header before checking if proxies are
+  // bypassed, to avoid misreporting cases where the Data Reduction Proxy was
+  // bypassed between the request being sent out and the response coming in.
+  if (request.response_info().headers.get() &&
+      HasDataReductionProxyViaHeader(request.response_info().headers.get(),
+                                     NULL)) {
+    return VIA_DATA_REDUCTION_PROXY;
+  }
+
   base::TimeDelta bypass_delay;
   if (params.AreDataReductionProxiesBypassed(
           request, data_reduction_proxy_config, &bypass_delay)) {
@@ -309,11 +323,19 @@ DataReductionProxyRequestType GetDataReductionProxyRequestType(
       return LONG_BYPASS;
     return SHORT_BYPASS;
   }
-  if (request.response_info().headers.get() &&
-      HasDataReductionProxyViaHeader(request.response_info().headers.get(),
-                                     NULL)) {
-    return VIA_DATA_REDUCTION_PROXY;
+
+  // Treat bypasses that only apply to the individual request as SHORT_BYPASS.
+  // This includes bypasses triggered by "Chrome-Proxy: block-once", bypasses
+  // due to other proxies overriding the Data Reduction Proxy, and bypasses due
+  // to local bypass rules.
+  if ((request.load_flags() & net::LOAD_BYPASS_PROXY) ||
+      (!request.proxy_server().IsEmpty() &&
+       !params.IsDataReductionProxy(request.proxy_server(), NULL)) ||
+      params.IsBypassedByDataReductionProxyLocalRules(
+          request, data_reduction_proxy_config)) {
+    return SHORT_BYPASS;
   }
+
   return UNKNOWN_TYPE;
 }
 
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics_unittest.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics_unittest.cc
index 4584d6c6..925149e 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics_unittest.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics_unittest.cc
@@ -2,21 +2,33 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics.h"
+
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/testing_pref_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/test/test_simple_task_runner.h"
 #include "base/time/time.h"
-#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_statistics_prefs.h"
+#include "components/data_reduction_proxy/core/common/data_reduction_proxy_params_test_utils.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_pref_names.h"
+#include "net/base/load_flags.h"
+#include "net/base/net_log.h"
+#include "net/proxy/proxy_server.h"
+#include "net/proxy/proxy_service.h"
+#include "net/socket/socket_test_util.h"
+#include "net/url_request/url_request.h"
+#include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using net::MockRead;
+
 namespace {
 
 const size_t kNumDaysInHistory = 60;
@@ -659,4 +671,139 @@ TEST_F(ChromeNetworkDailyDataSavingMetricsTest, BackwardTwoDays) {
       original, 1, received, 1);
 }
 
+TEST_F(ChromeNetworkDailyDataSavingMetricsTest,
+       GetDataReductionProxyRequestType) {
+  base::MessageLoopForIO loop;
+  const TestDataReductionProxyParams kParams(
+      DataReductionProxyParams::kAllowed,
+      TestDataReductionProxyParams::HAS_ORIGIN);
+
+  net::ProxyConfig data_reduction_proxy_config;
+  data_reduction_proxy_config.proxy_rules().ParseFromString(
+      "http=" + kParams.origin().host_port_pair().ToString() + ",direct://");
+  data_reduction_proxy_config.proxy_rules().bypass_rules.ParseFromString(
+      "localbypass.com");
+
+  struct TestCase {
+    GURL url;
+    net::ProxyServer proxy_server;
+    base::TimeDelta bypass_duration;  // 0 indicates not bypassed.
+    int load_flags;
+    const char* response_headers;
+    DataReductionProxyRequestType expected_request_type;
+  };
+  const TestCase test_cases[] = {
+    { GURL("http://foo.com"),
+      kParams.origin(),
+      base::TimeDelta(),
+      net::LOAD_NORMAL,
+      "HTTP/1.1 200 OK\r\nVia: 1.1 Chrome-Compression-Proxy\r\n\r\n",
+      VIA_DATA_REDUCTION_PROXY,
+    },
+    { GURL("https://foo.com"),
+      net::ProxyServer::Direct(),
+      base::TimeDelta(),
+      net::LOAD_NORMAL,
+      "HTTP/1.1 200 OK\r\n\r\n",
+      HTTPS,
+    },
+    { GURL("http://foo.com"),
+      net::ProxyServer::Direct(),
+      base::TimeDelta::FromSeconds(1),
+      net::LOAD_NORMAL,
+      "HTTP/1.1 200 OK\r\n\r\n",
+      SHORT_BYPASS,
+    },
+    { GURL("http://foo.com"),
+      net::ProxyServer::Direct(),
+      base::TimeDelta::FromMinutes(60),
+      net::LOAD_NORMAL,
+      "HTTP/1.1 200 OK\r\n\r\n",
+      LONG_BYPASS,
+    },
+    // Requests with LOAD_BYPASS_PROXY (e.g. block-once) should be classified as
+    // SHORT_BYPASS.
+    { GURL("http://foo.com"),
+      net::ProxyServer::Direct(),
+      base::TimeDelta(),
+      net::LOAD_BYPASS_PROXY,
+      "HTTP/1.1 200 OK\r\n\r\n",
+      SHORT_BYPASS,
+    },
+    // Another proxy overriding the Data Reduction Proxy should be classified as
+    // SHORT_BYPASS.
+    { GURL("http://foo.com"),
+      net::ProxyServer::FromPacString("PROXY otherproxy.net:80"),
+      base::TimeDelta(),
+      net::LOAD_NORMAL,
+      "HTTP/1.1 200 OK\r\n\r\n",
+      SHORT_BYPASS,
+    },
+    // Bypasses due to local bypass rules should be classified as SHORT_BYPASS.
+    { GURL("http://localbypass.com"),
+      net::ProxyServer::Direct(),
+      base::TimeDelta(),
+      net::LOAD_NORMAL,
+      "HTTP/1.1 200 OK\r\n\r\n",
+      SHORT_BYPASS,
+    },
+    // Responses that seem like they should have come through the Data Reduction
+    // Proxy, but did not, should be classified as UNKNOWN_TYPE.
+    { GURL("http://foo.com"),
+      net::ProxyServer::Direct(),
+      base::TimeDelta(),
+      net::LOAD_NORMAL,
+      "HTTP/1.1 200 OK\r\n\r\n",
+      UNKNOWN_TYPE,
+    },
+  };
+
+  for (const TestCase& test_case : test_cases) {
+    net::TestURLRequestContext context(true);
+    net::MockClientSocketFactory mock_socket_factory;
+    context.set_client_socket_factory(&mock_socket_factory);
+    // Set the |proxy_service| to use |test_case.proxy_server| for requests.
+    scoped_ptr<net::ProxyService> proxy_service(
+        net::ProxyService::CreateFixedFromPacResult(
+            test_case.proxy_server.ToPacString()));
+    context.set_proxy_service(proxy_service.get());
+    context.Init();
+
+    // Create a fake URLRequest and fill it with the appropriate response
+    // headers and proxy server by executing it with fake socket data.
+    net::SSLSocketDataProvider ssl_socket_data_provider(net::ASYNC, net::OK);
+    if (test_case.url.SchemeIsSecure())
+      mock_socket_factory.AddSSLSocketDataProvider(&ssl_socket_data_provider);
+    MockRead mock_reads[] = {
+        MockRead(test_case.response_headers),
+        MockRead("hello world"),
+        MockRead(net::SYNCHRONOUS, net::OK),
+    };
+    net::StaticSocketDataProvider socket_data_provider(
+        mock_reads, arraysize(mock_reads), nullptr, 0);
+    mock_socket_factory.AddSocketDataProvider(&socket_data_provider);
+
+    net::TestDelegate delegate;
+    scoped_ptr<net::URLRequest> request =
+        context.CreateRequest(test_case.url, net::IDLE, &delegate, nullptr);
+    request->SetLoadFlags(test_case.load_flags);
+    request->Start();
+    base::MessageLoop::current()->RunUntilIdle();
+
+    // Mark the Data Reduction Proxy as bad if the test specifies to.
+    if (test_case.bypass_duration > base::TimeDelta()) {
+      net::ProxyInfo proxy_info;
+      proxy_info.UseProxyList(
+          data_reduction_proxy_config.proxy_rules().proxies_for_http);
+      EXPECT_TRUE(context.proxy_service()->MarkProxiesAsBadUntil(
+          proxy_info, test_case.bypass_duration, net::ProxyServer(),
+          net::BoundNetLog::Make(context.net_log(), net::NetLog::SOURCE_NONE)));
+    }
+
+    EXPECT_EQ(test_case.expected_request_type,
+              GetDataReductionProxyRequestType(
+                  *request, data_reduction_proxy_config, kParams));
+  }
+}
+
 }  // namespace data_reduction_proxy