Check effective proxy config before adding data reduction proxies

components/data_reduction_proxy/core/browser/data_reduction_proxy_protocol.cc

Index: components/data_reduction_proxy/core/browser/data_reduction_proxy_protocol.cc
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_protocol.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_protocol.cc
index 967c897daddf2f9c8e6785ac329d2e44310fa976..d69691e3aa5f6fced9df96f93dbc2557e8009ca4 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_protocol.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_protocol.cc
@@ -130,11 +130,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)) {

[mef, 2014/10/28 17:01:32]

I'm not sure I understand why we need this check. Shouldn't data reduction proxy
be used if  |proxy_service_proxy_config| equals to
|data_reduction_proxy_config|?

I'm also a bit concerned that ProxyConfig::Equals has a non-trivial performance
(with multiple vector comparisons and whatnot).

[bengr, 2014/10/29 21:12:37]

The point of this code is to add the data reduction proxies to a config if that
config returns direct. If the config is already that of the data reduction
proxy, then the data reduction proxies should not be added to it.

As for performance, the case of Equals() == false requires at most 3 boolean
checks, a check that two empty lists are empty and a check that the first http
proxy is the data reduction proxy in one case and not the other. For Equals() ==
true, the story is the same, except that there would be two more string checks.
Is that too much? For comparison, the proxy resolver fires up v8 in many cases,
which takes milliseconds.

     net::ProxyInfo data_reduction_proxy_info;
     data_reduction_proxy_config.proxy_rules().Apply(
         url, &data_reduction_proxy_info);