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}diff --git a/chrome/browser/chromeos/policy/app_install_event_log_collector.cc b/chrome/browser/chromeos/policy/app_install_event_log_collector.cc
index a6fcbe2..7b00403 100644
--- a/chrome/browser/chromeos/policy/app_install_event_log_collector.cc
+++ b/chrome/browser/chromeos/policy/app_install_event_log_collector.cc
@@ -65,7 +65,8 @@
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 @@
}
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 @@
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;
diff --git a/chrome/browser/chromeos/policy/app_install_event_log_collector.h b/chrome/browser/chromeos/policy/app_install_event_log_collector.h
index 7394f7e..6f7ac761 100644
--- a/chrome/browser/chromeos/policy/app_install_event_log_collector.h
+++ b/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 @@
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 @@
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,
diff --git a/chrome/browser/chromeos/policy/app_install_event_log_collector_unittest.cc b/chrome/browser/chromeos/policy/app_install_event_log_collector_unittest.cc
index b6b6ed0..21f291f 100644
--- a/chrome/browser/chromeos/policy/app_install_event_log_collector_unittest.cc
+++ b/chrome/browser/chromeos/policy/app_install_event_log_collector_unittest.cc
@@ -120,8 +120,6 @@
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 @@
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 @@
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 @@
// 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 @@
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());
diff --git a/chrome/browser/chromeos/policy/auto_enrollment_client_impl.cc b/chrome/browser/chromeos/policy/auto_enrollment_client_impl.cc
index 9121532b..3bd6673 100644
--- a/chrome/browser/chromeos/policy/auto_enrollment_client_impl.cc
+++ b/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::~AutoEnrollmentClientImpl() {
- net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
+ g_browser_process->network_connection_tracker()
+ ->RemoveNetworkConnectionObserver(this);
}
// static
@@ -386,8 +388,10 @@
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 @@
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();
}
diff --git a/chrome/browser/chromeos/policy/auto_enrollment_client_impl.h b/chrome/browser/chromeos/policy/auto_enrollment_client_impl.h
index 56595c0..15742b72 100644
--- a/chrome/browser/chromeos/policy/auto_enrollment_client_impl.h
+++ b/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 @@
// 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 @@
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)(
diff --git a/chrome/browser/chromeos/policy/auto_enrollment_client_impl_unittest.cc b/chrome/browser/chromeos/policy/auto_enrollment_client_impl_unittest.cc
index d07e02b..050a8cc 100644
--- a/chrome/browser/chromeos/policy/auto_enrollment_client_impl_unittest.cc
+++ b/chrome/browser/chromeos/policy/auto_enrollment_client_impl_unittest.cc
@@ -460,7 +460,8 @@
// 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 @@
// 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 @@
// 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 @@
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 @@
"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 @@
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 @@
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 @@
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 @@
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",
