Commit 1dd7e2a7 authored by Matt Menke's avatar Matt Menke Committed by Commit Bot

Fix NetworkContextConfigurationBrowserTest.DnsCacheIsolation flake.

The test makes sure that HostCache entries aren't shared between
NetworkContexts. To do this, it does a DNS lookup in one NetworkContext,
which succeeds, and then cache-only DNS lookups in other NetworkContexts,
which should fail. At the end of the test, it does a cache-only lookup
in the first NetworkContext, which should succeed. It's generally best
to do this at the end of the test, since it makes sure that entries
aren't only usable once, or somesuch. Unfortunately, sometimes the
DNS cache entry is timing out before the last lookup. To fix that,
this CL moves the final cache-only lookup to be right after the
initial lookup that populates the cache.

This does mean the test can still flakily pass, however, flaky successes
are more tolerable than flaky failures, and the test currently passes
often enough, even with the flake, that I think we can live with it.

Bug: 1043084
Change-Id: Ibdcf77e7775943e18c21a9ddd2cd3d5bf127c248
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2008058
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: default avatarEric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733036}
parent c5b3331b
......@@ -1137,22 +1137,10 @@ IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest,
EXPECT_EQ(kAddress,
result.resolved_addresses.value()[0].ToStringWithoutPort());
// Make a cache-only request for the same hostname, for each other network
// context, and make sure no result is returned.
ForEachOtherContext(
base::BindLambdaForTesting([&](NetworkContextType network_context_type) {
network::mojom::ResolveHostParametersPtr params =
network::mojom::ResolveHostParameters::New();
// Cache only lookup.
params->source = net::HostResolverSource::LOCAL_ONLY;
network::DnsLookupResult result = network::BlockingDnsLookup(
GetNetworkContextForContextType(network_context_type),
host_port_pair, std::move(params), network_isolation_key);
EXPECT_EQ(net::ERR_DNS_CACHE_MISS, result.error);
}));
// Do a cache-only lookup using the original network context, which should
// return the same result it initially did.
// return the same result it initially did. This is done immediately after the
// lookup, as a race mitigation to minimize the chance of the cache entry
// expiring before this check.
network::mojom::ResolveHostParametersPtr params =
network::mojom::ResolveHostParameters::New();
// Cache only lookup.
......@@ -1164,6 +1152,22 @@ IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest,
ASSERT_EQ(1u, result.resolved_addresses->size());
EXPECT_EQ(kAddress,
result.resolved_addresses.value()[0].ToStringWithoutPort());
// Make a cache-only request for the same hostname, for each other network
// context, and make sure no result is returned. It's possible for the entry
// to expire during this test run, so on regression, this step may flakily
// pass.
ForEachOtherContext(
base::BindLambdaForTesting([&](NetworkContextType network_context_type) {
network::mojom::ResolveHostParametersPtr params =
network::mojom::ResolveHostParameters::New();
// Cache only lookup.
params->source = net::HostResolverSource::LOCAL_ONLY;
network::DnsLookupResult result = network::BlockingDnsLookup(
GetNetworkContextForContextType(network_context_type),
host_port_pair, std::move(params), network_isolation_key);
EXPECT_EQ(net::ERR_DNS_CACHE_MISS, result.error);
}));
}
// Visits a URL with an HSTS header, and makes sure it is respected.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment