Call OnResolveProxy in DataReductionProxyDelegate tests

This change simplifies setup in these tests as we already create a
DataReductionProxyDelegate instance with the appropriate configuration.

Bug: 721403
Change-Id: Iae6ae1364872c521f8da1b6ed5273b0af154ab25
Reviewed-on: https://chromium-review.googlesource.com/823179Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523642}

components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.cc

@@ -29,9 +29,57 @@
 namespace data_reduction_proxy {
 
 namespace {
+
 static const char kDataReductionCoreProxy[] = "proxy.googlezip.net";
+
+// Adds data reduction proxies to |result|, where applicable, if result
+// otherwise uses a direct connection for |url|, and the data reduction proxy is
+// not bypassed. Also, configures |result| to proceed directly to the origin if
+// |result|'s current proxy is the data reduction proxy.
+void OnResolveProxyHandler(
+    const GURL& url,
+    const std::string& method,
+    const net::ProxyConfig& proxy_config,
+    const net::ProxyRetryInfoMap& proxy_retry_info,
+    const DataReductionProxyConfig& data_reduction_proxy_config,
+    DataReductionProxyIOData* io_data,
+    net::ProxyInfo* result) {
+  DCHECK(result->is_empty() || result->is_direct() ||
+         !data_reduction_proxy_config.IsDataReductionProxy(
+             result->proxy_server(), nullptr));
+
+  if (!util::EligibleForDataReductionProxy(*result, url, method))
+    return;
+
+  net::ProxyInfo data_reduction_proxy_info;
+  bool data_saver_proxy_used = util::ApplyProxyConfigToProxyInfo(
+      proxy_config, proxy_retry_info, url, &data_reduction_proxy_info);
+  if (data_saver_proxy_used)
+    result->OverrideProxyList(data_reduction_proxy_info.proxy_list());
+
+  if (io_data && io_data->resource_type_provider()) {
+    ResourceTypeProvider::ContentType content_type =
+        io_data->resource_type_provider()->GetContentType(url);
+    DCHECK_GT(ResourceTypeProvider::CONTENT_TYPE_MAX, content_type);
+    UMA_HISTOGRAM_ENUMERATION("DataReductionProxy.ResourceContentType",
+                              content_type,
+                              ResourceTypeProvider::CONTENT_TYPE_MAX);
+  }
+
+  // The |proxy_config| must be valid otherwise the proxy cannot be used.
+  DCHECK(proxy_config.is_valid() || !data_saver_proxy_used);
+
+  if (data_reduction_proxy_config.enabled_by_user_and_reachable() &&
+      url.SchemeIs(url::kHttpScheme) && !net::IsLocalhost(url.host_piece()) &&
+      !params::IsIncludedInHoldbackFieldTrial()) {
+    UMA_HISTOGRAM_BOOLEAN(
+        "DataReductionProxy.ConfigService.HTTPRequests",
+        !data_reduction_proxy_config.GetProxiesForHttp().empty());
+  }
 }
 
+}  // namespace
+
 DataReductionProxyDelegate::DataReductionProxyDelegate(
     DataReductionProxyConfig* config,
     const DataReductionProxyConfigurator* configurator,
@@ -238,46 +286,4 @@ void DataReductionProxyDelegate::OnIPAddressChanged() {
   last_network_change_time_ = tick_clock_->NowTicks();
 }
 
-void OnResolveProxyHandler(
-    const GURL& url,
-    const std::string& method,
-    const net::ProxyConfig& proxy_config,
-    const net::ProxyRetryInfoMap& proxy_retry_info,
-    const DataReductionProxyConfig& data_reduction_proxy_config,
-    DataReductionProxyIOData* io_data,
-    net::ProxyInfo* result) {
-  DCHECK(result->is_empty() || result->is_direct() ||
-         !data_reduction_proxy_config.IsDataReductionProxy(
-             result->proxy_server(), nullptr));
-
-  if (!util::EligibleForDataReductionProxy(*result, url, method))
-    return;
-
-  net::ProxyInfo data_reduction_proxy_info;
-  bool data_saver_proxy_used = util::ApplyProxyConfigToProxyInfo(
-      proxy_config, proxy_retry_info, url, &data_reduction_proxy_info);
-  if (data_saver_proxy_used)
-    result->OverrideProxyList(data_reduction_proxy_info.proxy_list());
-
-  if (io_data && io_data->resource_type_provider()) {
-    ResourceTypeProvider::ContentType content_type =
-        io_data->resource_type_provider()->GetContentType(url);
-    DCHECK_GT(ResourceTypeProvider::CONTENT_TYPE_MAX, content_type);
-    UMA_HISTOGRAM_ENUMERATION("DataReductionProxy.ResourceContentType",
-                              content_type,
-                              ResourceTypeProvider::CONTENT_TYPE_MAX);
-  }
-
-  // The |proxy_config| must be valid otherwise the proxy cannot be used.
-  DCHECK(proxy_config.is_valid() || !data_saver_proxy_used);
-
-  if (data_reduction_proxy_config.enabled_by_user_and_reachable() &&
-      url.SchemeIs(url::kHttpScheme) && !net::IsLocalhost(url.host_piece()) &&
-      !params::IsIncludedInHoldbackFieldTrial()) {
-    UMA_HISTOGRAM_BOOLEAN(
-        "DataReductionProxy.ConfigService.HTTPRequests",
-        !data_reduction_proxy_config.GetProxiesForHttp().empty());
-  }
-}
-
 }  // namespace data_reduction_proxy

components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.h

@@ -23,7 +23,6 @@ class HostPortPair;
 class HttpRequestHeaders;
 class HttpResponseHeaders;
 class NetLog;
-class ProxyConfig;
 class ProxyInfo;
 class ProxyServer;
 }
