Concretely, remove:
ProxyConfig::id()
ProxyConfig::is_valid()
ProxyInfo::config_id()
And also remove ProxyResolutionService::ReconsiderProxyAfterError() which was the consumer of it.
The ProxyConfig::id() was an internal concept which relates a proxy configuration to a particular epoch for when it was fetched. However it is part of the public interface and is being abused by other layers (especially "is_valid()") to represent whether a ProxyConfig is optional or not, and using random IDs for the epoch.
This concept should be removed as it is confusing, and the underlying use-case for it is overly complicated (and was responsible for issue 723589). This awkwardness has come up during some recent refactorings for servicification, network traffic annotations, and cleanups as a source of confusion.
The feature that ProxyConfig::id supports today is to re-resolve the proxy for the URL in response to the proxy settings having changed since a previous attempt was made during fallback.
AFAICT there is no data showing this current mode to be useful, or why it is justified for such a narrow window. As I recall the original reason for doing it dates back to when notification of proxy settings was not async via a service, and hence there were more poll points to determine when settings had changed.
From a strict layering standpoint, it is cleaner to just do proxy resolution once, and for fallback to simply walk down that proxy list without the possibility of needing to do another asynchronous proxy resolve to get an entirely new list. This model is consistent with how we do things elsewhere in the stack (like connecting to one of many IP addresses from host resolution).
Comment 1 by eroman@chromium.org
, Feb 13 2018