Keep track of network error in ProxyRetryInfo.

Use it to log error type in BypassOneNetworkError UMA.
This fixes DataReductionProxy.BypassOnNetworkError UMA recording bug.

BUG=395769

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

Cr-Commit-Position: refs/heads/master@{#290054}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290054 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 110 insertions, 68 deletions

diff --git a/net/proxy/proxy_info.cc b/net/proxy/proxy_info.cc
index b6fcc867..96ae3ce 100644
--- a/net/proxy/proxy_info.cc
+++ b/net/proxy/proxy_info.cc
@@ -63,8 +63,8 @@ std::string ProxyInfo::ToPacString() const {
   return proxy_list_.ToPacString();
 }
 
-bool ProxyInfo::Fallback(const BoundNetLog& net_log) {
-  return proxy_list_.Fallback(&proxy_retry_info_, net_log);
+bool ProxyInfo::Fallback(int net_error, const BoundNetLog& net_log) {
+  return proxy_list_.Fallback(&proxy_retry_info_, net_error, net_log);
 }
 
 void ProxyInfo::DeprioritizeBadProxies(
diff --git a/net/proxy/proxy_info.h b/net/proxy/proxy_info.h
index 336cb3a..1e4fded 100644
--- a/net/proxy/proxy_info.h
+++ b/net/proxy/proxy_info.h
@@ -115,9 +115,12 @@ class NET_EXPORT ProxyInfo {
   // See description in ProxyList::ToPacString().
   std::string ToPacString() const;
 
-  // Marks the current proxy as bad. Returns true if there is another proxy
+  // Marks the current proxy as bad. |net_error| should contain the network
+  // error encountered when this proxy was tried, if any. If this fallback
+  // is not because of a network error, then |OK| should be passed in (eg. for
+  // reasons such as local policy). Returns true if there is another proxy is
   // available to try in proxy list_.
-  bool Fallback(const BoundNetLog& net_log);
+  bool Fallback(int net_error, const BoundNetLog& net_log);
 
   // De-prioritizes the proxies that we have cached as not working, by moving
   // them to the end of the proxy list.
diff --git a/net/proxy/proxy_info_unittest.cc b/net/proxy/proxy_info_unittest.cc
index 377cff34..165283d 100644
--- a/net/proxy/proxy_info_unittest.cc
+++ b/net/proxy/proxy_info_unittest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "net/base/net_errors.h"
 #include "net/proxy/proxy_info.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -32,7 +33,7 @@ TEST(ProxyInfoTest, ProxyInfoIsDirectOnly) {
   EXPECT_FALSE(info.is_direct());
   EXPECT_FALSE(info.is_direct_only());
   // After falling back to direct, we shouldn't consider it DIRECT only.
-  EXPECT_TRUE(info.Fallback(BoundNetLog()));
+  EXPECT_TRUE(info.Fallback(ERR_PROXY_CONNECTION_FAILED, BoundNetLog()));
   EXPECT_TRUE(info.is_direct());
   EXPECT_FALSE(info.is_direct_only());
 }
diff --git a/net/proxy/proxy_list.cc b/net/proxy/proxy_list.cc
index 2adea29..4dc4626 100644
--- a/net/proxy/proxy_list.cc
+++ b/net/proxy/proxy_list.cc
@@ -146,6 +146,7 @@ base::ListValue* ProxyList::ToValue() const {
 }
 
 bool ProxyList::Fallback(ProxyRetryInfoMap* proxy_retry_info,
+                         int net_error,
                          const BoundNetLog& net_log) {
 
   // TODO(eroman): It would be good if instead of removing failed proxies
@@ -171,6 +172,7 @@ bool ProxyList::Fallback(ProxyRetryInfoMap* proxy_retry_info,
                             TimeDelta::FromMinutes(5),
                             true,
                             ProxyServer(),
+                            net_error,
                             net_log);
 
   // Remove this proxy from our list.
@@ -182,6 +184,7 @@ void ProxyList::AddProxyToRetryList(ProxyRetryInfoMap* proxy_retry_info,
                                     base::TimeDelta retry_delay,
                                     bool try_while_bad,
                                     const ProxyServer& proxy_to_retry,
+                                    int net_error,
                                     const BoundNetLog& net_log) const {
   // Mark this proxy as bad.
   std::string proxy_key = proxy_to_retry.ToURI();
@@ -195,6 +198,7 @@ void ProxyList::AddProxyToRetryList(ProxyRetryInfoMap* proxy_retry_info,
     retry_info.current_delay = retry_delay;
     retry_info.bad_until = TimeTicks().Now() + retry_info.current_delay;
     retry_info.try_while_bad = try_while_bad;
+    retry_info.net_error = net_error;
     (*proxy_retry_info)[proxy_key] = retry_info;
   }
   net_log.AddEvent(NetLog::TYPE_PROXY_LIST_FALLBACK,
@@ -206,6 +210,7 @@ void ProxyList::UpdateRetryInfoOnFallback(
     base::TimeDelta retry_delay,
     bool reconsider,
     const ProxyServer& another_proxy_to_bypass,
+    int net_error,
     const BoundNetLog& net_log) const {
   DCHECK(retry_delay != base::TimeDelta());
 
@@ -215,14 +220,22 @@ void ProxyList::UpdateRetryInfoOnFallback(
   }
 
   if (!proxies_[0].is_direct()) {
-    AddProxyToRetryList(proxy_retry_info, retry_delay, reconsider, proxies_[0],
+    AddProxyToRetryList(proxy_retry_info,
+                        retry_delay,
+                        reconsider,
+                        proxies_[0],
+                        net_error,
                         net_log);
 
     // If an additional proxy to bypass is specified, add it to the retry map
     // as well.
     if (another_proxy_to_bypass.is_valid()) {
-      AddProxyToRetryList(proxy_retry_info, retry_delay, reconsider,
-                          another_proxy_to_bypass, net_log);
+      AddProxyToRetryList(proxy_retry_info,
+                          retry_delay,
+                          reconsider,
+                          another_proxy_to_bypass,
+                          net_error,
+                          net_log);
     }
   }
 }
diff --git a/net/proxy/proxy_list.h b/net/proxy/proxy_list.h
index ac9635a..d57743e 100644
--- a/net/proxy/proxy_list.h
+++ b/net/proxy/proxy_list.h
@@ -78,10 +78,14 @@ class NET_EXPORT_PRIVATE ProxyList {
   // Returns a serialized value for the list. The caller takes ownership of it.
   base::ListValue* ToValue() const;
 
-  // Marks the current proxy server as bad and deletes it from the list.  The
-  // list of known bad proxies is given by proxy_retry_info.  Returns true if
-  // there is another server available in the list.
+  // Marks the current proxy server as bad and deletes it from the list. The
+  // list of known bad proxies is given by |proxy_retry_info|. |net_error|
+  // should contain the network error encountered when this proxy was tried, if
+  // any. If this fallback is not because of a network error, then |OK| should
+  // be passed in (eg. for reasons such as local policy). Returns true if there
+  // is another server available in the list.
   bool Fallback(ProxyRetryInfoMap* proxy_retry_info,
+                int net_error,
                 const BoundNetLog& net_log);
 
   // Updates |proxy_retry_info| to indicate that the first proxy in the list
@@ -90,22 +94,28 @@ class NET_EXPORT_PRIVATE ProxyList {
   // retry after |retry_delay| if positive, and will use the default proxy retry
   // duration otherwise. It may reconsider the proxy beforehand if |reconsider|
   // is true. Additionally updates |proxy_retry_info| with
-  // |another_proxy_to_bypass| if non-empty.
+  // |another_proxy_to_bypass| if non-empty. |net_error| should contain the
+  // network error countered when this proxy was tried, or OK if the proxy retry
+  // info is being updated for a non-network related reason (e.g. local policy).
   void UpdateRetryInfoOnFallback(
       ProxyRetryInfoMap* proxy_retry_info,
       base::TimeDelta retry_delay,
       bool reconsider,
       const ProxyServer& another_proxy_to_bypass,
+      int net_error,
       const BoundNetLog& net_log) const;
 
  private:
   // Updates |proxy_retry_info| to indicate that the |proxy_to_retry| in
   // |proxies_| is bad for |retry_delay|, but may be reconsidered earlier if
-  // |try_while_bad| is true.
+  // |try_while_bad| is true. |net_error| should contain the network error
+  // countered when this proxy was tried, or OK if the proxy retry info is
+  // being updated for a non-network related reason (e.g. local policy).
   void AddProxyToRetryList(ProxyRetryInfoMap* proxy_retry_info,
                            base::TimeDelta retry_delay,
                            bool try_while_bad,
                            const ProxyServer& proxy_to_retry,
+                           int net_error,
                            const BoundNetLog& net_log) const;
 
   // List of proxies.
diff --git a/net/proxy/proxy_list_unittest.cc b/net/proxy/proxy_list_unittest.cc
index 9ccf9c4..23b823f 100644
--- a/net/proxy/proxy_list_unittest.cc
+++ b/net/proxy/proxy_list_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "net/proxy/proxy_list.h"
 
+#include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/proxy/proxy_retry_info.h"
 #include "net/proxy/proxy_server.h"
@@ -172,13 +173,38 @@ TEST(ProxyListTest, UpdateRetryInfoOnFallback) {
     ProxyList list;
     ProxyRetryInfoMap retry_info_map;
     BoundNetLog net_log;
+    ProxyServer proxy_server(
+        ProxyServer::FromURI("foopy1:80", ProxyServer::SCHEME_HTTP));
     list.SetFromPacString("PROXY foopy1:80;PROXY foopy2:80;PROXY foopy3:80");
     list.UpdateRetryInfoOnFallback(&retry_info_map,
                                    base::TimeDelta::FromSeconds(60),
                                    true,
-                                   ProxyServer(),
+                                   proxy_server,
+                                   ERR_PROXY_CONNECTION_FAILED,
+                                   net_log);
+    EXPECT_TRUE(retry_info_map.end() != retry_info_map.find("foopy1:80"));
+    EXPECT_EQ(ERR_PROXY_CONNECTION_FAILED,
+              retry_info_map[proxy_server.ToURI()].net_error);
+    EXPECT_TRUE(retry_info_map.end() == retry_info_map.find("foopy2:80"));
+    EXPECT_TRUE(retry_info_map.end() == retry_info_map.find("foopy3:80"));
+  }
+  // Retrying should put the first proxy on the retry list, even if there
+  // was no network error.
+  {
+    ProxyList list;
+    ProxyRetryInfoMap retry_info_map;
+    BoundNetLog net_log;
+    ProxyServer proxy_server(
+        ProxyServer::FromURI("foopy1:80", ProxyServer::SCHEME_HTTP));
+    list.SetFromPacString("PROXY foopy1:80;PROXY foopy2:80;PROXY foopy3:80");
+    list.UpdateRetryInfoOnFallback(&retry_info_map,
+                                   base::TimeDelta::FromSeconds(60),
+                                   true,
+                                   proxy_server,
+                                   OK,
                                    net_log);
     EXPECT_TRUE(retry_info_map.end() != retry_info_map.find("foopy1:80"));
+    EXPECT_EQ(OK, retry_info_map[proxy_server.ToURI()].net_error);
     EXPECT_TRUE(retry_info_map.end() == retry_info_map.find("foopy2:80"));
     EXPECT_TRUE(retry_info_map.end() == retry_info_map.find("foopy3:80"));
   }
@@ -195,8 +221,11 @@ TEST(ProxyListTest, UpdateRetryInfoOnFallback) {
                                    base::TimeDelta::FromSeconds(60),
                                    true,
                                    proxy_server,
+                                   ERR_NAME_RESOLUTION_FAILED,
                                    net_log);
     EXPECT_TRUE(retry_info_map.end() != retry_info_map.find("foopy1:80"));
+    EXPECT_EQ(ERR_NAME_RESOLUTION_FAILED,
+              retry_info_map[proxy_server.ToURI()].net_error);
     EXPECT_TRUE(retry_info_map.end() == retry_info_map.find("foopy2:80"));
     EXPECT_TRUE(retry_info_map.end() != retry_info_map.find("foopy3:80"));
   }
@@ -213,6 +242,7 @@ TEST(ProxyListTest, UpdateRetryInfoOnFallback) {
                                    base::TimeDelta::FromSeconds(60),
                                    true,
                                    proxy_server,
+                                   OK,
                                    net_log);
     EXPECT_TRUE(retry_info_map.end() == retry_info_map.find("foopy2:80"));
     EXPECT_TRUE(retry_info_map.end() == retry_info_map.find("foopy3:80"));
diff --git a/net/proxy/proxy_retry_info.h b/net/proxy/proxy_retry_info.h
index 0d073b9..a36fcd9 100644
--- a/net/proxy/proxy_retry_info.h
+++ b/net/proxy/proxy_retry_info.h
@@ -24,6 +24,11 @@ struct ProxyRetryInfo {
 
   // True if this proxy should be considered even if still bad.
   bool try_while_bad;
+
+  // The network error received when this proxy failed, or |OK| if the proxy
+  // was added to the retry list for a non-network related reason. (e.g. local
+  // policy).
+  int net_error;
 };
 
 // Map of proxy servers with the associated RetryInfo structures.
diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc
index 6bd519c..0de696f 100644
--- a/net/proxy/proxy_service.cc
+++ b/net/proxy/proxy_service.cc
@@ -1211,11 +1211,7 @@ int ProxyService::ReconsiderProxyAfterError(const GURL& url,
 
   // We don't have new proxy settings to try, try to fallback to the next proxy
   // in the list.
-  bool did_fallback = result->Fallback(net_log);
-
-  if (network_delegate) {
-      network_delegate->NotifyProxyFallback(bad_proxy, net_error, did_fallback);
-  }
+  bool did_fallback = result->Fallback(net_error, net_log);
 
   // Return synchronous failure if there is nothing left to fall-back to.
   // TODO(eroman): This is a yucky API, clean it up.
@@ -1227,9 +1223,11 @@ bool ProxyService::MarkProxiesAsBadUntil(
     base::TimeDelta retry_delay,
     const ProxyServer& another_bad_proxy,
     const BoundNetLog& net_log) {
-  result.proxy_list_.UpdateRetryInfoOnFallback(&proxy_retry_info_, retry_delay,
+  result.proxy_list_.UpdateRetryInfoOnFallback(&proxy_retry_info_,
+                                               retry_delay,
                                                false,
                                                another_bad_proxy,
+                                               OK,
                                                net_log);
   if (another_bad_proxy.is_valid())
     return result.proxy_list_.size() > 2;
@@ -1237,7 +1235,8 @@ bool ProxyService::MarkProxiesAsBadUntil(
     return result.proxy_list_.size() > 1;
 }
 
-void ProxyService::ReportSuccess(const ProxyInfo& result) {
+void ProxyService::ReportSuccess(const ProxyInfo& result,
+                                 NetworkDelegate* network_delegate) {
   DCHECK(CalledOnValidThread());
 
   const ProxyRetryInfoMap& new_retry_info = result.proxy_retry_info();
@@ -1247,8 +1246,14 @@ void ProxyService::ReportSuccess(const ProxyInfo& result) {
   for (ProxyRetryInfoMap::const_iterator iter = new_retry_info.begin();
        iter != new_retry_info.end(); ++iter) {
     ProxyRetryInfoMap::iterator existing = proxy_retry_info_.find(iter->first);
-    if (existing == proxy_retry_info_.end())
+    if (existing == proxy_retry_info_.end()) {
       proxy_retry_info_[iter->first] = iter->second;
+      if (network_delegate) {
+        const ProxyRetryInfo& proxy_retry_info = iter->second;
+        network_delegate->NotifyProxyFallback(result.proxy_server(),
+                                              proxy_retry_info.net_error);
+      }
+    }
     else if (existing->second.bad_until < iter->second.bad_until)
       existing->second.bad_until = iter->second.bad_until;
   }
diff --git a/net/proxy/proxy_service.h b/net/proxy/proxy_service.h
index 7b3e5d2..69aaf46 100644
--- a/net/proxy/proxy_service.h
+++ b/net/proxy/proxy_service.h
@@ -160,7 +160,10 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver,
   // and will use the default proxy retry duration otherwise. Proxies marked as
   // bad will not be retried until |retry_delay| has passed. Returns true if
   // there will be at least one proxy remaining in the list after fallback and
-  // false otherwise.
+  // false otherwise. This method should be used to add proxies to the bad
+  // proxy list only for reasons other than a network error. If a proxy needs
+  // to be added to the bad proxy list because a network error was encountered
+  // when trying to connect to it, use |ReconsiderProxyAfterError|.
   bool MarkProxiesAsBadUntil(const ProxyInfo& results,
                              base::TimeDelta retry_delay,
                              const ProxyServer& another_bad_proxy,
@@ -168,8 +171,10 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver,
 
   // Called to report that the last proxy connection succeeded.  If |proxy_info|
   // has a non empty proxy_retry_info map, the proxies that have been tried (and
-  // failed) for this request will be marked as bad.
-  void ReportSuccess(const ProxyInfo& proxy_info);
+  // failed) for this request will be marked as bad. |network_delegate| will
+  // be notified of any proxy fallbacks.
+  void ReportSuccess(const ProxyInfo& proxy_info,
+                     NetworkDelegate* network_delegate);
 
   // Call this method with a non-null |pac_request| to cancel the PAC request.
   void CancelPacRequest(PacRequest* pac_request);
diff --git a/net/proxy/proxy_service_unittest.cc b/net/proxy/proxy_service_unittest.cc
index 1d5b1c7..f153fad 100644
--- a/net/proxy/proxy_service_unittest.cc
+++ b/net/proxy/proxy_service_unittest.cc
@@ -207,17 +207,13 @@ class TestProxyFallbackNetworkDelegate : public NetworkDelegate {
  public:
   TestProxyFallbackNetworkDelegate()
       : on_proxy_fallback_called_(false),
-        proxy_fallback_net_error_(0),
-        proxy_did_fallback_(false) {
+        proxy_fallback_net_error_(OK) {
   }
 
-  virtual void OnProxyFallback(
-      const ProxyServer& proxy_server,
-      int net_error,
-      bool did_fallback) OVERRIDE {
+  virtual void OnProxyFallback(const ProxyServer& proxy_server,
+                               int net_error) OVERRIDE {
     proxy_server_ = proxy_server;
     proxy_fallback_net_error_ = net_error;
-    proxy_did_fallback_ = did_fallback;
     on_proxy_fallback_called_ = true;
   }
 
@@ -233,15 +229,10 @@ class TestProxyFallbackNetworkDelegate : public NetworkDelegate {
     return proxy_fallback_net_error_;
   }
 
-  bool proxy_did_fallback() const {
-    return proxy_did_fallback_;
-  }
-
  private:
   bool on_proxy_fallback_called_;
   ProxyServer proxy_server_;
   int proxy_fallback_net_error_;
-  bool proxy_did_fallback_;
 };
 
 }  // namespace
@@ -514,7 +505,7 @@ TEST_F(ProxyServiceTest, PAC_FailoverWithoutDirect) {
   // Now, imagine that connecting to foopy:8080 fails: there is nothing
   // left to fallback to, since our proxy list was NOT terminated by
   // DIRECT.
-  TestProxyFallbackNetworkDelegate network_delegate;
+  NetworkDelegate network_delegate;
   TestCompletionCallback callback2;
   ProxyServer expected_proxy_server = info.proxy_server();
   rv = service.ReconsiderProxyAfterError(
@@ -523,11 +514,6 @@ TEST_F(ProxyServiceTest, PAC_FailoverWithoutDirect) {
   // ReconsiderProxyAfterError returns error indicating nothing left.
   EXPECT_EQ(ERR_FAILED, rv);
   EXPECT_TRUE(info.is_empty());
-  EXPECT_TRUE(network_delegate.on_proxy_fallback_called());
-  EXPECT_EQ(expected_proxy_server, network_delegate.proxy_server());
-  EXPECT_EQ(net::ERR_PROXY_CONNECTION_FAILED,
-            network_delegate.proxy_fallback_net_error());
-  EXPECT_FALSE(network_delegate.proxy_did_fallback());
 }
 
 // Test that if the execution of the PAC script fails (i.e. javascript runtime
@@ -620,7 +606,6 @@ TEST_F(ProxyServiceTest, PAC_FailoverAfterDirect) {
   EXPECT_TRUE(info.is_direct());
 
   // Fallback 1.
-  TestProxyFallbackNetworkDelegate network_delegate2;
   TestCompletionCallback callback2;
   rv = service.ReconsiderProxyAfterError(url, net::LOAD_NORMAL,
                                          net::ERR_PROXY_CONNECTION_FAILED,
@@ -629,57 +614,38 @@ TEST_F(ProxyServiceTest, PAC_FailoverAfterDirect) {
   EXPECT_EQ(OK, rv);
   EXPECT_FALSE(info.is_direct());
   EXPECT_EQ("foobar:10", info.proxy_server().ToURI());
-  // No network delegate provided.
-  EXPECT_FALSE(network_delegate2.on_proxy_fallback_called());
 
   // Fallback 2.
-  TestProxyFallbackNetworkDelegate network_delegate3;
+  NetworkDelegate network_delegate;
   ProxyServer expected_proxy_server3 = info.proxy_server();
   TestCompletionCallback callback3;
   rv = service.ReconsiderProxyAfterError(url, net::LOAD_NORMAL,
                                          net::ERR_PROXY_CONNECTION_FAILED,
                                          &info, callback3.callback(), NULL,
-                                         &network_delegate3, BoundNetLog());
+                                         &network_delegate, BoundNetLog());
   EXPECT_EQ(OK, rv);
   EXPECT_TRUE(info.is_direct());
-  EXPECT_TRUE(network_delegate3.on_proxy_fallback_called());
-  EXPECT_EQ(expected_proxy_server3, network_delegate3.proxy_server());
-  EXPECT_EQ(net::ERR_PROXY_CONNECTION_FAILED,
-            network_delegate3.proxy_fallback_net_error());
-  EXPECT_TRUE(network_delegate3.proxy_did_fallback());
 
   // Fallback 3.
-  TestProxyFallbackNetworkDelegate network_delegate4;
   ProxyServer expected_proxy_server4 = info.proxy_server();
   TestCompletionCallback callback4;
   rv = service.ReconsiderProxyAfterError(url, net::LOAD_NORMAL,
                                          net::ERR_PROXY_CONNECTION_FAILED,
                                          &info, callback4.callback(), NULL,
-                                         &network_delegate4, BoundNetLog());
+                                         &network_delegate, BoundNetLog());
   EXPECT_EQ(OK, rv);
   EXPECT_FALSE(info.is_direct());
   EXPECT_EQ("foobar:20", info.proxy_server().ToURI());
-  EXPECT_TRUE(network_delegate4.on_proxy_fallback_called());
-  EXPECT_EQ(expected_proxy_server4, network_delegate4.proxy_server());
-  EXPECT_EQ(net::ERR_PROXY_CONNECTION_FAILED,
-            network_delegate4.proxy_fallback_net_error());
-  EXPECT_TRUE(network_delegate4.proxy_did_fallback());
 
   // Fallback 4 -- Nothing to fall back to!
-  TestProxyFallbackNetworkDelegate network_delegate5;
   ProxyServer expected_proxy_server5 = info.proxy_server();
   TestCompletionCallback callback5;
   rv = service.ReconsiderProxyAfterError(url, net::LOAD_NORMAL,
                                          net::ERR_PROXY_CONNECTION_FAILED,
                                          &info, callback5.callback(), NULL,
-                                         &network_delegate5, BoundNetLog());
+                                         &network_delegate, BoundNetLog());
   EXPECT_EQ(ERR_FAILED, rv);
   EXPECT_TRUE(info.is_empty());
-  EXPECT_TRUE(network_delegate5.on_proxy_fallback_called());
-  EXPECT_EQ(expected_proxy_server5, network_delegate5.proxy_server());
-  EXPECT_EQ(net::ERR_PROXY_CONNECTION_FAILED,
-            network_delegate5.proxy_fallback_net_error());
-  EXPECT_FALSE(network_delegate5.proxy_did_fallback());
 }
 
 TEST_F(ProxyServiceTest, PAC_ConfigSourcePropagates) {
@@ -994,7 +960,11 @@ TEST_F(ProxyServiceTest, ProxyFallback) {
   EXPECT_EQ("foopy2:9090", info.proxy_server().ToURI());
   // Report back that the second proxy worked.  This will globally mark the
   // first proxy as bad.
-  service.ReportSuccess(info);
+  TestProxyFallbackNetworkDelegate test_delegate;
+  service.ReportSuccess(info, &test_delegate);
+  EXPECT_EQ(info.proxy_server(), test_delegate.proxy_server());
+  EXPECT_EQ(net::ERR_PROXY_CONNECTION_FAILED,
+            test_delegate.proxy_fallback_net_error());
 
   TestCompletionCallback callback3;
   rv = service.ResolveProxy(