Migrate chrome/browser/chromeos/policy to NetworkConnectionTracker

This migrates AutoEnrollmentClientImpl and AppInstallEventLoCollector
from using net::NetworkChangeNotifier to
content::NetworkConnectionTracker, which works with the network service
enabled.

Bug: 821009
Change-Id: I33dbd4ab0102d5a55bf34affdb7366b444e842ac
Reviewed-on: https://chromium-review.googlesource.com/1147602
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579187}

chrome/browser/chromeos/policy/app_install_event_log_collector.cc

@@ -65,7 +65,8 @@ AppInstallEventLogCollector::AppInstallEventLogCollector(
       pending_packages_(pending_packages) {
   chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->AddObserver(
       this);
-  net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
+  g_browser_process->network_connection_tracker()->AddNetworkConnectionObserver(
+      this);
   // Might not be available in unit test.
   arc::ArcPolicyBridge* const policy_bridge =
       arc::ArcPolicyBridge::GetForBrowserContext(profile_);
@@ -85,7 +86,8 @@ AppInstallEventLogCollector::~AppInstallEventLogCollector() {
   }
   chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->RemoveObserver(
       this);
-  net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
+  g_browser_process->network_connection_tracker()
+      ->RemoveNetworkConnectionObserver(this);
   arc::ArcPolicyBridge* const policy_bridge =
       arc::ArcPolicyBridge::GetForBrowserContext(profile_);
   if (policy_bridge) {
@@ -133,8 +135,8 @@ void AppInstallEventLogCollector::SuspendDone(
       CreateSessionChangeEvent(em::AppInstallReportLogEvent::RESUME));
 }
 
-void AppInstallEventLogCollector::OnNetworkChanged(
-    net::NetworkChangeNotifier::ConnectionType type) {
+void AppInstallEventLogCollector::OnConnectionChanged(
+    network::mojom::ConnectionType type) {
   const bool currently_online = GetOnlineState();
   if (currently_online == online_) {
     return;

chrome/browser/chromeos/policy/app_install_event_log_collector.h

@@ -18,7 +18,7 @@
 #include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
 #include "chromeos/dbus/power_manager_client.h"
 #include "components/arc/common/policy.mojom.h"
-#include "net/base/network_change_notifier.h"
+#include "content/public/browser/network_connection_tracker.h"
 
 class Profile;
 
@@ -31,7 +31,7 @@ namespace policy {
 class AppInstallEventLogCollector
     : public chromeos::PowerManagerClient::Observer,
       public arc::ArcPolicyBridge::Observer,
-      public net::NetworkChangeNotifier::NetworkChangeObserver,
+      public content::NetworkConnectionTracker::NetworkConnectionObserver,
       public ArcAppListPrefs::Observer {
  public:
   // The delegate that events are forwarded to for inclusion in the log.
@@ -76,9 +76,8 @@ class AppInstallEventLogCollector
   void SuspendImminent(power_manager::SuspendImminent::Reason reason) override;
   void SuspendDone(const base::TimeDelta& sleep_duration) override;
 
-  // net::NetworkChangeNotifier::NetworkChangeObserver:
-  void OnNetworkChanged(
-      net::NetworkChangeNotifier::ConnectionType type) override;
+  // content::NetworkConnectionTracker::NetworkConnectionObserver:
+  void OnConnectionChanged(network::mojom::ConnectionType type) override;
 
   // arc::ArcPolicyBridge::Observer:
   void OnCloudDpsRequested(base::Time time,

chrome/browser/chromeos/policy/app_install_event_log_collector_unittest.cc

@@ -120,8 +120,6 @@ class AppInstallEventLogCollectorTest : public testing::Test {
     chromeos::DBusThreadManager::Initialize();
     chromeos::NetworkHandler::Initialize();
     profile_ = std::make_unique<TestingProfile>();
-    network_change_notifier_ =
-        base::WrapUnique(net::NetworkChangeNotifier::CreateMock());
 
     service_test_ = chromeos::DBusThreadManager::Get()
                         ->GetShillServiceClient()
@@ -146,27 +144,29 @@ class AppInstallEventLogCollectorTest : public testing::Test {
     TestingBrowserProcess::GetGlobal()->SetLocalState(nullptr);
   }
 
-  void SetNetworkState(const std::string& service_path,
-                       const std::string& state) {
+  void SetNetworkState(
+      content::NetworkConnectionTracker::NetworkConnectionObserver* observer,
+      const std::string& service_path,
+      const std::string& state) {
     service_test_->SetServiceProperty(service_path, shill::kStateProperty,
                                       base::Value(state));
     base::RunLoop().RunUntilIdle();
 
-    net::NetworkChangeNotifier::ConnectionType connection_type =
-        net::NetworkChangeNotifier::CONNECTION_NONE;
+    network::mojom::ConnectionType connection_type =
+        network::mojom::ConnectionType::CONNECTION_NONE;
     std::string network_state;
     service_test_->GetServiceProperties(kWifiServicePath)
         ->GetString(shill::kStateProperty, &network_state);
     if (network_state == shill::kStateOnline) {
-      connection_type = net::NetworkChangeNotifier::CONNECTION_WIFI;
+      connection_type = network::mojom::ConnectionType::CONNECTION_WIFI;
     }
     service_test_->GetServiceProperties(kEthernetServicePath)
         ->GetString(shill::kStateProperty, &network_state);
     if (network_state == shill::kStateOnline) {
-      connection_type = net::NetworkChangeNotifier::CONNECTION_ETHERNET;
+      connection_type = network::mojom::ConnectionType::CONNECTION_ETHERNET;
     }
-    net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
-        connection_type);
+    if (observer)
+      observer->OnConnectionChanged(connection_type);
     base::RunLoop().RunUntilIdle();
   }
 
@@ -179,7 +179,6 @@ class AppInstallEventLogCollectorTest : public testing::Test {
 
   const std::set<std::string> packages_ = {kPackageName};
 
-  std::unique_ptr<net::NetworkChangeNotifier> network_change_notifier_;
   chromeos::ShillServiceClient::TestInterface* service_test_ = nullptr;
 
  private:
@@ -301,7 +300,7 @@ TEST_F(AppInstallEventLogCollectorTest, SuspendResume) {
 // to WiFi with a pending captive portal. Verify that no event is recorded.
 // Then, pass the captive portal. Verify that a connectivity change is recorded.
 TEST_F(AppInstallEventLogCollectorTest, ConnectivityChanges) {
-  SetNetworkState(kEthernetServicePath, shill::kStateOnline);
+  SetNetworkState(nullptr, kEthernetServicePath, shill::kStateOnline);
 
   std::unique_ptr<AppInstallEventLogCollector> collector =
       std::make_unique<AppInstallEventLogCollector>(delegate(), profile(),
@@ -317,22 +316,22 @@ TEST_F(AppInstallEventLogCollectorTest, ConnectivityChanges) {
             delegate()->last_event().session_state_change_type());
   EXPECT_TRUE(delegate()->last_event().online());
 
-  SetNetworkState(kWifiServicePath, shill::kStateOnline);
+  SetNetworkState(collector.get(), kWifiServicePath, shill::kStateOnline);
   EXPECT_EQ(1, delegate()->add_for_all_count());
 
-  SetNetworkState(kEthernetServicePath, shill::kStateOffline);
+  SetNetworkState(collector.get(), kEthernetServicePath, shill::kStateOffline);
   EXPECT_EQ(1, delegate()->add_for_all_count());
 
-  SetNetworkState(kWifiServicePath, shill::kStateOffline);
+  SetNetworkState(collector.get(), kWifiServicePath, shill::kStateOffline);
   EXPECT_EQ(2, delegate()->add_for_all_count());
   EXPECT_EQ(em::AppInstallReportLogEvent::CONNECTIVITY_CHANGE,
             delegate()->last_event().event_type());
   EXPECT_FALSE(delegate()->last_event().online());
 
-  SetNetworkState(kWifiServicePath, shill::kStatePortal);
+  SetNetworkState(collector.get(), kWifiServicePath, shill::kStatePortal);
   EXPECT_EQ(2, delegate()->add_for_all_count());
 
-  SetNetworkState(kWifiServicePath, shill::kStateOnline);
+  SetNetworkState(collector.get(), kWifiServicePath, shill::kStateOnline);
   EXPECT_EQ(3, delegate()->add_for_all_count());
   EXPECT_EQ(em::AppInstallReportLogEvent::CONNECTIVITY_CHANGE,
             delegate()->last_event().event_type());

chrome/browser/chromeos/policy/auto_enrollment_client_impl.cc

@@ -14,6 +14,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/optional.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/policy/server_backed_device_state.h"
 #include "chrome/common/chrome_content_client.h"
 #include "chrome/common/pref_names.h"
@@ -375,7 +376,8 @@ AutoEnrollmentClientImpl::FactoryImpl::CreateForInitialEnrollment(
 }
 
 AutoEnrollmentClientImpl::~AutoEnrollmentClientImpl() {
-  net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
+  g_browser_process->network_connection_tracker()
+      ->RemoveNetworkConnectionObserver(this);
 }
 
 // static
@@ -386,8 +388,10 @@ void AutoEnrollmentClientImpl::RegisterPrefs(PrefRegistrySimple* registry) {
 
 void AutoEnrollmentClientImpl::Start() {
   // (Re-)register the network change observer.
-  net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
-  net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
+  g_browser_process->network_connection_tracker()
+      ->RemoveNetworkConnectionObserver(this);
+  g_browser_process->network_connection_tracker()->AddNetworkConnectionObserver(
+      this);
 
   // Drop the previous job and reset state.
   request_job_.reset();
@@ -425,9 +429,9 @@ AutoEnrollmentState AutoEnrollmentClientImpl::state() const {
   return state_;
 }
 
-void AutoEnrollmentClientImpl::OnNetworkChanged(
-    net::NetworkChangeNotifier::ConnectionType type) {
-  if (type != net::NetworkChangeNotifier::CONNECTION_NONE &&
+void AutoEnrollmentClientImpl::OnConnectionChanged(
+    network::mojom::ConnectionType type) {
+  if (type != network::mojom::ConnectionType::CONNECTION_NONE &&
       !progress_callback_.is_null()) {
     RetryStep();
   }

chrome/browser/chromeos/policy/auto_enrollment_client_impl.h

@@ -15,7 +15,7 @@
 #include "base/time/time.h"
 #include "chrome/browser/chromeos/policy/auto_enrollment_client.h"
 #include "components/policy/core/common/cloud/cloud_policy_constants.h"
-#include "net/base/network_change_notifier.h"
+#include "content/public/browser/network_connection_tracker.h"
 #include "third_party/protobuf/src/google/protobuf/repeated_field.h"
 
 class PrefRegistrySimple;
@@ -35,7 +35,7 @@ class DeviceManagementRequestJob;
 // OOBE.
 class AutoEnrollmentClientImpl
     : public AutoEnrollmentClient,
-      public net::NetworkChangeNotifier::NetworkChangeObserver {
+      public content::NetworkConnectionTracker::NetworkConnectionObserver {
  public:
   // Subclasses of this class provide an identifier and specify the identifier
   // set for the DeviceAutoEnrollmentRequest,
@@ -86,9 +86,8 @@ class AutoEnrollmentClientImpl
   std::string device_id() const override;
   AutoEnrollmentState state() const override;
 
-  // Implementation of net::NetworkChangeNotifier::NetworkChangeObserver:
-  void OnNetworkChanged(
-      net::NetworkChangeNotifier::ConnectionType type) override;
+  // content::NetworkConnectionTracker::NetworkConnectionObserver:
+  void OnConnectionChanged(network::mojom::ConnectionType type) override;
 
  private:
   typedef bool (AutoEnrollmentClientImpl::*RequestCompletionHandler)(

chrome/browser/chromeos/policy/auto_enrollment_client_impl_unittest.cc

@@ -460,7 +460,8 @@ TEST_P(AutoEnrollmentClientImplTest, ConsumerDevice) {
 
   // Network changes don't trigger retries after obtaining a response from
   // the server.
-  client()->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+  client()->OnConnectionChanged(
+      network::mojom::ConnectionType::CONNECTION_ETHERNET);
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_NO_ENROLLMENT, state_);
 }
 
@@ -479,7 +480,8 @@ TEST_P(AutoEnrollmentClientImplTest, ForcedReEnrollment) {
 
   // Network changes don't trigger retries after obtaining a response from
   // the server.
-  client()->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+  client()->OnConnectionChanged(
+      network::mojom::ConnectionType::CONNECTION_ETHERNET);
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT, state_);
 }
 
@@ -502,7 +504,8 @@ TEST_P(AutoEnrollmentClientImplTest, ForcedReEnrollmentZeroTouch) {
 
   // Network changes don't trigger retries after obtaining a response from
   // the server.
-  client()->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+  client()->OnConnectionChanged(
+      network::mojom::ConnectionType::CONNECTION_ETHERNET);
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_TRIGGER_ZERO_TOUCH, state_);
 }
 
@@ -648,7 +651,8 @@ TEST_P(AutoEnrollmentClientImplTest, NetworkChangeRetryAfterErrors) {
   EXPECT_FALSE(HasServerBackedState());
 
   // The client doesn't retry if no new connection became available.
-  client()->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_NONE);
+  client()->OnConnectionChanged(
+      network::mojom::ConnectionType::CONNECTION_NONE);
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_SERVER_ERROR, state_);
   EXPECT_FALSE(HasCachedDecision());
   EXPECT_FALSE(HasServerBackedState());
@@ -659,7 +663,8 @@ TEST_P(AutoEnrollmentClientImplTest, NetworkChangeRetryAfterErrors) {
       "example.com",
       em::DeviceStateRetrievalResponse::RESTORE_MODE_REENROLLMENT_ENFORCED,
       kDisabledMessage);
-  client()->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+  client()->OnConnectionChanged(
+      network::mojom::ConnectionType::CONNECTION_ETHERNET);
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT, state_);
   EXPECT_TRUE(HasCachedDecision());
   VerifyServerBackedState("example.com",
@@ -667,8 +672,10 @@ TEST_P(AutoEnrollmentClientImplTest, NetworkChangeRetryAfterErrors) {
                           kDisabledMessage);
 
   // Subsequent network changes don't trigger retries.
-  client()->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_NONE);
-  client()->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+  client()->OnConnectionChanged(
+      network::mojom::ConnectionType::CONNECTION_NONE);
+  client()->OnConnectionChanged(
+      network::mojom::ConnectionType::CONNECTION_ETHERNET);
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT, state_);
   EXPECT_TRUE(HasCachedDecision());
   VerifyServerBackedState("example.com",
@@ -712,7 +719,8 @@ TEST_P(AutoEnrollmentClientImplTest, NetworkChangedAfterCancelAndDeleteSoon) {
   EXPECT_TRUE(base::MessageLoopCurrent::Get()->IsIdleForTesting());
 
   // Network change events are ignored while a request is pending.
-  client->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+  client->OnConnectionChanged(
+      network::mojom::ConnectionType::CONNECTION_ETHERNET);
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_PENDING, state_);
 
   // The client cleans itself up once a reply is received.
@@ -723,7 +731,8 @@ TEST_P(AutoEnrollmentClientImplTest, NetworkChangedAfterCancelAndDeleteSoon) {
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_PENDING, state_);
 
   // Network changes that have been posted before are also ignored:
-  client->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+  client->OnConnectionChanged(
+      network::mojom::ConnectionType::CONNECTION_ETHERNET);
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_PENDING, state_);
 }
 
@@ -786,7 +795,8 @@ TEST_P(AutoEnrollmentClientImplTest, NetworkFailureThenRequireUpdatedModulus) {
   EXPECT_CALL(*service_, StartJob(_, _, _, _, _, _));
 
   // Trigger a network change event.
-  client()->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+  client()->OnConnectionChanged(
+      network::mojom::ConnectionType::CONNECTION_ETHERNET);
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT, state_);
   EXPECT_TRUE(HasCachedDecision());
   VerifyServerBackedState("example.com",