Add DnsConfigChangeManager mojo API.

Allows network service clients to listen for when changes are detected
to the system DNS configuration. Will be useful for the DnsProbeService
usecase where the probe service needs to use config change as a signal
to clear cached probe results.

Only signals that changes have happenned, not what the changes were, and
there is no current plan for the network service to support retrieving
DNS config or details on changes. Recent work on DnsConfigOverrides has
negated all known service-external usecases for such capabilities.

Bug: 846423
Change-Id: Iee980205a2d3d0ddc80b39fb2a0a043a593db0a4
Reviewed-on: https://chromium-review.googlesource.com/c/1309334Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605441}

services/network/BUILD.gn

@@ -35,6 +35,8 @@ jumbo_component("network_service") {
     "cross_origin_read_blocking.h",
     "data_pipe_element_reader.cc",
     "data_pipe_element_reader.h",
+    "dns_config_change_manager.cc",
+    "dns_config_change_manager.h",
     "empty_url_loader_client.cc",
     "empty_url_loader_client.h",
     "host_resolver.cc",
@@ -270,6 +272,7 @@ source_set("tests") {
     "cors/preflight_controller_unittest.cc",
     "cross_origin_read_blocking_unittest.cc",
     "data_pipe_element_reader_unittest.cc",
+    "dns_config_change_manager_unittest.cc",
     "host_resolver_unittest.cc",
     "http_cache_data_counter_unittest.cc",
     "http_cache_data_remover_unittest.cc",

services/network/dns_config_change_manager.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "services/network/dns_config_change_manager.h"
+
+#include <utility>
+
+namespace network {
+
+DnsConfigChangeManager::DnsConfigChangeManager() {
+  net::NetworkChangeNotifier::AddDNSObserver(this);
+}
+
+DnsConfigChangeManager::~DnsConfigChangeManager() {
+  net::NetworkChangeNotifier::RemoveDNSObserver(this);
+}
+
+void DnsConfigChangeManager::AddBinding(
+    mojom::DnsConfigChangeManagerRequest request) {
+  bindings_.AddBinding(this, std::move(request));
+}
+
+void DnsConfigChangeManager::RequestNotifications(
+    mojom::DnsConfigChangeManagerClientPtr client) {
+  clients_.AddPtr(std::move(client));
+}
+
+void DnsConfigChangeManager::OnDNSChanged() {
+  clients_.ForAllPtrs([](mojom::DnsConfigChangeManagerClient* client) {
+    client->OnSystemDnsConfigChanged();
+  });
+}
+
+void DnsConfigChangeManager::OnInitialDNSConfigRead() {
+  // Service API makes no distinction between initial read and change.
+  OnDNSChanged();
+}
+
+}  // namespace network

services/network/dns_config_change_manager.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SERVICES_NETWORK_DNS_CONFIG_CHANGE_MANAGER_H_
+#define SERVICES_NETWORK_DNS_CONFIG_CHANGE_MANAGER_H_
+
+#include <memory>
+#include <set>
+
+#include "base/component_export.h"
+#include "base/macros.h"
+#include "mojo/public/cpp/bindings/binding_set.h"
+#include "mojo/public/cpp/bindings/interface_ptr_set.h"
+#include "net/base/network_change_notifier.h"
+#include "services/network/public/mojom/host_resolver.mojom.h"
+
+namespace network {
+
+class COMPONENT_EXPORT(NETWORK_SERVICE) DnsConfigChangeManager
+    : public mojom::DnsConfigChangeManager,
+      public net::NetworkChangeNotifier::DNSObserver {
+ public:
+  DnsConfigChangeManager();
+  ~DnsConfigChangeManager() override;
+
+  void AddBinding(mojom::DnsConfigChangeManagerRequest request);
+
+  // mojom::DnsConfigChangeManager implementation:
+  void RequestNotifications(
+      mojom::DnsConfigChangeManagerClientPtr client) override;
+
+ private:
+  // net::NetworkChangeNotifier::DNSObserver implementation:
+  void OnDNSChanged() override;
+  void OnInitialDNSConfigRead() override;
+
+  mojo::BindingSet<mojom::DnsConfigChangeManager> bindings_;
+  mojo::InterfacePtrSet<mojom::DnsConfigChangeManagerClient> clients_;
+
+  DISALLOW_COPY_AND_ASSIGN(DnsConfigChangeManager);
+};
+
+}  // namespace network
+
+#endif  // SERVICES_NETWORK_DNS_CONFIG_CHANGE_MANAGER_H_

services/network/dns_config_change_manager_unittest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "services/network/dns_config_change_manager.h"
+
+#include <climits>
+#include <utility>
+
+#include "base/run_loop.h"
+#include "base/test/scoped_task_environment.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace network {
+namespace {
+
+class TestDnsConfigChangeManagerClient
+    : public mojom::DnsConfigChangeManagerClient {
+ public:
+  explicit TestDnsConfigChangeManagerClient(DnsConfigChangeManager* manager) {
+    mojom::DnsConfigChangeManagerPtr manager_ptr;
+    mojom::DnsConfigChangeManagerRequest manager_request(
+        mojo::MakeRequest(&manager_ptr));
+    manager->AddBinding(std::move(manager_request));
+
+    mojom::DnsConfigChangeManagerClientPtr client_ptr;
+    mojom::DnsConfigChangeManagerClientRequest client_request(
+        mojo::MakeRequest(&client_ptr));
+    binding_.Bind(std::move(client_request));
+
+    manager_ptr->RequestNotifications(std::move(client_ptr));
+  }
+
+  void OnSystemDnsConfigChanged() override {
+    num_notifications_++;
+    if (num_notifications_ >= num_notifications_expected_)
+      run_loop_.Quit();
+  }
+
+  int num_notifications() { return num_notifications_; }
+
+  void WaitForNotification(int num_notifications_expected) {
+    num_notifications_expected_ = num_notifications_expected;
+    if (num_notifications_ < num_notifications_expected_)
+      run_loop_.Run();
+  }
+
+ private:
+  int num_notifications_ = 0;
+  int num_notifications_expected_ = INT_MAX;
+  base::RunLoop run_loop_;
+  mojo::Binding<mojom::DnsConfigChangeManagerClient> binding_{this};
+
+  DISALLOW_COPY_AND_ASSIGN(TestDnsConfigChangeManagerClient);
+};
+
+class DnsConfigChangeManagerTest : public testing::Test {
+ public:
+  DnsConfigChangeManagerTest() {}
+
+  DnsConfigChangeManager* manager() { return &manager_; }
+  TestDnsConfigChangeManagerClient* client() { return &client_; }
+
+ private:
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+  std::unique_ptr<net::NetworkChangeNotifier> notifier_mock_{
+      net::NetworkChangeNotifier::CreateMock()};
+  DnsConfigChangeManager manager_;
+  TestDnsConfigChangeManagerClient client_{&manager_};
+
+  DISALLOW_COPY_AND_ASSIGN(DnsConfigChangeManagerTest);
+};
+
+TEST_F(DnsConfigChangeManagerTest, Notification) {
+  EXPECT_EQ(0, client()->num_notifications());
+
+  net::NetworkChangeNotifier::NotifyObserversOfDNSChangeForTests();
+  client()->WaitForNotification(1);
+  EXPECT_EQ(1, client()->num_notifications());
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1, client()->num_notifications());
+}
+
+TEST_F(DnsConfigChangeManagerTest, Notification_InitialRead) {
+  EXPECT_EQ(0, client()->num_notifications());
+
+  net::NetworkChangeNotifier::NotifyObserversOfInitialDNSConfigReadForTests();
+  client()->WaitForNotification(1);
+  EXPECT_EQ(1, client()->num_notifications());
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1, client()->num_notifications());
+}
+
+TEST_F(DnsConfigChangeManagerTest, MultipleNotification) {
+  EXPECT_EQ(0, client()->num_notifications());
+
+  net::NetworkChangeNotifier::NotifyObserversOfDNSChangeForTests();
+  net::NetworkChangeNotifier::NotifyObserversOfDNSChangeForTests();
+  client()->WaitForNotification(2);
+  EXPECT_EQ(2, client()->num_notifications());
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(2, client()->num_notifications());
+}
+
+TEST_F(DnsConfigChangeManagerTest, MultipleClients) {
+  TestDnsConfigChangeManagerClient client2(manager());
+
+  EXPECT_EQ(0, client()->num_notifications());
+  EXPECT_EQ(0, client2.num_notifications());
+
+  net::NetworkChangeNotifier::NotifyObserversOfDNSChangeForTests();
+  client()->WaitForNotification(1);
+  client2.WaitForNotification(1);
+  EXPECT_EQ(1, client()->num_notifications());
+  EXPECT_EQ(1, client2.num_notifications());
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1, client()->num_notifications());
+  EXPECT_EQ(1, client2.num_notifications());
+}
+
+}  // namespace
+}  // namespace network