@@ -132,19 +131,6 @@ class DataReductionProxyDelegate
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyDelegate);
 };
 
-// Adds data reduction proxies to |result|, where applicable, if result
-// otherwise uses a direct connection for |url|, and the data reduction proxy is
-// not bypassed. Also, configures |result| to proceed directly to the origin if
-// |result|'s current proxy is the data reduction proxy
-// This is visible for test purposes.
-void OnResolveProxyHandler(
-    const GURL& url,
-    const std::string& method,
-    const net::ProxyConfig& proxy_config,
-    const net::ProxyRetryInfoMap& proxy_retry_info,
-    const DataReductionProxyConfig& data_reduction_proxy_config,
-    DataReductionProxyIOData* io_data,
-    net::ProxyInfo* result);
 }  // namespace data_reduction_proxy
 
 #endif  // COMPONENTS_DATA_REDUCTION_PROXY_CORE_BROWSER_DATA_REDUCTION_PROXY_DELEGATE_H_

components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc

@@ -583,29 +583,9 @@ class DataReductionProxyDelegateTest : public testing::Test {
   std::unique_ptr<DataReductionProxyTestContext> test_context_;
 };
 
-TEST_F(DataReductionProxyDelegateTest, OnResolveProxyHandler) {
+TEST_F(DataReductionProxyDelegateTest, OnResolveProxy) {
   GURL url("http://www.google.com/");
   params()->UseNonSecureProxiesForHttp();
-  net::ProxyList proxy_list;
-  proxy_list.AddProxyServer(
-      params()->proxies_for_http().front().proxy_server());
-  proxy_list.AddProxyServer(net::ProxyServer::Direct());
-  net::ProxyInfo data_reduction_proxy_info;
-  data_reduction_proxy_info.UseProxyList(proxy_list);
-  EXPECT_FALSE(data_reduction_proxy_info.is_empty());
-
-  // Data reduction proxy config
-  net::ProxyConfig data_reduction_proxy_config;
-  data_reduction_proxy_config.proxy_rules().ParseFromString(
-      "http=" +
-      params()
-          ->proxies_for_http()
-          .front()
-          .proxy_server()
-          .host_port_pair()
-          .ToString() +
-      ",direct://;");
-  data_reduction_proxy_config.set_id(1);
 
   // Other proxy info
   net::ProxyInfo other_proxy_info;
@@ -626,14 +606,14 @@ TEST_F(DataReductionProxyDelegateTest, OnResolveProxyHandler) {
   retry_info.current_delay = base::TimeDelta::FromSeconds(1000);
   retry_info.bad_until = base::TimeTicks().Now() + retry_info.current_delay;
   retry_info.try_while_bad = false;
-  data_reduction_proxy_retry_info[data_reduction_proxy_info.proxy_server()
-                                      .ToURI()] = retry_info;
+  data_reduction_proxy_retry_info
+      [params()->proxies_for_http().front().proxy_server().ToURI()] =
+          retry_info;
 
   net::ProxyInfo result;
   // Another proxy is used. It should be used afterwards.
   result.Use(other_proxy_info);
-  OnResolveProxyHandler(url, "GET", data_reduction_proxy_config,
-                        empty_proxy_retry_info, *config(), nullptr, &result);
+  proxy_delegate()->OnResolveProxy(url, "GET", empty_proxy_retry_info, &result);
   EXPECT_EQ(other_proxy_info.proxy_server(), result.proxy_server());
 
   // A direct connection is used. The data reduction proxy should be used
@@ -641,9 +621,9 @@ TEST_F(DataReductionProxyDelegateTest, OnResolveProxyHandler) {
   // Another proxy is used. It should be used afterwards.
   result.Use(direct_proxy_info);
   net::ProxyConfig::ID prev_id = result.config_id();
-  OnResolveProxyHandler(url, "GET", data_reduction_proxy_config,
-                        empty_proxy_retry_info, *config(), nullptr, &result);
-  EXPECT_EQ(data_reduction_proxy_info.proxy_server(), result.proxy_server());
+  proxy_delegate()->OnResolveProxy(url, "GET", empty_proxy_retry_info, &result);
+  EXPECT_EQ(params()->proxies_for_http().front().proxy_server(),
+            result.proxy_server());
   // Only the proxy list should be updated, not the proxy info.
   EXPECT_EQ(result.config_id(), prev_id);
 
@@ -651,41 +631,36 @@ TEST_F(DataReductionProxyDelegateTest, OnResolveProxyHandler) {
   // list. A direct connection should be used afterwards.
   result.Use(direct_proxy_info);
   prev_id = result.config_id();
-  OnResolveProxyHandler(
-      GURL("ws://echo.websocket.org/"), "GET", data_reduction_proxy_config,
-      data_reduction_proxy_retry_info, *config(), nullptr, &result);
+  proxy_delegate()->OnResolveProxy(GURL("ws://echo.websocket.org/"), "GET",
+                                   data_reduction_proxy_retry_info, &result);
   EXPECT_TRUE(result.proxy_server().is_direct());
   EXPECT_EQ(result.config_id(), prev_id);
 
   // Test that ws:// and wss:// URLs bypass the data reduction proxy.
   result.UseDirect();
-  OnResolveProxyHandler(GURL("wss://echo.websocket.org/"), "GET",
-                        data_reduction_proxy_config, empty_proxy_retry_info,
-                        *config(), nullptr, &result);
+  proxy_delegate()->OnResolveProxy(GURL("wss://echo.websocket.org/"), "GET",
+                                   empty_proxy_retry_info, &result);
   EXPECT_TRUE(result.is_direct());
 
   result.UseDirect();
-  OnResolveProxyHandler(GURL("wss://echo.websocket.org/"), "GET",
-                        data_reduction_proxy_config, empty_proxy_retry_info,
-                        *config(), nullptr, &result);
+  proxy_delegate()->OnResolveProxy(GURL("wss://echo.websocket.org/"), "GET",
+                                   empty_proxy_retry_info, &result);
   EXPECT_TRUE(result.is_direct());
 
   // POST methods go direct.
   result.UseDirect();
-  OnResolveProxyHandler(url, "POST", data_reduction_proxy_config,
-                        empty_proxy_retry_info, *config(), nullptr, &result);
+  proxy_delegate()->OnResolveProxy(url, "POST", empty_proxy_retry_info,
+                                   &result);
   EXPECT_TRUE(result.is_direct());
 
   // Without DataCompressionProxyCriticalBypass Finch trial set, the
   // BYPASS_DATA_REDUCTION_PROXY load flag should be ignored.
   result.UseDirect();
-  OnResolveProxyHandler(url, "GET", data_reduction_proxy_config,
-                        empty_proxy_retry_info, *config(), nullptr, &result);
+  proxy_delegate()->OnResolveProxy(url, "GET", empty_proxy_retry_info, &result);
   EXPECT_FALSE(result.is_direct());
 
-  OnResolveProxyHandler(url, "GET", data_reduction_proxy_config,
-                        empty_proxy_retry_info, *config(), nullptr,
-                        &other_proxy_info);
+  proxy_delegate()->OnResolveProxy(url, "GET", empty_proxy_retry_info,
+                                   &other_proxy_info);
   EXPECT_FALSE(other_proxy_info.is_direct());
 }
 
@@ -695,69 +670,34 @@ TEST_F(DataReductionProxyDelegateTest, HTTPRequests) {
   const struct {
     const char* url;
     bool enabled_by_user;
-    bool use_direct_proxy;
     bool expect_histogram;
   } test_cases[] = {
       {
           // Request should not be logged because data saver is disabled.
-          "http://www.example.com/", false, true, false,
+          "http://www.example.com/", false, false,
       },
       {
-          "http://www.example.com/", true, true, true,
-      },
-      {
-          "http://www.example.com/", true, false, true,
-      },
-      {
-          "https://www.example.com/", false, true, false,
+          "http://www.example.com/", true, true,
       },
       {
           // Request should not be logged because request is HTTPS.
-          "https://www.example.com/", true, true, false,
+          "https://www.example.com/", true, false,
       },
       {
           // Request to localhost should not be logged.
-          "http://127.0.0.1/", true, true, false,
+          "http://127.0.0.1/", true, false,
       },
       {
           // Special use IPv4 address for testing purposes (RFC 5735).
-          "http://198.51.100.1/", true, true, true,
+          "http://198.51.100.1/", true, true,
       },
   };
 
   for (const auto& test : test_cases) {
-    ASSERT_TRUE(test.use_direct_proxy || test.enabled_by_user);
     ASSERT_TRUE(test.enabled_by_user || !test.expect_histogram);
     base::HistogramTester histogram_tester;
     GURL url(test.url);
 
-    net::ProxyInfo data_reduction_proxy_info;
-    if (!test.use_direct_proxy) {
-      net::ProxyList proxy_list;
-      proxy_list.AddProxyServer(
-          params()->proxies_for_http().front().proxy_server());
-      proxy_list.AddProxyServer(net::ProxyServer::Direct());
-      data_reduction_proxy_info.UseProxyList(proxy_list);
-    }
-    EXPECT_EQ(test.use_direct_proxy, data_reduction_proxy_info.is_empty());
-
-    net::ProxyConfig data_reduction_proxy_config;
-    if (test.use_direct_proxy) {
-      data_reduction_proxy_config = net::ProxyConfig::CreateDirect();
-
-    } else {
-      data_reduction_proxy_config.proxy_rules().ParseFromString(
-          "http=" +
-          params()
-              ->proxies_for_http()
-              .front()
-              .proxy_server()
-              .host_port_pair()
-              .ToString() +
-          ",direct://;");
-      data_reduction_proxy_config.set_id(1);
-    }
-    EXPECT_NE(test.use_direct_proxy, data_reduction_proxy_config.is_valid());
     config()->UpdateConfigForTesting(test.enabled_by_user /* enabled */,
                                      false /* secure_proxies_allowed */,
                                      true /* insecure_proxies_allowed */);
@@ -770,8 +710,8 @@ TEST_F(DataReductionProxyDelegateTest, HTTPRequests) {
 
     net::ProxyInfo result;
     result.Use(direct_proxy_info);
-    OnResolveProxyHandler(url, "GET", data_reduction_proxy_config,
-                          empty_proxy_retry_info, *config(), nullptr, &result);
+    proxy_delegate()->OnResolveProxy(url, "GET", empty_proxy_retry_info,
+                                     &result);
     histogram_tester.ExpectTotalCount(
         "DataReductionProxy.ConfigService.HTTPRequests",
         test.expect_histogram ? 1 : 0);