Move NetworkStateHandlerObserver clients in chromeos/ash/ to

base::ScopedObservation.

Bug: 1332551
Change-Id: I76df0aab689bbcd611f46e84fa5c39f8f2a6c31c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688073
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Auto-Submit: Andrew Rayskiy <greengrape@google.com>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1067624}

chromeos/ash/components/device_activity/device_activity_client.cc

@@ -13,10 +13,12 @@
 #include "base/strings/strcat.h"
 #include "base/time/time.h"
 #include "chromeos/ash/components/dbus/system_clock/system_clock_sync_observation.h"
+#include "chromeos/ash/components/network/network_state.h"
+#include "chromeos/ash/components/network/network_state_handler.h"
+#include "services/network/public/cpp/resource_request.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "services/network/public/cpp/simple_url_loader.h"
 #include "services/network/public/mojom/url_response_head.mojom.h"
-#include "third_party/private_membership/src/private_membership_rlwe_client.h"
 
 namespace ash::device_activity {
 
@@ -229,15 +231,13 @@ DeviceActivityClient::DeviceActivityClient(
   report_timer_->Start(FROM_HERE, kTimeToRepeat, this,
                        &DeviceActivityClient::ReportingTriggeredByTimer);
 
-  network_state_handler_->AddObserver(this, FROM_HERE);
+  network_state_handler_observer_.Observe(network_state_handler_);
   DefaultNetworkChanged(network_state_handler_->DefaultNetwork());
 }
 
 DeviceActivityClient::~DeviceActivityClient() {
   RecordDeviceActivityMethodCalled(DeviceActivityClient::DeviceActivityMethod::
                                        kDeviceActivityClientDestructor);
-
-  network_state_handler_->RemoveObserver(this, FROM_HERE);
 }
 
 base::RepeatingTimer* DeviceActivityClient::GetReportTimer() {
@@ -259,6 +259,10 @@ void DeviceActivityClient::DefaultNetworkChanged(const NetworkState* network) {
     OnNetworkOffline();
 }
 
+void DeviceActivityClient::OnShuttingDown() {
+  network_state_handler_observer_.Reset();
+}
+
 DeviceActivityClient::State DeviceActivityClient::GetState() const {
   return state_;
 }

chromeos/ash/components/device_activity/device_activity_client.h

@@ -6,18 +6,15 @@
 #define CHROMEOS_ASH_COMPONENTS_DEVICE_ACTIVITY_DEVICE_ACTIVITY_CLIENT_H_
 
 #include <memory>
+#include <queue>
 
 #include "base/component_export.h"
-#include "base/containers/queue.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
 #include "base/timer/elapsed_timer.h"
 #include "base/timer/timer.h"
-#include "chromeos/ash/components/network/network_state.h"
-#include "chromeos/ash/components/network/network_state_handler.h"
 #include "chromeos/ash/components/network/network_state_handler_observer.h"
-#include "services/network/public/cpp/resource_request.h"
 #include "third_party/private_membership/src/private_membership_rlwe_client.h"
 #include "url/gurl.h"
 
@@ -28,6 +25,8 @@ class SharedURLLoaderFactory;
 
 namespace ash {
 
+class NetworkState;
+class NetworkStateHandler;
 class SystemClockSyncObservation;
 
 namespace device_activity {
@@ -129,6 +128,7 @@ class COMPONENT_EXPORT(CHROMEOS_ASH_COMPONENTS_DEVICE_ACTIVITY)
 
   // NetworkStateHandlerObserver overridden method.
   void DefaultNetworkChanged(const NetworkState* network) override;
+  void OnShuttingDown() override;
 
   State GetState() const;
 
@@ -265,6 +265,8 @@ class COMPONENT_EXPORT(CHROMEOS_ASH_COMPONENTS_DEVICE_ACTIVITY)
   // Used to wait until the system clock to be synchronized.
   std::unique_ptr<SystemClockSyncObservation> system_clock_sync_observation_;
 
+  NetworkStateHandlerScopedObservation network_state_handler_observer_{this};
+
   // Automatically cancels callbacks when the referent of weakptr gets
   // destroyed.
   base::WeakPtrFactory<DeviceActivityClient> weak_factory_{this};

chromeos/ash/components/network/auto_connect_handler.cc

@@ -20,6 +20,7 @@
 #include "chromeos/ash/components/network/network_connection_handler.h"
 #include "chromeos/ash/components/network/network_event_log.h"
 #include "chromeos/ash/components/network/network_state.h"
+#include "chromeos/ash/components/network/network_state_handler.h"
 #include "chromeos/ash/components/network/network_type_pattern.h"
 #include "dbus/object_path.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"

chromeos/ash/components/network/auto_connect_handler.h

@@ -10,17 +10,17 @@
 #include "base/component_export.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
-#include "base/scoped_observation.h"
 #include "chromeos/ash/components/network/client_cert_resolver.h"
 #include "chromeos/ash/components/network/network_connection_observer.h"
-#include "chromeos/ash/components/network/network_handler.h"
 #include "chromeos/ash/components/network/network_policy_observer.h"
-#include "chromeos/ash/components/network/network_state_handler.h"
 #include "chromeos/ash/components/network/network_state_handler_observer.h"
 #include "chromeos/login/login_state/login_state.h"
 
 namespace ash {
 
+class NetworkHandler;
+class NetworkStateHandler;
+
 class COMPONENT_EXPORT(CHROMEOS_NETWORK) AutoConnectHandler
     : public LoginState::Observer,
       public NetworkPolicyObserver,
@@ -138,8 +138,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) AutoConnectHandler
   ClientCertResolver* client_cert_resolver_;
   NetworkConnectionHandler* network_connection_handler_;
   NetworkStateHandler* network_state_handler_;
-  base::ScopedObservation<NetworkStateHandler, NetworkStateHandlerObserver>
-      network_state_handler_observer_{this};
+  NetworkStateHandlerScopedObservation network_state_handler_observer_{this};
   ManagedNetworkConfigurationHandler* managed_configuration_handler_;
 
   // Whether a request to connect to the best network is pending. If true, once

chromeos/ash/components/network/metrics/connection_info_metrics_logger.cc

@@ -38,8 +38,6 @@ bool ConnectionInfoMetricsLogger::ConnectionInfo::operator==(
 ConnectionInfoMetricsLogger::ConnectionInfoMetricsLogger() = default;
 
 ConnectionInfoMetricsLogger::~ConnectionInfoMetricsLogger() {
-  if (network_state_handler_)
-    network_state_handler_->RemoveObserver(this, FROM_HERE);
   if (network_connection_handler_)
     network_connection_handler_->RemoveObserver(this);
 }
@@ -54,7 +52,7 @@ void ConnectionInfoMetricsLogger::Init(
 
   if (network_state_handler) {
     network_state_handler_ = network_state_handler;
-    network_state_handler_->AddObserver(this, FROM_HERE);
+    network_state_handler_observer_.Observe(network_state_handler_);
     NetworkListChanged();
   }
 }
@@ -95,6 +93,10 @@ void ConnectionInfoMetricsLogger::NetworkConnectionStateChanged(
   UpdateConnectionInfo(network);
 }
 
+void ConnectionInfoMetricsLogger::OnShuttingDown() {
+  network_state_handler_observer_.Reset();
+}
+
 void ConnectionInfoMetricsLogger::ConnectSucceeded(
     const std::string& service_path) {
   const NetworkState* network =

chromeos/ash/components/network/metrics/connection_info_metrics_logger.h

@@ -90,6 +90,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) ConnectionInfoMetricsLogger
   // NetworkStateHandlerObserver::
   void NetworkListChanged() override;
   void NetworkConnectionStateChanged(const NetworkState* network) override;
+  void OnShuttingDown() override;
 
   // NetworkConnectionObserver::
   void ConnectSucceeded(const std::string& service_path) override;
@@ -111,6 +112,8 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) ConnectionInfoMetricsLogger
   NetworkStateHandler* network_state_handler_ = nullptr;
   NetworkConnectionHandler* network_connection_handler_ = nullptr;
 
+  NetworkStateHandlerScopedObservation network_state_handler_observer_{this};
+
   // Stores connection information for all networks.
   base::flat_map<std::string, ConnectionInfo> guid_to_connection_info_;
 

chromeos/ash/components/network/metrics/esim_policy_login_metrics_logger.cc

@@ -6,7 +6,6 @@
 
 #include "base/metrics/histogram_macros.h"
 #include "chromeos/ash/components/network/managed_network_configuration_handler.h"
-#include "chromeos/ash/components/network/network_event_log.h"
 #include "chromeos/ash/components/network/network_state_handler.h"
 
 namespace ash {
@@ -64,11 +63,6 @@ void ESimPolicyLoginMetricsLogger::RecordBlockNonManagedCellularBehavior(
 ESimPolicyLoginMetricsLogger::ESimPolicyLoginMetricsLogger() = default;
 
 ESimPolicyLoginMetricsLogger::~ESimPolicyLoginMetricsLogger() {
-  if (network_state_handler_) {
-    network_state_handler_->RemoveObserver(this, FROM_HERE);
-    network_state_handler_ = nullptr;
-  }
-
   if (initialized_ && LoginState::IsInitialized()) {
     LoginState::Get()->RemoveObserver(this);
   }
@@ -80,7 +74,7 @@ void ESimPolicyLoginMetricsLogger::Init(
   network_state_handler_ = network_state_handler;
   managed_network_configuration_handler_ =
       managed_network_configuration_handler;
-  network_state_handler_->AddObserver(this, FROM_HERE);
+  network_state_handler_observer_.Observe(network_state_handler_);
 
   if (LoginState::IsInitialized())
     LoginState::Get()->AddObserver(this);
@@ -148,6 +142,10 @@ void ESimPolicyLoginMetricsLogger::DeviceListChanged() {
       &ESimPolicyLoginMetricsLogger::LogESimPolicyStatusAtLogin);
 }
 
+void ESimPolicyLoginMetricsLogger::OnShuttingDown() {
+  network_state_handler_observer_.Reset();
+}
+
 void ESimPolicyLoginMetricsLogger::SetIsEnterpriseManaged(
     bool is_enterprise_managed) {
   is_enterprise_managed_ = is_enterprise_managed;

chromeos/ash/components/network/metrics/esim_policy_login_metrics_logger.h

@@ -59,6 +59,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) ESimPolicyLoginMetricsLogger
 
   // NetworkStateHandlerObserver::
   void DeviceListChanged() override;
+  void OnShuttingDown() override;
 
   void SetIsEnterpriseManaged(bool is_enterprise_managed);
 
@@ -95,6 +96,8 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) ESimPolicyLoginMetricsLogger
   ManagedNetworkConfigurationHandler* managed_network_configuration_handler_ =
       nullptr;
 
+  NetworkStateHandlerScopedObservation network_state_handler_observer_{this};
+
   // A timer to wait for cellular initialization. This is useful
   // to avoid tracking intermediate states when cellular network is
   // starting up.

chromeos/ash/components/network/network_state_handler_observer.h

@@ -9,12 +9,14 @@
 #include <vector>
 
 #include "base/component_export.h"
+#include "base/scoped_observation.h"
 #include "chromeos/ash/components/network/network_state.h"
 #include "chromeos/ash/components/network/network_type_pattern.h"
 
 namespace ash {
 
 class DeviceState;
+class NetworkStateHandler;
 
 // Observer class for all network state changes, including changes to
 // active (connecting or connected) services.
@@ -91,11 +93,15 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkStateHandlerObserver {
   virtual void OnShuttingDown();
 };
 
+using NetworkStateHandlerScopedObservation =
+    base::ScopedObservation<NetworkStateHandler, NetworkStateHandlerObserver>;
+
 }  // namespace ash
 
 // TODO(https://crbug.com/1164001): remove when the migration is finished.
 namespace chromeos {
 using ::ash::NetworkStateHandlerObserver;
-}
+using ::ash::NetworkStateHandlerScopedObservation;
+}  // namespace chromeos
 
 #endif  // CHROMEOS_ASH_COMPONENTS_NETWORK_NETWORK_STATE_HANDLER_OBSERVER_H_

chromeos/ash/components/network/proxy/proxy_config_service_impl.cc

@@ -77,17 +77,12 @@ ProxyConfigServiceImpl::ProxyConfigServiceImpl(
     // Register for changes to the default network.
     NetworkStateHandler* state_handler =
         NetworkHandler::Get()->network_state_handler();
-    state_handler->AddObserver(this, FROM_HERE);
+    network_state_handler_observer_.Observe(state_handler);
     DefaultNetworkChanged(state_handler->DefaultNetwork());
   }
 }
 
-ProxyConfigServiceImpl::~ProxyConfigServiceImpl() {
-  if (NetworkHandler::IsInitialized()) {
-    NetworkHandler::Get()->network_state_handler()->RemoveObserver(this,
-                                                                   FROM_HERE);
-  }
-}
+ProxyConfigServiceImpl::~ProxyConfigServiceImpl() = default;
 
 void ProxyConfigServiceImpl::OnProxyConfigChanged(
     ProxyPrefs::ConfigState config_state,
@@ -122,8 +117,7 @@ void ProxyConfigServiceImpl::DefaultNetworkChanged(
 void ProxyConfigServiceImpl::OnShuttingDown() {
   // Ownership of this class is complicated. Stop observing NetworkStateHandler
   // when the class shuts down.
-  NetworkHandler::Get()->network_state_handler()->RemoveObserver(this,
-                                                                 FROM_HERE);
+  network_state_handler_observer_.Reset();
 }
 
 // static

chromeos/ash/components/network/proxy/proxy_config_service_impl.h

@@ -7,7 +7,6 @@
 
 #include <string>
 
-#include "base/compiler_specific.h"
 #include "base/component_export.h"
 #include "base/task/single_thread_task_runner.h"
 #include "chromeos/ash/components/network/network_state_handler_observer.h"
@@ -18,6 +17,7 @@
 namespace ash {
 
 class NetworkState;
+class NetworkStateHandler;
 
 // Implementation of proxy config service for chromeos that:
 // - extends PrefProxyConfigTrackerImpl (and so lives and runs entirely on UI
@@ -102,6 +102,8 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) ProxyConfigServiceImpl
   // Not owned.
   PrefService* local_state_prefs_;
 
+  NetworkStateHandlerScopedObservation network_state_handler_observer_{this};
+
   base::WeakPtrFactory<ProxyConfigServiceImpl> pointer_factory_{this};
 };
 

chromeos/ash/components/sync_wifi/synced_network_metrics_logger.cc

@@ -8,7 +8,6 @@
 #include "base/metrics/histogram_functions.h"
 #include "chromeos/ash/components/network/network_configuration_handler.h"
 #include "chromeos/ash/components/network/network_connection_handler.h"
-#include "chromeos/ash/components/network/network_event_log.h"
 #include "chromeos/ash/components/network/network_metadata_store.h"
 #include "chromeos/ash/components/network/network_state_handler.h"
 #include "chromeos/ash/components/sync_wifi/network_eligibility_checker.h"
@@ -118,7 +117,7 @@ SyncedNetworkMetricsLogger::SyncedNetworkMetricsLogger(
 
   if (network_state_handler) {
     network_state_handler_ = network_state_handler;
-    network_state_handler_->AddObserver(this, FROM_HERE);
+    network_state_handler_observer_.Observe(network_state_handler_);
   }
 
   if (network_connection_handler) {
@@ -139,10 +138,8 @@ SyncedNetworkMetricsLogger::~SyncedNetworkMetricsLogger() {
 }
 
 void SyncedNetworkMetricsLogger::OnShuttingDown() {
-  if (network_state_handler_) {
-    network_state_handler_->RemoveObserver(this, FROM_HERE);
-    network_state_handler_ = nullptr;
-  }
+  network_state_handler_observer_.Reset();
+  network_state_handler_ = nullptr;
 
   if (network_connection_handler_) {
     network_connection_handler_->RemoveObserver(this);

chromeos/ash/components/sync_wifi/synced_network_metrics_logger.h

@@ -135,6 +135,8 @@ class SyncedNetworkMetricsLogger : public NetworkConnectionObserver,
   NetworkStateHandler* network_state_handler_ = nullptr;
   NetworkConnectionHandler* network_connection_handler_ = nullptr;
 
+  NetworkStateHandlerScopedObservation network_state_handler_observer_{this};
+
   // Contains the guids of networks which are currently connecting.
   base::flat_set<std::string> connecting_guids_;