Remove the implicit fallback to DIRECT when proxies fail. This better matches...

net/proxy/proxy_info.h

 // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef NET_PROXY_PROXY_INFO_H_
 #define NET_PROXY_PROXY_INFO_H_
 
 #include <string>
 
 #include "net/proxy/proxy_config.h"
@@ skipping 25 matching lines @@
 
   // Set the proxy list to a single entry, |proxy_server|.
   void UseProxyServer(const ProxyServer& proxy_server);
 
   // Parse from the given PAC result.
   void UsePacString(const std::string& pac_string) {
     proxy_list_.SetFromPacString(pac_string);
   }
 
   // Returns true if this proxy info specifies a direct connection.
-  bool is_direct() const { return proxy_list_.Get().is_direct(); }
+  bool is_direct() const {
+    // We don't implicitly fallback to DIRECT unless it was added to the list.

[wtc, 2010/01/07 20:12:27]

Why don't you change ProxyList::Get() to do the right thing?
Is it because that's too much work (I think it requires
changing ProxyList::Get() to return a pointer so that it
can report failure)?

[eroman, 2010/01/07 21:08:19]

I don't understand what you mean by this.

[wtc, 2010/01/07 21:22:09]

Right now, you're asking the caller to call ProxyList::Get()
only if ProxyList is not empty.  Instead, I think we can
change ProxyList::Get() to return a pointer:
  const ProxyServer* Get() const;

Then, ProxyList::Get() can return NULL if ProxyList is
empty.  This way the caller can call ProxyList::Get()
without having to check if ProxyList is empty first.
This seems like a better fix to me, although I think
it's more intrusive.

+    if (is_empty())
+      return false;
+    return proxy_list_.Get().is_direct();
+  }
 
-  // Returns the first valid proxy server.
+  // Returns true if this proxy info has no proxies left to try.
+  bool is_empty() const {
+    return proxy_list_.IsEmpty();
+  }
+
+  // Returns the first valid proxy server. is_empty() must be false to be able
+  // to call this function.
   ProxyServer proxy_server() const { return proxy_list_.Get(); }
 
   // See description in ProxyList::ToPacString().
   std::string ToPacString();
 
   // Marks the current proxy as bad. Returns true if there is another proxy
   // available to try in proxy list_.
   bool Fallback(ProxyRetryInfoMap* proxy_retry_info) {
     return proxy_list_.Fallback(proxy_retry_info);
   }
 
   // Remove all proxies known to be bad from the proxy list.
   void RemoveBadProxies(const ProxyRetryInfoMap& proxy_retry_info) {
     proxy_list_.RemoveBadProxies(proxy_retry_info);
   }
 
   // Delete any entry which doesn't have one of the specified proxy schemes.
   void RemoveProxiesWithoutScheme(int scheme_bit_field) {
     proxy_list_.RemoveProxiesWithoutScheme(scheme_bit_field);
   }
 
  private:
   friend class ProxyService;
 
-  // If proxy_list_ is set to empty, then a "direct" connection is indicated.
+  // The ordered list of proxy servers (including DIRECT attempts) remaining to

[wtc, 2010/01/07 20:12:27]

Nit: the word "attempts" is confusing.  You can just remove it.

+  // try. If proxy_list_ is empty, then there is nothing left to fall back to.
   ProxyList proxy_list_;
 
   // This value identifies the proxy config used to initialize this object.
   ProxyConfig::ID config_id_;
-
-  // This flag is false when the proxy configuration was known to be bad when
-  // this proxy info was initialized.  In such cases, we know that if this
-  // proxy info does not yield a connection that we might want to reconsider
-  // the proxy config given by config_id_.
-  bool config_was_tried_;
 };
 
 }  // namespace net
 
 #endif  // NET_PROXY_PROXY_INFO_H_