Add system DNS config refresh capabilities

Used to trigger config update in SystemDnsConfigChangeNotifier on
Android and ChromeOS where the update is triggered external to the
network stack.

Primarily needed for SystemDnsConfigChangeNotifier, but updated the
current soon-to-be-modified code in NetworkChangeNotifierPosix to use
the new unified method on DnsConfigService and removed its special
DnsConfigService overrides in order to simplify and prepare for the
transition to SystemDnsConfigChangeNotifier.

Bug: 971411
Change-Id: I944073d4ff86e43aae40d0e1404a2cfbb1bb902a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1668147Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672632}

net/base/network_change_notifier_posix.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include <string>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/sequenced_task_runner.h"
@@ -18,29 +19,6 @@
 
 namespace net {
 
-// DNS config services on Chrome OS and Android are signalled by the network
-// state handler rather than relying on watching files in /etc.
-class NetworkChangeNotifierPosix::DnsConfigService
-    : public net::internal::DnsConfigServicePosix {
- public:
-  DnsConfigService() = default;
-  ~DnsConfigService() override = default;
-
-  // net::internal::DnsConfigService() overrides.
-  bool StartWatching() override {
-    CreateReaders();
-    // DNS config changes are handled and notified by the network
-    // state handlers.
-    return true;
-  }
-
-  void OnNetworkChange() {
-    InvalidateConfig();
-    InvalidateHosts();
-    ReadNow();
-  }
-};
-
 NetworkChangeNotifierPosix::NetworkChangeNotifierPosix(
     NetworkChangeNotifier::ConnectionType initial_connection_type,
     NetworkChangeNotifier::ConnectionSubtype initial_connection_subtype)
@@ -48,7 +26,7 @@ NetworkChangeNotifierPosix::NetworkChangeNotifierPosix(
       dns_config_service_runner_(
           base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()})),
       dns_config_service_(
-          new DnsConfigService(),
+          nullptr,
           // Ensure DnsConfigService lives on |dns_config_service_runner_|
           // to prevent races where NetworkChangeNotifierPosix outlives
           // ScopedTaskEnvironment. https://crbug.com/938126
@@ -57,12 +35,7 @@ NetworkChangeNotifierPosix::NetworkChangeNotifierPosix(
       max_bandwidth_mbps_(
           NetworkChangeNotifier::GetMaxBandwidthMbpsForConnectionSubtype(
               initial_connection_subtype)) {
-  dns_config_service_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(
-          &NetworkChangeNotifierPosix::DnsConfigService::WatchConfig,
-          base::Unretained(dns_config_service_.get()),
-          base::BindRepeating(&NetworkChangeNotifier::SetDnsConfig)));
+  SetAndStartDnsConfigService(DnsConfigService::CreateSystemService());
   OnDNSChanged();
 }
 
@@ -73,10 +46,8 @@ NetworkChangeNotifierPosix::~NetworkChangeNotifierPosix() {
 void NetworkChangeNotifierPosix::OnDNSChanged() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   dns_config_service_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(
-          &NetworkChangeNotifierPosix::DnsConfigService::OnNetworkChange,
-          base::Unretained(dns_config_service_.get())));
+      FROM_HERE, base::BindOnce(&DnsConfigService::RefreshConfig,
+                                base::Unretained(dns_config_service_.get())));
 }
 
 void NetworkChangeNotifierPosix::OnIPAddressChanged() {
@@ -108,6 +79,11 @@ void NetworkChangeNotifierPosix::OnConnectionSubtypeChanged(
                                                              connection_type);
 }
 
+void NetworkChangeNotifierPosix::SetDnsConfigServiceForTesting(
+    std::unique_ptr<DnsConfigService> dns_config_service) {
+  SetAndStartDnsConfigService(std::move(dns_config_service));
+}
+
 NetworkChangeNotifier::ConnectionType
 NetworkChangeNotifierPosix::GetCurrentConnectionType() const {
   base::AutoLock scoped_lock(lock_);
@@ -122,6 +98,17 @@ void NetworkChangeNotifierPosix::GetCurrentMaxBandwidthAndConnectionType(
   *max_bandwidth_mbps = max_bandwidth_mbps_;
 }
 
+void NetworkChangeNotifierPosix::SetAndStartDnsConfigService(
+    std::unique_ptr<DnsConfigService> dns_config_service) {
+  // Reset/release to use the deleter already set up in |dns_config_service_|.
+  dns_config_service_.reset(dns_config_service.release());
+  dns_config_service_runner_->PostTask(
+      FROM_HERE, base::BindOnce(&DnsConfigService::WatchConfig,
+                                base::Unretained(dns_config_service_.get()),
+                                base::BindRepeating(
+                                    &NetworkChangeNotifier::SetDnsConfig)));
+}
+
 // static
 NetworkChangeNotifier::NetworkChangeCalculatorParams
 NetworkChangeNotifierPosix::NetworkChangeCalculatorParamsPosix() {

net/base/network_change_notifier_posix.h

@@ -25,6 +25,8 @@ struct OnTaskRunnerDeleter;
 
 namespace net {
 
+class DnsConfigService;
+
 // A NetworkChangeNotifier that needs to be told about network changes by some
 // other object. This class can't directly listen for network changes because on
 // ChromeOS and Android only objects running in the browser process can listen
@@ -46,6 +48,10 @@ class NET_EXPORT NetworkChangeNotifierPosix : public NetworkChangeNotifier {
       NetworkChangeNotifier::ConnectionType connection_type,
       NetworkChangeNotifier::ConnectionSubtype connection_subtype);
 
+  // |dns_config_service| must support RefreshConfig().
+  void SetDnsConfigServiceForTesting(
+      std::unique_ptr<DnsConfigService> dns_config_service);
+
  protected:
   // NetworkChangeNotifier overrides.
   NetworkChangeNotifier::ConnectionType GetCurrentConnectionType()
@@ -57,7 +63,8 @@ class NET_EXPORT NetworkChangeNotifierPosix : public NetworkChangeNotifier {
  private:
   friend class NetworkChangeNotifierPosixTest;
 
-  class DnsConfigService;
+  void SetAndStartDnsConfigService(
+      std::unique_ptr<DnsConfigService> dns_config_service);
 
   // |dns_config_service_| will live on this runner.
   scoped_refptr<base::SequencedTaskRunner> dns_config_service_runner_;

net/base/network_change_notifier_posix_unittest.cc

@@ -4,8 +4,11 @@
 
 #include "net/base/network_change_notifier_posix.h"
 
+#include <utility>
+
 #include "base/test/scoped_task_environment.h"
 #include "net/base/network_change_notifier.h"
+#include "net/dns/test_dns_config_service.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace net {
@@ -17,18 +20,24 @@ class NetworkChangeNotifierPosixTest : public testing::Test {
             base::test::ScopedTaskEnvironment::MainThreadType::MOCK_TIME),
         notifier_(new NetworkChangeNotifierPosix(
             NetworkChangeNotifier::CONNECTION_UNKNOWN,
-            NetworkChangeNotifier::SUBTYPE_UNKNOWN)) {}
+            NetworkChangeNotifier::SUBTYPE_UNKNOWN)) {
+    auto dns_config_service = std::make_unique<TestDnsConfigService>();
+    dns_config_service_ = dns_config_service.get();
+    notifier_->SetDnsConfigServiceForTesting(std::move(dns_config_service));
+  }
 
   void FastForwardUntilIdle() {
     scoped_task_environment_.FastForwardUntilNoTasksRemain();
   }
 
   NetworkChangeNotifierPosix* notifier() { return notifier_.get(); }
+  TestDnsConfigService* dns_config_service() { return dns_config_service_; }
 
  private:
   base::test::ScopedTaskEnvironment scoped_task_environment_;
   net::NetworkChangeNotifier::DisableForTest mock_notifier_disabler_;
   std::unique_ptr<NetworkChangeNotifierPosix> notifier_;
+  TestDnsConfigService* dns_config_service_;
 };
 
 class MockIPAddressObserver : public NetworkChangeNotifier::IPAddressObserver {
@@ -86,4 +95,17 @@ TEST_F(NetworkChangeNotifierPosixTest, OnMaxBandwidthChanged) {
   NetworkChangeNotifier::RemoveMaxBandwidthObserver(&observer);
 }
 
+TEST_F(NetworkChangeNotifierPosixTest, OnDNSChanged) {
+  DnsConfig expected_config;
+  expected_config.nameservers = {IPEndPoint(IPAddress(1, 2, 3, 4), 233)};
+  dns_config_service()->SetConfigForRefresh(expected_config);
+
+  notifier()->OnDNSChanged();
+  FastForwardUntilIdle();
+
+  DnsConfig actual_config;
+  NetworkChangeNotifier::GetDnsConfig(&actual_config);
+  EXPECT_EQ(expected_config, actual_config);
+}
+
 }  // namespace net

net/dns/dns_config_service.cc

@@ -41,6 +41,11 @@ void DnsConfigService::WatchConfig(const CallbackType& callback) {
   ReadNow();
 }
 
+void DnsConfigService::RefreshConfig() {
+  // Overridden on supported platforms.
+  NOTREACHED();
+}
+
 void DnsConfigService::InvalidateConfig() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   base::TimeTicks now = base::TimeTicks::Now();

net/dns/dns_config_service.h

@@ -45,6 +45,11 @@ class NET_EXPORT_PRIVATE DnsConfigService {
   // Might require MessageLoopForIO.
   void WatchConfig(const CallbackType& callback);
 
+  // Triggers invalidation and re-read of the current configuration (followed by
+  // invocation of the callback). For use only on platforms expecting
+  // network-stack-external notifications of DNS config changes.
+  virtual void RefreshConfig();
+
  protected:
   enum WatchStatus {
     DNS_CONFIG_WATCH_STARTED = 0,

net/dns/dns_config_service_posix.cc

@@ -412,11 +412,25 @@ DnsConfigServicePosix::~DnsConfigServicePosix() {
   hosts_reader_->Cancel();
 }
 
+void DnsConfigServicePosix::RefreshConfig() {
+  InvalidateConfig();
+  InvalidateHosts();
+  ReadNow();
+}
+
 void DnsConfigServicePosix::ReadNow() {
   config_reader_->WorkNow();
   hosts_reader_->WorkNow();
 }
 
+#if defined(OS_ANDROID) || defined(OS_CHROMEOS)
+bool DnsConfigServicePosix::StartWatching() {
+  CreateReaders();
+  // DNS config changes are handled and notified by the network
+  // state handlers.
+  return true;
+}
+#else   // defined(OS_ANDROID) || defined(OS_CHROMEOS)
 bool DnsConfigServicePosix::StartWatching() {
   CreateReaders();
   // TODO(szym): re-start watcher if that makes sense. http://crbug.com/116139
@@ -425,6 +439,7 @@ bool DnsConfigServicePosix::StartWatching() {
                             DNS_CONFIG_WATCH_MAX);
   return watcher_->Watch();
 }
+#endif  // defined(OS_ANDROID) || defined(OS_CHROMEOS)
 
 void DnsConfigServicePosix::OnConfigChanged(bool succeeded) {
   InvalidateConfig();

net/dns/dns_config_service_posix.h

@@ -5,6 +5,8 @@
 #ifndef NET_DNS_DNS_CONFIG_SERVICE_POSIX_H_
 #define NET_DNS_DNS_CONFIG_SERVICE_POSIX_H_
 
+#include <memory>
+
 #if !defined(OS_ANDROID)
 #include <sys/types.h>
 #include <netinet/in.h>
@@ -34,6 +36,8 @@ class NET_EXPORT_PRIVATE DnsConfigServicePosix : public DnsConfigService {
   DnsConfigServicePosix();
   ~DnsConfigServicePosix() override;
 
+  void RefreshConfig() override;
+
  protected:
   // DnsConfigService:
   void ReadNow() override;

net/dns/dns_config_service_posix_unittest.cc

@@ -256,7 +256,7 @@ TEST_F(DnsConfigServicePosixTest, ChangeConfigMultipleTimes) {
   scoped_task_environment_.RunUntilIdle();
 
   for (int i = 0; i < 5; i++) {
-    service_->OnConfigChanged(true);
+    service_->RefreshConfig();
     // Wait for config read after the change. OnConfigChanged() will only be
     // called if the new config is different from the old one, so this can't be
     // ExpectChange().

net/dns/system_dns_config_change_notifier.cc

@@ -118,6 +118,12 @@ class SystemDnsConfigChangeNotifier::Core {
     }
   }
 
+  void RefreshConfig() {
+    task_runner_->PostTask(FROM_HERE,
+                           base::BindOnce(&Core::TriggerRefreshConfig,
+                                          weak_ptr_factory_.GetWeakPtr()));
+  }
+
  private:
   void StartWatching() {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -147,6 +153,11 @@ class SystemDnsConfigChangeNotifier::Core {
     }
   }
 
+  void TriggerRefreshConfig() {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    dns_config_service_->RefreshConfig();
+  }
+
   // Fields that may be accessed from any sequence. Must protect access using
   // |lock_|.
   mutable base::Lock lock_;
@@ -185,4 +196,8 @@ void SystemDnsConfigChangeNotifier::RemoveObserver(Observer* observer) {
   core_->RemoveObserver(observer);
 }
 
+void SystemDnsConfigChangeNotifier::RefreshConfig() {
+  core_->RefreshConfig();
+}
+
 }  // namespace net

net/dns/system_dns_config_change_notifier.h

@@ -62,6 +62,11 @@ class NET_EXPORT_PRIVATE SystemDnsConfigChangeNotifier {
   // AddObserver() was called.
   void RemoveObserver(Observer* observer);
 
+  // Triggers invalidation and re-read of the current configuration (followed by
+  // notifications to registered Observers). For use only on platforms
+  // expecting network-stack-external notifications of DNS config changes.
+  void RefreshConfig();
+
  private:
   class Core;
 

net/dns/system_dns_config_change_notifier_unittest.cc

@@ -308,4 +308,20 @@ TEST_F(SystemDnsConfigChangeNotifierTest, InitialConfigInvalid) {
   notifier_->RemoveObserver(&observer);
 }
 
+TEST_F(SystemDnsConfigChangeNotifierTest, RefreshConfig) {
+  test_config_service_->SetConfigForRefresh(kConfig);
+
+  TestObserver observer;
+  notifier_->AddObserver(&observer);
+
+  notifier_->RefreshConfig();
+  observer.WaitForNotification();
+
+  EXPECT_THAT(observer.configs_received(),
+              testing::ElementsAre(testing::Optional(kConfig)));
+  observer.ExpectNoMoreNotifications();
+
+  notifier_->RemoveObserver(&observer);
+}
+
 }  // namespace net

net/dns/test_dns_config_service.cc

@@ -6,8 +6,21 @@
 
 namespace net {
 
+TestDnsConfigService::TestDnsConfigService() = default;
+
+TestDnsConfigService::~TestDnsConfigService() = default;
+
 bool TestDnsConfigService::StartWatching() {
   return true;
 }
 
+void TestDnsConfigService::RefreshConfig() {
+  DCHECK(config_for_refresh_);
+  InvalidateConfig();
+  InvalidateHosts();
+  OnConfigRead(config_for_refresh_.value());
+  OnHostsRead(config_for_refresh_.value().hosts);
+  config_for_refresh_ = base::nullopt;
+}
+
 }  // namespace net

net/dns/test_dns_config_service.h

@@ -2,17 +2,24 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "net/dns/dns_config_service.h"
-
 #ifndef NET_DNS_TEST_DNS_CONFIG_SERVICE_H_
 #define NET_DNS_TEST_DNS_CONFIG_SERVICE_H_
 
+#include <utility>
+
+#include "base/logging.h"
+#include "base/optional.h"
+#include "net/dns/dns_config_service.h"
+
 namespace net {
 
 // Simple test implementation of DnsConfigService that will trigger
 // notifications only on explicitly calling On...() methods.
 class TestDnsConfigService : public DnsConfigService {
  public:
+  TestDnsConfigService();
+  ~TestDnsConfigService() override;
+
   void ReadNow() override {}
   bool StartWatching() override;
 
@@ -32,6 +39,16 @@ class TestDnsConfigService : public DnsConfigService {
   void set_watch_failed(bool value) {
     DnsConfigService::set_watch_failed(value);
   }
+
+  void RefreshConfig() override;
+
+  void SetConfigForRefresh(DnsConfig config) {
+    DCHECK(!config_for_refresh_);
+    config_for_refresh_ = std::move(config);
+  }
+
+ private:
+  base::Optional<DnsConfig> config_for_refresh_;
 };
 
 }  // namespace net