Reland: "Allow content shell to enable the built-in DNS resolver"

The original CL (https://crrev.com/c/5224044) was reverted because
it made some tests flaky. The diff from the original CL is not to
call NetworkService::ConfigureStubHostResolver() when running
browser_tests to work around the flakiness.

Not that the following original commit message is outdated because
this CL isn't useful to write browser_tests any longer (but it's
useful for running content_shell).

[Original commit message]
> Allow content shell to enable the built-in DNS resolver
>
> It's useful for local testing and writing browser_tests in //content.
>
> Move the feature flag from //chrome to //net so that
> ShellContentBrowserClient can use it to determine using the built-in
> DNS resolver.

Bug: chromium:1521190
Change-Id: Ic02c1a69f5f302aaa346654393579946b9053de4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5232354
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1253657}

chrome/browser/about_flags.cc

@@ -6820,7 +6820,7 @@ const FeatureEntry kFeatureEntries[] = {
 #if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX)
     {"enable-async-dns", flag_descriptions::kAsyncDnsName,
      flag_descriptions::kAsyncDnsDescription, kOsWin | kOsLinux,
-     FEATURE_VALUE_TYPE(features::kAsyncDns)},
+     FEATURE_VALUE_TYPE(net::features::kAsyncDns)},
 #endif  // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX)
 
     {"downloads-migrate-to-jobs-api",

chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_private_browsertest.cc

@@ -32,6 +32,7 @@
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test.h"
 #include "extensions/browser/api_test_utils.h"
+#include "net/base/features.h"
 #include "net/cert/x509_certificate.h"
 #include "net/cert/x509_util.h"
 #include "net/ssl/client_cert_identity_test_util.h"
@@ -162,7 +163,7 @@ class EnterpriseReportingPrivateGetContextInfoBrowserTest
     if (browser_managed()) {
       SetupDMToken();
     }
-    feature_list_.InitAndEnableFeature(features::kAsyncDns);
+    feature_list_.InitAndEnableFeature(net::features::kAsyncDns);
   }
 
   bool browser_managed() const { return testing::get<0>(GetParam()); }

chrome/browser/net/stub_resolver_config_reader.cc

@@ -31,6 +31,7 @@
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/network_service_instance.h"
+#include "net/base/features.h"
 #include "net/dns/public/dns_over_https_config.h"
 #include "net/dns/public/secure_dns_mode.h"
 #include "net/dns/public/util.h"
@@ -92,13 +93,13 @@ bool ShouldDisableDohForWindowsParentalControls() {
 bool ShouldEnableAsyncDns() {
   bool feature_can_be_enabled = true;
 #if BUILDFLAG(IS_ANDROID)
-  int min_sdk =
-      base::GetFieldTrialParamByFeatureAsInt(features::kAsyncDns, "min_sdk", 0);
+  int min_sdk = base::GetFieldTrialParamByFeatureAsInt(net::features::kAsyncDns,
+                                                       "min_sdk", 0);
   if (base::android::BuildInfo::GetInstance()->sdk_int() < min_sdk)
     feature_can_be_enabled = false;
 #endif
   return feature_can_be_enabled &&
-         base::FeatureList::IsEnabled(features::kAsyncDns);
+         base::FeatureList::IsEnabled(net::features::kAsyncDns);
 }
 
 }  // namespace

chrome/browser/net/stub_resolver_config_reader_browsertest.cc

@@ -26,6 +26,7 @@
 #include "components/policy/policy_constants.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/test/browser_test.h"
+#include "net/base/features.h"
 #include "net/dns/public/secure_dns_mode.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -45,7 +46,8 @@ class StubResolverConfigReaderBrowsertest
       public testing::WithParamInterface<bool> {
  public:
   StubResolverConfigReaderBrowsertest() {
-    scoped_feature_list_.InitWithFeatureState(features::kAsyncDns, GetParam());
+    scoped_feature_list_.InitWithFeatureState(net::features::kAsyncDns,
+                                              GetParam());
   }
   ~StubResolverConfigReaderBrowsertest() override = default;
 

chrome/common/chrome_features.cc

@@ -74,17 +74,6 @@ BASE_FEATURE(kAppShimNotificationAttribution,
              base::FEATURE_DISABLED_BY_DEFAULT);
 #endif  // BUILDFLAG(IS_MAC)
 
-// Enables the built-in DNS resolver.
-BASE_FEATURE(kAsyncDns,
-             "AsyncDns",
-#if BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_ANDROID) || \
-    BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX)
-             base::FEATURE_ENABLED_BY_DEFAULT
-#else
-             base::FEATURE_DISABLED_BY_DEFAULT
-#endif
-);
-
 #if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) || \
     BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_FUCHSIA)
 // Enables or disables the Autofill survey triggered by opening a prompt to

chrome/common/chrome_features.h

@@ -57,8 +57,6 @@ COMPONENT_EXPORT(CHROME_FEATURES)
 BASE_DECLARE_FEATURE(kAppShimNotificationAttribution);
 #endif  // BUILDFLAG(IS_MAC)
 
