Check effective proxy config before adding data reduction proxies

Before this change, the data reduction proxies were added to the
list of acceptable proxies for a URL, if the effective proxy
configuration resolved to direct for it. This change further checks
that the effective configuration is not the data reduction proxy's.

BUG=425826

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

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

chrome/browser/net/chrome_network_delegate.cc

@@ -441,6 +441,7 @@ void ChromeNetworkDelegate::OnResolveProxy(
       !proxy_config_getter_.is_null()) {
     on_resolve_proxy_handler_.Run(url, load_flags,
                                   proxy_config_getter_.Run(),
+                                  proxy_service.config(),
                                   proxy_service.proxy_retry_info(),
                                   data_reduction_proxy_params_, result);
   }

chrome/browser/net/chrome_network_delegate.h

@@ -72,12 +72,15 @@ class PrerenderTracker;
 class ChromeNetworkDelegate : public net::NetworkDelegate {
  public:
   // Provides an opportunity to interpose on proxy resolution. Called before
-  // ProxyService.ResolveProxy() returns. |proxy_info| contains information
-  // about the proxy being used, and may be modified by this callback.
+  // ProxyService.ResolveProxy() returns. Two proxy configurations are provided
+  // that specify the data reduction proxy's configuration and the effective
+  // configuration according to the proxy service, respectively. Retry info is
+  // presumed to be from the proxy service.
   typedef base::Callback<void(
       const GURL& url,
       int load_flags,
       const net::ProxyConfig& data_reduction_proxy_config,
+      const net::ProxyConfig& proxy_service_proxy_config,
       const net::ProxyRetryInfoMap& proxy_retry_info_map,
       const data_reduction_proxy::DataReductionProxyParams* params,
       net::ProxyInfo* result)> OnResolveProxyHandler;

components/data_reduction_proxy/core/browser/data_reduction_proxy_protocol.cc

@@ -140,11 +140,13 @@ bool MaybeBypassProxyAndPrepareToRetry(
 void OnResolveProxyHandler(const GURL& url,
                            int load_flags,
                            const net::ProxyConfig& data_reduction_proxy_config,
+                           const net::ProxyConfig& proxy_service_proxy_config,
                            const net::ProxyRetryInfoMap& proxy_retry_info,
                            const DataReductionProxyParams* params,
                            net::ProxyInfo* result) {
   if (data_reduction_proxy_config.is_valid() &&
-      result->proxy_server().is_direct()) {
+      result->proxy_server().is_direct() &&
+      !data_reduction_proxy_config.Equals(proxy_service_proxy_config)) {
     net::ProxyInfo data_reduction_proxy_info;
     data_reduction_proxy_config.proxy_rules().Apply(
         url, &data_reduction_proxy_info);

components/data_reduction_proxy/core/browser/data_reduction_proxy_protocol.h

@@ -38,15 +38,17 @@ bool MaybeBypassProxyAndPrepareToRetry(
     scoped_refptr<net::HttpResponseHeaders>* override_response_headers,
     DataReductionProxyBypassType* proxy_bypass_type);
 
-// Configure |result| to proceed directly to the origin if |result|'s current
-// proxy is the data reduction proxy, the
-// |net::LOAD_BYPASS_DATA_REDUCTION_PROXY| |load_flag| is set, and the
-// DataCompressionProxyCriticalBypass Finch trial is set.
-// This handler is intended to be invoked only by
-// |ChromeNetworkDelegate.NotifyResolveProxy|.
+// Adds data reduction proxies to |result|, where applicable, if result
+// otherwise uses a direct connection for |url|, the proxy service's effective
+// proxy configuration is not the data reduction proxy configuration, 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, the |net::LOAD_BYPASS_DATA_REDUCTION_PROXY| |load_flag| is
+// set, and the DataCompressionProxyCriticalBypass Finch trial is set.
 void OnResolveProxyHandler(const GURL& url,
                            int load_flags,
                            const net::ProxyConfig& data_reduction_proxy_config,
+                           const net::ProxyConfig& proxy_service_proxy_config,
                            const net::ProxyRetryInfoMap& proxy_retry_info,
                            const DataReductionProxyParams* params,
                            net::ProxyInfo* result);

components/data_reduction_proxy/core/browser/data_reduction_proxy_protocol_unittest.cc

@@ -884,6 +884,7 @@ class BadEntropyProvider : public base::FieldTrial::EntropyProvider {
 TEST_F(DataReductionProxyProtocolTest, OnResolveProxyHandler) {
   int load_flags = net::LOAD_NORMAL;
   GURL url("http://www.google.com/");
+  net::ProxyConfig proxy_config_direct = net::ProxyConfig::CreateDirect();
 
   TestDataReductionProxyParams test_params(
             DataReductionProxyParams::kAllowed |
@@ -933,13 +934,15 @@ TEST_F(DataReductionProxyProtocolTest, OnResolveProxyHandler) {
    // The data reduction proxy is used. It should be used afterwards.
    result.Use(data_reduction_proxy_info);
    OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                         empty_proxy_retry_info, &test_params, &result);
+                         proxy_config_direct, empty_proxy_retry_info,
+                         &test_params, &result);
    EXPECT_EQ(data_reduction_proxy_info.proxy_server(), result.proxy_server());
 
    // Another proxy is used. It should be used afterwards.
    result.Use(other_proxy_info);
    OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                         empty_proxy_retry_info, &test_params, &result);
+                         proxy_config_direct, empty_proxy_retry_info,
+                         &test_params, &result);
    EXPECT_EQ(other_proxy_info.proxy_server(), result.proxy_server());
 
    // A direct connection is used. The data reduction proxy should be used
@@ -948,7 +951,8 @@ TEST_F(DataReductionProxyProtocolTest, OnResolveProxyHandler) {
    result.Use(direct_proxy_info);
    net::ProxyConfig::ID prev_id = result.config_id();
    OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                         empty_proxy_retry_info, &test_params, &result);
+                         proxy_config_direct, empty_proxy_retry_info,
+                         &test_params, &result);
    EXPECT_EQ(data_reduction_proxy_info.proxy_server(), result.proxy_server());
    // Only the proxy list should be updated, not he proxy info.
    EXPECT_EQ(result.config_id(), prev_id);
@@ -958,8 +962,8 @@ TEST_F(DataReductionProxyProtocolTest, OnResolveProxyHandler) {
    result.Use(direct_proxy_info);
    prev_id = result.config_id();
    OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                         data_reduction_proxy_retry_info, &test_params,
-                         &result);
+                         proxy_config_direct, data_reduction_proxy_retry_info,
+                         &test_params, &result);
    EXPECT_TRUE(result.proxy_server().is_direct());
    EXPECT_EQ(result.config_id(), prev_id);
 
@@ -967,25 +971,25 @@ TEST_F(DataReductionProxyProtocolTest, OnResolveProxyHandler) {
   // Without DataCompressionProxyCriticalBypass Finch trial set, should never
   // bypass.
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                        empty_proxy_retry_info, &test_params,
-                        &data_reduction_proxy_info);
+                        proxy_config_direct, empty_proxy_retry_info,
+                        &test_params, &data_reduction_proxy_info);
   EXPECT_FALSE(data_reduction_proxy_info.is_direct());
 
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                        empty_proxy_retry_info, &test_params,
-                        &other_proxy_info);
+                        proxy_config_direct, empty_proxy_retry_info,
+                        &test_params, &other_proxy_info);
   EXPECT_FALSE(other_proxy_info.is_direct());
 
   load_flags |= net::LOAD_BYPASS_DATA_REDUCTION_PROXY;
 
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                        empty_proxy_retry_info, &test_params,
-                        &data_reduction_proxy_info);
+                        proxy_config_direct, empty_proxy_retry_info,
+                        &test_params, &data_reduction_proxy_info);
   EXPECT_FALSE(data_reduction_proxy_info.is_direct());
 
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                        empty_proxy_retry_info, &test_params,
-                        &other_proxy_info);
+                        proxy_config_direct, empty_proxy_retry_info,
+                        &test_params, &other_proxy_info);
   EXPECT_FALSE(other_proxy_info.is_direct());
 
   // With Finch trial set, should only bypass if LOAD flag is set and the
