Get NetworkChangeNotifier working on ChromeOS with network service.

The current NetworkChangeNotifierChromeOS implementation won't work
with the network service enabled because it listens for network changes
from Shill, which only supports a single client. The browser process
has other dependencies on Shill, so it has to be the client for the time
being, meaning the network service can't directly listen for network
changes and has to be notified of them from the browser process.

This CL splits the NetworkChangeNotifierChromeOS into two parts;
the first will live in the browser process and listen to Shill, while
the second is the actual NetworkChangeNotifierChromeOS implementation
which will have an instance in both the browser and network processes.
Since NetworkChangeNotifierChromeOS can't actually listen for network
changes itself, it just gets notified of changes from part 1 via a new
method in NetworkChangeManager. When the thing listening to Shill sees
a network change, it notifies its local NetworkChangeNotifierChromeOS
instance, and calls the new NetworkChangeManager method, the
implementation of which then notifies the network service's
NetworkChangeNotifierChromeOS instance.

Bug: 882610
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ib220575fbe42f026b1e5cb90d3bb6be9d0345414
Reviewed-on: https://chromium-review.googlesource.com/c/1274445Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606665}

chrome/browser/chromeos/BUILD.gn

@@ -1394,6 +1394,8 @@ source_set("chromeos") {
     "net/wake_on_wifi_connection_observer.h",
     "net/wake_on_wifi_manager.cc",
     "net/wake_on_wifi_manager.h",
+    "network_change_manager_client.cc",
+    "network_change_manager_client.h",
     "night_light/night_light_client.cc",
     "night_light/night_light_client.h",
     "note_taking_controller_client.cc",
@@ -2239,6 +2241,7 @@ source_set("unit_tests") {
     "net/network_pref_state_observer_unittest.cc",
     "net/network_throttling_observer_unittest.cc",
     "net/wake_on_wifi_manager_unittest.cc",
+    "network_change_manager_client_unittest.cc",
     "night_light/night_light_client_unittest.cc",
     "note_taking_helper_unittest.cc",
     "oauth2_token_service_delegate_unittest.cc",

chrome/browser/chromeos/chrome_browser_main_chromeos.cc

@@ -78,6 +78,7 @@
 #include "chrome/browser/chromeos/net/network_pref_state_observer.h"
 #include "chrome/browser/chromeos/net/network_throttling_observer.h"
 #include "chrome/browser/chromeos/net/wake_on_wifi_manager.h"
+#include "chrome/browser/chromeos/network_change_manager_client.h"
 #include "chrome/browser/chromeos/note_taking_helper.h"
 #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos_factory.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
@@ -135,8 +136,6 @@
 #include "chromeos/login/login_state.h"
 #include "chromeos/login_event_recorder.h"
 #include "chromeos/network/network_cert_loader.h"
-#include "chromeos/network/network_change_notifier_chromeos.h"
-#include "chromeos/network/network_change_notifier_factory_chromeos.h"
 #include "chromeos/network/network_handler.h"
 #include "chromeos/network/portal_detector/network_portal_detector_stub.h"
 #include "chromeos/settings/install_attributes.h"
@@ -156,6 +155,7 @@
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/media_capture_devices.h"
+#include "content/public/browser/network_service_instance.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/main_function_params.h"
@@ -167,6 +167,7 @@
 #include "device/bluetooth/dbus/bluez_dbus_manager.h"
 #include "media/audio/sounds/sounds_manager.h"
 #include "net/base/network_change_notifier.h"
+#include "net/base/network_change_notifier_chromeos.h"
 #include "net/cert/nss_cert_database.h"
 #include "net/cert/nss_cert_database_chromeos.h"
 #include "printing/backend/print_backend.h"
@@ -349,11 +350,6 @@ class DBusServices {
 
     NetworkHandler::Initialize();
 
-    // Initialize the network change notifier for Chrome OS. The network
-    // change notifier starts to monitor changes from the power manager and
-    // the network manager.
-    NetworkChangeNotifierFactoryChromeos::GetInstance()->Initialize();
-
     // Likewise, initialize the upgrade detector for Chrome OS. The upgrade
     // detector starts to monitor changes from the update engine.
     UpgradeDetectorChromeos::GetInstance()->Init();
@@ -572,11 +568,6 @@ void ChromeBrowserMainPartsChromeos::PreMainMessageLoopStart() {
   // to session state change right after browser is started.
   g_browser_process->platform_part()->InitializeSessionManager();
 
-  // Replace the default NetworkChangeNotifierFactory with ChromeOS specific
-  // implementation. This must be done before BrowserMainLoop calls
-  // net::NetworkChangeNotifier::Create() in MainMessageLoopStart().
-  net::NetworkChangeNotifier::SetFactory(
-      new NetworkChangeNotifierFactoryChromeos());
   ChromeBrowserMainPartsLinux::PreMainMessageLoopStart();
 }
 
@@ -599,6 +590,10 @@ void ChromeBrowserMainPartsChromeos::PostMainMessageLoopStart() {
 // Threads are initialized between MainMessageLoopStart and MainMessageLoopRun.
 // about_flags settings are applied in ChromeBrowserMainParts::PreCreateThreads.
 void ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun() {
+  network_change_manager_client_.reset(new NetworkChangeManagerClient(
+      static_cast<net::NetworkChangeNotifierChromeos*>(
+          content::GetNetworkChangeNotifier())));
+
   // Set the crypto thread after the IO thread has been created/started.
   TPMTokenLoader::Get()->SetCryptoTaskRunner(
       base::CreateSingleThreadTaskRunnerWithTraits(
@@ -1057,12 +1052,6 @@ void ChromeBrowserMainPartsChromeos::PostMainMessageLoopRun() {
   if (UpgradeDetectorChromeos::GetInstance())
     UpgradeDetectorChromeos::GetInstance()->Shutdown();
 
-  // Shutdown the network change notifier for Chrome OS. The network
-  // change notifier stops monitoring changes from the power manager and
-  // the network manager.
-  if (NetworkChangeNotifierFactoryChromeos::GetInstance())
-    NetworkChangeNotifierFactoryChromeos::GetInstance()->Shutdown();
-
   // Tell DeviceSettingsService to stop talking to session_manager. Do not
   // shutdown DeviceSettingsService yet, it might still be accessed by
   // BrowserPolicyConnector (owned by g_browser_process).
@@ -1171,6 +1160,8 @@ void ChromeBrowserMainPartsChromeos::PostDestroyThreads() {
   // held by any task.
   crosvm_metrics_.reset();
 
+  network_change_manager_client_.reset();
+
   // Destroy DBus services immediately after threads are stopped.
   dbus_services_.reset();
 

chrome/browser/chromeos/chrome_browser_main_chromeos.h

@@ -40,6 +40,7 @@ class DiscoverManager;
 class EventRewriterDelegateImpl;
 class IdleActionWarningObserver;
 class LowDiskNotification;
+class NetworkChangeManagerClient;
 class NetworkPrefStateObserver;
 class NetworkThrottlingObserver;
 class PowerMetricsReporter;
@@ -98,6 +99,7 @@ class ChromeBrowserMainPartsChromeos : public ChromeBrowserMainPartsLinux {
   std::unique_ptr<PowerMetricsReporter> power_metrics_reporter_;
   std::unique_ptr<WakeOnWifiManager> wake_on_wifi_manager_;
   std::unique_ptr<NetworkThrottlingObserver> network_throttling_observer_;
+  std::unique_ptr<NetworkChangeManagerClient> network_change_manager_client_;
 
   // Indicates whether the DBus has been initialized before. It is possible that
   // the DBus has been initialized in ChromeFeatureListCreator.

chromeos/network/network_change_notifier_chromeos.h → chrome/browser/chromeos/network_change_manager_client.h

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// 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 CHROMEOS_NETWORK_NETWORK_CHANGE_NOTIFIER_CHROMEOS_H_
-#define CHROMEOS_NETWORK_NETWORK_CHANGE_NOTIFIER_CHROMEOS_H_
+#ifndef CHROME_BROWSER_CHROMEOS_NETWORK_CHANGE_MANAGER_CLIENT_H_
+#define CHROME_BROWSER_CHROMEOS_NETWORK_CHANGE_MANAGER_CLIENT_H_
 
-#include <memory>
 #include <string>
 
-#include "base/callback.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
-#include "base/memory/weak_ptr.h"
-#include "base/single_thread_task_runner.h"
 #include "chromeos/chromeos_export.h"
 #include "chromeos/dbus/power_manager_client.h"
 #include "chromeos/network/network_state_handler_observer.h"
 #include "net/base/network_change_notifier.h"
+#include "services/network/public/mojom/network_change_manager.mojom.h"
+
+namespace net {
+class NetworkChangeNotifierChromeos;
+}
 
 namespace chromeos {
 
-class CHROMEOS_EXPORT NetworkChangeNotifierChromeos
-    : public net::NetworkChangeNotifier,
-      public chromeos::PowerManagerClient::Observer,
+// This class listens to Shill for network change events and notifies both
+// the local NetworkChangeNotifierChromeos, and the network service via
+// the NetworkChangeManager if the network service is enabled.
+class CHROMEOS_EXPORT NetworkChangeManagerClient
+    : public chromeos::PowerManagerClient::Observer,
       public chromeos::NetworkStateHandlerObserver {
  public:
-  NetworkChangeNotifierChromeos();
-  ~NetworkChangeNotifierChromeos() override;
-
-  // Starts observing changes from the network state handler.
-  void Initialize();
-
-  // Stops observing changes from the network state handler.
-  void Shutdown();
-
-  // NetworkChangeNotifier overrides.
-  net::NetworkChangeNotifier::ConnectionType GetCurrentConnectionType()
-      const override;
-  void GetCurrentMaxBandwidthAndConnectionType(
-      double* max_bandwidth_mbps,
-      ConnectionType* connection_type) const override;
+  NetworkChangeManagerClient(
+      net::NetworkChangeNotifierChromeos* network_change_notifier);
+  ~NetworkChangeManagerClient() override;
 
   // PowerManagerClient::Observer overrides.
   void SuspendDone(const base::TimeDelta& sleep_duration) override;
@@ -49,47 +40,44 @@ class CHROMEOS_EXPORT NetworkChangeNotifierChromeos
       const chromeos::NetworkState* default_network) override;
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(NetworkChangeNotifierChromeosTest,
+  friend class NetworkChangeManagerClientUpdateTest;
+  FRIEND_TEST_ALL_PREFIXES(NetworkChangeManagerClientTest,
                            ConnectionTypeFromShill);
-  friend class NetworkChangeNotifierChromeosUpdateTest;
 
-  class DnsConfigService;
+  void ConnectToNetworkChangeManager();
+  void ReconnectToNetworkChangeManager();
 
   // Updates the notifier state based on a default network update.
   // |connection_type_changed| is set to true if we must report a connection
   // type change.
   // |ip_address_changed| is set to true if we must report an IP address change.
   // |dns_changed| is set to true if we must report a DNS config change.
-  // |max_bandwidth_changed| is set to true if we must report a max bandwidth
-  // change.
+  // |connection_subtype_changed| is set to true if we must report a connection
+  // subtype change.
   void UpdateState(const chromeos::NetworkState* default_network,
-                   bool* connection_type_changed,
-                   bool* ip_address_changed,
                    bool* dns_changed,
-                   bool* max_bandwidth_changed);
-
-  // Proactively retrieves current network state from the network
-  // state handler and calls UpdateState with the result.
-  void PollForState();
+                   bool* ip_address_changed,
+                   bool* connection_type_changed,
+                   bool* connection_subtype_changed);
 
   // Maps the shill network type and technology to its NetworkChangeNotifier
   // equivalent.
-  static net::NetworkChangeNotifier::ConnectionType
-      ConnectionTypeFromShill(const std::string& type,
-                              const std::string& technology);
+  //
+  // This is a static member function for testing.
+  static net::NetworkChangeNotifier::ConnectionType ConnectionTypeFromShill(
+      const std::string& type,
+      const std::string& technology);
 
   // Maps the shill network type and technology to its NetworkChangeNotifier
   // subtype equivalent.
+  //
+  // This is a static member function for testing.
   static net::NetworkChangeNotifier::ConnectionSubtype GetConnectionSubtype(
       const std::string& type,
       const std::string& technology);
 
-  // Calculates parameters used for network change notifier online/offline
-  // signals.
-  static net::NetworkChangeNotifier::NetworkChangeCalculatorParams
-      NetworkChangeCalculatorParamsChromeos();
-
-  NetworkChangeNotifier::ConnectionType connection_type_;
+  net::NetworkChangeNotifier::ConnectionType connection_type_;
+  net::NetworkChangeNotifier::ConnectionSubtype connection_subtype_;
   // IP address for the current default network.
   std::string ip_address_;
   // DNS servers for the current default network.
@@ -97,22 +85,12 @@ class CHROMEOS_EXPORT NetworkChangeNotifierChromeos
   // Service path for the current default network.
   std::string service_path_;
 
-  // The maximum theoretical bandwidth in megabits per second for the current
-  // default network.
-  double max_bandwidth_mbps_;
-
-  std::unique_ptr<DnsConfigService> dns_config_service_;
-
-  // Callback for refreshing network state.
-  base::Closure poll_callback_;
-
-  // For setting up network refresh polling callbacks.
-  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
-  base::WeakPtrFactory<NetworkChangeNotifierChromeos> weak_ptr_factory_;
+  net::NetworkChangeNotifierChromeos* network_change_notifier_;
+  network::mojom::NetworkChangeManagerPtr network_change_manager_;
 
-  DISALLOW_COPY_AND_ASSIGN(NetworkChangeNotifierChromeos);
+  DISALLOW_COPY_AND_ASSIGN(NetworkChangeManagerClient);
 };
 
 }  // namespace chromeos
 
-#endif  // CHROMEOS_NETWORK_NETWORK_CHANGE_NOTIFIER_CHROMEOS_H_
+#endif  // CHROME_BROWSER_CHROMEOS_NETWORK_CHANGE_MANAGER_CLIENT_H_

chrome/browser/chromeos/network_change_manager_client_browsertest.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 "base/run_loop.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/shill_device_client.h"
+#include "chromeos/dbus/shill_service_client.h"
+#include "content/public/browser/network_service_instance.h"
+#include "net/base/network_change_notifier.h"
+#include "services/network/public/cpp/features.h"
+#include "services/network/public/cpp/network_connection_tracker.h"
+#include "third_party/cros_system_api/dbus/shill/dbus-constants.h"
+
+namespace chromeos {
+
+namespace {
+
+class NetObserver : public net::NetworkChangeNotifier::NetworkChangeObserver {
+ public:
+  NetObserver() { net::NetworkChangeNotifier::AddNetworkChangeObserver(this); }
+
+  ~NetObserver() override {
+    net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
+  }
+
+  void WaitForConnectionType(net::NetworkChangeNotifier::ConnectionType type) {
+    while (last_connection_type != type) {
+      run_loop_.reset(new base::RunLoop());
+      run_loop_->Run();
+      run_loop_.reset();
+    }
+  }
+
+  // net::NetworkChangeNotifier:NetworkChangeObserver:
+  void OnNetworkChanged(
+      net::NetworkChangeNotifier::ConnectionType type) override {
+    change_count++;
+    last_connection_type = type;
+    if (run_loop_)
+      run_loop_->Quit();
+  }
+
+  int change_count = 0;
+  net::NetworkChangeNotifier::ConnectionType last_connection_type =
+      net::NetworkChangeNotifier::CONNECTION_UNKNOWN;
+
+ private:
+  std::unique_ptr<base::RunLoop> run_loop_;
+};
+
+class NetworkServiceObserver
+    : public network::NetworkConnectionTracker::NetworkConnectionObserver {
+ public:
+  NetworkServiceObserver() {
+    content::GetNetworkConnectionTracker()->AddNetworkConnectionObserver(this);
+  }
+
+  ~NetworkServiceObserver() override {
+    content::GetNetworkConnectionTracker()->RemoveNetworkConnectionObserver(
+        this);
+  }
+
+  void WaitForConnectionType(network::mojom::ConnectionType type) {
+    while (last_connection_type != type) {
+      run_loop_.reset(new base::RunLoop());
+      run_loop_->Run();
+      run_loop_.reset();
+    }
+  }
+
+  // network::NetworkConnectionTracker::NetworkConnectionObserver:
+  void OnConnectionChanged(network::mojom::ConnectionType type) override {
+    change_count++;
+    last_connection_type = type;
+    if (run_loop_)
+      run_loop_->Quit();
+  }
+
+  int change_count = 0;
+  network::mojom::ConnectionType last_connection_type =
+      network::mojom::ConnectionType::CONNECTION_UNKNOWN;
+
+ private:
+  std::unique_ptr<base::RunLoop> run_loop_;
+};
+
+}  // namespace
+
+class NetworkChangeManagerClientBrowserTest : public InProcessBrowserTest {
+ public:
+  void SetUpOnMainThread() override {
+    InProcessBrowserTest::SetUpOnMainThread();
+    service_client_ =
+        DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface();
+    service_client_->ClearServices();
+  }
+
+  ShillServiceClient::TestInterface* service_client() {
+    return service_client_;
+  }
+
+ private:
+  ShillServiceClient::TestInterface* service_client_;
+};
+
+// Tests that network changes from shill are received by both the
+// NetworkChangeNotifier and NetworkConnectionTracker.
+IN_PROC_BROWSER_TEST_F(NetworkChangeManagerClientBrowserTest,
+                       ReceiveNotifications) {
+  NetObserver net_observer;
+  NetworkServiceObserver network_service_observer;
+
+  service_client()->AddService("wifi", "wifi", "wifi", shill::kTypeWifi,
+                               shill::kStateOnline, true);
+
+  net_observer.WaitForConnectionType(
+      net::NetworkChangeNotifier::CONNECTION_WIFI);
+  // NetworkChangeNotifier will send a CONNECTION_NONE notification before
+  // the CONNECTION_WIFI one.
+  EXPECT_EQ(2, net_observer.change_count);
+  EXPECT_EQ(net::NetworkChangeNotifier::CONNECTION_WIFI,
+            net_observer.last_connection_type);
+
+  network_service_observer.WaitForConnectionType(
+      network::mojom::ConnectionType::CONNECTION_WIFI);
+  EXPECT_EQ(2, network_service_observer.change_count);
+  EXPECT_EQ(network::mojom::ConnectionType::CONNECTION_WIFI,
+            network_service_observer.last_connection_type);
+};
+
+// Tests that the NetworkChangeManagerClient reconnects to the network service
+// after it gets disconnected.
+IN_PROC_BROWSER_TEST_F(NetworkChangeManagerClientBrowserTest,
+                       ReconnectToNetworkService) {
+  if (!base::FeatureList::IsEnabled(network::features::kNetworkService))
+    return;
+
+  {
+    // Make sure everyone thinks we have an ethernet connection.
+    NetObserver net_observer;
+    NetworkServiceObserver network_service_observer;
+    net_observer.WaitForConnectionType(
+        net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+    network_service_observer.WaitForConnectionType(
+        network::mojom::ConnectionType::CONNECTION_ETHERNET);
+  }
+
+  NetObserver net_observer;
+  NetworkServiceObserver network_service_observer;
+
+  SimulateNetworkServiceCrash();
+  service_client()->AddService("wifi", "wifi", "wifi", shill::kTypeWifi,
+                               shill::kStateOnline, true);
+
+  net_observer.WaitForConnectionType(
+      net::NetworkChangeNotifier::CONNECTION_WIFI);
+  // NetworkChangeNotifier will send a CONNECTION_NONE notification before
+  // the CONNECTION_WIFI one.
+  EXPECT_EQ(2, net_observer.change_count);
+  EXPECT_EQ(net::NetworkChangeNotifier::CONNECTION_WIFI,
+            net_observer.last_connection_type);
+
+  network_service_observer.WaitForConnectionType(
+      network::mojom::ConnectionType::CONNECTION_WIFI);
+  EXPECT_EQ(2, network_service_observer.change_count);
+  EXPECT_EQ(network::mojom::ConnectionType::CONNECTION_WIFI,
+            network_service_observer.last_connection_type);
+};
+
+}  // namespace chromeos

chrome/browser/net/network_context_configuration_browsertest.cc

@@ -42,6 +42,7 @@
 #include "components/proxy_config/proxy_config_pref_names.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/network_service_instance.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/url_constants.h"
@@ -68,6 +69,7 @@
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "net/url_request/url_request.h"
 #include "services/network/public/cpp/features.h"
+#include "services/network/public/cpp/network_connection_tracker.h"
 #include "services/network/public/cpp/resource_response.h"
 #include "services/network/public/cpp/resource_response_info.h"
 #include "services/network/public/cpp/simple_url_loader.h"
@@ -1490,7 +1492,8 @@ IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationHttpPacBrowserTest, HttpPac) {
 // Make sure the system URLRequestContext can handle fetching PAC scripts from
 // file URLs.
 class NetworkContextConfigurationFilePacBrowserTest
-    : public NetworkContextConfigurationBrowserTest {
+    : public NetworkContextConfigurationBrowserTest,
+      public network::NetworkConnectionTracker::NetworkConnectionObserver {
  public:
   NetworkContextConfigurationFilePacBrowserTest() {}
 
@@ -1512,8 +1515,30 @@ class NetworkContextConfigurationFilePacBrowserTest
         switches::kProxyPacUrl, net::FilePathToFileURL(pac_file_path).spec());
   }
 
+  void SetUpOnMainThread() override {
+    NetworkContextConfigurationBrowserTest::SetUpOnMainThread();
+
+    // The network service will have just been killed if network_service_state
+    // is kRestarted. Make sure it knows about the correct network state before
+    // continuing.
+    content::GetNetworkConnectionTracker()->AddNetworkConnectionObserver(this);
+    while (content::GetNetworkConnectionTracker()->IsOffline()) {
+      run_loop_.reset(new base::RunLoop());
+      run_loop_->Run();
+    }
+    content::GetNetworkConnectionTracker()->RemoveNetworkConnectionObserver(
+        this);
+  }
+
+  // network::NetworkConnectionTracker::NetworkConnectionObserver
+  void OnConnectionChanged(network::mojom::ConnectionType type) override {
+    if (run_loop_)
+      run_loop_->Quit();
+  }
+
  private:
   base::ScopedTempDir temp_dir_;
+  std::unique_ptr<base::RunLoop> run_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkContextConfigurationFilePacBrowserTest);
 };

chrome/test/BUILD.gn

@@ -1733,6 +1733,7 @@ test("browser_tests") {
         "../browser/chromeos/login/webview_login_browsertest.cc",
         "../browser/chromeos/login/wizard_controller_browsertest.cc",
         "../browser/chromeos/net/network_portal_detector_impl_browsertest.cc",
+        "../browser/chromeos/network_change_manager_client_browsertest.cc",
         "../browser/chromeos/policy/affiliation_test_helper.cc",
         "../browser/chromeos/policy/affiliation_test_helper.h",
         "../browser/chromeos/policy/blocking_login_browsertest.cc",

chromeos/network/BUILD.gn

@@ -13,12 +13,12 @@ source_set("network") {
   deps = [
     "//base",
     "//base:i18n",
-    "//chromeos/dbus",
-    "//chromeos/login",
     "//chromeos:chromeos_constants",
     "//chromeos:chromeos_export",
     "//chromeos:policy_certificate_provider",
     "//chromeos:tools",
+    "//chromeos/dbus",
+    "//chromeos/login",
     "//components/account_id",
     "//components/device_event_log",
     "//components/onc",
@@ -70,10 +70,6 @@ source_set("network") {
     "network_cert_migrator.h",
     "network_certificate_handler.cc",
     "network_certificate_handler.h",
-    "network_change_notifier_chromeos.cc",
-    "network_change_notifier_chromeos.h",
-    "network_change_notifier_factory_chromeos.cc",
-    "network_change_notifier_factory_chromeos.h",
     "network_configuration_handler.cc",
     "network_configuration_handler.h",
     "network_configuration_observer.h",
@@ -171,13 +167,13 @@ source_set("unit_tests") {
   deps = [
     ":network",
     "//base",
-    "//base/test:test_support",
     "//base:i18n",
-    "//chromeos/dbus",
-    "//chromeos/login",
+    "//base/test:test_support",
     "//chromeos:chromeos_constants",
     "//chromeos:policy_certificate_provider",
     "//chromeos:test_support_source_set",
+    "//chromeos/dbus",
+    "//chromeos/login",
     "//components/onc",
     "//components/prefs",
     "//components/prefs:test_support",
@@ -200,7 +196,6 @@ source_set("unit_tests") {
     "managed_network_configuration_handler_unittest.cc",
     "network_cert_loader_unittest.cc",
     "network_cert_migrator_unittest.cc",
-    "network_change_notifier_chromeos_unittest.cc",
     "network_configuration_handler_unittest.cc",
     "network_connect_unittest.cc",
     "network_connection_handler_impl_unittest.cc",

chromeos/network/managed_state.h

@@ -89,6 +89,9 @@ class CHROMEOS_EXPORT ManagedState {
     update_requested_ = update_requested;
   }
 
+  void set_path_for_testing(const std::string& path) { path_ = path; }
+  void set_type_for_testing(const std::string& type) { type_ = type; }
+
   // Returns true if |type_| matches |pattern|.
   bool Matches(const NetworkTypePattern& pattern) const;
 
@@ -120,7 +123,6 @@ class CHROMEOS_EXPORT ManagedState {
   void set_type(const std::string& type) { type_ = type; }
 
  private:
-  friend class NetworkChangeNotifierChromeosUpdateTest;
   friend class NetworkStateHandler;
   friend class ash::network_icon::NetworkIconTest;
   friend class chromeos::tether::NetworkListSorterTest;

chromeos/network/network_change_notifier_factory_chromeos.cc

-// Copyright (c) 2012 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 "chromeos/network/network_change_notifier_factory_chromeos.h"
-
-#include "chromeos/network/network_change_notifier_chromeos.h"
-
-namespace chromeos {
-
-namespace {
-
-NetworkChangeNotifierChromeos* g_network_change_notifier_chromeos = NULL;
-
-}  // namespace
-
-net::NetworkChangeNotifier*
-NetworkChangeNotifierFactoryChromeos::CreateInstance() {
-  DCHECK(!g_network_change_notifier_chromeos);
-  g_network_change_notifier_chromeos = new NetworkChangeNotifierChromeos();
-  return g_network_change_notifier_chromeos;
-}
-
-// static
-NetworkChangeNotifierChromeos*
-NetworkChangeNotifierFactoryChromeos::GetInstance() {
-  return g_network_change_notifier_chromeos;
-}
-
-}  // namespace chromeos

chromeos/network/network_change_notifier_factory_chromeos.h

-// Copyright (c) 2012 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 CHROMEOS_NETWORK_NETWORK_CHANGE_NOTIFIER_FACTORY_CHROMEOS_H_
-#define CHROMEOS_NETWORK_NETWORK_CHANGE_NOTIFIER_FACTORY_CHROMEOS_H_
-
-#include "base/compiler_specific.h"
-#include "chromeos/chromeos_export.h"
-#include "net/base/network_change_notifier_factory.h"
-
-namespace chromeos {
-
-class NetworkChangeNotifierChromeos;
-
-class CHROMEOS_EXPORT NetworkChangeNotifierFactoryChromeos
-    : public net::NetworkChangeNotifierFactory {
- public:
-  NetworkChangeNotifierFactoryChromeos() {}
-
-  // net::NetworkChangeNotifierFactory overrides.
-  net::NetworkChangeNotifier* CreateInstance() override;
-
-  static NetworkChangeNotifierChromeos* GetInstance();
-};
-
-}  // namespace chromeos
-
-#endif  // CHROMEOS_NETWORK_NETWORK_CHANGE_NOTIFIER_FACTORY_CHROMEOS_H_

chromeos/network/network_state.h

@@ -194,6 +194,11 @@ class CHROMEOS_EXPORT NetworkState : public ManagedState {
   // Returns |error_| if valid, otherwise returns |last_error_|.
   std::string GetErrorState() const;
 
+  // Setters for testing.
+  void set_network_technology_for_testing(const std::string& technology) {
+    network_technology_ = technology;
+  }
+
   // Helpers (used e.g. when a state, error, or shill dictionary is cached)
   static bool StateIsConnected(const std::string& connection_state);
   static bool StateIsConnecting(const std::string& connection_state);
@@ -205,7 +210,6 @@ class CHROMEOS_EXPORT NetworkState : public ManagedState {
  private:
   friend class MobileActivatorTest;
   friend class NetworkStateHandler;
-  friend class NetworkChangeNotifierChromeosUpdateTest;
   FRIEND_TEST_ALL_PREFIXES(NetworkStateTest, TetherProperties);
 
   // Updates |name_| from the 'WiFi.HexSSID' entry in |properties|, which must

content/browser/browser_main_loop.h

@@ -174,6 +174,9 @@ class CONTENT_EXPORT BrowserMainLoop {
   media::UserInputMonitor* user_input_monitor() const {
     return user_input_monitor_.get();
   }
+  net::NetworkChangeNotifier* network_change_notifier() const {
+    return network_change_notifier_.get();
+  }
 
 #if defined(OS_CHROMEOS)
   KeyboardMicRegistration* keyboard_mic_registration() {

content/browser/net_info_browsertest.cc

@@ -109,13 +109,6 @@ class NetInfoBrowserTest : public content::ContentBrowserTest {
   void SetUp() override {
     net::NetworkChangeNotifier::SetTestNotificationsOnly(true);
 
-#if defined(OS_CHROMEOS)
-    // ChromeOS's NetworkChangeNotifier isn't known to content and therefore
-    // doesn't get created in content_browsertests. Insert a mock
-    // NetworkChangeNotifier.
-    net::NetworkChangeNotifier::CreateMock();
-#endif
-
     content::ContentBrowserTest::SetUp();
   }
 

content/browser/network_service_instance.cc

@@ -16,6 +16,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/post_task.h"
 #include "build/build_config.h"
+#include "content/browser/browser_main_loop.h"
 #include "content/browser/network_service_client.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
@@ -187,6 +188,10 @@ network::NetworkService* GetNetworkServiceImpl() {
   return g_network_service;
 }
 
+net::NetworkChangeNotifier* GetNetworkChangeNotifier() {
+  return BrowserMainLoop::GetInstance()->network_change_notifier();
+}
+
 void FlushNetworkServiceInstanceForTesting() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 

content/public/browser/network_service_instance.h

@@ -11,6 +11,10 @@
 #include "base/callback_list.h"
 #include "content/common/content_export.h"
 
+namespace net {
+class NetworkChangeNotifier;
+}  // namespace net
+
 namespace network {
 class NetworkConnectionTracker;
 class NetworkService;
@@ -55,6 +59,9 @@ RegisterNetworkServiceCrashHandler(base::RepeatingClosure handler);
 // service is enabled.
 CONTENT_EXPORT network::NetworkService* GetNetworkServiceImpl();
 
+// Returns the global NetworkChangeNotifier instance.
+CONTENT_EXPORT net::NetworkChangeNotifier* GetNetworkChangeNotifier();
+
 // Call |FlushForTesting()| on cached |NetworkServicePtr|. For testing only.
 // Must only be called on the UI thread.
 CONTENT_EXPORT void FlushNetworkServiceInstanceForTesting();

content/public/test/browser_test_base.cc

@@ -61,6 +61,12 @@
 #include "base/process/process_handle.h"
 #endif
 
+#if defined(OS_CHROMEOS)
+#include "content/public/browser/network_service_instance.h"
+#include "net/base/network_change_notifier.h"
+#include "net/base/network_change_notifier_chromeos.h"
+#endif
+
 #if defined(USE_AURA)
 #include "content/browser/compositor/image_transport_factory.h"
 #include "ui/aura/test/event_generator_delegate_aura.h"  // nogncheck
@@ -379,6 +385,17 @@ void BrowserTestBase::ProxyRunTestOnMainThreadLoop() {
     signal(SIGTERM, DumpStackTraceSignalHandler);
 #endif  // defined(OS_POSIX)
 
+#if defined(OS_CHROMEOS)
+  // Manually set the connection type since ChromeOS's NetworkChangeNotifier
+  // implementation relies on some other class controlling it (normally
+  // NetworkChangeManagerClient), which may not be set up in all browser tests.
+  net::NetworkChangeNotifierChromeos* network_change_notifier =
+      static_cast<net::NetworkChangeNotifierChromeos*>(
+          content::GetNetworkChangeNotifier());
+  network_change_notifier->OnConnectionChanged(
+      net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+#endif
+
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableTracing)) {
     base::trace_event::TraceConfig trace_config(

net/BUILD.gn

@@ -476,6 +476,8 @@ component("net") {
       "base/network_activity_monitor.h",
       "base/network_change_notifier.cc",
       "base/network_change_notifier.h",
+      "base/network_change_notifier_chromeos.cc",
+      "base/network_change_notifier_chromeos.h",
       "base/network_change_notifier_factory.h",
       "base/network_change_notifier_linux.cc",
       "base/network_change_notifier_linux.h",
@@ -4678,6 +4680,7 @@ test("net_unittests") {
     "base/mime_util_unittest.cc",
     "base/net_string_util_unittest.cc",
     "base/network_activity_monitor_unittest.cc",
+    "base/network_change_notifier_chromeos_unittest.cc",
     "base/network_change_notifier_unittest.cc",
     "base/network_change_notifier_win_unittest.cc",
     "base/network_interfaces_linux_unittest.cc",

net/base/network_change_notifier.cc

@@ -33,6 +33,8 @@
 #include "net/base/network_change_notifier_linux.h"
 #elif defined(OS_MACOSX)
 #include "net/base/network_change_notifier_mac.h"
+#elif defined(OS_CHROMEOS)
+#include "net/base/network_change_notifier_chromeos.h"
 #elif defined(OS_FUCHSIA)
 #include "net/base/network_change_notifier_fuchsia.h"
 #endif
@@ -206,15 +208,12 @@ NetworkChangeNotifier* NetworkChangeNotifier::Create() {
       new NetworkChangeNotifierWin();
   network_change_notifier->WatchForAddressChange();
   return network_change_notifier;
-#elif defined(OS_CHROMEOS) || defined(OS_ANDROID)
-  // ChromeOS and Android builds MUST use their own class factory.
-#if !defined(OS_CHROMEOS)
-  // TODO(oshima): ash_shell do not have access to chromeos'es
-  // notifier yet. Re-enable this when chromeos'es notifier moved to
-  // chromeos root directory. crbug.com/119298.
+#elif defined(OS_ANDROID)
+  // Android builds MUST use their own class factory.
   CHECK(false);
-#endif
   return NULL;
+#elif defined(OS_CHROMEOS)
+  return new NetworkChangeNotifierChromeos();
 #elif defined(OS_LINUX)
   return new NetworkChangeNotifierLinux(std::unordered_set<std::string>());
 #elif defined(OS_MACOSX)

net/base/network_change_notifier_chromeos.cc

+// Copyright (c) 2012 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 <string>
+
+#include "base/bind.h"
+#include "net/base/network_change_notifier_chromeos.h"
+#include "net/dns/dns_config_service_posix.h"
+
+namespace net {
+
+// DNS config services on Chrome OS are signalled by the network state handler
+// rather than relying on watching files in /etc.
+class NetworkChangeNotifierChromeos::DnsConfigService
+    : public net::internal::DnsConfigServicePosix {
+ public:
+  DnsConfigService() = default;
+  ~DnsConfigService() override = default;
+
+  // net::internal::DnsConfigService() overrides.
+  bool StartWatching() override {
+    // DNS config changes are handled and notified by the network
+    // state handlers.
+    return true;
+  }
+
+  void OnNetworkChange() {
+    InvalidateConfig();
+    InvalidateHosts();
+    ReadNow();
+  }
+};
+
+NetworkChangeNotifierChromeos::NotifierThread::NotifierThread()
+    : base::Thread("NetworkChangeNotifier") {
+  DETACH_FROM_SEQUENCE(sequence_checker_);
+}
+
+NetworkChangeNotifierChromeos::NotifierThread::~NotifierThread() {
+  DCHECK(!Thread::IsRunning());
+}
+
+void NetworkChangeNotifierChromeos::NotifierThread::OnNetworkChange() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  dns_config_service_->OnNetworkChange();
+}
+
+void NetworkChangeNotifierChromeos::NotifierThread::Init() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  dns_config_service_.reset(new DnsConfigService());
+  dns_config_service_->WatchConfig(
+      base::BindRepeating(&NetworkChangeNotifier::SetDnsConfig));
+  dns_config_service_->OnNetworkChange();
+}
+
+void NetworkChangeNotifierChromeos::NotifierThread::CleanUp() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  dns_config_service_.reset();
+}
+
+NetworkChangeNotifierChromeos::NetworkChangeNotifierChromeos()
+    : NetworkChangeNotifier(NetworkChangeCalculatorParamsChromeos()),
+      connection_type_(CONNECTION_NONE),
+      max_bandwidth_mbps_(
+          NetworkChangeNotifier::GetMaxBandwidthMbpsForConnectionSubtype(
+              SUBTYPE_NONE)) {
+  notifier_thread_.StartWithOptions(
+      base::Thread::Options(base::MessageLoop::TYPE_IO, 0));
+}
+
+NetworkChangeNotifierChromeos::~NetworkChangeNotifierChromeos() {
+  notifier_thread_.Stop();
+}
+
+void NetworkChangeNotifierChromeos::OnDNSChanged() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  // The Unretained thread pointer is ok here because if the thread gets
+  // deleted, the callback won't be called.
+  notifier_thread_.task_runner()->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          &NetworkChangeNotifierChromeos::NotifierThread::OnNetworkChange,
+          base::Unretained(&notifier_thread_)));
+}
+
+void NetworkChangeNotifierChromeos::OnIPAddressChanged() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  NetworkChangeNotifier::NotifyObserversOfIPAddressChange();
+}
+
+void NetworkChangeNotifierChromeos::OnConnectionChanged(
+    NetworkChangeNotifier::ConnectionType connection_type) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  {
+    base::AutoLock scoped_lock(lock_);
+    connection_type_ = connection_type;
+  }
+  NetworkChangeNotifier::NotifyObserversOfConnectionTypeChange();
+}
+
+void NetworkChangeNotifierChromeos::OnConnectionSubtypeChanged(
+    NetworkChangeNotifier::ConnectionType connection_type,
+    NetworkChangeNotifier::ConnectionSubtype connection_subtype) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  double max_bandwidth_mbps =
+      GetMaxBandwidthMbpsForConnectionSubtype(connection_subtype);
+  {
+    base::AutoLock scoped_lock(lock_);
+    max_bandwidth_mbps_ = max_bandwidth_mbps;
+  }
+  NetworkChangeNotifier::NotifyObserversOfMaxBandwidthChange(max_bandwidth_mbps,
+                                                             connection_type);
+}
+
+NetworkChangeNotifier::ConnectionType
+NetworkChangeNotifierChromeos::GetCurrentConnectionType() const {
+  base::AutoLock scoped_lock(lock_);
+  return connection_type_;
+}
+
+void NetworkChangeNotifierChromeos::GetCurrentMaxBandwidthAndConnectionType(
+    double* max_bandwidth_mbps,
+    ConnectionType* connection_type) const {
+  base::AutoLock scoped_lock(lock_);
+  *connection_type = connection_type_;
+  *max_bandwidth_mbps = max_bandwidth_mbps_;
+}
+
+// static
+NetworkChangeNotifier::NetworkChangeCalculatorParams
+NetworkChangeNotifierChromeos::NetworkChangeCalculatorParamsChromeos() {
+  NetworkChangeCalculatorParams params;
+  // Delay values arrived at by simple experimentation and adjusted so as to
+  // produce a single signal when switching between network connections.
+  params.ip_address_offline_delay_ = base::TimeDelta::FromMilliseconds(4000);
+  params.ip_address_online_delay_ = base::TimeDelta::FromMilliseconds(1000);
+  params.connection_type_offline_delay_ =
+      base::TimeDelta::FromMilliseconds(500);
+  params.connection_type_online_delay_ = base::TimeDelta::FromMilliseconds(500);
+  return params;
+}
+
+}  // namespace net

net/base/network_change_notifier_chromeos.h

+// Copyright (c) 2012 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 NET_BASE_NETWORK_CHANGE_NOTIFIER_CHROMEOS_H_
+#define NET_BASE_NETWORK_CHANGE_NOTIFIER_CHROMEOS_H_
+
+#include "base/gtest_prod_util.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/sequence_checker.h"
+#include "base/synchronization/lock.h"
+#include "base/threading/thread.h"
+#include "base/threading/thread_checker.h"
+#include "net/base/net_export.h"
+#include "net/base/network_change_notifier.h"
+
+namespace net {
+
+// 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 only objects running in the browser process can listen for
+// network state changes.
+class NET_EXPORT NetworkChangeNotifierChromeos : public NetworkChangeNotifier {
+ public:
+  NetworkChangeNotifierChromeos();
+  ~NetworkChangeNotifierChromeos() override;
+
+  // These methods are used to notify this object that a network property has
+  // changed. These must be called from the thread that owns this object.
+  void OnDNSChanged();
+  void OnIPAddressChanged();
+  void OnConnectionChanged(
+      NetworkChangeNotifier::ConnectionType connection_type);
+  void OnConnectionSubtypeChanged(
+      NetworkChangeNotifier::ConnectionType connection_type,
+      NetworkChangeNotifier::ConnectionSubtype connection_subtype);
+
+ protected:
+  // NetworkChangeNotifier overrides.
+  NetworkChangeNotifier::ConnectionType GetCurrentConnectionType()
+      const override;
+  void GetCurrentMaxBandwidthAndConnectionType(
+      double* max_bandwidth_mbps,
+      ConnectionType* connection_type) const override;
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(NetworkChangeNotifierChromeosTest,
+                           ConnectionTypeFromShill);
+  friend class NetworkChangeNotifierChromeosTest;
+
+  class DnsConfigService;
+
+  // Thread on which we can run DnsConfigService, which requires a TYPE_IO
+  // message loop.
+  class NotifierThread : public base::Thread {
+   public:
+    NotifierThread();
+    ~NotifierThread() override;
+
+    void OnNetworkChange();
+
+   protected:
+    // base::Thread
+    void Init() override;
+    void CleanUp() override;
+
+   private:
+    std::unique_ptr<DnsConfigService> dns_config_service_;
+    SEQUENCE_CHECKER(sequence_checker_);
+
+    DISALLOW_COPY_AND_ASSIGN(NotifierThread);
+  };
+
+  // Calculates parameters used for network change notifier online/offline
+  // signals.
+  static NetworkChangeNotifier::NetworkChangeCalculatorParams
+  NetworkChangeCalculatorParamsChromeos();
+
+  THREAD_CHECKER(thread_checker_);
+
+  mutable base::Lock lock_;
+  NetworkChangeNotifier::ConnectionType
+      connection_type_;        // Guarded by |lock_|.
+  double max_bandwidth_mbps_;  // Guarded by |lock_|.
+
+  NotifierThread notifier_thread_;
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkChangeNotifierChromeos);
+};
+
+}  // namespace net
+
+#endif  // NET_BASE_NETWORK_CHANGE_NOTIFIER_CHROMEOS_H_

net/base/network_change_notifier_chromeos_unittest.cc

+// Copyright (c) 2012 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 "net/base/network_change_notifier_chromeos.h"
+
+#include "base/test/scoped_task_environment.h"
+#include "net/base/network_change_notifier.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace net {
+
+class NetworkChangeNotifierChromeosTest : public testing::Test {
+ public:
+  NetworkChangeNotifierChromeosTest()
+      : scoped_task_environment_(
+            base::test::ScopedTaskEnvironment::MainThreadType::MOCK_TIME),
+        notifier_(new NetworkChangeNotifierChromeos()) {}
+
+  void FastForwardUntilIdle() {
+    scoped_task_environment_.FastForwardUntilNoTasksRemain();
+  }
+
+  NetworkChangeNotifierChromeos* notifier() { return notifier_.get(); }
+
+ private:
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+  net::NetworkChangeNotifier::DisableForTest mock_notifier_disabler_;
+  std::unique_ptr<NetworkChangeNotifierChromeos> notifier_;
+};
+
+class MockIPAddressObserver : public NetworkChangeNotifier::IPAddressObserver {
+ public:
+  MOCK_METHOD0(OnIPAddressChanged, void());
+};
+
+TEST_F(NetworkChangeNotifierChromeosTest, OnIPAddressChanged) {
+  testing::StrictMock<MockIPAddressObserver> observer;
+  NetworkChangeNotifier::AddIPAddressObserver(&observer);
+
+  EXPECT_CALL(observer, OnIPAddressChanged());
+  notifier()->OnIPAddressChanged();
+  FastForwardUntilIdle();
+
+  NetworkChangeNotifier::RemoveIPAddressObserver(&observer);
+}
+
+class MockNetworkChangeObserver
+    : public NetworkChangeNotifier::NetworkChangeObserver {
+ public:
+  MOCK_METHOD1(OnNetworkChanged, void(NetworkChangeNotifier::ConnectionType));
+};
+
+TEST_F(NetworkChangeNotifierChromeosTest, OnNetworkChanged) {
+  testing::StrictMock<MockNetworkChangeObserver> observer;
+  NetworkChangeNotifier::AddNetworkChangeObserver(&observer);
+
+  EXPECT_CALL(observer,
+              OnNetworkChanged(NetworkChangeNotifier::CONNECTION_NONE));
+  EXPECT_CALL(observer, OnNetworkChanged(NetworkChangeNotifier::CONNECTION_3G));
+  notifier()->OnConnectionChanged(NetworkChangeNotifier::CONNECTION_3G);
+  FastForwardUntilIdle();
+
+  NetworkChangeNotifier::RemoveNetworkChangeObserver(&observer);
+}
+
+class MockMaxBandwidthObserver
+    : public NetworkChangeNotifier::MaxBandwidthObserver {
+ public:
+  MOCK_METHOD2(OnMaxBandwidthChanged,
+               void(double, NetworkChangeNotifier::ConnectionType));
+};
+
+TEST_F(NetworkChangeNotifierChromeosTest, OnMaxBandwidthChanged) {
+  testing::StrictMock<MockMaxBandwidthObserver> observer;
+  NetworkChangeNotifier::AddMaxBandwidthObserver(&observer);
+
+  EXPECT_CALL(observer,
+              OnMaxBandwidthChanged(3.6, NetworkChangeNotifier::CONNECTION_4G));
+  notifier()->OnConnectionSubtypeChanged(NetworkChangeNotifier::CONNECTION_4G,
+                                         NetworkChangeNotifier::SUBTYPE_HSPA);
+  FastForwardUntilIdle();
+
+  NetworkChangeNotifier::RemoveMaxBandwidthObserver(&observer);
+}
+
+}  // namespace net

services/network/network_change_manager.cc

@@ -9,6 +9,7 @@
 
 #include "base/logging.h"
 #include "net/base/network_change_notifier.h"
+#include "net/base/network_change_notifier_chromeos.h"
 
 namespace network {
 
@@ -43,6 +44,34 @@ void NetworkChangeManager::RequestNotifications(
   clients_.push_back(std::move(client_ptr));
 }
 
+#if defined(OS_CHROMEOS)
+void NetworkChangeManager::OnNetworkChanged(
+    bool dns_changed,
+    bool ip_address_changed,
+    bool connection_type_changed,
+    mojom::ConnectionType new_connection_type,
+    bool connection_subtype_changed,
+    mojom::ConnectionSubtype new_connection_subtype) {
+  DCHECK(network_change_notifier_);
+  net::NetworkChangeNotifierChromeos* notifier =
+      static_cast<net::NetworkChangeNotifierChromeos*>(
+          network_change_notifier_.get());
+  if (dns_changed)
+    notifier->OnDNSChanged();
+  if (ip_address_changed)
+    notifier->OnIPAddressChanged();
+  if (connection_type_changed) {
+    notifier->OnConnectionChanged(
+        net::NetworkChangeNotifier::ConnectionType(new_connection_type));
+  }
+  if (connection_type_changed || connection_subtype_changed) {
+    notifier->OnConnectionSubtypeChanged(
+        net::NetworkChangeNotifier::ConnectionType(new_connection_type),
+        net::NetworkChangeNotifier::ConnectionSubtype(new_connection_subtype));
+  }
+}
+#endif
+
 size_t NetworkChangeManager::GetNumClientsForTesting() const {
   return clients_.size();
 }

services/network/network_change_manager.h

@@ -41,6 +41,16 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkChangeManager
   void RequestNotifications(
       mojom::NetworkChangeManagerClientPtr client_ptr) override;
 
+#if defined(OS_CHROMEOS)
+  void OnNetworkChanged(
+      bool dns_changed,
+      bool ip_address_changed,
+      bool connection_type_changed,
+      mojom::ConnectionType new_connection_type,
+      bool connection_subtype_changed,
+      mojom::ConnectionSubtype new_connection_subtype) override;
+#endif
+
   size_t GetNumClientsForTesting() const;
 
  private:

services/network/network_context_unittest.cc

@@ -238,6 +238,7 @@ class NetworkContextTest : public testing::Test,
   NetworkContextTest()
       : scoped_task_environment_(
             base::test::ScopedTaskEnvironment::MainThreadType::IO),
+        network_change_notifier_(net::NetworkChangeNotifier::CreateMock()),
         network_service_(NetworkService::CreateForTesting()) {}
   ~NetworkContextTest() override {}
 
@@ -324,6 +325,7 @@ class NetworkContextTest : public testing::Test,
 
  protected:
   base::test::ScopedTaskEnvironment scoped_task_environment_;
+  std::unique_ptr<net::NetworkChangeNotifier> network_change_notifier_;
   std::unique_ptr<NetworkService> network_service_;
   // Stores the NetworkContextPtr of the most recently created NetworkContext.
   // Not strictly needed, but seems best to mimic real-world usage.

services/network/network_service.cc

@@ -89,11 +89,9 @@ CreateNetworkChangeNotifierIfNeeded() {
     // On Android, NetworkChangeNotifier objects are always set up in process
     // before NetworkService is run.
     return nullptr;
-#elif defined(OS_CHROMEOS) || defined(OS_IOS) || defined(OS_FUCHSIA)
-    // ChromeOS has its own implementation of NetworkChangeNotifier that lives
-    // outside of //net. iOS doesn't embed //content. Fuchsia doesn't have an
-    // implementation yet.
-    // TODO(xunjieli): Figure out what to do for these 3 platforms.
+#elif defined(OS_IOS) || defined(OS_FUCHSIA)
+    // iOS doesn't embed //content. Fuchsia doesn't have an implementation yet.
+    // TODO(xunjieli): Figure out what to do for these 2 platforms.
     NOTIMPLEMENTED();
     return nullptr;
 #endif

services/network/public/mojom/network_change_manager.mojom

@@ -18,6 +18,44 @@ enum ConnectionType {
   CONNECTION_LAST = CONNECTION_BLUETOOTH
 };
 
+// This needs to match the definition of net::ConnectionSubtype.
+enum ConnectionSubtype {
+  SUBTYPE_UNKNOWN = 0,
+  SUBTYPE_NONE,
+  SUBTYPE_OTHER,
+  SUBTYPE_GSM,
+  SUBTYPE_IDEN,
+  SUBTYPE_CDMA,
+  SUBTYPE_1XRTT,
+  SUBTYPE_GPRS,
+  SUBTYPE_EDGE,
+  SUBTYPE_UMTS,
+  SUBTYPE_EVDO_REV_0,
+  SUBTYPE_EVDO_REV_A,
+  SUBTYPE_HSPA,
+  SUBTYPE_EVDO_REV_B,
+  SUBTYPE_HSDPA,
+  SUBTYPE_HSUPA,
+  SUBTYPE_EHRPD,
+  SUBTYPE_HSPAP,
+  SUBTYPE_LTE,
+  SUBTYPE_LTE_ADVANCED,
+  SUBTYPE_BLUETOOTH_1_2,
+  SUBTYPE_BLUETOOTH_2_1,
+  SUBTYPE_BLUETOOTH_3_0,
+  SUBTYPE_BLUETOOTH_4_0,
+  SUBTYPE_ETHERNET,
+  SUBTYPE_FAST_ETHERNET,
+  SUBTYPE_GIGABIT_ETHERNET,
+  SUBTYPE_10_GIGABIT_ETHERNET,
+  SUBTYPE_WIFI_B,
+  SUBTYPE_WIFI_G,
+  SUBTYPE_WIFI_N,
+  SUBTYPE_WIFI_AC,
+  SUBTYPE_WIFI_AD,
+  SUBTYPE_LAST = SUBTYPE_WIFI_AD
+};
+
 // A client interface that subscribes to network change events from
 // NetworkChangeManager.
 interface NetworkChangeManagerClient {
@@ -43,8 +81,22 @@ interface NetworkChangeManagerClient {
   OnNetworkChanged(ConnectionType type);
 };
 
-// An interface that broadcasts network change events.
+// An interface for facilitating notifications of network change events.
 interface NetworkChangeManager {
   // Requests to receive notification when there is a network change.
   RequestNotifications(NetworkChangeManagerClient client_ptr);
+
+  // Notifies the network service that the network configuration has changed.
+  // This is needed for ChromeOS because only one process can listen for network
+  // changes from Shill, and currently that has to be the browser process. This
+  // allows the browser process to forward the network changes to the network
+  // service.
+  [EnableIf=is_chromeos]
+  OnNetworkChanged(
+      bool dns_changed,
+      bool ip_address_changed,
+      bool connection_type_changed,
+      ConnectionType new_connection_type,
+      bool connection_subtype_changed,
+      ConnectionSubtype new_connection_subtype);
 };

testing/buildbot/filters/mojo.fyi.chromeos.network_browser_tests.filter

@@ -13,26 +13,6 @@
 # https://crbug.com/892357
 -KioskUpdateTest.LaunchCachedOfflineEnabledAppNoNetwork
 
-# |NetworkChangeNotifier()| Not implemented.
-# https://crbug.com/882610
--ChromeBrowserMainBrowserTest.VariationsServiceStartsRequestOnNetworkChange
--NetworkConnectionTrackerBrowserTest.SimulateNetworkServiceCrash
--BackgroundFetchBrowserTest.FetchCanBePausedAndResumed
--BackgroundFetchBrowserTest.FetchesRunToCompletionAndUpdateTitle_Failed
--BackgroundFetchBrowserTest.FetchesRunToCompletionAndUpdateTitle_Fetched
--BackgroundFetchBrowserTest.FetchFromChildFrameWithPermissions
--BackgroundFetchBrowserTest.FetchFromServiceWorker
--BackgroundFetchBrowserTest.OfflineItemCollection_VerifyResourceDownloadedWhenDownloadTotalLargerThanActualSize
--BackgroundFetchBrowserTest.OfflineItemCollection_VerifyResourceDownloadedWhenDownloadTotalSmallerThanActualSize
-
-# Stuck at 'Network not available' page, should be related to the
-# |NetworkChangeNotifier()| issue.
-# https://crbug.com/882610
--WizardControllerDeviceStateExplicitRequirement/WizardControllerDeviceStateExplicitRequirementTest.ControlFlowForcedReEnrollment/0
--WizardControllerDeviceStateExplicitRequirement/WizardControllerDeviceStateExplicitRequirementTest.ControlFlowForcedReEnrollment/1
--WizardControllerDeviceStateTest.ControlFlowDeviceDisabled
--WizardControllerDeviceStateWithInitialEnrollmentTest.ControlFlowInitialEnrollment
-
 # Relies on net::URLRequestInterceptor.
 # https://crbug.com/884782
 # Convert the production code using example of how we add/remove