-COMPONENT_EXPORT(CHROME_FEATURES) BASE_DECLARE_FEATURE(kAsyncDns);
-
 #if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) || \
     BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_FUCHSIA)
 COMPONENT_EXPORT(CHROME_FEATURES) BASE_DECLARE_FEATURE(kAutofillAddressSurvey);

content/public/test/content_browser_test_content_browser_client.cc

@@ -39,4 +39,12 @@ bool ContentBrowserTestContentBrowserClient::CreateThreadPool(
   return true;
 }
 
+void ContentBrowserTestContentBrowserClient::OnNetworkServiceCreated(
+    network::mojom::NetworkService* network_service) {
+  // Override ShellContentBrowserClient::OnNetworkServiceCreated() not to call
+  // NetworkService::ConfigureStubHostResolver(), because some tests are flaky
+  // when configuring the stub host resolver.
+  // TODO(crbug.com/1521190): Remove this override once the flakiness is fixed.
+}
+
 }  // namespace content

content/public/test/content_browser_test_content_browser_client.h

@@ -20,6 +20,8 @@ class ContentBrowserTestContentBrowserClient
   ~ContentBrowserTestContentBrowserClient() override;
 
   bool CreateThreadPool(base::StringPiece name) override;
+  void OnNetworkServiceCreated(
+      network::mojom::NetworkService* network_service) override;
 };
 
 }  // namespace content

content/shell/browser/shell_content_browser_client.cc

@@ -77,6 +77,9 @@
 #include "media/mojo/mojom/media_service.mojom.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
+#include "net/base/features.h"
+#include "net/dns/public/dns_over_https_config.h"
+#include "net/dns/public/secure_dns_mode.h"
 #include "net/ssl/client_cert_identity.h"
 #include "services/device/public/cpp/geolocation/location_system_permission_status.h"
 #include "services/network/public/cpp/features.h"
@@ -705,6 +708,25 @@ void ShellContentBrowserClient::GetAdditionalMappedFilesForChildProcess(
 #endif  // BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) ||
         // BUILDFLAG(IS_ANDROID)
 
+// Note that ShellContentBrowserClient overrides this method to work around
+// test flakiness that happens when NetworkService::SetTestDohConfigForTesting()
+// is used.
+// TODO(crbug.com/1521190): Remove that override once the flakiness is fixed.
+void ShellContentBrowserClient::OnNetworkServiceCreated(
+    network::mojom::NetworkService* network_service) {
+  // TODO(bashi): Consider enabling this for Android. Excluded because the
+  // built-in resolver may not work on older SDK versions.
+#if !BUILDFLAG(IS_ANDROID)
+  if (base::FeatureList::IsEnabled(net::features::kAsyncDns)) {
+    network_service->ConfigureStubHostResolver(
+        /*insecure_dns_client_enabled=*/true,
+        /*secure_dns_mode=*/net::SecureDnsMode::kAutomatic,
+        net::DnsOverHttpsConfig(),
+        /*additional_dns_types_enabled=*/true);
+  }
+#endif
+}
+
 void ShellContentBrowserClient::ConfigureNetworkContextParams(
     BrowserContext* context,
     bool in_memory,

content/shell/browser/shell_content_browser_client.h

@@ -143,6 +143,8 @@ class ShellContentBrowserClient : public ContentBrowserClient {
 #endif  // BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) ||
         // BUILDFLAG(IS_ANDROID)
   device::GeolocationManager* GetGeolocationManager() override;
+  void OnNetworkServiceCreated(
+      network::mojom::NetworkService* network_service) override;
   void ConfigureNetworkContextParams(
       BrowserContext* context,
       bool in_memory,

net/base/features.cc

@@ -23,6 +23,16 @@ BASE_FEATURE(kCapReferrerToOriginOnCrossOrigin,
              "CapReferrerToOriginOnCrossOrigin",
              base::FEATURE_DISABLED_BY_DEFAULT);
 
+BASE_FEATURE(kAsyncDns,
+             "AsyncDns",
+#if BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_ANDROID) || \
+    BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX)
+             base::FEATURE_ENABLED_BY_DEFAULT
+#else
+             base::FEATURE_DISABLED_BY_DEFAULT
+#endif
+);
+
 BASE_FEATURE(kDnsTransactionDynamicTimeouts,
              "DnsTransactionDynamicTimeouts",
              base::FEATURE_DISABLED_BY_DEFAULT);

net/base/features.h

@@ -30,6 +30,9 @@ NET_EXPORT BASE_DECLARE_FEATURE(kAvoidH2Reprioritization);
 // origin requests are restricted to contain at most the source origin.
 NET_EXPORT BASE_DECLARE_FEATURE(kCapReferrerToOriginOnCrossOrigin);
 
+// Enables the built-in DNS resolver.
+NET_EXPORT BASE_DECLARE_FEATURE(kAsyncDns);
+
 // Support for altering the parameters used for DNS transaction timeout. See
 // ResolveContext::SecureTransactionTimeout().
 NET_EXPORT BASE_DECLARE_FEATURE(kDnsTransactionDynamicTimeouts);