Retry proxies which were cached as bad before giving up....

Retry proxies which were cached as bad before giving up.

This morphs ProxyList::RemoveBadProxies() into ProxyList::DeprioritizeBadProxies(), such that "bad proxies" are moved to the end of the fallback list rather than removed alltogether.

BUG=31983
TEST=ProxyListTest.DeprioritizeBadProxies


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=36054

[darin (slow to review), 2010-01-12 18:00:40]

nice, LGTM!

http://codereview.chromium.org/542029/diff/1/7
File net/proxy/proxy_list.cc (right):

http://codereview.chromium.org/542029/diff/1/7#newcode57
net/proxy/proxy_list.cc:57: proxies_ = good_proxies;
minor optimization: use .swap here.

[wtc, 2010-01-12 19:00:52]

LGTM.

http://codereview.chromium.org/542029/diff/1/8
File net/http/http_network_transaction.cc (right):

http://codereview.chromium.org/542029/diff/1/8#newcode616
net/http/http_network_transaction.cc:616: return ERR_EMPTY_PROXY_LIST;
Nit: I still think this error code should be renamed to be
more descriptive of the error condition ("we don't support
any of the proxies in the returned list").  The name
ERR_EMPTY_PROXY_LIST implies that the returned list is empty.

http://codereview.chromium.org/542029/diff/1/2
File net/proxy/proxy_info.h (right):

http://codereview.chromium.org/542029/diff/1/2#newcode74
net/proxy/proxy_info.h:74: proxy_list_.ReorderBadProxies(proxy_retry_info);
Nit: perhaps "Demote" or "Deprioritize" would be more
self-documenting than "Reorder"?

[eroman, 2010-01-12 20:10:00]

Thankyou for the reviews.

http://codereview.chromium.org/542029/diff/1/8
File net/http/http_network_transaction.cc (right):

http://codereview.chromium.org/542029/diff/1/8#newcode616
net/http/http_network_transaction.cc:616: return ERR_EMPTY_PROXY_LIST;
On 2010/01/12 19:00:52, wtc wrote:
> Nit: I still think this error code should be renamed to be
> more descriptive of the error condition ("we don't support
> any of the proxies in the returned list").  The name
> ERR_EMPTY_PROXY_LIST implies that the returned list is empty.

Renamed ERR_EMPTY_PROXY_LIST --> ERR_NO_SUPPORTED_PROXIES.

(Note I hadn't addressed your review comment earlier since I wanted to first get
rid of the other cases when it could be empty, i.e. when all the "bad" proxies
had been removed. Thankyou for your patience).

http://codereview.chromium.org/542029/diff/1/2
File net/proxy/proxy_info.h (right):

http://codereview.chromium.org/542029/diff/1/2#newcode74
net/proxy/proxy_info.h:74: proxy_list_.ReorderBadProxies(proxy_retry_info);
On 2010/01/12 19:00:52, wtc wrote:
> Nit: perhaps "Demote" or "Deprioritize" would be more
> self-documenting than "Reorder"?

Done. Renamed to DeprioritizeBadProxies().

http://codereview.chromium.org/542029/diff/1/7
File net/proxy/proxy_list.cc (right):

http://codereview.chromium.org/542029/diff/1/7#newcode57
net/proxy/proxy_list.cc:57: proxies_ = good_proxies;
On 2010/01/12 18:00:40, darin wrote:
> minor optimization: use .swap here.

Done.