Skip to content

Commit

Permalink
Remove optionality from result lists in HostCache::Entry
Browse files Browse the repository at this point in the history
The ability to track nullopt/nullptr vs empty is soon going away as a
simplification with the switch to HostResolverInternalResult, so soon
anything going through HostResolverInternalResult will just return
empty lists for what would have previously been nullopt or nullptr.
Make this change now with HostCache::Entry too, just to separate off
all of the test/plumbing churn that would occur when the switch is
made to start converting some internal results to
HostResolverInternalResult.

This is technically a behavior change for the overall HostResolver
API (and mojo API), because while I'm leaving things nullable/optional
in those APIs (to be cleaned up in a future CL), they may now sometimes
now return empty instead of null, depending on whether or not results
went through HostCache::Entry. But the behavior change doesn't actually
matter because nothing outside of tests was ever actually calling the
Get*Results() methods in situations where it would have been
nullopt/nullptr (e.g. nobody was calling GetTextResults() after address
requests).

Bug: 1381506, 1290920
Change-Id: If31219449d4a93b2e5071d5001720c3333a6198c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4545558
Auto-Submit: Eric Orth <ericorth@chromium.org>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Commit-Queue: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1146488}
  • Loading branch information
Eric Orth authored and Chromium LUCI CQ committed May 19, 2023
1 parent 2f4d81e commit c60143b
Show file tree
Hide file tree
Showing 15 changed files with 1,408 additions and 1,080 deletions.
12 changes: 5 additions & 7 deletions components/cronet/stale_host_resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,8 @@ class StaleHostResolver::RequestImpl
int Start(net::CompletionOnceCallback result_callback) override;
const net::AddressList* GetAddressResults() const override;
const net::HostResolverEndpointResults* GetEndpointResults() const override;
const absl::optional<std::vector<std::string>>& GetTextResults()
const override;
const absl::optional<std::vector<net::HostPortPair>>& GetHostnameResults()
const override;
const std::vector<std::string>* GetTextResults() const override;
const std::vector<net::HostPortPair>* GetHostnameResults() const override;
const std::set<std::string>* GetDnsAliasResults() const override;
net::ResolveErrorInfo GetResolveErrorInfo() const override;
const absl::optional<net::HostCache::EntryStaleness>& GetStaleInfo()
Expand Down Expand Up @@ -201,16 +199,16 @@ StaleHostResolver::RequestImpl::GetEndpointResults() const {
return cache_request_->GetEndpointResults();
}

const absl::optional<std::vector<std::string>>&
StaleHostResolver::RequestImpl::GetTextResults() const {
const std::vector<std::string>* StaleHostResolver::RequestImpl::GetTextResults()
const {
if (network_request_)
return network_request_->GetTextResults();

DCHECK(cache_request_);
return cache_request_->GetTextResults();
}

const absl::optional<std::vector<net::HostPortPair>>&
const std::vector<net::HostPortPair>*
StaleHostResolver::RequestImpl::GetHostnameResults() const {
if (network_request_)
return network_request_->GetHostnameResults();
Expand Down
324 changes: 151 additions & 173 deletions net/dns/dns_response_result_extractor_unittest.cc

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion net/dns/dns_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1088,7 +1088,7 @@ class DnsOverHttpsProbeRunner : public DnsProbeRunner {

if (extraction_error ==
DnsResponseResultExtractor::ExtractionError::kOk &&
results.ip_endpoints() && !results.ip_endpoints()->empty()) {
!results.ip_endpoints().empty()) {
// The DoH probe queries don't go through the standard DnsAttempt
// path, so the ServerStats have not been updated yet.
context_->RecordServerSuccess(
Expand Down

0 comments on commit c60143b

Please sign in to comment.