@@ -999,25 +1003,25 @@ TEST_F(DataReductionProxyProtocolTest, OnResolveProxyHandler) {
   load_flags = net::LOAD_NORMAL;
 
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                        empty_proxy_retry_info, &test_params,
-                        &data_reduction_proxy_info);
+                        proxy_config_direct, empty_proxy_retry_info,
+                        &test_params, &data_reduction_proxy_info);
   EXPECT_FALSE(data_reduction_proxy_info.is_direct());
 
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                        empty_proxy_retry_info, &test_params,
-                        &other_proxy_info);
+                        proxy_config_direct, empty_proxy_retry_info,
+                        &test_params, &other_proxy_info);
   EXPECT_FALSE(other_proxy_info.is_direct());
 
   load_flags |= net::LOAD_BYPASS_DATA_REDUCTION_PROXY;
 
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                        empty_proxy_retry_info, &test_params,
-                        &other_proxy_info);
+                        proxy_config_direct, empty_proxy_retry_info,
+                        &test_params, &other_proxy_info);
   EXPECT_FALSE(other_proxy_info.is_direct());
 
   OnResolveProxyHandler(url, load_flags, data_reduction_proxy_config,
-                        empty_proxy_retry_info, &test_params,
-                        &data_reduction_proxy_info);
+                        proxy_config_direct, empty_proxy_retry_info,
+                        &test_params, &data_reduction_proxy_info);
   EXPECT_TRUE(data_reduction_proxy_info.is_direct());
 }
 

net/proxy/proxy_service.h

@@ -210,7 +210,7 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver,
   }
 
   // Returns the current configuration being used by ProxyConfigService.
-  const ProxyConfig& config() {
+  const ProxyConfig& config() const {
     return config_;
   }