Fix infinite proxy retries that could happen when PAC script threw javascript runtime error.

Problem was the config_id_ was not getting set (was getting overridden by ProxyInfo::UseDirect), so it looked like it was seeing an new configuration each time.

BUG=143984


Review URL: https://chromiumcodereview.appspot.com/11344029

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

3 files changed, 42 insertions, 1 deletions

diff --git a/net/proxy/proxy_info.h b/net/proxy/proxy_info.h
index e02ba8b2..d0fa523 100644
--- a/net/proxy/proxy_info.h
+++ b/net/proxy/proxy_info.h
@@ -117,6 +117,8 @@ class NET_EXPORT ProxyInfo {
   // Deletes any entry which doesn't have one of the specified proxy schemes.
   void RemoveProxiesWithoutScheme(int scheme_bit_field);
 
+  ProxyConfig::ID config_id() const { return config_id_; }
+
  private:
   friend class ProxyService;
 
diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc
index a834d82..17e46ed 100644
--- a/net/proxy/proxy_service.cc
+++ b/net/proxy/proxy_service.cc
@@ -800,6 +800,9 @@ class ProxyService::PacRequest
   int QueryDidComplete(int result_code) {
     DCHECK(!was_cancelled());
 
+    // Note that DidFinishResolvingProxy might modify |results_|.
+    int rv = service_->DidFinishResolvingProxy(results_, result_code, net_log_);
+
     // Make a note in the results which configuration was in use at the
     // time of the resolve.
     results_->config_id_ = config_id_;
@@ -811,7 +814,7 @@ class ProxyService::PacRequest
     config_id_ = ProxyConfig::kInvalidConfigID;
     config_source_ = PROXY_CONFIG_SOURCE_UNKNOWN;
 
-    return service_->DidFinishResolvingProxy(results_, result_code, net_log_);
+    return rv;
   }
 
   BoundNetLog* net_log() { return &net_log_; }
diff --git a/net/proxy/proxy_service_unittest.cc b/net/proxy/proxy_service_unittest.cc
index cec0035..ebe3043 100644
--- a/net/proxy/proxy_service_unittest.cc
+++ b/net/proxy/proxy_service_unittest.cc
@@ -306,6 +306,42 @@ TEST_F(ProxyServiceTest, PAC_FailoverWithoutDirect) {
   EXPECT_TRUE(info.is_empty());
 }
 
+// Test that if the execution of the PAC script fails (i.e. javascript runtime
+// error), and the PAC settings are non-mandatory, that we fall-back to direct.
+TEST_F(ProxyServiceTest, PAC_RuntimeError) {
+  MockProxyConfigService* config_service =
+      new MockProxyConfigService("http://foopy/proxy.pac");
+  MockAsyncProxyResolver* resolver = new MockAsyncProxyResolver;
+
+  ProxyService service(config_service, resolver, NULL);
+
+  GURL url("http://this-causes-js-error/");
+
+  ProxyInfo info;
+  TestCompletionCallback callback1;
+  int rv = service.ResolveProxy(
+      url, &info, callback1.callback(), NULL, BoundNetLog());
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  EXPECT_EQ(GURL("http://foopy/proxy.pac"),
+            resolver->pending_set_pac_script_request()->script_data()->url());
+  resolver->pending_set_pac_script_request()->CompleteNow(OK);
+
+  ASSERT_EQ(1u, resolver->pending_requests().size());
+  EXPECT_EQ(url, resolver->pending_requests()[0]->url());
+
+  // Simulate a failure in the PAC executor.
+  resolver->pending_requests()[0]->CompleteNow(ERR_PAC_SCRIPT_FAILED);
+
+  EXPECT_EQ(OK, callback1.WaitForResult());
+
+  // Since the PAC script was non-mandatory, we should have fallen-back to
+  // DIRECT.
+  EXPECT_TRUE(info.is_direct());
+  EXPECT_TRUE(info.did_use_pac_script());
+  EXPECT_EQ(1, info.config_id());
+}
+
 // The proxy list could potentially contain the DIRECT fallback choice
 // in a location other than the very end of the list, and could even
 // specify it multiple times.