Commit 6113c487 authored by Matt Menke's avatar Matt Menke Committed by Commit Bot

Revert "Fix NetworkContextConfigurationBrowserTest.DnsCacheIsolation flake."

This reverts commit 1dd7e2a7.

Reason for revert: STill flaky.  Reverting both CLs.
NOTRY=true

Original change's description:
> 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: Eric Orth <ericorth@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#733036}

TBR=mmenke@chromium.org,ericorth@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1043084
Change-Id: Ia4bffaad8793e7f660807c39d8e0cdfa95d8766b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2012080Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733564}
parent 4f250a0f
...@@ -1137,26 +1137,8 @@ IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest, ...@@ -1137,26 +1137,8 @@ IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest,
EXPECT_EQ(kAddress, EXPECT_EQ(kAddress,
result.resolved_addresses.value()[0].ToStringWithoutPort()); result.resolved_addresses.value()[0].ToStringWithoutPort());
// Do a cache-only lookup using the original network context, which should
// 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.
params->source = net::HostResolverSource::LOCAL_ONLY;
result = network::BlockingDnsLookup(network_context(), host_port_pair,
std::move(params), network_isolation_key);
EXPECT_EQ(net::OK, result.error);
ASSERT_TRUE(result.resolved_addresses.has_value());
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 // 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 // context, and make sure no result is returned.
// to expire during this test run, so on regression, this step may flakily
// pass.
ForEachOtherContext( ForEachOtherContext(
base::BindLambdaForTesting([&](NetworkContextType network_context_type) { base::BindLambdaForTesting([&](NetworkContextType network_context_type) {
network::mojom::ResolveHostParametersPtr params = network::mojom::ResolveHostParametersPtr params =
...@@ -1168,6 +1150,20 @@ IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest, ...@@ -1168,6 +1150,20 @@ IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest,
host_port_pair, std::move(params), network_isolation_key); host_port_pair, std::move(params), network_isolation_key);
EXPECT_EQ(net::ERR_DNS_CACHE_MISS, result.error); 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.
network::mojom::ResolveHostParametersPtr params =
network::mojom::ResolveHostParameters::New();
// Cache only lookup.
params->source = net::HostResolverSource::LOCAL_ONLY;
result = network::BlockingDnsLookup(network_context(), host_port_pair,
std::move(params), network_isolation_key);
EXPECT_EQ(net::OK, result.error);
ASSERT_TRUE(result.resolved_addresses.has_value());
ASSERT_EQ(1u, result.resolved_addresses->size());
EXPECT_EQ(kAddress,
result.resolved_addresses.value()[0].ToStringWithoutPort());
} }
// Visits a URL with an HSTS header, and makes sure it is respected. // 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