Display the "effective" proxy settings in about:net-internals.

The "effective" settings is what you get after applying the various fallbacks between automatic and manual settings.

This display makes it easier to notice whether "auto-detect" actually took effect, and if so what was the PAC URL it used.

BUG=53549
TEST=On windows change your proxy settings to include both auto-detect, a custom pac script, and some manually configured proxy servers. Now run chrome and go to the proxy tab on about:net-internals. Check that the "original" settings is what you entered in the dialog box, however the "effective" settings will only be a subset of them.
Review URL: http://codereview.chromium.org/3241002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57767 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 92 insertions, 38 deletions

diff --git a/net/proxy/init_proxy_resolver.cc b/net/proxy/init_proxy_resolver.cc
index 9cffa67..1046173 100644
--- a/net/proxy/init_proxy_resolver.cc
+++ b/net/proxy/init_proxy_resolver.cc
@@ -16,6 +16,10 @@
 
 namespace net {
 
+// This is the hard-coded location used by the DNS portion of web proxy
+// auto-discovery.
+static const char kWpadUrl[] = "http://wpad/wpad.dat";
+
 InitProxyResolver::InitProxyResolver(ProxyResolver* resolver,
                                      ProxyScriptFetcher* proxy_script_fetcher,
                                      NetLog* net_log)
@@ -27,7 +31,8 @@ InitProxyResolver::InitProxyResolver(ProxyResolver* resolver,
       current_pac_url_index_(0u),
       next_state_(STATE_NONE),
       net_log_(BoundNetLog::Make(
-          net_log, NetLog::SOURCE_INIT_PROXY_RESOLVER)) {
+          net_log, NetLog::SOURCE_INIT_PROXY_RESOLVER)),
+      effective_config_(NULL) {
 }
 
 InitProxyResolver::~InitProxyResolver() {
@@ -37,6 +42,7 @@ InitProxyResolver::~InitProxyResolver() {
 
 int InitProxyResolver::Init(const ProxyConfig& config,
                             const base::TimeDelta wait_delay,
+                            ProxyConfig* effective_config,
                             CompletionCallback* callback) {
   DCHECK_EQ(STATE_NONE, next_state_);
   DCHECK(callback);
@@ -49,6 +55,8 @@ int InitProxyResolver::Init(const ProxyConfig& config,
   if (wait_delay_ < base::TimeDelta())
     wait_delay_ = base::TimeDelta();
 
+  effective_config_ = effective_config;
+
   pac_urls_ = BuildPacUrlsFallbackList(config);
   DCHECK(!pac_urls_.empty());
 
@@ -157,7 +165,7 @@ int InitProxyResolver::DoFetchPacScript() {
   const PacURL& pac_url = current_pac_url();
 
   const GURL effective_pac_url =
-      pac_url.auto_detect ? GURL("http://wpad/wpad.dat") : pac_url.url;
+      pac_url.auto_detect ? GURL(kWpadUrl) : pac_url.url;
 
   net_log_.BeginEvent(
       NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT,
@@ -218,6 +226,20 @@ int InitProxyResolver::DoSetPacScriptComplete(int result) {
     return TryToFallbackPacUrl(result);
   }
 
+  // Let the caller know which automatic setting we ended up initializing the
+  // resolver for (there may have been multiple fallbacks to choose from.)
+  if (effective_config_) {
+    if (current_pac_url().auto_detect && resolver_->expects_pac_bytes()) {
+      *effective_config_ =
+          ProxyConfig::CreateFromCustomPacURL(GURL(kWpadUrl));
+    } else if (current_pac_url().auto_detect) {
+      *effective_config_ = ProxyConfig::CreateAutoDetect();
+    } else {
+      *effective_config_ =
+          ProxyConfig::CreateFromCustomPacURL(current_pac_url().url);
+    }
+  }
+
   net_log_.EndEvent(NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT, NULL);
   return result;
 }
diff --git a/net/proxy/init_proxy_resolver.h b/net/proxy/init_proxy_resolver.h
index 1bdb4e8..2691be5 100644
--- a/net/proxy/init_proxy_resolver.h
+++ b/net/proxy/init_proxy_resolver.h
@@ -48,11 +48,18 @@ class InitProxyResolver {
   // Aborts any in-progress request.
   ~InitProxyResolver();
 
-  // Apply the PAC settings of |config| to |resolver_|.
+  // Applies the PAC settings of |config| to |resolver_|.
   // If |wait_delay| is positive, the initialization will pause for this
   // amount of time before getting started.
+  // If |effective_config| is non-NULL, then on successful initialization of
+  // |resolver_| the "effective" proxy settings we ended up using will be
+  // written out to |*effective_config|. Note that this may differ from
+  // |config| since we will have stripped any manual settings, and decided
+  // whether to use auto-detect or the custom PAC URL. Finally, if auto-detect
+  // was used we may now have resolved that to a specific script URL.
   int Init(const ProxyConfig& config,
            const base::TimeDelta wait_delay,
+           ProxyConfig* effective_config,
            CompletionCallback* callback);
 
  private:
@@ -127,6 +134,8 @@ class InitProxyResolver {
   base::TimeDelta wait_delay_;
   base::OneShotTimer<InitProxyResolver> wait_timer_;
 
+  ProxyConfig* effective_config_;
+
   DISALLOW_COPY_AND_ASSIGN(InitProxyResolver);
 };
 
diff --git a/net/proxy/init_proxy_resolver_unittest.cc b/net/proxy/init_proxy_resolver_unittest.cc
index 11d7c5f..0c7e8a1 100644
--- a/net/proxy/init_proxy_resolver_unittest.cc
+++ b/net/proxy/init_proxy_resolver_unittest.cc
@@ -177,7 +177,7 @@ TEST(InitProxyResolverTest, CustomPacSucceeds) {
   TestCompletionCallback callback;
   CapturingNetLog log(CapturingNetLog::kUnbounded);
   InitProxyResolver init(&resolver, &fetcher, &log);
-  EXPECT_EQ(OK, init.Init(config, base::TimeDelta(), &callback));
+  EXPECT_EQ(OK, init.Init(config, base::TimeDelta(), NULL, &callback));
   EXPECT_EQ(rule.text(), resolver.script_data()->utf16());
 
   // Check the NetLog was filled correctly.
@@ -211,7 +211,7 @@ TEST(InitProxyResolverTest, CustomPacFails1) {
   CapturingNetLog log(CapturingNetLog::kUnbounded);
   InitProxyResolver init(&resolver, &fetcher, &log);
   EXPECT_EQ(kFailedDownloading,
-            init.Init(config, base::TimeDelta(), &callback));
+            init.Init(config, base::TimeDelta(), NULL, &callback));
   EXPECT_EQ(NULL, resolver.script_data());
 
   // Check the NetLog was filled correctly.
@@ -239,7 +239,8 @@ TEST(InitProxyResolverTest, CustomPacFails2) {
 
   TestCompletionCallback callback;
   InitProxyResolver init(&resolver, &fetcher, NULL);
-  EXPECT_EQ(kFailedParsing, init.Init(config, base::TimeDelta(), &callback));
+  EXPECT_EQ(kFailedParsing,
+            init.Init(config, base::TimeDelta(), NULL, &callback));
   EXPECT_EQ(NULL, resolver.script_data());
 }
 
@@ -253,7 +254,8 @@ TEST(InitProxyResolverTest, HasNullProxyScriptFetcher) {
 
   TestCompletionCallback callback;
   InitProxyResolver init(&resolver, NULL, NULL);
-  EXPECT_EQ(ERR_UNEXPECTED, init.Init(config, base::TimeDelta(), &callback));
+  EXPECT_EQ(ERR_UNEXPECTED,
+            init.Init(config, base::TimeDelta(), NULL, &callback));
   EXPECT_EQ(NULL, resolver.script_data());
 }
 
@@ -270,7 +272,7 @@ TEST(InitProxyResolverTest, AutodetectSuccess) {
 
   TestCompletionCallback callback;
   InitProxyResolver init(&resolver, &fetcher, NULL);
-  EXPECT_EQ(OK, init.Init(config, base::TimeDelta(), &callback));
+  EXPECT_EQ(OK, init.Init(config, base::TimeDelta(), NULL, &callback));
   EXPECT_EQ(rule.text(), resolver.script_data()->utf16());
 }
 
@@ -289,7 +291,7 @@ TEST(InitProxyResolverTest, AutodetectFailCustomSuccess1) {
 
   TestCompletionCallback callback;
   InitProxyResolver init(&resolver, &fetcher, NULL);
-  EXPECT_EQ(OK, init.Init(config, base::TimeDelta(), &callback));
+  EXPECT_EQ(OK, init.Init(config, base::TimeDelta(), NULL, &callback));
   EXPECT_EQ(rule.text(), resolver.script_data()->utf16());
 }
 
@@ -302,16 +304,25 @@ TEST(InitProxyResolverTest, AutodetectFailCustomSuccess2) {
   ProxyConfig config;
   config.set_auto_detect(true);
   config.set_pac_url(GURL("http://custom/proxy.pac"));
+  config.proxy_rules().ParseFromString("unused-manual-proxy:99");
 
   rules.AddFailParsingRule("http://wpad/wpad.dat");
   Rules::Rule rule = rules.AddSuccessRule("http://custom/proxy.pac");
 
   TestCompletionCallback callback;
   CapturingNetLog log(CapturingNetLog::kUnbounded);
+
+  ProxyConfig effective_config;
   InitProxyResolver init(&resolver, &fetcher, &log);
-  EXPECT_EQ(OK, init.Init(config, base::TimeDelta(), &callback));
+  EXPECT_EQ(OK, init.Init(config, base::TimeDelta(),
+                          &effective_config, &callback));
   EXPECT_EQ(rule.text(), resolver.script_data()->utf16());
 
+  // Verify that the effective configuration no longer contains auto detect or
+  // any of the manual settings.
+  EXPECT_TRUE(effective_config.Equals(
+      ProxyConfig::CreateFromCustomPacURL(GURL("http://custom/proxy.pac"))));
+
   // Check the NetLog was filled correctly.
   // (Note that the Fetch and Set states are repeated since both WPAD and custom
   // PAC scripts are tried).
@@ -358,7 +369,7 @@ TEST(InitProxyResolverTest, AutodetectFailCustomFails1) {
   TestCompletionCallback callback;
   InitProxyResolver init(&resolver, &fetcher, NULL);
   EXPECT_EQ(kFailedDownloading,
-            init.Init(config, base::TimeDelta(), &callback));
+            init.Init(config, base::TimeDelta(), NULL, &callback));
   EXPECT_EQ(NULL, resolver.script_data());
 }
 
@@ -377,7 +388,8 @@ TEST(InitProxyResolverTest, AutodetectFailCustomFails2) {
 
   TestCompletionCallback callback;
   InitProxyResolver init(&resolver, &fetcher, NULL);
-  EXPECT_EQ(kFailedParsing, init.Init(config, base::TimeDelta(), &callback));
+  EXPECT_EQ(kFailedParsing,
+            init.Init(config, base::TimeDelta(), NULL, &callback));
   EXPECT_EQ(NULL, resolver.script_data());
 }
 
@@ -398,7 +410,7 @@ TEST(InitProxyResolverTest, AutodetectFailCustomSuccess2_NoFetch) {
 
   TestCompletionCallback callback;
   InitProxyResolver init(&resolver, &fetcher, NULL);
-  EXPECT_EQ(OK, init.Init(config, base::TimeDelta(), &callback));
+  EXPECT_EQ(OK, init.Init(config, base::TimeDelta(), NULL, &callback));
   EXPECT_EQ(rule.url, resolver.script_data()->url());
 }
 
@@ -420,7 +432,7 @@ TEST(InitProxyResolverTest, CustomPacFails1_WithPositiveDelay) {
   InitProxyResolver init(&resolver, &fetcher, &log);
   EXPECT_EQ(ERR_IO_PENDING,
             init.Init(config, base::TimeDelta::FromMilliseconds(1),
-                      &callback));
+                      NULL, &callback));
 
   EXPECT_EQ(kFailedDownloading, callback.WaitForResult());
   EXPECT_EQ(NULL, resolver.script_data());
@@ -458,7 +470,8 @@ TEST(InitProxyResolverTest, CustomPacFails1_WithNegativeDelay) {
   CapturingNetLog log(CapturingNetLog::kUnbounded);
   InitProxyResolver init(&resolver, &fetcher, &log);
   EXPECT_EQ(kFailedDownloading,
-            init.Init(config, base::TimeDelta::FromSeconds(-5), &callback));
+            init.Init(config, base::TimeDelta::FromSeconds(-5),
+                      NULL, &callback));
   EXPECT_EQ(NULL, resolver.script_data());
 
   // Check the NetLog was filled correctly.
diff --git a/net/proxy/proxy_config.h b/net/proxy/proxy_config.h
index 9cae51a..c5c68b6 100644
--- a/net/proxy/proxy_config.h
+++ b/net/proxy/proxy_config.h
@@ -120,8 +120,15 @@ class ProxyConfig {
 
   // Returns true if this config could possibly require the proxy service to
   // use a PAC resolver.
+  // TODO(eroman): rename to HasAutomaticSettings() for consistency with
+  //               ClearAutomaticSettings().
   bool MayRequirePACResolver() const;
 
+  void ClearAutomaticSettings() {
+    auto_detect_ = false;
+    pac_url_ = GURL();
+  }
+
   // Creates a textual dump of the configuration.
   std::string ToString() const;
 
diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc
index e3fc422..9eedabb 100644
--- a/net/proxy/proxy_service.cc
+++ b/net/proxy/proxy_service.cc
@@ -369,7 +369,6 @@ ProxyService::ProxyService(ProxyConfigService* config_service,
                            NetLog* net_log)
     : resolver_(resolver),
       next_config_id_(1),
-      should_use_proxy_resolver_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(init_proxy_resolver_callback_(
           this, &ProxyService::OnInitProxyResolverComplete)),
       current_state_(STATE_NONE) ,
@@ -504,11 +503,12 @@ int ProxyService::TryToCompleteSynchronously(const GURL& url,
   if (current_state_ != STATE_READY)
     return ERR_IO_PENDING;  // Still initializing.
 
-  if (should_use_proxy_resolver_)
+  DCHECK_NE(config_.id(), ProxyConfig::INVALID_ID);
+
+  if (config_.MayRequirePACResolver())
     return ERR_IO_PENDING;  // Must submit the request to the proxy resolver.
 
   // Use the manual proxy settings.
-  DCHECK(config_.id() != ProxyConfig::INVALID_ID);
   config_.proxy_rules().Apply(url, result);
   result->config_id_ = config_.id();
   return OK;
@@ -583,17 +583,18 @@ void ProxyService::ApplyProxyConfigIfAvailable() {
 void ProxyService::OnInitProxyResolverComplete(int result) {
   DCHECK_EQ(STATE_WAITING_FOR_INIT_PROXY_RESOLVER, current_state_);
   DCHECK(init_proxy_resolver_.get());
-  DCHECK(config_.MayRequirePACResolver());
-  DCHECK(!should_use_proxy_resolver_);
+  DCHECK(fetched_config_.MayRequirePACResolver());
   init_proxy_resolver_.reset();
 
-  should_use_proxy_resolver_ = result == OK;
-
   if (result != OK) {
     LOG(INFO) << "Failed configuring with PAC script, falling-back to manual "
                  "proxy servers.";
+    config_ = fetched_config_;
+    config_.ClearAutomaticSettings();
   }
 
+  config_.set_id(fetched_config_.id());
+
   // Resume any requests which we had to defer until the PAC script was
   // downloaded.
   SetReady();
@@ -694,7 +695,6 @@ ProxyService::State ProxyService::ResetProxyConfig() {
 
   proxy_retry_info_.clear();
   init_proxy_resolver_.reset();
-  should_use_proxy_resolver_ = false;
   SuspendAllPendingRequests();
   config_ = ProxyConfig();
   current_state_ = STATE_NONE;
@@ -767,17 +767,17 @@ ProxyConfigService* ProxyService::CreateSystemProxyConfigService(
 }
 
 void ProxyService::OnProxyConfigChanged(const ProxyConfig& config) {
-  ProxyConfig old_config = config_;
+  ProxyConfig old_fetched_config = fetched_config_;
   ResetProxyConfig();
 
   // Increment the ID to reflect that the config has changed.
-  config_ = config;
-  config_.set_id(next_config_id_++);
+  fetched_config_ = config;
+  fetched_config_.set_id(next_config_id_++);
 
   // Emit the proxy settings change to the NetLog stream.
   if (net_log_) {
     scoped_refptr<NetLog::EventParameters> params =
-        new ProxyConfigChangedNetLogParam(old_config, config);
+        new ProxyConfigChangedNetLogParam(old_fetched_config, fetched_config_);
     net_log_->AddEntry(net::NetLog::TYPE_PROXY_CONFIG_CHANGED,
                        base::TimeTicks::Now(),
                        NetLog::Source(),
@@ -785,7 +785,8 @@ void ProxyService::OnProxyConfigChanged(const ProxyConfig& config) {
                        params);
   }
 
-  if (!config_.MayRequirePACResolver()) {
+  if (!fetched_config_.MayRequirePACResolver()) {
+    config_ = fetched_config_;
     SetReady();
     return;
   }
@@ -802,7 +803,7 @@ void ProxyService::OnProxyConfigChanged(const ProxyConfig& config) {
       stall_proxy_autoconfig_until_ - base::TimeTicks::Now();
 
   int rv = init_proxy_resolver_->Init(
-      config_, wait_delay, &init_proxy_resolver_callback_);
+      fetched_config_, wait_delay, &config_, &init_proxy_resolver_callback_);
 
   if (rv != ERR_IO_PENDING)
     OnInitProxyResolverComplete(rv);
diff --git a/net/proxy/proxy_service.h b/net/proxy/proxy_service.h
index 3b61fbb..990830e 100644
--- a/net/proxy/proxy_service.h
+++ b/net/proxy/proxy_service.h
@@ -109,12 +109,13 @@ class ProxyService : public base::RefCountedThreadSafe<ProxyService>,
   // Tells the resolver to purge any memory it does not need.
   void PurgeMemory();
 
-  // Returns true if we have called UpdateConfig() at least once.
-  bool config_has_been_initialized() const {
-    return config_.id() != ProxyConfig::INVALID_ID;
-  }
 
   // Returns the last configuration fetched from ProxyConfigService.
+  const ProxyConfig& fetched_config() {
+    return fetched_config_;
+  }
+
+  // Returns the current configuration being used by ProxyConfigService.
   const ProxyConfig& config() {
     return config_;
   }
@@ -269,16 +270,17 @@ class ProxyService : public base::RefCountedThreadSafe<ProxyService>,
   scoped_ptr<ProxyConfigService> config_service_;
   scoped_ptr<ProxyResolver> resolver_;
 
-  // We store the proxy config and a counter (ID) that is incremented each time
-  // the config changes.
+  // We store the proxy configuration that was last fetched from the
+  // ProxyConfigService, as well as the resulting "effective" configuration.
+  // The effective configuration is what we condense the original fetched
+  // settings to after testing the various automatic settings (auto-detect
+  // and custom PAC url).
+  ProxyConfig fetched_config_;
   ProxyConfig config_;
 
   // Increasing ID to give to the next ProxyConfig that we set.
   int next_config_id_;
 
-  // Indicates whether the ProxyResolver should be sent requests.
-  bool should_use_proxy_resolver_;
-
   // The time when the proxy configuration was last read from the system.
   base::TimeTicks config_last_update_time_;