Skip to content

Commit

Permalink
Revert "Browser test to exercise DoH against external servers"
Browse files Browse the repository at this point in the history
This reverts commit 261096c.

Reason for revert: Speculative revert of this CL since it may be causing many test failures.  See issue 1038953.

BUG=1038953

Original change's description:
> Browser test to exercise DoH against external servers
> 
> This change adds a non-hermetic manual browser test to verify
> Chrome's DoH implementation against a handful of external
> DoH servers. The test loops through each server, and
> attempts to load a test URL. Fallback to Do53 is
> disabled, so a successful navigation indicates that DoH passed.
> 
> A new setter set_allow_network_access_to_host_resolutions has
> been added to BrowserTestBase to allow browser tests to opt-in
> to using the full host resolver stack (instead of the mock host
> resolver). This mode then invokes
> SetAllowNetworkAccessToHostResolutions() on the NetworkServiceTest
> interface to disable the test host resolver.
> 
> Bug: 1032710
> Change-Id: I8e9e029e69a9e8281adb23e55179d9dd94707ef5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1961006
> Commit-Queue: Kaustubha Govind <kaustubhag@chromium.org>
> Reviewed-by: Eric Orth <ericorth@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#727778}

TBR=sky@chromium.org,ericorth@chromium.org,kaustubhag@chromium.org,dalyk@chromium.org

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

Bug: 1032710
Change-Id: I1e7628f22b55f9a884926b05e47fff25e2368a27
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1987274
Reviewed-by: Donn Denman <donnd@chromium.org>
Commit-Queue: Donn Denman <donnd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728374}
  • Loading branch information
Donn Denman authored and Commit Bot committed Jan 4, 2020
1 parent 39d6b89 commit 9338add
Show file tree
Hide file tree
Showing 6 changed files with 5 additions and 117 deletions.
74 changes: 0 additions & 74 deletions chrome/browser/net/dns_over_https_browsertest.cc

This file was deleted.

1 change: 0 additions & 1 deletion chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1014,7 +1014,6 @@ if (!is_android) {
"../browser/net/chrome_network_service_browsertest.cc",
"../browser/net/chrome_network_service_restart_browsertest.cc",
"../browser/net/cookie_policy_browsertest.cc",
"../browser/net/dns_over_https_browsertest.cc",
"../browser/net/dns_probe_browsertest.cc",
"../browser/net/errorpage_browsertest.cc",
"../browser/net/ftp_browsertest.cc",
Expand Down
20 changes: 5 additions & 15 deletions content/public/test/browser_test_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,7 @@ void BrowserTestBase::SetUp() {
// not affect the results.
command_line->AppendSwitchASCII(switches::kForceDisplayColorProfile, "srgb");

if (!allow_network_access_to_host_resolutions_)
test_host_resolver_ = std::make_unique<TestHostResolver>();
test_host_resolver_ = std::make_unique<TestHostResolver>();

ContentBrowserSanityChecker scoped_enable_sanity_checks;

Expand Down Expand Up @@ -752,26 +751,13 @@ void BrowserTestBase::InitializeNetworkProcess() {
return;

initialized_network_process_ = true;

host_resolver()->DisableModifications();

// Send the host resolver rules to the network service if it's in use. No need
// to do this if it's running in the browser process though.
if (!IsOutOfProcessNetworkService())
return;

mojo::Remote<network::mojom::NetworkServiceTest> network_service_test;
content::GetNetworkService()->BindTestInterface(
network_service_test.BindNewPipeAndPassReceiver());

// Do not set up host resolver rules if we allow the test to access
// the network.
if (allow_network_access_to_host_resolutions_) {
mojo::ScopedAllowSyncCallForTesting allow_sync_call;
network_service_test->SetAllowNetworkAccessToHostResolutions();
return;
}

net::RuleBasedHostResolverProc::RuleList rules = host_resolver()->GetRules();
std::vector<network::mojom::RulePtr> mojo_rules;
for (const auto& rule : rules) {
Expand Down Expand Up @@ -823,6 +809,10 @@ void BrowserTestBase::InitializeNetworkProcess() {
if (mojo_rules.empty())
return;

mojo::Remote<network::mojom::NetworkServiceTest> network_service_test;
content::GetNetworkService()->BindTestInterface(
network_service_test.BindNewPipeAndPassReceiver());

// Send the DNS rules to network service process. Android needs the RunLoop
// to dispatch a Java callback that makes network process to enter native
// code.
Expand Down
8 changes: 0 additions & 8 deletions content/public/test/browser_test_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,6 @@ class BrowserTestBase : public testing::Test {
// Sets expected browser exit code, in case it's different than 0 (success).
void set_expected_exit_code(int code) { expected_exit_code_ = code; }

// Sets flag to allow host resolutions to reach the network. Must be called
// before Setup() to take effect.
void set_allow_network_access_to_host_resolutions() {
allow_network_access_to_host_resolutions_ = true;
}

const net::SpawnedTestServer* spawned_test_server() const {
return spawned_test_server_.get();
}
Expand Down Expand Up @@ -219,8 +213,6 @@ class BrowserTestBase : public testing::Test {

bool initialized_network_process_ = false;

bool allow_network_access_to_host_resolutions_ = false;

#if defined(OS_POSIX)
bool handle_sigterm_;
#endif
Expand Down
14 changes: 0 additions & 14 deletions net/dns/dns_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -477,20 +477,6 @@ std::string GetDohProviderIdForHistogramFromNameserver(
return entries[0]->provider;
}

std::map<std::string, std::string> GetDohServerTemplatesListForTesting() {
const std::vector<DohUpgradeEntry>& upgradable_servers = GetDohUpgradeList();
std::map<std::string, std::string> server_templates;
for (const auto& upgrade_entry : upgradable_servers) {
auto return_val = server_templates.insert(
std::make_pair(upgrade_entry.provider,
upgrade_entry.dns_over_https_config.server_template));
// Check that the new element was inserted. The map's key is the DoH
// provider name which should be unique.
DCHECK(return_val.second);
}
return server_templates;
}

std::string SecureDnsModeToString(
const DnsConfig::SecureDnsMode secure_dns_mode) {
switch (secure_dns_mode) {
Expand Down
5 changes: 0 additions & 5 deletions net/dns/dns_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,6 @@ NET_EXPORT_PRIVATE std::string GetDohProviderIdForHistogramFromNameserver(
NET_EXPORT_PRIVATE std::string SecureDnsModeToString(
const DnsConfig::SecureDnsMode secure_dns_mode);

// Returns a map of DoH provider names and server templates
// from the auto-upgrade list for testing.
NET_EXPORT_PRIVATE std::map<std::string, std::string>
GetDohServerTemplatesListForTesting();

} // namespace net

#endif // NET_DNS_DNS_UTIL_H_

0 comments on commit 9338add

Please sign in to comment.