Skip to content

Commit

Permalink
Add a struct of optional params to net/ host resolution API.
Browse files Browse the repository at this point in the history 
Everything in the struct should have a reasonable default value and the
struct param itself is optional using base::Optional.

Include DNS qtype and initial priority support in the new parameter.
Lots of tests added in now that qtype and priority are supported by the
new API.

I'll add a similar struct of optional params to the mojo API in the next
CL.

Bug: 821021
Cq-Include-Trybots: luci.chromium.try:linux_mojo;master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I4b628364e53f1c3fdf34819071b6562eaedc9c14
Reviewed-on: https://chromium-review.googlesource.com/1171574
Commit-Queue: Eric Orth <ericorth@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583418}
  • Loading branch information
Eric Orth authored and Commit Bot committed Aug 15, 2018
1 parent 5a7c36f commit 00fe5a6
Show file tree
Hide file tree
Showing 19 changed files with 756 additions and 331 deletions.
6 changes: 4 additions & 2 deletions components/cronet/stale_host_resolver.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,10 @@ StaleHostResolver::StaleHostResolver(
StaleHostResolver::~StaleHostResolver() {}

std::unique_ptr<net::HostResolver::ResolveHostRequest>
StaleHostResolver::CreateRequest(const net::HostPortPair& host,
const net::NetLogWithSource& net_log) {
StaleHostResolver::CreateRequest(
const net::HostPortPair& host,
const net::NetLogWithSource& net_log,
const base::Optional<ResolveHostParameters>& optional_parameters) {
// TODO(crbug.com/821021): Implement.
NOTIMPLEMENTED();
return nullptr;
Expand Down
4 changes: 3 additions & 1 deletion components/cronet/stale_host_resolver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ class StaleHostResolver : public net::HostResolver {

std::unique_ptr<ResolveHostRequest> CreateRequest(
const net::HostPortPair& host,
const net::NetLogWithSource& net_log) override;
const net::NetLogWithSource& net_log,
const base::Optional<ResolveHostParameters>& optional_parameters)
override;

// Resolves as a regular HostResolver, but if stale data is available and
// usable (according to the options passed to the constructor), and fresh data
Expand Down
47 changes: 47 additions & 0 deletions net/dns/host_resolver.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,53 @@ std::unique_ptr<HostResolverImpl> HostResolver::CreateDefaultResolverImpl(
return CreateSystemResolverImpl(Options(), net_log);
}

// static
AddressFamily HostResolver::DnsQueryTypeToAddressFamily(
DnsQueryType dns_query_type) {
switch (dns_query_type) {
case DnsQueryType::UNSPECIFIED:
return ADDRESS_FAMILY_UNSPECIFIED;
case DnsQueryType::A:
return ADDRESS_FAMILY_IPV4;
case DnsQueryType::AAAA:
return ADDRESS_FAMILY_IPV6;
default:
// |dns_query_type| should be an address type (A or AAAA) or UNSPECIFIED.
NOTREACHED();
return ADDRESS_FAMILY_UNSPECIFIED;
}
}

// static
HostResolver::DnsQueryType HostResolver::AddressFamilyToDnsQueryType(
AddressFamily address_family) {
switch (address_family) {
case ADDRESS_FAMILY_UNSPECIFIED:
return DnsQueryType::UNSPECIFIED;
case ADDRESS_FAMILY_IPV4:
return DnsQueryType::A;
case ADDRESS_FAMILY_IPV6:
return DnsQueryType::AAAA;
default:
NOTREACHED();
return DnsQueryType::UNSPECIFIED;
}
}

// static
HostResolver::ResolveHostParameters
HostResolver::RequestInfoToResolveHostParameters(
const HostResolver::RequestInfo& request_info,
RequestPriority priority) {
ResolveHostParameters parameters;

parameters.dns_query_type =
AddressFamilyToDnsQueryType(request_info.address_family());
parameters.initial_priority = priority;

return parameters;
}

HostResolver::HostResolver() = default;

} // namespace net
45 changes: 44 additions & 1 deletion net/dns/host_resolver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <string>
#include <vector>

#include "base/optional.h"
#include "net/base/address_family.h"
#include "net/base/completion_once_callback.h"
#include "net/base/host_port_pair.h"
Expand Down Expand Up @@ -113,6 +114,9 @@ class NET_EXPORT HostResolver {

// The parameters for doing a Resolve(). A hostname and port are
// required; the rest are optional (and have reasonable defaults).
//
// TODO(crbug.com/821021): Delete this class once all usage has been
// converted to the new CreateRequest() API.
class NET_EXPORT RequestInfo {
public:
explicit RequestInfo(const HostPortPair& host_port_pair);
Expand Down Expand Up @@ -171,6 +175,29 @@ class NET_EXPORT HostResolver {
bool is_my_ip_address_;
};

// DNS query type for a ResolveHostRequest.
// See:
// https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-4
//
// TODO(crbug.com/846423): Add support for non-address types.
enum class DnsQueryType {
UNSPECIFIED,
A,
AAAA,
};

// Parameter-grouping struct for additional optional parameters for
// CreateRequest() calls. All fields are optional and have a reasonable
// default.
struct ResolveHostParameters {
// Requested DNS query type. If UNSPECIFIED, resolver will pick A or AAAA
// (or both) based on IPv4/IPv6 settings.
DnsQueryType dns_query_type = DnsQueryType::UNSPECIFIED;

// The initial net priority for the host resolution request.
RequestPriority initial_priority = RequestPriority::DEFAULT_PRIORITY;
};

// Set Options.max_concurrent_resolves to this to select a default level
// of concurrency.
static const size_t kDefaultParallelism = 0;
Expand All @@ -186,14 +213,18 @@ class NET_EXPORT HostResolver {
// Creates a request to resolve the given hostname (or IP address literal).
// Profiling information for the request is saved to |net_log| if non-NULL.
//
// Additional parameters may be set using |optional_parameters|. Reasonable
// defaults will be used if passed |base::nullopt|.
//
// This method is intended as a direct replacement for the old Resolve()
// method, but it may not yet cover all the capabilities of the old method.
//
// TODO(crbug.com/821021): Implement more complex functionality to meet
// capabilities of Resolve() and M/DnsClient functionality.
virtual std::unique_ptr<ResolveHostRequest> CreateRequest(
const HostPortPair& host,
const NetLogWithSource& net_log) = 0;
const NetLogWithSource& net_log,
const base::Optional<ResolveHostParameters>& optional_parameters) = 0;

// DEPRECATION NOTE: This method is being replaced by CreateRequest(). New
// callers should prefer CreateRequest() if it works for their needs.
Expand Down Expand Up @@ -300,6 +331,18 @@ class NET_EXPORT HostResolver {
static std::unique_ptr<HostResolverImpl> CreateDefaultResolverImpl(
NetLog* net_log);

static AddressFamily DnsQueryTypeToAddressFamily(DnsQueryType query_type);

// Helpers for converting old Resolve() API parameters to new CreateRequest()
// parameters.
//
// TODO(crbug.com/821021): Delete these methods once all usage has been
// converted to the new CreateRequest() API.
static DnsQueryType AddressFamilyToDnsQueryType(AddressFamily address_family);
static ResolveHostParameters RequestInfoToResolveHostParameters(
const RequestInfo& request_info,
RequestPriority priority);

protected:
HostResolver();

Expand Down
71 changes: 38 additions & 33 deletions net/dns/host_resolver_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -617,35 +617,34 @@ class HostResolverImpl::RequestImpl
public:
RequestImpl(const NetLogWithSource& source_net_log,
const HostPortPair& request_host,
const base::Optional<ResolveHostParameters>& optional_parameters,
bool is_speculative,
RequestPriority priority,
base::WeakPtr<HostResolverImpl> resolver)
: RequestImpl(source_net_log,
request_host,
ADDRESS_FAMILY_UNSPECIFIED,
optional_parameters,
0 /* host_resolver_flags */,
true /* allow_cached_response */,
is_speculative,
priority,
resolver) {}

// Overload for use by the legacy Resolve() API. Has more advanced parameters
// not yet supported by the CreateRequest() API.
RequestImpl(const NetLogWithSource& source_net_log,
const HostPortPair& request_host,
AddressFamily address_family,
const base::Optional<ResolveHostParameters>& optional_parameters,
HostResolverFlags host_resolver_flags,
bool allow_cached_response,
bool is_speculative,
RequestPriority priority,
base::WeakPtr<HostResolverImpl> resolver)
: source_net_log_(source_net_log),
request_host_(request_host),
address_family_(address_family),
host_resolver_flags_(host_resolver_flags),
allow_cached_response_(allow_cached_response),
is_speculative_(is_speculative),
priority_(priority),
parameters_(optional_parameters ? optional_parameters.value()
: ResolveHostParameters()),
priority_(parameters_.initial_priority),
job_(nullptr),
resolver_(resolver),
complete_(false) {}
Expand Down Expand Up @@ -730,14 +729,14 @@ class HostResolverImpl::RequestImpl

const HostPortPair& request_host() const { return request_host_; }

AddressFamily address_family() const { return address_family_; }

HostResolverFlags host_resolver_flags() const { return host_resolver_flags_; }

bool allow_cached_response() const { return allow_cached_response_; }

bool is_speculative() const { return is_speculative_; }

const ResolveHostParameters& parameters() const { return parameters_; }

RequestPriority priority() const { return priority_; }
void set_priority(RequestPriority priority) { priority_ = priority; }

Expand All @@ -757,10 +756,10 @@ class HostResolverImpl::RequestImpl
const NetLogWithSource source_net_log_;

const HostPortPair request_host_;
const AddressFamily address_family_;
const HostResolverFlags host_resolver_flags_;
const bool allow_cached_response_;
const bool is_speculative_;
const ResolveHostParameters parameters_;

RequestPriority priority_;

Expand Down Expand Up @@ -2141,11 +2140,13 @@ void HostResolverImpl::SetDnsClient(std::unique_ptr<DnsClient> dns_client) {
}

std::unique_ptr<HostResolver::ResolveHostRequest>
HostResolverImpl::CreateRequest(const HostPortPair& host,
const NetLogWithSource& net_log) {
return std::make_unique<RequestImpl>(
net_log, host, false /* is_speculative */,
RequestPriority::DEFAULT_PRIORITY, weak_ptr_factory_.GetWeakPtr());
HostResolverImpl::CreateRequest(
const HostPortPair& host,
const NetLogWithSource& net_log,
const base::Optional<ResolveHostParameters>& optional_parameters) {
return std::make_unique<RequestImpl>(net_log, host, optional_parameters,
false /* is_speculative */,
weak_ptr_factory_.GetWeakPtr());
}

int HostResolverImpl::Resolve(const RequestInfo& info,
Expand All @@ -2160,9 +2161,10 @@ int HostResolverImpl::Resolve(const RequestInfo& info,
DCHECK(out_req);

auto request = std::make_unique<RequestImpl>(
source_net_log, info.host_port_pair(), info.address_family(),
source_net_log, info.host_port_pair(),
RequestInfoToResolveHostParameters(info, priority),
info.host_resolver_flags(), info.allow_cached_response(),
info.is_speculative(), priority, weak_ptr_factory_.GetWeakPtr());
info.is_speculative(), weak_ptr_factory_.GetWeakPtr());
auto wrapped_request =
std::make_unique<LegacyRequestImpl>(std::move(request));

Expand Down Expand Up @@ -2190,9 +2192,10 @@ int HostResolverImpl::ResolveFromCache(const RequestInfo& info,

Key key;
int rv = ResolveLocally(
info.host_port_pair(), info.address_family(), info.host_resolver_flags(),
info.allow_cached_response(), false /* allow_stale */,
nullptr /* stale_info */, source_net_log, addresses, &key);
info.host_port_pair(), AddressFamilyToDnsQueryType(info.address_family()),
info.host_resolver_flags(), info.allow_cached_response(),
false /* allow_stale */, nullptr /* stale_info */, source_net_log,
addresses, &key);

LogFinishRequest(source_net_log, rv);
return rv;
Expand All @@ -2211,10 +2214,10 @@ int HostResolverImpl::ResolveStaleFromCache(
LogStartRequest(source_net_log, info);

Key key;
int rv = ResolveLocally(info.host_port_pair(), info.address_family(),
info.host_resolver_flags(),
info.allow_cached_response(), true /* allow_stale */,
stale_info, source_net_log, addresses, &key);
int rv = ResolveLocally(
info.host_port_pair(), AddressFamilyToDnsQueryType(info.address_family()),
info.host_resolver_flags(), info.allow_cached_response(),
true /* allow_stale */, stale_info, source_net_log, addresses, &key);
LogFinishRequest(source_net_log, rv);
return rv;
}
Expand Down Expand Up @@ -2349,11 +2352,11 @@ int HostResolverImpl::Resolve(RequestImpl* request) {

AddressList addresses;
Key key;
int rv = ResolveLocally(request->request_host(), request->address_family(),
request->host_resolver_flags(),
request->allow_cached_response(),
false /* allow_stale */, nullptr /* stale_info */,
request->source_net_log(), &addresses, &key);
int rv = ResolveLocally(
request->request_host(), request->parameters().dns_query_type,
request->host_resolver_flags(), request->allow_cached_response(),
false /* allow_stale */, nullptr /* stale_info */,
request->source_net_log(), &addresses, &key);
if (rv == OK) {
request->set_address_results(
EnsurePortOnAddressList(addresses, request->request_host().port()));
Expand All @@ -2373,7 +2376,7 @@ int HostResolverImpl::Resolve(RequestImpl* request) {
}

int HostResolverImpl::ResolveLocally(const HostPortPair& host,
AddressFamily requested_address_family,
DnsQueryType dns_query_type,
HostResolverFlags flags,
bool allow_cache,
bool allow_stale,
Expand All @@ -2393,7 +2396,7 @@ int HostResolverImpl::ResolveLocally(const HostPortPair& host,

// Build a key that identifies the request in the cache and in the
// outstanding jobs map.
*key = GetEffectiveKeyForRequest(host.host(), requested_address_family, flags,
*key = GetEffectiveKeyForRequest(host.host(), dns_query_type, flags,
ip_address_ptr, source_net_log);

DCHECK(allow_stale == !!stale_info);
Expand Down Expand Up @@ -2620,12 +2623,14 @@ std::unique_ptr<HostResolverImpl::Job> HostResolverImpl::RemoveJob(Job* job) {

HostResolverImpl::Key HostResolverImpl::GetEffectiveKeyForRequest(
const std::string& hostname,
AddressFamily requested_address_family,
DnsQueryType dns_query_type,
HostResolverFlags flags,
const IPAddress* ip_address,
const NetLogWithSource& net_log) {
HostResolverFlags effective_flags = flags | additional_resolver_flags_;
AddressFamily effective_address_family = requested_address_family;

AddressFamily effective_address_family =
DnsQueryTypeToAddressFamily(dns_query_type);

if (effective_address_family == ADDRESS_FAMILY_UNSPECIFIED &&
// When resolving IPv4 literals, there's no need to probe for IPv6.
Expand Down
8 changes: 5 additions & 3 deletions net/dns/host_resolver_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,9 @@ class NET_EXPORT HostResolverImpl
// HostResolver methods:
std::unique_ptr<ResolveHostRequest> CreateRequest(
const HostPortPair& host,
const NetLogWithSource& net_log) override;
const NetLogWithSource& net_log,
const base::Optional<ResolveHostParameters>& optional_parameters)
override;
int Resolve(const RequestInfo& info,
RequestPriority priority,
AddressList* addresses,
Expand Down Expand Up @@ -228,7 +230,7 @@ class NET_EXPORT HostResolverImpl
// If |allow_stale| is false, then stale cache entries will not be returned,
// and |stale_info| must be null.
int ResolveLocally(const HostPortPair& host,
AddressFamily requested_address_family,
DnsQueryType requested_address_family,
HostResolverFlags flags,
bool allow_cache,
bool allow_stale,
Expand Down Expand Up @@ -283,7 +285,7 @@ class NET_EXPORT HostResolverImpl
// "effective" address family by inheriting the resolver's default address
// family when the request leaves it unspecified.
Key GetEffectiveKeyForRequest(const std::string& hostname,
AddressFamily requested_address_family,
DnsQueryType dns_query_type,
HostResolverFlags flags,
const IPAddress* ip_address,
const NetLogWithSource& net_log);
Expand Down
Loading

0 comments on commit 00fe5a6

Please sign in to comment.