There's another question here:  When a network request uses LOAD_BYPASS_PROXY, should DoH bypass the proxy, too?  It's not entirely clear what the caller's intention is with the flag.

How are proxies being configured for DoH?

DoH uses the system NetworkContext to make normal requests.  Which brings up another issue - should we be using the NetworkContext that needs to do the DNS lookup instead?  If so, that would require per-NetworContext HostResolvers.

Yes, the host resolver should be scoped to the NetworkContext using it. And its DoH requests should go through that NetworkContext and its proxy settings.

Except the request for the proxy's address itself...which means DoH needs to know it's looking up the proxy's host name.

Since we don't have a list of proxies (For the PAC case, at least), that means we'd probably need a way to tell HostResolverImpl to bypass DoH for a particular lookup, and that such lookups must not be merged with other lookups with that option not set.

Same goes for certificate network fetches (if we were doing the right thing, which we aren't when using platform verifiers).

To bypass DoH, can we set HOST_RESOLVER_SYSTEM_ONLY?  I think this avoids AsyncDNS which includes DoH.  I think it also avoids merging jobs or cache entries with requests without that flag set.

That works at the DNS layer, but the issue is getting it there.  If we add a load flag, and push it all the way down to the socket pool layer, we'd have to bifurcate socket pools to do it.

When you say "looking up a proxy's host name" do you mean "resolving the proxy's host name to an IP address"?

Rather than plumb it through the socket pools and bifurcate them, I could imagine a hacky solution where we always attach HOST_RESOLVER_SYSTEM_ONLY to DNS requests for the proxy.  I don't know how this would work for PAC though where the proxy isn't readily known, though perhaps there's some other way to identify those requests.

Sorry, there are two issues:

* Requests for the PAC script, which makes HTTP requests, rather than doing DNS lookups.
* Connections to proxies.  These go HTTP proxy socket pool -> SSL socket pool -> TCP socket pool.

Also, proxies can change - which means we may have pre-existing normal DNS lookups for a hostname.

The hack I mentioned would work like so: as ConnectJob goes to create HostResolverImpl request, compare host being resolved to PAC host name and to some list of HTTP proxies (i.e. if system is configured with one HTTP proxy, then that proxy's host name; with PACs, keep some list of all proxies returned by the PAC script) and set HOST_RESOLVER_SYSTEM_ONLY if this comparison matched.  Redirects would be uglier as they'd need to add the hostname redirected to into the list of HTTP proxies.

As for handling when proxies change, shouldn't we abort all requests (and ConnectJobs and DNS requests) at this point so there won't be any pre-existing normal DNS lookups?

If we do it before we merge requests into the same job at the HostResolver layer, that works for PAC scripts (Assuming we split HostResolvers to be per-NetworkContext), modulo redirects, as you say.

We don't abort all DNS requests when proxies change - I don't think we even abort all HTTP requests.

Eric, can you explain the reasoning behind your earlier comment, "Yes, the host resolver should be scoped to the NetworkContext using it. And its DoH requests should go through that NetworkContext and its proxy settings."?

Proxy settings are per-NetworkContext, not global.  DoH requests should go through proxies, so they then need to be per-NetworkContext, too.

I was talking about this with bemasc@, who raised this point: if there is a proxy configured, wouldn't the DNS lookup for the eventual destination performed by the proxy?

Not with socks4.  I'm also not sure about H2 (Which SSL connections to different servers, if available).  There are also direct lookups, which are done by various consumers in Chrome (With PAC scripts being the most obvious).

Err...by PAC scripts, I mean DNS lookups done by PAC scripts, not DNS lookups for the host serving the PAC scripts.

Also, I think SSL uses DNS for something (Cert Transparency logs?).

Regarding using HOST_RESOLVER_SYSTEM_ONLY to disable DOH: This would work (at the DNS layer) but I don't think its ideal.  Eg, such requests should probably still use non-DOH async resolution when otherwise enabled.  I would be in favor of a separate doh-specific option.  |skip_secure_dns| proposed in the DOH design sounds reasonable to me and could be set for these requests.  Should be fairly easy to implement.

Regarding per-context HostResolver instances: I can see the strong argument to separate config and cache, but another big responsibility of HostResolver is task scheduling.  Not breaking crappy routers with too many concurrent DNS requests is naturally a global resource.  Should we maybe work to have one system HostResolver with per-context config and cache? Or, alternately, maybe the separate per-context resolvers are locked to only handle DOH (since I'm assuming DOH-enabled servers will be much more resilient than your standard crappy router), and either enforce that callers use the correct one or delegate when needed from per-context resolvers to the system resolver.

For anyone following this bug but not dalyk@'s DoH document, the current proposal there (I'm summarizing):
*HostResolver lookups for the proxy hostname should set a |skip_proxy| flag (to be added to HostResolver) to signal that flags should be set to bypass the proxy for DoH requests.
*To handle proxied contexts, keep a single global HostResolver, but instead of containing a single URLRequestContext for handling DoH requests, context will be set per-request (possibly using a wrapper that automatically uses the context from which the HostResolver was retrieved to avoid making all callers actually specify the context).
*Single shared HostCache but adding new is_doh and proxy_url values to the cache key to separate off those results.

Eric: Do you have a link to this document? Got linked to this bug from a CL, so I suspect there's missing context.

The doc Eric is talking about is the DoH setting design doc (internal only: https://docs.google.com/document/d/1ifeqS0VJ0AFo76ubhmgEcM3khFbK7Xt5tBV0yOFKpIU/edit?usp=sharing).

Is there a public doc for sharing?

Not currently. I think my favored approach would be to take specific aspects of the DoH design to public lists/bugs when wider input would be helpful. I'm not convinced that it would be worth the effort to maintain a public version of the entire design doc scrubbed of any product/legal discussions that should remain internal.