[CrOS Cellular] Cond. reset service roaming property.

This change introduces a helper class that causes the
Service.AllowRoaming property to be reset to its default value when the
feature flag to enable per-network cellular roaming configuration is off
to prevent it from blocking any attempts to set Device.AllowRoaming. The
helper class provides an observer interface to allow observers to
receive notifications of each found cellular network and its roaming
state.

Bug: 1220691
Test: Verified on-device.
Change-Id: I8497a45cca7685b81565aa330ef22268ebb404bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3046577
Commit-Queue: Chad Duffin <chadduffin@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#908948}

chrome/browser/ash/policy/networking/device_network_configuration_updater.cc

@@ -6,11 +6,14 @@
 
 #include <map>
 
+#include "ash/constants/ash_features.h"
 #include "ash/constants/ash_switches.h"
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
+#include "base/feature_list.h"
 #include "chrome/browser/ash/policy/core/browser_policy_connector_ash.h"
+#include "chrome/browser/ash/policy/networking/network_roaming_state_migration_handler_impl.h"
 #include "chrome/browser/ash/settings/cros_settings.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_process_platform_part.h"
@@ -80,6 +83,11 @@ DeviceNetworkConfigurationUpdater::DeviceNetworkConfigurationUpdater(
           base::Unretained(this)));
   if (device_asset_id_fetcher_.is_null())
     device_asset_id_fetcher_ = base::BindRepeating(&GetDeviceAssetID);
