diff options
18 files changed, 650 insertions, 178 deletions
diff --git a/chrome/browser/chromeos/policy/device_network_configuration_updater.cc b/chrome/browser/chromeos/policy/device_network_configuration_updater.cc new file mode 100644 index 0000000..a68cd31a3 --- /dev/null +++ b/chrome/browser/chromeos/policy/device_network_configuration_updater.cc @@ -0,0 +1,112 @@ +// Copyright 2013 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/chromeos/policy/device_network_configuration_updater.h" + +#include "base/bind.h" +#include "base/bind_helpers.h" +#include "chrome/browser/chromeos/settings/cros_settings.h" +#include "chromeos/network/managed_network_configuration_handler.h" +#include "chromeos/network/network_device_handler.h" +#include "chromeos/network/onc/onc_certificate_importer.h" +#include "chromeos/settings/cros_settings_names.h" +#include "chromeos/settings/cros_settings_provider.h" +#include "policy/policy_constants.h" + +namespace policy { + +DeviceNetworkConfigurationUpdater::~DeviceNetworkConfigurationUpdater() {} + +// static +scoped_ptr<DeviceNetworkConfigurationUpdater> +DeviceNetworkConfigurationUpdater::CreateForDevicePolicy( + scoped_ptr<chromeos::onc::CertificateImporter> certificate_importer, + PolicyService* policy_service, + chromeos::ManagedNetworkConfigurationHandler* network_config_handler, + chromeos::NetworkDeviceHandler* network_device_handler, + chromeos::CrosSettings* cros_settings) { + scoped_ptr<DeviceNetworkConfigurationUpdater> updater( + new DeviceNetworkConfigurationUpdater(certificate_importer.Pass(), + policy_service, + network_config_handler, + network_device_handler, + cros_settings)); + updater->Init(); + return updater.Pass(); +} + +DeviceNetworkConfigurationUpdater::DeviceNetworkConfigurationUpdater( + scoped_ptr<chromeos::onc::CertificateImporter> certificate_importer, + PolicyService* policy_service, + chromeos::ManagedNetworkConfigurationHandler* network_config_handler, + chromeos::NetworkDeviceHandler* network_device_handler, + chromeos::CrosSettings* cros_settings) + : NetworkConfigurationUpdater(onc::ONC_SOURCE_DEVICE_POLICY, + key::kDeviceOpenNetworkConfiguration, + certificate_importer.Pass(), + policy_service, + network_config_handler), + network_device_handler_(network_device_handler), + cros_settings_(cros_settings), + weak_factory_(this) { + DCHECK(network_device_handler_); + data_roaming_setting_subscription_ = cros_settings->AddSettingsObserver( + chromeos::kSignedDataRoamingEnabled, + base::Bind( + &DeviceNetworkConfigurationUpdater::OnDataRoamingSettingChanged, + base::Unretained(this))); +} + +void DeviceNetworkConfigurationUpdater::Init() { + NetworkConfigurationUpdater::Init(); + + // Apply the roaming setting initially. + OnDataRoamingSettingChanged(); +} + +void DeviceNetworkConfigurationUpdater::ImportCertificates( + const base::ListValue& certificates_onc) { + certificate_importer_->ImportCertificates( + certificates_onc, onc_source_, NULL); +} + +void DeviceNetworkConfigurationUpdater::ApplyNetworkPolicy( + base::ListValue* network_configs_onc, + base::DictionaryValue* global_network_config) { + network_config_handler_->SetPolicy(onc_source_, + std::string() /* no username hash */, + *network_configs_onc, + *global_network_config); +} + +void DeviceNetworkConfigurationUpdater::OnDataRoamingSettingChanged() { + chromeos::CrosSettingsProvider::TrustedStatus trusted_status = + cros_settings_->PrepareTrustedValues(base::Bind( + &DeviceNetworkConfigurationUpdater::OnDataRoamingSettingChanged, + weak_factory_.GetWeakPtr())); + + if (trusted_status == chromeos::CrosSettingsProvider::TEMPORARILY_UNTRUSTED) { + // Return, this function will be called again later by + // PrepareTrustedValues. + return; + } + + bool data_roaming_setting = false; + if (trusted_status == chromeos::CrosSettingsProvider::TRUSTED) { + if (!cros_settings_->GetBoolean(chromeos::kSignedDataRoamingEnabled, + &data_roaming_setting)) { + LOG(ERROR) << "Couldn't get device setting " + << chromeos::kSignedDataRoamingEnabled; + data_roaming_setting = false; + } + } else { + DCHECK_EQ(trusted_status, + chromeos::CrosSettingsProvider::PERMANENTLY_UNTRUSTED); + // Roaming is disabled as we can't determine the correct setting. + } + + network_device_handler_->SetCellularAllowRoaming(data_roaming_setting); +} + +} // namespace policy diff --git a/chrome/browser/chromeos/policy/device_network_configuration_updater.h b/chrome/browser/chromeos/policy/device_network_configuration_updater.h new file mode 100644 index 0000000..6b9b762f --- /dev/null +++ b/chrome/browser/chromeos/policy/device_network_configuration_updater.h @@ -0,0 +1,83 @@ +// Copyright 2013 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_CHROMEOS_POLICY_DEVICE_NETWORK_CONFIGURATION_UPDATER_H_ +#define CHROME_BROWSER_CHROMEOS_POLICY_DEVICE_NETWORK_CONFIGURATION_UPDATER_H_ + +#include <string> + +#include "base/basictypes.h" +#include "base/callback_list.h" +#include "base/compiler_specific.h" +#include "base/memory/scoped_ptr.h" +#include "base/memory/weak_ptr.h" +#include "chrome/browser/chromeos/policy/network_configuration_updater.h" +#include "components/onc/onc_constants.h" + +namespace base { +class DictionaryValue; +class ListValue; +class Value; +} + +namespace chromeos { +class CrosSettings; +class ManagedNetworkConfigurationHandler; +class NetworkDeviceHandler; + +namespace onc { +class CertificateImporter; +} +} + +namespace policy { + +class PolicyService; + +// Implements addtional special handling of ONC device policies, which requires +// listening for notifications from CrosSettings. +class DeviceNetworkConfigurationUpdater : public NetworkConfigurationUpdater { + public: + virtual ~DeviceNetworkConfigurationUpdater(); + + // Creates an updater that applies the ONC device policy from |policy_service| + // once the policy service is completely initialized and on each policy + // change. The argument objects must outlive the returned updater. + static scoped_ptr<DeviceNetworkConfigurationUpdater> CreateForDevicePolicy( + scoped_ptr<chromeos::onc::CertificateImporter> certificate_importer, + PolicyService* policy_service, + chromeos::ManagedNetworkConfigurationHandler* network_config_handler, + chromeos::NetworkDeviceHandler* network_device_handler, + chromeos::CrosSettings* cros_settings); + + private: + DeviceNetworkConfigurationUpdater( + scoped_ptr<chromeos::onc::CertificateImporter> certificate_importer, + PolicyService* policy_service, + chromeos::ManagedNetworkConfigurationHandler* network_config_handler, + chromeos::NetworkDeviceHandler* network_device_handler, + chromeos::CrosSettings* cros_settings); + + virtual void Init() OVERRIDE; + virtual void ImportCertificates(const base::ListValue& certificates_onc) + OVERRIDE; + virtual void ApplyNetworkPolicy(base::ListValue* network_configs_onc, + base::DictionaryValue* global_network_config) + OVERRIDE; + void OnDataRoamingSettingChanged(); + + chromeos::NetworkDeviceHandler* network_device_handler_; + chromeos::CrosSettings* cros_settings_; + scoped_ptr<base::CallbackList<void(void)>::Subscription> + data_roaming_setting_subscription_; + + base::WeakPtrFactory<DeviceNetworkConfigurationUpdater> weak_factory_; + + DISALLOW_COPY_AND_ASSIGN(DeviceNetworkConfigurationUpdater); +}; + +} // namespace policy + +#endif // CHROME_BROWSER_CHROMEOS_POLICY_DEVICE_NETWORK_CONFIGURATION_UPDATER_H_ + diff --git a/chrome/browser/chromeos/policy/network_configuration_updater.cc b/chrome/browser/chromeos/policy/network_configuration_updater.cc index 25bd4b4..ce72dd8 100644 --- a/chrome/browser/chromeos/policy/network_configuration_updater.cc +++ b/chrome/browser/chromeos/policy/network_configuration_updater.cc @@ -8,7 +8,6 @@ #include "base/bind_helpers.h" #include "base/logging.h" #include "base/values.h" -#include "chromeos/network/managed_network_configuration_handler.h" #include "chromeos/network/onc/onc_certificate_importer.h" #include "chromeos/network/onc/onc_utils.h" #include "components/policy/core/common/policy_map.h" @@ -20,22 +19,6 @@ NetworkConfigurationUpdater::~NetworkConfigurationUpdater() { policy_service_->RemoveObserver(POLICY_DOMAIN_CHROME, this); } -// static -scoped_ptr<NetworkConfigurationUpdater> -NetworkConfigurationUpdater::CreateForDevicePolicy( - scoped_ptr<chromeos::onc::CertificateImporter> certificate_importer, - PolicyService* policy_service, - chromeos::ManagedNetworkConfigurationHandler* network_config_handler) { - scoped_ptr<NetworkConfigurationUpdater> updater( - new NetworkConfigurationUpdater(onc::ONC_SOURCE_DEVICE_POLICY, - key::kDeviceOpenNetworkConfiguration, - certificate_importer.Pass(), - policy_service, - network_config_handler)); - updater->Init(); - return updater.Pass(); -} - void NetworkConfigurationUpdater::OnPolicyUpdated(const PolicyNamespace& ns, const PolicyMap& previous, const PolicyMap& current) { @@ -84,21 +67,6 @@ void NetworkConfigurationUpdater::Init() { } } -void NetworkConfigurationUpdater::ImportCertificates( - const base::ListValue& certificates_onc) { - certificate_importer_->ImportCertificates( - certificates_onc, onc_source_, NULL); -} - -void NetworkConfigurationUpdater::ApplyNetworkPolicy( - base::ListValue* network_configs_onc, - base::DictionaryValue* global_network_config) { - network_config_handler_->SetPolicy(onc_source_, - std::string() /* no username hash */, - *network_configs_onc, - *global_network_config); -} - void NetworkConfigurationUpdater::OnPolicyChanged( const base::Value* previous, const base::Value* current) { diff --git a/chrome/browser/chromeos/policy/network_configuration_updater.h b/chrome/browser/chromeos/policy/network_configuration_updater.h index 18cd350..9665fcc 100644 --- a/chrome/browser/chromeos/policy/network_configuration_updater.h +++ b/chrome/browser/chromeos/policy/network_configuration_updater.h @@ -41,14 +41,6 @@ class NetworkConfigurationUpdater : public PolicyService::Observer { public: virtual ~NetworkConfigurationUpdater(); - // Creates an updater that applies the ONC device policy from |policy_service| - // once the policy service is completely initialized and on each policy - // change. - static scoped_ptr<NetworkConfigurationUpdater> CreateForDevicePolicy( - scoped_ptr<chromeos::onc::CertificateImporter> certificate_importer, - PolicyService* policy_service, - chromeos::ManagedNetworkConfigurationHandler* network_config_handler); - // PolicyService::Observer overrides virtual void OnPolicyUpdated(const PolicyNamespace& ns, const PolicyMap& previous, @@ -63,16 +55,17 @@ class NetworkConfigurationUpdater : public PolicyService::Observer { PolicyService* policy_service, chromeos::ManagedNetworkConfigurationHandler* network_config_handler); - void Init(); + virtual void Init(); // Imports the certificates part of the policy. - virtual void ImportCertificates(const base::ListValue& certificates_onc); + virtual void ImportCertificates(const base::ListValue& certificates_onc) = 0; // Pushes the network part of the policy to the // ManagedNetworkConfigurationHandler. This can be overridden by subclasses to // modify |network_configs_onc| before the actual application. - virtual void ApplyNetworkPolicy(base::ListValue* network_configs_onc, - base::DictionaryValue* global_network_config); + virtual void ApplyNetworkPolicy( + base::ListValue* network_configs_onc, + base::DictionaryValue* global_network_config) = 0; onc::ONCSource onc_source_; diff --git a/chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc b/chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc index 4571072..dcb55ed 100644 --- a/chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc +++ b/chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc @@ -10,7 +10,12 @@ #include "base/run_loop.h" #include "base/values.h" #include "chrome/browser/chromeos/login/user.h" +#include "chrome/browser/chromeos/policy/device_network_configuration_updater.h" #include "chrome/browser/chromeos/policy/user_network_configuration_updater.h" +#include "chrome/browser/chromeos/settings/cros_settings.h" +#include "chrome/browser/chromeos/settings/device_settings_service.h" +#include "chrome/browser/chromeos/settings/stub_cros_settings_provider.h" +#include "chromeos/network/fake_network_device_handler.h" #include "chromeos/network/mock_managed_network_configuration_handler.h" #include "chromeos/network/onc/mock_certificate_importer.h" #include "chromeos/network/onc/onc_test_utils.h" @@ -71,6 +76,17 @@ class FakeWebTrustedCertsObserver net::CertificateList trust_anchors_; }; +class FakeNetworkDeviceHandler : public chromeos::FakeNetworkDeviceHandler { + public: + FakeNetworkDeviceHandler() : allow_roaming_(false) {} + + virtual void SetCellularAllowRoaming(bool allow_roaming) OVERRIDE { + allow_roaming_ = allow_roaming; + } + + bool allow_roaming_; +}; + const char kFakeONC[] = "{ \"NetworkConfigurations\": [" " { \"GUID\": \"{485d6076-dd44-6b6d-69787465725f5040}\"," @@ -188,10 +204,12 @@ class NetworkConfigurationUpdaterTest : public testing::Test { void CreateNetworkConfigurationUpdaterForDevicePolicy() { network_configuration_updater_ = - NetworkConfigurationUpdater::CreateForDevicePolicy( + DeviceNetworkConfigurationUpdater::CreateForDevicePolicy( certificate_importer_owned_.Pass(), policy_service_.get(), - &network_config_handler_); + &network_config_handler_, + &network_device_handler_, + chromeos::CrosSettings::Get()); } base::ListValue fake_network_configs_; @@ -199,6 +217,11 @@ class NetworkConfigurationUpdaterTest : public testing::Test { base::ListValue fake_certificates_; StrictMock<chromeos::MockManagedNetworkConfigurationHandler> network_config_handler_; + FakeNetworkDeviceHandler network_device_handler_; + + // Not used directly. Required for CrosSettings. + chromeos::ScopedTestDeviceSettingsService scoped_device_settings_service_; + chromeos::ScopedTestCrosSettings scoped_cros_settings_; // Ownership of certificate_importer_owned_ is passed to the // NetworkConfigurationUpdater. When that happens, |certificate_importer_| @@ -215,6 +238,36 @@ class NetworkConfigurationUpdaterTest : public testing::Test { content::TestBrowserThreadBundle thread_bundle_; }; +TEST_F(NetworkConfigurationUpdaterTest, CellularAllowRoaming) { + // Ignore networ config updates. + EXPECT_CALL(network_config_handler_, SetPolicy(_, _, _, _)).Times(AtLeast(1)); + EXPECT_CALL(*certificate_importer_, ImportCertificates(_, _, _)) + .Times(AtLeast(1)); + + // Setup the DataRoaming device setting. + chromeos::CrosSettings* cros_settings = chromeos::CrosSettings::Get(); + chromeos::CrosSettingsProvider* device_settings_provider = + cros_settings->GetProvider(chromeos::kSignedDataRoamingEnabled); + cros_settings->RemoveSettingsProvider(device_settings_provider); + delete device_settings_provider; + chromeos::StubCrosSettingsProvider* stub_settings_provider = + new chromeos::StubCrosSettingsProvider; + cros_settings->AddSettingsProvider(stub_settings_provider); + + chromeos::CrosSettings::Get()->Set(chromeos::kSignedDataRoamingEnabled, + base::FundamentalValue(false)); + EXPECT_FALSE(network_device_handler_.allow_roaming_); + + CreateNetworkConfigurationUpdaterForDevicePolicy(); + chromeos::CrosSettings::Get()->Set(chromeos::kSignedDataRoamingEnabled, + base::FundamentalValue(true)); + EXPECT_TRUE(network_device_handler_.allow_roaming_); + + chromeos::CrosSettings::Get()->Set(chromeos::kSignedDataRoamingEnabled, + base::FundamentalValue(false)); + EXPECT_FALSE(network_device_handler_.allow_roaming_); +} + TEST_F(NetworkConfigurationUpdaterTest, PolicyIsValidatedAndRepaired) { scoped_ptr<base::DictionaryValue> onc_repaired = chromeos::onc::test_utils::ReadTestDictionary( diff --git a/chrome/browser/chromeos/settings/device_settings_provider.cc b/chrome/browser/chromeos/settings/device_settings_provider.cc index a07da1f..9ece972 100644 --- a/chrome/browser/chromeos/settings/device_settings_provider.cc +++ b/chrome/browser/chromeos/settings/device_settings_provider.cc @@ -8,11 +8,9 @@ #include "base/bind_helpers.h" #include "base/callback.h" #include "base/command_line.h" -#include "base/location.h" #include "base/logging.h" #include "base/metrics/histogram.h" #include "base/prefs/pref_service.h" -#include "base/strings/string_util.h" #include "base/threading/thread_restrictions.h" #include "base/values.h" #include "chrome/browser/browser_process.h" @@ -23,16 +21,9 @@ #include "chrome/browser/ui/options/options_util.h" #include "chrome/installer/util/google_update_settings.h" #include "chromeos/chromeos_switches.h" -#include "chromeos/network/device_state.h" -#include "chromeos/network/network_device_handler.h" -#include "chromeos/network/network_event_log.h" -#include "chromeos/network/network_handler.h" -#include "chromeos/network/network_state_handler.h" -#include "chromeos/network/shill_property_util.h" #include "chromeos/settings/cros_settings_names.h" #include "components/policy/core/common/cloud/cloud_policy_constants.h" #include "policy/proto/device_management_backend.pb.h" -#include "third_party/cros_system_api/dbus/service_constants.h" using google::protobuf::RepeatedField; using google::protobuf::RepeatedPtrField; @@ -94,12 +85,6 @@ bool HasOldMetricsFile() { return GoogleUpdateSettings::GetCollectStatsConsent(); } -void LogShillError( - const std::string& name, - scoped_ptr<base::DictionaryValue> error_data) { - NET_LOG_ERROR("Shill error: " + name, "Network operation failed."); -} - } // namespace DeviceSettingsProvider::DeviceSettingsProvider( @@ -111,11 +96,6 @@ DeviceSettingsProvider::DeviceSettingsProvider( ownership_status_(device_settings_service_->GetOwnershipStatus()), store_callback_factory_(this) { device_settings_service_->AddObserver(this); - if (NetworkHandler::IsInitialized()) { - NetworkHandler::Get()->network_state_handler()->AddObserver(this, - FROM_HERE); - } - if (!UpdateFromService()) { // Make sure we have at least the cache data immediately. RetrieveCachedData(); @@ -124,10 +104,6 @@ DeviceSettingsProvider::DeviceSettingsProvider( DeviceSettingsProvider::~DeviceSettingsProvider() { device_settings_service_->RemoveObserver(this); - if (NetworkHandler::IsInitialized()) { - NetworkHandler::Get()->network_state_handler()->RemoveObserver(this, - FROM_HERE); - } } // static @@ -325,7 +301,6 @@ void DeviceSettingsProvider::SetInPolicy() { roam->set_data_roaming_enabled(roaming_value); else NOTREACHED(); - ApplyRoamingSetting(roaming_value); } else if (prop == kReleaseChannel) { em::ReleaseChannelProto* release_channel = device_settings_.mutable_release_channel(); @@ -459,7 +434,6 @@ void DeviceSettingsProvider::DecodeLoginPolicies( // For all our boolean settings the following is applicable: // true is default permissive value and false is safe prohibitive value. // Exceptions: - // kSignedDataRoamingEnabled has a default value of false. // kAccountsPrefEphemeralUsersEnabled has a default value of false. if (policy.has_allow_new_users() && policy.allow_new_users().has_allow_new_users()) { @@ -646,6 +620,7 @@ void DeviceSettingsProvider::DecodeKioskPolicies( void DeviceSettingsProvider::DecodeNetworkPolicies( const em::ChromeDeviceSettingsProto& policy, PrefValueMap* new_values_cache) const { + // kSignedDataRoamingEnabled has a default value of false. new_values_cache->SetBoolean( kSignedDataRoamingEnabled, policy.has_data_roaming_enabled() && @@ -841,49 +816,6 @@ void DeviceSettingsProvider::ApplyMetricsSetting(bool use_file, OptionsUtil::ResolveMetricsReportingEnabled(new_value); } -void DeviceSettingsProvider::ApplyRoamingSetting(bool new_value) { - // TODO(pneubeck): Move this application of the roaming policy to - // NetworkConfigurationUpdater and ManagedNetworkConfigurationHandler. See - // http://crbug.com/323537 . - // TODO(armansito): Look up the device by explicitly using the device path. - const DeviceState* cellular = - NetworkHandler::Get()->network_state_handler()->GetDeviceStateByType( - NetworkTypePattern::Cellular()); - if (!cellular) { - NET_LOG_DEBUG("No cellular device is available", - "Roaming is only supported by cellular devices."); - return; - } - bool current_value; - if (!cellular->properties().GetBooleanWithoutPathExpansion( - shill::kCellularAllowRoamingProperty, ¤t_value)) { - NET_LOG_ERROR("Could not get \"allow roaming\" property from cellular " - "device.", cellular->path()); - return; - } - - // Only set the value if the current value is different from |new_value|. - // If roaming is required by the provider, always try to set to true. - new_value = (cellular->provider_requires_roaming() ? true : new_value); - if (new_value == current_value) - return; - - NetworkHandler::Get()->network_device_handler()->SetDeviceProperty( - cellular->path(), - shill::kCellularAllowRoamingProperty, - base::FundamentalValue(new_value), - base::Bind(&base::DoNothing), - base::Bind(&LogShillError)); -} - -void DeviceSettingsProvider::ApplyRoamingSettingFromProto( - const em::ChromeDeviceSettingsProto& settings) { - ApplyRoamingSetting( - settings.has_data_roaming_enabled() ? - settings.data_roaming_enabled().data_roaming_enabled() : - false); -} - void DeviceSettingsProvider::ApplySideEffects( const em::ChromeDeviceSettingsProto& settings) { // First migrate metrics settings as needed. @@ -891,9 +823,6 @@ void DeviceSettingsProvider::ApplySideEffects( ApplyMetricsSetting(false, settings.metrics_enabled().metrics_enabled()); else ApplyMetricsSetting(true, false); - - // Next set the roaming setting as needed. - ApplyRoamingSettingFromProto(settings); } bool DeviceSettingsProvider::MitigateMissingPolicy() { @@ -948,10 +877,6 @@ bool DeviceSettingsProvider::HandlesSetting(const std::string& path) const { return IsDeviceSetting(path); } -void DeviceSettingsProvider::DeviceListChanged() { - ApplyRoamingSettingFromProto(device_settings_); -} - DeviceSettingsProvider::TrustedStatus DeviceSettingsProvider::RequestTrustedEntity() { if (ownership_status_ == DeviceSettingsService::OWNERSHIP_NONE) diff --git a/chrome/browser/chromeos/settings/device_settings_provider.h b/chrome/browser/chromeos/settings/device_settings_provider.h index 665d4655..ff46884 100644 --- a/chrome/browser/chromeos/settings/device_settings_provider.h +++ b/chrome/browser/chromeos/settings/device_settings_provider.h @@ -17,7 +17,6 @@ #include "base/prefs/pref_value_map.h" #include "chrome/browser/chromeos/policy/proto/chrome_device_policy.pb.h" #include "chrome/browser/chromeos/settings/device_settings_service.h" -#include "chromeos/network/network_state_handler_observer.h" #include "chromeos/settings/cros_settings_provider.h" namespace base { @@ -32,8 +31,7 @@ namespace chromeos { // CrosSettingsProvider implementation that works with device settings. class DeviceSettingsProvider : public CrosSettingsProvider, - public DeviceSettingsService::Observer, - public NetworkStateHandlerObserver { + public DeviceSettingsService::Observer { public: DeviceSettingsProvider(const NotifyObserversCallback& notify_cb, DeviceSettingsService* device_settings_service); @@ -48,9 +46,6 @@ class DeviceSettingsProvider : public CrosSettingsProvider, const base::Closure& callback) OVERRIDE; virtual bool HandlesSetting(const std::string& path) const OVERRIDE; - // NetworkStateHandlerObserver implementation. - virtual void DeviceListChanged() OVERRIDE; - private: // CrosSettingsProvider implementation: virtual void DoSet(const std::string& path, diff --git a/chrome/browser/policy/browser_policy_connector.cc b/chrome/browser/policy/browser_policy_connector.cc index d7b996e..f3fbf13 100644 --- a/chrome/browser/policy/browser_policy_connector.cc +++ b/chrome/browser/policy/browser_policy_connector.cc @@ -68,9 +68,9 @@ #include "chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.h" #include "chrome/browser/chromeos/policy/device_local_account.h" #include "chrome/browser/chromeos/policy/device_local_account_policy_service.h" +#include "chrome/browser/chromeos/policy/device_network_configuration_updater.h" #include "chrome/browser/chromeos/policy/device_status_collector.h" #include "chrome/browser/chromeos/policy/enterprise_install_attributes.h" -#include "chrome/browser/chromeos/policy/network_configuration_updater.h" #include "chrome/browser/chromeos/settings/cros_settings.h" #include "chrome/browser/chromeos/settings/device_settings_service.h" #include "chromeos/chromeos_paths.h" @@ -341,12 +341,14 @@ void BrowserPolicyConnector::Init( #if defined(OS_CHROMEOS) network_configuration_updater_ = - NetworkConfigurationUpdater::CreateForDevicePolicy( + DeviceNetworkConfigurationUpdater::CreateForDevicePolicy( scoped_ptr<chromeos::onc::CertificateImporter>( new chromeos::onc::CertificateImporterImpl), GetPolicyService(), chromeos::NetworkHandler::Get() - ->managed_network_configuration_handler()); + ->managed_network_configuration_handler(), + chromeos::NetworkHandler::Get()->network_device_handler(), + chromeos::CrosSettings::Get()); #endif is_initialized_ = true; diff --git a/chrome/chrome_browser_chromeos.gypi b/chrome/chrome_browser_chromeos.gypi index 8030676..d68c055 100644 --- a/chrome/chrome_browser_chromeos.gypi +++ b/chrome/chrome_browser_chromeos.gypi @@ -712,6 +712,8 @@ 'browser/chromeos/policy/device_local_account_policy_service.h', 'browser/chromeos/policy/device_local_account_policy_store.cc', 'browser/chromeos/policy/device_local_account_policy_store.h', + 'browser/chromeos/policy/device_network_configuration_updater.cc', + 'browser/chromeos/policy/device_network_configuration_updater.h', 'browser/chromeos/policy/device_policy_decoder_chromeos.cc', 'browser/chromeos/policy/device_policy_decoder_chromeos.h', 'browser/chromeos/policy/device_status_collector.cc', diff --git a/chromeos/chromeos.gyp b/chromeos/chromeos.gyp index 2279bb4..e85f255 100644 --- a/chromeos/chromeos.gyp +++ b/chromeos/chromeos.gyp @@ -403,6 +403,8 @@ 'disks/mock_disk_mount_manager.h', 'ime/mock_component_extension_ime_manager_delegate.cc', 'ime/mock_component_extension_ime_manager_delegate.h', + 'network/fake_network_device_handler.cc', + 'network/fake_network_device_handler.h', 'network/mock_managed_network_configuration_handler.cc', 'network/mock_managed_network_configuration_handler.h', 'network/onc/mock_certificate_importer.cc', diff --git a/chromeos/dbus/fake_shill_device_client.cc b/chromeos/dbus/fake_shill_device_client.cc index f1c0128..c698277 100644 --- a/chromeos/dbus/fake_shill_device_client.cc +++ b/chromeos/dbus/fake_shill_device_client.cc @@ -11,6 +11,7 @@ #include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/shill_manager_client.h" #include "chromeos/dbus/shill_property_changed_observer.h" +#include "chromeos/network/shill_property_util.h" #include "dbus/bus.h" #include "dbus/message.h" #include "dbus/object_path.h" @@ -214,15 +215,16 @@ void FakeShillDeviceClient::AddDevice(const std::string& device_path, AddDevice(device_path); base::DictionaryValue* properties = GetDeviceProperties(device_path); + properties->SetWithoutPathExpansion(shill::kTypeProperty, + base::Value::CreateStringValue(type)); properties->SetWithoutPathExpansion( - shill::kTypeProperty, - base::Value::CreateStringValue(type)); - properties->SetWithoutPathExpansion( - shill::kDBusObjectProperty, - base::Value::CreateStringValue(object_path)); - properties->SetWithoutPathExpansion( - shill::kDBusConnectionProperty, - base::Value::CreateStringValue("/stub")); + shill::kDBusObjectProperty, base::Value::CreateStringValue(object_path)); + properties->SetWithoutPathExpansion(shill::kDBusConnectionProperty, + base::Value::CreateStringValue("/stub")); + if (NetworkTypePattern::Cellular().MatchesType(type)) { + properties->SetWithoutPathExpansion(shill::kCellularAllowRoamingProperty, + new base::FundamentalValue(false)); + } } void FakeShillDeviceClient::RemoveDevice(const std::string& device_path) { diff --git a/chromeos/network/fake_network_device_handler.cc b/chromeos/network/fake_network_device_handler.cc new file mode 100644 index 0000000..dd737ac --- /dev/null +++ b/chromeos/network/fake_network_device_handler.cc @@ -0,0 +1,76 @@ +// Copyright 2013 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/fake_network_device_handler.h" + +namespace chromeos { + +FakeNetworkDeviceHandler::FakeNetworkDeviceHandler() {} + +FakeNetworkDeviceHandler::~FakeNetworkDeviceHandler() {} + +void FakeNetworkDeviceHandler::GetDeviceProperties( + const std::string& device_path, + const network_handler::DictionaryResultCallback& callback, + const network_handler::ErrorCallback& error_callback) const {} + +void FakeNetworkDeviceHandler::SetDeviceProperty( + const std::string& device_path, + const std::string& property_name, + const base::Value& value, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) {} + +void FakeNetworkDeviceHandler::RequestRefreshIPConfigs( + const std::string& device_path, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) {} + +void FakeNetworkDeviceHandler::ProposeScan( + const std::string& device_path, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) {} + +void FakeNetworkDeviceHandler::RegisterCellularNetwork( + const std::string& device_path, + const std::string& network_id, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) {} + +void FakeNetworkDeviceHandler::SetCarrier( + const std::string& device_path, + const std::string& carrier, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) {} + +void FakeNetworkDeviceHandler::RequirePin( + const std::string& device_path, + bool require_pin, + const std::string& pin, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) {} + +void FakeNetworkDeviceHandler::EnterPin( + const std::string& device_path, + const std::string& pin, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) {} + +void FakeNetworkDeviceHandler::UnblockPin( + const std::string& device_path, + const std::string& puk, + const std::string& new_pin, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) {} + +void FakeNetworkDeviceHandler::ChangePin( + const std::string& device_path, + const std::string& old_pin, + const std::string& new_pin, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) {} + +void FakeNetworkDeviceHandler::SetCellularAllowRoaming(bool allow_roaming) {} + +} // namespace chromeos diff --git a/chromeos/network/fake_network_device_handler.h b/chromeos/network/fake_network_device_handler.h new file mode 100644 index 0000000..f3bf145 --- /dev/null +++ b/chromeos/network/fake_network_device_handler.h @@ -0,0 +1,92 @@ +// Copyright 2013 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_FAKE_NETWORK_DEVICE_HANDLER_H_ +#define CHROMEOS_NETWORK_FAKE_NETWORK_DEVICE_HANDLER_H_ + +#include "base/basictypes.h" +#include "base/compiler_specific.h" +#include "chromeos/chromeos_export.h" +#include "chromeos/network/network_device_handler.h" + +namespace chromeos { + +// This is a fake implementation which does nothing. Use this as a base class +// for concrete fake handlers. +class CHROMEOS_EXPORT FakeNetworkDeviceHandler : public NetworkDeviceHandler { + public: + FakeNetworkDeviceHandler(); + virtual ~FakeNetworkDeviceHandler(); + + // NetworkDeviceHandler overrides + virtual void GetDeviceProperties( + const std::string& device_path, + const network_handler::DictionaryResultCallback& callback, + const network_handler::ErrorCallback& error_callback) const OVERRIDE; + + virtual void SetDeviceProperty( + const std::string& device_path, + const std::string& property_name, + const base::Value& value, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) OVERRIDE; + + virtual void RequestRefreshIPConfigs( + const std::string& device_path, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) OVERRIDE; + + virtual void ProposeScan(const std::string& device_path, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) + OVERRIDE; + + virtual void RegisterCellularNetwork( + const std::string& device_path, + const std::string& network_id, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) OVERRIDE; + + virtual void SetCarrier(const std::string& device_path, + const std::string& carrier, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) + OVERRIDE; + + virtual void RequirePin(const std::string& device_path, + bool require_pin, + const std::string& pin, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) + OVERRIDE; + + virtual void EnterPin(const std::string& device_path, + const std::string& pin, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) + OVERRIDE; + + virtual void UnblockPin(const std::string& device_path, + const std::string& puk, + const std::string& new_pin, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) + OVERRIDE; + + virtual void ChangePin(const std::string& device_path, + const std::string& old_pin, + const std::string& new_pin, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) + OVERRIDE; + + virtual void SetCellularAllowRoaming(bool allow_roaming) OVERRIDE; + + private: + DISALLOW_COPY_AND_ASSIGN(FakeNetworkDeviceHandler); +}; + +} // namespace chromeos + +#endif // CHROMEOS_NETWORK_FAKE_NETWORK_DEVICE_HANDLER_H_ diff --git a/chromeos/network/network_device_handler.h b/chromeos/network/network_device_handler.h index fe454f4..9b69399 100644 --- a/chromeos/network/network_device_handler.h +++ b/chromeos/network/network_device_handler.h @@ -52,10 +52,12 @@ class CHROMEOS_EXPORT NetworkDeviceHandler { const network_handler::ErrorCallback& error_callback) const = 0; // Sets the value of property |name| on device with id |device_path| to - // |value|. + // |value|. This function provides a generic setter to be used by the UI or + // network API and doesn't allow changes to protected settings like cellular + // roaming. virtual void SetDeviceProperty( const std::string& device_path, - const std::string& name, + const std::string& property_name, const base::Value& value, const base::Closure& callback, const network_handler::ErrorCallback& error_callback) = 0; @@ -185,6 +187,10 @@ class CHROMEOS_EXPORT NetworkDeviceHandler { const base::Closure& callback, const network_handler::ErrorCallback& error_callback) = 0; + // Enables/disables roaming of all cellular devices. This happens + // asychronously in the background and applies also to devices which become + // available in the future. + virtual void SetCellularAllowRoaming(bool allow_roaming) = 0; private: DISALLOW_COPY_AND_ASSIGN(NetworkDeviceHandler); diff --git a/chromeos/network/network_device_handler_impl.cc b/chromeos/network/network_device_handler_impl.cc index cf8f036..02fbdb9 100644 --- a/chromeos/network/network_device_handler_impl.cc +++ b/chromeos/network/network_device_handler_impl.cc @@ -5,12 +5,16 @@ #include "chromeos/network/network_device_handler_impl.h" #include "base/bind.h" +#include "base/location.h" #include "base/values.h" #include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/shill_device_client.h" #include "chromeos/dbus/shill_ipconfig_client.h" #include "chromeos/network/device_state.h" #include "chromeos/network/network_event_log.h" +#include "chromeos/network/network_handler_callbacks.h" +#include "chromeos/network/network_state_handler.h" +#include "chromeos/network/shill_property_util.h" #include "dbus/object_path.h" #include "third_party/cros_system_api/dbus/service_constants.h" @@ -40,6 +44,15 @@ std::string GetErrorNameForShillError(const std::string& shill_error_name) { return NetworkDeviceHandler::kErrorUnknown; } +void InvokeErrorCallback(const std::string& service_path, + const network_handler::ErrorCallback& error_callback, + const std::string& error_name) { + std::string error_msg = "Device Error: " + error_name; + NET_LOG_ERROR(error_msg, service_path); + network_handler::RunErrorCallback( + error_callback, service_path, error_name, error_msg); +} + void HandleShillCallFailure( const std::string& device_path, const network_handler::ErrorCallback& error_callback, @@ -103,9 +116,6 @@ void ProposeScanCallback( const network_handler::ErrorCallback& error_callback, DBusMethodCallStatus call_status) { if (call_status != DBUS_METHOD_CALL_SUCCESS) { - NET_LOG_ERROR( - base::StringPrintf("Device.ProposeScan failed: %d", call_status), - device_path); network_handler::ShillErrorCallbackFunction( "Device.ProposeScan Failed", device_path, @@ -118,9 +128,25 @@ void ProposeScanCallback( callback.Run(); } +void SetDevicePropertyInternal( + const std::string& device_path, + const std::string& property_name, + const base::Value& value, + const base::Closure& callback, + const network_handler::ErrorCallback& error_callback) { + DBusThreadManager::Get()->GetShillDeviceClient()->SetProperty( + dbus::ObjectPath(device_path), + property_name, + value, + callback, + base::Bind(&HandleShillCallFailure, device_path, error_callback)); +} + } // namespace -NetworkDeviceHandlerImpl::~NetworkDeviceHandlerImpl() {} +NetworkDeviceHandlerImpl::~NetworkDeviceHandlerImpl() { + network_state_handler_->RemoveObserver(this, FROM_HERE); +} void NetworkDeviceHandlerImpl::GetDeviceProperties( const std::string& device_path, @@ -134,16 +160,27 @@ void NetworkDeviceHandlerImpl::GetDeviceProperties( void NetworkDeviceHandlerImpl::SetDeviceProperty( const std::string& device_path, - const std::string& name, + const std::string& property_name, const base::Value& value, const base::Closure& callback, const network_handler::ErrorCallback& error_callback) { - DBusThreadManager::Get()->GetShillDeviceClient()->SetProperty( - dbus::ObjectPath(device_path), - name, - value, - callback, - base::Bind(&HandleShillCallFailure, device_path, error_callback)); + const char* const property_blacklist[] = { + // Must only be changed by policy/owner through. + shill::kCellularAllowRoamingProperty + }; + + for (size_t i = 0; i < arraysize(property_blacklist); ++i) { + if (property_name == property_blacklist[i]) { + InvokeErrorCallback( + device_path, + error_callback, + "SetDeviceProperty called on blacklisted property " + property_name); + return; + } + } + + SetDevicePropertyInternal( + device_path, property_name, value, callback, error_callback); } void NetworkDeviceHandlerImpl::RequestRefreshIPConfigs( @@ -243,6 +280,64 @@ void NetworkDeviceHandlerImpl::ChangePin( base::Bind(&HandleShillCallFailure, device_path, error_callback)); } -NetworkDeviceHandlerImpl::NetworkDeviceHandlerImpl() {} +void NetworkDeviceHandlerImpl::SetCellularAllowRoaming( + const bool allow_roaming) { + cellular_allow_roaming_ = allow_roaming; + ApplyCellularAllowRoamingToShill(); +} + +void NetworkDeviceHandlerImpl::DeviceListChanged() { + ApplyCellularAllowRoamingToShill(); +} + +NetworkDeviceHandlerImpl::NetworkDeviceHandlerImpl() + : network_state_handler_(NULL), + cellular_allow_roaming_(false) {} + +void NetworkDeviceHandlerImpl::Init( + NetworkStateHandler* network_state_handler) { + DCHECK(network_state_handler); + network_state_handler_ = network_state_handler; + network_state_handler_->AddObserver(this, FROM_HERE); +} + +void NetworkDeviceHandlerImpl::ApplyCellularAllowRoamingToShill() { + NetworkStateHandler::DeviceStateList list; + network_state_handler_->GetDeviceListByType(NetworkTypePattern::Cellular(), + &list); + if (list.empty()) { + NET_LOG_DEBUG("No cellular device is available", + "Roaming is only supported by cellular devices."); + return; + } + for (NetworkStateHandler::DeviceStateList::const_iterator it = list.begin(); + it != list.end(); ++it) { + const DeviceState* device_state = *it; + bool current_device_value; + if (!device_state->properties().GetBooleanWithoutPathExpansion( + shill::kCellularAllowRoamingProperty, ¤t_device_value)) { + NET_LOG_ERROR( + "Could not get \"allow roaming\" property from cellular " + "device.", + device_state->path()); + continue; + } + + // If roaming is required by the provider, always try to set to true. + bool new_device_value = + device_state->provider_requires_roaming() || cellular_allow_roaming_; + + // Only set the value if the current value is different from + // |new_device_value|. + if (new_device_value == current_device_value) + continue; + + SetDevicePropertyInternal(device_state->path(), + shill::kCellularAllowRoamingProperty, + base::FundamentalValue(new_device_value), + base::Bind(&base::DoNothing), + network_handler::ErrorCallback()); + } +} } // namespace chromeos diff --git a/chromeos/network/network_device_handler_impl.h b/chromeos/network/network_device_handler_impl.h index 358b64b..226e7a9 100644 --- a/chromeos/network/network_device_handler_impl.h +++ b/chromeos/network/network_device_handler_impl.h @@ -34,7 +34,7 @@ class CHROMEOS_EXPORT NetworkDeviceHandlerImpl virtual void SetDeviceProperty( const std::string& device_path, - const std::string& name, + const std::string& property_name, const base::Value& value, const base::Closure& callback, const network_handler::ErrorCallback& error_callback) OVERRIDE; @@ -88,12 +88,27 @@ class CHROMEOS_EXPORT NetworkDeviceHandlerImpl const network_handler::ErrorCallback& error_callback) OVERRIDE; + virtual void SetCellularAllowRoaming(bool allow_roaming) OVERRIDE; + + // NetworkStateHandlerObserver overrides + virtual void DeviceListChanged() OVERRIDE; + private: friend class NetworkHandler; friend class NetworkDeviceHandlerTest; NetworkDeviceHandlerImpl(); + void Init(NetworkStateHandler* network_state_handler); + + // Apply the current value of |cellular_allow_roaming_| to all existing + // cellular devices of Shill. + void ApplyCellularAllowRoamingToShill(); + + NetworkStateHandler* network_state_handler_; + + bool cellular_allow_roaming_; + DISALLOW_COPY_AND_ASSIGN(NetworkDeviceHandlerImpl); }; diff --git a/chromeos/network/network_device_handler_unittest.cc b/chromeos/network/network_device_handler_unittest.cc index 4185b75..60a264b 100644 --- a/chromeos/network/network_device_handler_unittest.cc +++ b/chromeos/network/network_device_handler_unittest.cc @@ -10,6 +10,7 @@ #include "chromeos/dbus/fake_shill_device_client.h" #include "chromeos/dbus/fake_shill_manager_client.h" #include "chromeos/network/network_device_handler_impl.h" +#include "chromeos/network/network_state_handler.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/cros_system_api/dbus/service_constants.h" @@ -57,11 +58,15 @@ class NetworkDeviceHandlerTest : public testing::Test { error_callback_ = base::Bind(&NetworkDeviceHandlerTest::ErrorCallback, base::Unretained(this)); - network_device_handler_.reset(new NetworkDeviceHandlerImpl); + network_state_handler_.reset(NetworkStateHandler::InitializeForTest()); + NetworkDeviceHandlerImpl* device_handler = new NetworkDeviceHandlerImpl; + device_handler->Init(network_state_handler_.get()); + network_device_handler_.reset(device_handler); } virtual void TearDown() OVERRIDE { network_device_handler_.reset(); + network_state_handler_.reset(); DBusThreadManager::Shutdown(); } @@ -85,6 +90,7 @@ class NetworkDeviceHandlerTest : public testing::Test { FakeShillDeviceClient* fake_device_client_; scoped_ptr<NetworkDeviceHandler> network_device_handler_; + scoped_ptr<NetworkStateHandler> network_state_handler_; base::MessageLoopForUI message_loop_; base::Closure success_callback_; network_handler::DictionaryResultCallback properties_success_callback_; @@ -106,15 +112,13 @@ TEST_F(NetworkDeviceHandlerTest, GetDeviceProperties) { } TEST_F(NetworkDeviceHandlerTest, SetDeviceProperty) { - - // Set the shill::kCellularAllowRoamingProperty to true. The call + // Set the shill::kScanIntervalProperty to true. The call // should succeed and the value should be set. - network_device_handler_->SetDeviceProperty( - kDefaultCellularDevicePath, - shill::kCellularAllowRoamingProperty, - base::FundamentalValue(true), - success_callback_, - error_callback_); + network_device_handler_->SetDeviceProperty(kDefaultCellularDevicePath, + shill::kScanIntervalProperty, + base::FundamentalValue(1), + success_callback_, + error_callback_); message_loop_.RunUntilIdle(); EXPECT_EQ(kResultSuccess, result_); @@ -124,18 +128,17 @@ TEST_F(NetworkDeviceHandlerTest, SetDeviceProperty) { error_callback_); message_loop_.RunUntilIdle(); EXPECT_EQ(kResultSuccess, result_); - bool allow_roaming = false; - EXPECT_TRUE(properties_->GetBooleanWithoutPathExpansion( - shill::kCellularAllowRoamingProperty, &allow_roaming)); - EXPECT_TRUE(allow_roaming); + int interval = 0; + EXPECT_TRUE(properties_->GetIntegerWithoutPathExpansion( + shill::kScanIntervalProperty, &interval)); + EXPECT_EQ(1, interval); // Repeat the same with value false. - network_device_handler_->SetDeviceProperty( - kDefaultCellularDevicePath, - shill::kCellularAllowRoamingProperty, - base::FundamentalValue(false), - success_callback_, - error_callback_); + network_device_handler_->SetDeviceProperty(kDefaultCellularDevicePath, + shill::kScanIntervalProperty, + base::FundamentalValue(2), + success_callback_, + error_callback_); message_loop_.RunUntilIdle(); EXPECT_EQ(kResultSuccess, result_); @@ -144,19 +147,66 @@ TEST_F(NetworkDeviceHandlerTest, SetDeviceProperty) { error_callback_); message_loop_.RunUntilIdle(); EXPECT_EQ(kResultSuccess, result_); - EXPECT_TRUE(properties_->GetBooleanWithoutPathExpansion( - shill::kCellularAllowRoamingProperty, &allow_roaming)); - EXPECT_FALSE(allow_roaming); + EXPECT_TRUE(properties_->GetIntegerWithoutPathExpansion( + shill::kScanIntervalProperty, &interval)); + EXPECT_EQ(2, interval); // Set property on an invalid path. + network_device_handler_->SetDeviceProperty(kUnknownCellularDevicePath, + shill::kScanIntervalProperty, + base::FundamentalValue(1), + success_callback_, + error_callback_); + message_loop_.RunUntilIdle(); + EXPECT_EQ(NetworkDeviceHandler::kErrorFailure, result_); + + // Setting a owner-protected device property through SetDeviceProperty must + // fail. network_device_handler_->SetDeviceProperty( - kUnknownCellularDevicePath, + kDefaultCellularDevicePath, shill::kCellularAllowRoamingProperty, base::FundamentalValue(true), success_callback_, error_callback_); message_loop_.RunUntilIdle(); - EXPECT_EQ(NetworkDeviceHandler::kErrorFailure, result_); + EXPECT_NE(kResultSuccess, result_); + +} + +TEST_F(NetworkDeviceHandlerTest, CellularAllowRoaming) { + // Start with disabled data roaming. + ShillDeviceClient::TestInterface* device_test = + fake_device_client_->GetTestInterface(); + device_test->SetDeviceProperty(kDefaultCellularDevicePath, + shill::kCellularAllowRoamingProperty, + base::FundamentalValue(false)); + + network_device_handler_->SetCellularAllowRoaming(true); + message_loop_.RunUntilIdle(); + + // Roaming should be enabled now. + network_device_handler_->GetDeviceProperties(kDefaultCellularDevicePath, + properties_success_callback_, + error_callback_); + message_loop_.RunUntilIdle(); + EXPECT_EQ(kResultSuccess, result_); + bool allow_roaming; + EXPECT_TRUE(properties_->GetBooleanWithoutPathExpansion( + shill::kCellularAllowRoamingProperty, &allow_roaming)); + EXPECT_TRUE(allow_roaming); + + network_device_handler_->SetCellularAllowRoaming(false); + message_loop_.RunUntilIdle(); + + // Roaming should be disable again. + network_device_handler_->GetDeviceProperties(kDefaultCellularDevicePath, + properties_success_callback_, + error_callback_); + message_loop_.RunUntilIdle(); + EXPECT_EQ(kResultSuccess, result_); + EXPECT_TRUE(properties_->GetBooleanWithoutPathExpansion( + shill::kCellularAllowRoamingProperty, &allow_roaming)); + EXPECT_FALSE(allow_roaming); } TEST_F(NetworkDeviceHandlerTest, RequestRefreshIPConfigs) { diff --git a/chromeos/network/network_handler.cc b/chromeos/network/network_handler.cc index 2875862..ada863d 100644 --- a/chromeos/network/network_handler.cc +++ b/chromeos/network/network_handler.cc @@ -53,6 +53,7 @@ NetworkHandler::~NetworkHandler() { void NetworkHandler::Init() { network_state_handler_->InitShillPropertyHandler(); + network_device_handler_->Init(network_state_handler_.get()); network_profile_handler_->Init(network_state_handler_.get()); network_configuration_handler_->Init(network_state_handler_.get()); managed_network_configuration_handler_->Init( |