Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(291)

Unified Diff: net/proxy/proxy_list.cc

Issue 502068: Remove the implicit fallback to DIRECT when proxies fail. This better matches... (Closed) Base URL: svn://chrome-svn/chrome/trunk/src/
Patch Set: Fix a comment typo Created 10 years, 12 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: net/proxy/proxy_list.cc
===================================================================
--- net/proxy/proxy_list.cc (revision 35522)
+++ net/proxy/proxy_list.cc (working copy)
@@ -61,12 +61,15 @@
}
}
-ProxyServer ProxyList::Get() const {
- if (!proxies_.empty())
- return proxies_[0];
- return ProxyServer(ProxyServer::SCHEME_DIRECT, std::string(), -1);
+bool ProxyList::IsEmpty() const {
+ return proxies_.empty();
}
+const ProxyServer& ProxyList::Get() const {
+ DCHECK(!proxies_.empty());
+ return proxies_[0];
+}
+
std::string ProxyList::ToPacString() const {
std::string proxy_list;
std::vector<ProxyServer>::const_iterator iter = proxies_.begin();
@@ -75,7 +78,7 @@
proxy_list += ";";
proxy_list += iter->ToPacString();
}
- return proxy_list.empty() ? "DIRECT" : proxy_list;
+ return proxy_list.empty() ? std::string() : proxy_list;
}
void ProxyList::SetFromPacString(const std::string& pac_string) {
@@ -88,30 +91,51 @@
if (uri.is_valid())
proxies_.push_back(uri);
}
+
+ // If we failed to parse anything from the PAC results list, fallback to
+ // DIRECT (this basically means an error in the PAC script).
+ if (proxies_.empty()) {
+ proxies_.push_back(ProxyServer::Direct());
+ }
}
bool ProxyList::Fallback(ProxyRetryInfoMap* proxy_retry_info) {
// Number of minutes to wait before retrying a bad proxy server.
const TimeDelta kProxyRetryDelay = TimeDelta::FromMinutes(5);
+ // TODO(eroman): It would be good if instead of removing failed proxies
+ // from the list, we simply annotated them with the error code they failed
+ // with. Of course, ProxyService::ReconsiderProxyAfterError() would need to
+ // be given this information by the network transaction.
+ //
+ // The advantage of this approach is when the network transaction
+ // fails, we could output the full list of proxies that were attempted, and
+ // why each one of those failed (as opposed to just the last failure).
+ //
+ // And also, before failing the transaction wholesale, we could go back and
+ // retry the "bad proxies" which we never tried to begin with.
+ // (RemoveBadProxies would annotate them as 'expected bad' rather then delete
+ // them from the list, so we would know what they were).
+
if (proxies_.empty()) {
NOTREACHED();
return false;
}
- std::string key = proxies_[0].ToURI();
-
- // Mark this proxy as bad.
- ProxyRetryInfoMap::iterator iter = proxy_retry_info->find(key);
- if (iter != proxy_retry_info->end()) {
- // TODO(nsylvain): This is not the first time we get this. We should
- // double the retry time. Bug 997660.
- iter->second.bad_until = TimeTicks::Now() + iter->second.current_delay;
- } else {
- ProxyRetryInfo retry_info;
- retry_info.current_delay = kProxyRetryDelay;
- retry_info.bad_until = TimeTicks().Now() + retry_info.current_delay;
- (*proxy_retry_info)[key] = retry_info;
+ if (!proxies_[0].is_direct()) {
+ std::string key = proxies_[0].ToURI();
+ // Mark this proxy as bad.
+ ProxyRetryInfoMap::iterator iter = proxy_retry_info->find(key);
+ if (iter != proxy_retry_info->end()) {
+ // TODO(nsylvain): This is not the first time we get this. We should
+ // double the retry time. Bug 997660.
+ iter->second.bad_until = TimeTicks::Now() + iter->second.current_delay;
+ } else {
+ ProxyRetryInfo retry_info;
+ retry_info.current_delay = kProxyRetryDelay;
+ retry_info.bad_until = TimeTicks().Now() + retry_info.current_delay;
+ (*proxy_retry_info)[key] = retry_info;
+ }
}
// Remove this proxy from our list.

Powered by Google App Engine
This is Rietveld 408576698