+  if (!base::FeatureList::IsEnabled(
+          ash::features::kCellularAllowPerNetworkRoaming)) {
+    network_roaming_state_migration_handler_ =
+        std::make_unique<NetworkRoamingStateMigrationHandlerImpl>();
+  }
 }
 
 void DeviceNetworkConfigurationUpdater::Init() {

chrome/browser/ash/policy/networking/device_network_configuration_updater.h

@@ -33,6 +33,7 @@ class NetworkDeviceHandler;
 namespace policy {
 
 class PolicyService;
+class NetworkRoamingStateMigrationHandler;
 
 // Implements addtional special handling of ONC device policies, which requires
 // listening for notifications from CrosSettings.
@@ -78,6 +79,11 @@ class DeviceNetworkConfigurationUpdater : public NetworkConfigurationUpdater {
   ash::CrosSettings* cros_settings_;
   base::CallbackListSubscription data_roaming_setting_subscription_;
 
+  // This is only instantiated when the |kCellularAllowPerNetworkRoaming|
+  // feature flag is disabled.
+  std::unique_ptr<NetworkRoamingStateMigrationHandler>
+      network_roaming_state_migration_handler_;
+
   // Returns the device's administrator-set asset id.
   DeviceAssetIDFetcher device_asset_id_fetcher_;
 

chrome/browser/ash/policy/networking/fake_network_roaming_state_migration_handler.cc

@@ -0,0 +1,15 @@
+// Copyright 2021 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 "chrome/browser/ash/policy/networking/fake_network_roaming_state_migration_handler.h"
+
+namespace policy {
+
+FakeNetworkRoamingStateMigrationHandler::
+    FakeNetworkRoamingStateMigrationHandler() = default;
+
+FakeNetworkRoamingStateMigrationHandler::
+    ~FakeNetworkRoamingStateMigrationHandler() = default;
+
+}  // namespace policy

chrome/browser/ash/policy/networking/fake_network_roaming_state_migration_handler.h

@@ -0,0 +1,24 @@
+// Copyright 2021 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 CHROME_BROWSER_ASH_POLICY_NETWORKING_FAKE_NETWORK_ROAMING_STATE_MIGRATION_HANDLER_H_
+#define CHROME_BROWSER_ASH_POLICY_NETWORKING_FAKE_NETWORK_ROAMING_STATE_MIGRATION_HANDLER_H_
+
+#include "chrome/browser/ash/policy/networking/network_roaming_state_migration_handler.h"
+
+namespace policy {
+
+// Fake NetworkRoamingStateMigrationHandler implementation.
+class FakeNetworkRoamingStateMigrationHandler
+    : public NetworkRoamingStateMigrationHandler {
+ public:
+  FakeNetworkRoamingStateMigrationHandler();
+  ~FakeNetworkRoamingStateMigrationHandler() override;
+
+  using NetworkRoamingStateMigrationHandler::NotifyFoundCellularNetwork;
+};
+
+}  // namespace policy
+
+#endif  // CHROME_BROWSER_ASH_POLICY_NETWORKING_FAKE_NETWORK_ROAMING_STATE_MIGRATION_HANDLER_H_

chrome/browser/ash/policy/networking/network_roaming_state_migration_handler.cc

@@ -0,0 +1,30 @@
+// Copyright 2021 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 "chrome/browser/ash/policy/networking/network_roaming_state_migration_handler.h"
+
+namespace policy {
+
+NetworkRoamingStateMigrationHandler::NetworkRoamingStateMigrationHandler() =
+    default;
+
+NetworkRoamingStateMigrationHandler::~NetworkRoamingStateMigrationHandler() =
+    default;
+
+void NetworkRoamingStateMigrationHandler::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void NetworkRoamingStateMigrationHandler::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+void NetworkRoamingStateMigrationHandler::NotifyFoundCellularNetwork(
+    bool roaming_enabled) {
+  for (auto& observer : observers_) {
+    observer.OnFoundCellularNetwork(roaming_enabled);
+  }
+}
+
+}  // namespace policy

chrome/browser/ash/policy/networking/network_roaming_state_migration_handler.h

@@ -0,0 +1,47 @@
+// Copyright 2021 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 CHROME_BROWSER_ASH_POLICY_NETWORKING_NETWORK_ROAMING_STATE_MIGRATION_HANDLER_H_
+#define CHROME_BROWSER_ASH_POLICY_NETWORKING_NETWORK_ROAMING_STATE_MIGRATION_HANDLER_H_
+
+#include "base/observer_list.h"
+#include "base/observer_list_types.h"
+
+namespace policy {
+
+// This class is responsible for clearing the Service.AllowRoaming Shill
+// property of cellular networks, and notifying observers of whether cellular
+// roaming was enabled for each network handled. This class is only enabled when
+// per-network cellular roaming is disabled.
+// TODO(crbug.com/1232818): Remove when per-network cellular roaming is fully
+// launched.
+class NetworkRoamingStateMigrationHandler {
+ public:
+  class Observer : public base::CheckedObserver {
+   public:
+    ~Observer() override = default;
+
+    virtual void OnFoundCellularNetwork(bool roaming_enabled) = 0;
+  };
+
+  virtual ~NetworkRoamingStateMigrationHandler();
+
+  // Allows clients and register and de-register themselves.
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+ protected:
+  NetworkRoamingStateMigrationHandler();
+
+  // Notifies all observers that a cellular network was found, and whether or
+  // not roaming was enabled for the network.
+  void NotifyFoundCellularNetwork(bool roaming_enabled);
+
+ private:
+  base::ObserverList<Observer, /*check_empty=*/true> observers_;
+};
+
+}  // namespace policy
+
+#endif  // CHROME_BROWSER_ASH_POLICY_NETWORKING_NETWORK_ROAMING_STATE_MIGRATION_HANDLER_H_

chrome/browser/ash/policy/networking/network_roaming_state_migration_handler_impl.cc

@@ -0,0 +1,75 @@
+// Copyright 2021 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 "chrome/browser/ash/policy/networking/network_roaming_state_migration_handler_impl.h"
+
+#include "ash/constants/ash_features.h"
+#include "base/callback_helpers.h"
+#include "base/feature_list.h"
+#include "chromeos/dbus/shill/shill_service_client.h"
+#include "chromeos/network/network_handler.h"
+#include "chromeos/network/network_handler_callbacks.h"
+#include "chromeos/network/network_state_handler.h"
+#include "chromeos/network/network_type_pattern.h"
+#include "components/device_event_log/device_event_log.h"
+#include "dbus/object_path.h"
+#include "third_party/cros_system_api/dbus/shill/dbus-constants.h"
+
+namespace policy {
+
+NetworkRoamingStateMigrationHandlerImpl::
+    NetworkRoamingStateMigrationHandlerImpl() {
+  DCHECK(!base::FeatureList::IsEnabled(
+      ash::features::kCellularAllowPerNetworkRoaming));
+  if (chromeos::NetworkHandler::IsInitialized()) {
+    network_state_handler_ =
+        chromeos::NetworkHandler::Get()->network_state_handler();
+    network_state_handler_->AddObserver(this, FROM_HERE);
+  }
+}
+
+NetworkRoamingStateMigrationHandlerImpl::
+    ~NetworkRoamingStateMigrationHandlerImpl() {
+  if (network_state_handler_) {
+    network_state_handler_->RemoveObserver(this, FROM_HERE);
+  }
+}
+
+void NetworkRoamingStateMigrationHandlerImpl::NetworkListChanged() {
+  DCHECK(network_state_handler_);
+
+  chromeos::NetworkStateHandler::NetworkStateList network_state_list;
+  network_state_handler_->GetNetworkListByType(
+      chromeos::NetworkTypePattern::Cellular(), /*configured_only=*/false,
+      /*visible_only=*/false, /*limit=*/0, &network_state_list);
+
+  for (const chromeos::NetworkState* network_state : network_state_list) {
+    if (network_state->IsNonShillCellularNetwork() ||
+        processed_cellular_guids_.contains(network_state->guid())) {
+      continue;
+    }
+    NET_LOG(USER) << "Service.ClearProperty: "
+                  << shill::kCellularAllowRoamingProperty;
+
+    ash::ShillServiceClient::Get()->ClearProperty(
+        dbus::ObjectPath(network_state->path()),
+        shill::kCellularAllowRoamingProperty, base::DoNothing(),
+        base::BindOnce(
+            &ash::network_handler::ShillErrorCallbackFunction,
+            "NetworkRoamingStateMigrationHandlerImpl.ClearProperty Failed",
+            network_state->path(), ash::network_handler::ErrorCallback()));
+    processed_cellular_guids_.insert(network_state->guid());
+
+    NotifyFoundCellularNetwork(network_state->allow_roaming());
+  }
+}
+
+void NetworkRoamingStateMigrationHandlerImpl::OnShuttingDown() {
+  if (network_state_handler_) {
+    network_state_handler_->RemoveObserver(this, FROM_HERE);
+    network_state_handler_ = nullptr;
+  }
+}
+
+}  // namespace policy

chrome/browser/ash/policy/networking/network_roaming_state_migration_handler_impl.h

@@ -0,0 +1,41 @@
+// Copyright 2021 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 CHROME_BROWSER_ASH_POLICY_NETWORKING_NETWORK_ROAMING_STATE_MIGRATION_HANDLER_IMPL_H_
+#define CHROME_BROWSER_ASH_POLICY_NETWORKING_NETWORK_ROAMING_STATE_MIGRATION_HANDLER_IMPL_H_
+
+#include <string>
+
+#include "base/containers/flat_set.h"
+#include "chrome/browser/ash/policy/networking/network_roaming_state_migration_handler.h"
+#include "chromeos/network/network_state_handler_observer.h"
+
+namespace chromeos {
+class NetworkStateHandler;
+}  // namespace chromeos
+
+namespace policy {
+
+// AdapterStateController implementation which uses NetworkStateHandlerObserver.
+class NetworkRoamingStateMigrationHandlerImpl
+    : public chromeos::NetworkStateHandlerObserver,
+      public NetworkRoamingStateMigrationHandler {
+ public:
+  NetworkRoamingStateMigrationHandlerImpl();
+  ~NetworkRoamingStateMigrationHandlerImpl() override;
+
+ private:
+  // NetworkStateHandlerObserver:
+  void NetworkListChanged() override;
+  void OnShuttingDown() override;
+
+  // The cellular networks that have already been processed.
+  base::flat_set<std::string> processed_cellular_guids_;
+
+  chromeos::NetworkStateHandler* network_state_handler_ = nullptr;
+};
+
+}  // namespace policy
+
+#endif  // CHROME_BROWSER_ASH_POLICY_NETWORKING_NETWORK_ROAMING_STATE_MIGRATION_HANDLER_IMPL_H_