services/network/network_service.cc

@@ -38,6 +38,7 @@
 #include "net/url_request/url_request_context_builder.h"
 #include "services/network/crl_set_distributor.h"
 #include "services/network/cross_origin_read_blocking.h"
+#include "services/network/dns_config_change_manager.h"
 #include "services/network/net_log_capture_mode_type_converter.h"
 #include "services/network/net_log_exporter.h"
 #include "services/network/network_context.h"
@@ -197,6 +198,8 @@ NetworkService::NetworkService(
   network_quality_estimator_manager_ =
       std::make_unique<NetworkQualityEstimatorManager>(net_log_);
 
+  dns_config_change_manager_ = std::make_unique<DnsConfigChangeManager>();
+
   host_resolver_ = CreateHostResolver(net_log_);
 
   network_usage_accumulator_ = std::make_unique<NetworkUsageAccumulator>();
@@ -441,6 +444,11 @@ void NetworkService::GetNetworkQualityEstimatorManager(
   network_quality_estimator_manager_->AddRequest(std::move(request));
 }
 
+void NetworkService::GetDnsConfigChangeManager(
+    mojom::DnsConfigChangeManagerRequest request) {
+  dns_config_change_manager_->AddBinding(std::move(request));
+}
+
 void NetworkService::GetTotalNetworkUsages(
     mojom::NetworkService::GetTotalNetworkUsagesCallback callback) {
   std::move(callback).Run(network_usage_accumulator_->GetTotalNetworkUsages());

services/network/network_service.h

@@ -25,6 +25,7 @@
 #include "services/network/network_change_manager.h"
 #include "services/network/network_quality_estimator_manager.h"
 #include "services/network/public/cpp/network_service_buildflags.h"
+#include "services/network/public/mojom/host_resolver.mojom.h"
 #include "services/network/public/mojom/net_log.mojom.h"
 #include "services/network/public/mojom/network_change_manager.mojom.h"
 #include "services/network/public/mojom/network_quality_estimator_manager.mojom.h"
@@ -51,6 +52,7 @@ class STHReporter;
 namespace network {
 
 class CRLSetDistributor;
+class DnsConfigChangeManager;
 class NetworkContext;
 class NetworkUsageAccumulator;
 class URLRequestContextBuilderMojo;
@@ -147,6 +149,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
       mojom::NetworkChangeManagerRequest request) override;
   void GetNetworkQualityEstimatorManager(
       mojom::NetworkQualityEstimatorManagerRequest request) override;
+  void GetDnsConfigChangeManager(
+      mojom::DnsConfigChangeManagerRequest request) override;
   void GetTotalNetworkUsages(
       mojom::NetworkService::GetTotalNetworkUsagesCallback callback) override;
 #if BUILDFLAG(IS_CT_SUPPORTED)
@@ -249,6 +253,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
   std::unique_ptr<NetworkQualityEstimatorManager>
       network_quality_estimator_manager_;
 
+  std::unique_ptr<DnsConfigChangeManager> dns_config_change_manager_;
+
   std::unique_ptr<net::HostResolver> host_resolver_;
   std::unique_ptr<NetworkUsageAccumulator> network_usage_accumulator_;
 

services/network/network_service_unittest.cc

@@ -1172,6 +1172,14 @@ TEST_F(NetworkServiceTestWithService,
   EXPECT_TRUE(network_context.encountered_error());
 }
 
+TEST_F(NetworkServiceTestWithService, GetDnsConfigChangeManager) {
+  mojom::DnsConfigChangeManagerPtr ptr;
+  ASSERT_FALSE(ptr.is_bound());
+
+  network_service_->GetDnsConfigChangeManager(mojo::MakeRequest(&ptr));
+  EXPECT_TRUE(ptr.is_bound());
+}
+
 class TestNetworkChangeManagerClient
     : public mojom::NetworkChangeManagerClient {
  public:

services/network/public/mojom/host_resolver.mojom

@@ -199,3 +199,17 @@ interface HostResolver {
               ResolveHostParameters? optional_parameters,
               ResolveHostClient response_client);
 };
+
+// A client interface that subscribes to DNS config change events from
+// DnsConfigChangeManager.
+interface DnsConfigChangeManagerClient {
+  // Notifies that a potential change has been detected in the DNS settings of
+  // the system that may affect results of host resolution.
+  OnSystemDnsConfigChanged();
+};
+
+// An interface that broadcasts DNS config change events.
+interface DnsConfigChangeManager {
+  // Requests to receive notification when there is a DNS config change.
+  RequestNotifications(DnsConfigChangeManagerClient client_ptr);
+};

services/network/public/mojom/network_service.mojom

@@ -295,6 +295,9 @@ interface NetworkService {
   GetNetworkQualityEstimatorManager(
       NetworkQualityEstimatorManager& network_quality_estimator_manager);
 
+  // Gets the DnsConfigChangeManager.
+  GetDnsConfigChangeManager(DnsConfigChangeManager& dns_config_change_manager);
+
   // Gets the accumulated network usage since the start/restart of the service.
   GetTotalNetworkUsages() => (array<NetworkUsage> total_network_usages);