diff options
author | stevenjb@chromium.org <stevenjb@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-05 19:54:40 +0000 |
---|---|---|
committer | stevenjb@chromium.org <stevenjb@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-05 19:54:40 +0000 |
commit | 4d201ec63db39d03f5928550d3777859965d8759 (patch) | |
tree | 0fb7348ab6552fe6a4fbaa10af731e8cf358a930 | |
parent | 0d911ae9a10844fbe092696d5c4dae102fd3f184 (diff) | |
download | chromium_src-4d201ec63db39d03f5928550d3777859965d8759.zip chromium_src-4d201ec63db39d03f5928550d3777859965d8759.tar.gz chromium_src-4d201ec63db39d03f5928550d3777859965d8759.tar.bz2 |
Chrome OS: Use Manager.DefaultService for Default Network
This changes Chrome to use Manager.DefaultService to provide the default
network to Chrome (i.e. NetworkChangeNotifier) instead of relying on
Manager.Services[0]. This should fix some timing issues and make the NCS
more in sync with Shill.
One change in behavior that should be benign is that Shill considers a
connecting network to be the default network, whereas previously only
a connected network would be considered.
BUG=159540, 330873
R=gauravsh@chromium.org, pneubeck@chromium.org, tbarzic@chromium.org
Review URL: https://codereview.chromium.org/175243004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@255124 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc | 24 | ||||
-rw-r--r-- | chrome/browser/chromeos/proxy_config_service_impl_unittest.cc | 2 | ||||
-rw-r--r-- | chromeos/dbus/fake_shill_manager_client.cc | 118 | ||||
-rw-r--r-- | chromeos/dbus/fake_shill_manager_client.h | 14 | ||||
-rw-r--r-- | chromeos/dbus/fake_shill_service_client.cc | 25 | ||||
-rw-r--r-- | chromeos/dbus/fake_shill_service_client.h | 4 | ||||
-rw-r--r-- | chromeos/dbus/shill_manager_client.h | 15 | ||||
-rw-r--r-- | chromeos/network/network_state_handler.cc | 92 | ||||
-rw-r--r-- | chromeos/network/network_state_handler.h | 20 | ||||
-rw-r--r-- | chromeos/network/network_state_handler_unittest.cc | 186 | ||||
-rw-r--r-- | chromeos/network/shill_property_handler.cc | 6 | ||||
-rw-r--r-- | chromeos/network/shill_property_handler.h | 4 | ||||
-rw-r--r-- | chromeos/network/shill_property_handler_unittest.cc | 43 |
13 files changed, 382 insertions, 171 deletions
diff --git a/chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc b/chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc index dea2197..f5898d6 100644 --- a/chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc +++ b/chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc @@ -31,6 +31,7 @@ #include "testing/gtest/include/gtest/gtest.h" #include "third_party/cros_system_api/dbus/service_constants.h" +using testing::AnyNumber; using testing::Mock; using testing::_; @@ -388,12 +389,22 @@ TEST_F(NetworkPortalDetectorImplTest, Online2Offline) { MockObserver observer; network_portal_detector()->AddObserver(&observer); + NetworkPortalDetector::CaptivePortalState offline_state; + offline_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_OFFLINE; + // WiFi is in online state. { - NetworkPortalDetector::CaptivePortalState state; - state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE; - state.response_code = 204; - EXPECT_CALL(observer, OnPortalDetectionCompleted(_, state)).Times(1); + // When transitioning to a connected state, the network will transition to + // connecting states which will set the default network to NULL. This may + // get triggered multiple times. + EXPECT_CALL(observer, OnPortalDetectionCompleted(_, offline_state)) + .Times(AnyNumber()); + + // Expect a single transition to an online state. + NetworkPortalDetector::CaptivePortalState online_state; + online_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE; + online_state.response_code = 204; + EXPECT_CALL(observer, OnPortalDetectionCompleted(_, online_state)).Times(1); SetConnected(kStubWireless1); ASSERT_TRUE(is_state_checking_for_portal()); @@ -407,9 +418,8 @@ TEST_F(NetworkPortalDetectorImplTest, Online2Offline) { // WiFi is turned off. { - NetworkPortalDetector::CaptivePortalState state; - state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_OFFLINE; - EXPECT_CALL(observer, OnPortalDetectionCompleted(NULL, state)).Times(1); + EXPECT_CALL(observer, OnPortalDetectionCompleted(NULL, offline_state)) + .Times(1); SetDisconnected(kStubWireless1); ASSERT_TRUE(is_state_idle()); diff --git a/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc b/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc index 060f00b..b4cf433 100644 --- a/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc +++ b/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc @@ -242,6 +242,8 @@ class ProxyConfigServiceImplTest : public testing::Test { ShillServiceClient::TestInterface* service_test = DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface(); + // Process any pending notifications before clearing services. + loop_.RunUntilIdle(); service_test->ClearServices(); // Sends a notification about the added profile. diff --git a/chromeos/dbus/fake_shill_manager_client.cc b/chromeos/dbus/fake_shill_manager_client.cc index d3ad566..beafbcd 100644 --- a/chromeos/dbus/fake_shill_manager_client.cc +++ b/chromeos/dbus/fake_shill_manager_client.cc @@ -73,6 +73,16 @@ void AppendServicesForType( } } +void LogErrorCallback(const std::string& error_name, + const std::string& error_message) { + LOG(ERROR) << error_name << ": " << error_message; +} + +bool IsConnectedState(const std::string& state) { + return state == shill::kStateOnline || state == shill::kStatePortal || + state == shill::kStateReady; +} + } // namespace FakeShillManagerClient::FakeShillManagerClient() @@ -118,7 +128,7 @@ void FakeShillManagerClient::SetProperty(const std::string& name, const base::Closure& callback, const ErrorCallback& error_callback) { stub_properties_.SetWithoutPathExpansion(name, value.DeepCopy()); - CallNotifyObserversPropertyChanged(name, 0); + CallNotifyObserversPropertyChanged(name); base::MessageLoop::current()->PostTask(FROM_HERE, callback); } @@ -324,7 +334,7 @@ ShillManagerClient::TestInterface* FakeShillManagerClient::GetTestInterface() { void FakeShillManagerClient::AddDevice(const std::string& device_path) { if (GetListProperty(shill::kDevicesProperty)->AppendIfNotPresent( base::Value::CreateStringValue(device_path))) { - CallNotifyObserversPropertyChanged(shill::kDevicesProperty, 0); + CallNotifyObserversPropertyChanged(shill::kDevicesProperty); } } @@ -332,13 +342,13 @@ void FakeShillManagerClient::RemoveDevice(const std::string& device_path) { base::StringValue device_path_value(device_path); if (GetListProperty(shill::kDevicesProperty)->Remove( device_path_value, NULL)) { - CallNotifyObserversPropertyChanged(shill::kDevicesProperty, 0); + CallNotifyObserversPropertyChanged(shill::kDevicesProperty); } } void FakeShillManagerClient::ClearDevices() { GetListProperty(shill::kDevicesProperty)->Clear(); - CallNotifyObserversPropertyChanged(shill::kDevicesProperty, 0); + CallNotifyObserversPropertyChanged(shill::kDevicesProperty); } void FakeShillManagerClient::AddTechnology(const std::string& type, @@ -346,13 +356,13 @@ void FakeShillManagerClient::AddTechnology(const std::string& type, if (GetListProperty(shill::kAvailableTechnologiesProperty)-> AppendIfNotPresent(base::Value::CreateStringValue(type))) { CallNotifyObserversPropertyChanged( - shill::kAvailableTechnologiesProperty, 0); + shill::kAvailableTechnologiesProperty); } if (enabled && GetListProperty(shill::kEnabledTechnologiesProperty)-> AppendIfNotPresent(base::Value::CreateStringValue(type))) { CallNotifyObserversPropertyChanged( - shill::kEnabledTechnologiesProperty, 0); + shill::kEnabledTechnologiesProperty); } } @@ -361,12 +371,12 @@ void FakeShillManagerClient::RemoveTechnology(const std::string& type) { if (GetListProperty(shill::kAvailableTechnologiesProperty)->Remove( type_value, NULL)) { CallNotifyObserversPropertyChanged( - shill::kAvailableTechnologiesProperty, 0); + shill::kAvailableTechnologiesProperty); } if (GetListProperty(shill::kEnabledTechnologiesProperty)->Remove( type_value, NULL)) { CallNotifyObserversPropertyChanged( - shill::kEnabledTechnologiesProperty, 0); + shill::kEnabledTechnologiesProperty); } } @@ -376,13 +386,13 @@ void FakeShillManagerClient::SetTechnologyInitializing(const std::string& type, if (GetListProperty(shill::kUninitializedTechnologiesProperty)-> AppendIfNotPresent(base::Value::CreateStringValue(type))) { CallNotifyObserversPropertyChanged( - shill::kUninitializedTechnologiesProperty, 0); + shill::kUninitializedTechnologiesProperty); } } else { if (GetListProperty(shill::kUninitializedTechnologiesProperty)->Remove( base::StringValue(type), NULL)) { CallNotifyObserversPropertyChanged( - shill::kUninitializedTechnologiesProperty, 0); + shill::kUninitializedTechnologiesProperty); } } } @@ -391,6 +401,12 @@ void FakeShillManagerClient::ClearProperties() { stub_properties_.Clear(); } +void FakeShillManagerClient::SetManagerProperty(const std::string& key, + const base::Value& value) { + SetProperty(key, value, + base::Bind(&base::DoNothing), base::Bind(&LogErrorCallback)); +} + void FakeShillManagerClient::AddManagerService(const std::string& service_path, bool add_to_visible_list, bool add_to_watch_list) { @@ -400,8 +416,8 @@ void FakeShillManagerClient::AddManagerService(const std::string& service_path, // If visible, add to Services and notify if new. if (add_to_visible_list && GetListProperty(shill::kServicesProperty)->AppendIfNotPresent( - base::Value::CreateStringValue(service_path))) { - CallNotifyObserversPropertyChanged(shill::kServicesProperty, 0); + base::Value::CreateStringValue(service_path))) { + CallNotifyObserversPropertyChanged(shill::kServicesProperty); } if (add_to_watch_list) AddServiceToWatchList(service_path); @@ -412,14 +428,14 @@ void FakeShillManagerClient::RemoveManagerService( base::StringValue service_path_value(service_path); if (GetListProperty(shill::kServicesProperty)->Remove( service_path_value, NULL)) { - CallNotifyObserversPropertyChanged(shill::kServicesProperty, 0); + CallNotifyObserversPropertyChanged(shill::kServicesProperty); } GetListProperty(shill::kServiceCompleteListProperty)->Remove( service_path_value, NULL); if (GetListProperty(shill::kServiceWatchListProperty)->Remove( service_path_value, NULL)) { CallNotifyObserversPropertyChanged( - shill::kServiceWatchListProperty, 0); + shill::kServiceWatchListProperty); } } @@ -427,8 +443,19 @@ void FakeShillManagerClient::ClearManagerServices() { GetListProperty(shill::kServicesProperty)->Clear(); GetListProperty(shill::kServiceCompleteListProperty)->Clear(); GetListProperty(shill::kServiceWatchListProperty)->Clear(); - CallNotifyObserversPropertyChanged(shill::kServicesProperty, 0); - CallNotifyObserversPropertyChanged(shill::kServiceWatchListProperty, 0); + CallNotifyObserversPropertyChanged(shill::kServicesProperty); + CallNotifyObserversPropertyChanged(shill::kServiceWatchListProperty); +} + +void FakeShillManagerClient::ServiceStateChanged( + const std::string& service_path, + const std::string& state) { + if (service_path == default_service_ && !IsConnectedState(state)) { + // Default service is no longer connected; clear. + default_service_.clear(); + base::StringValue default_service_value(default_service_); + SetManagerProperty(shill::kDefaultServiceProperty, default_service_value); + } } void FakeShillManagerClient::SortManagerServices() { @@ -440,8 +467,14 @@ void FakeShillManagerClient::SortManagerServices() { shill::kTypeVPN }; base::ListValue* service_list = GetListProperty(shill::kServicesProperty); - if (!service_list || service_list->empty()) + if (!service_list || service_list->empty()) { + if (!default_service_.empty()) { + default_service_.clear(); + base::StringValue empty_value(""); + SetManagerProperty(shill::kDefaultServiceProperty, empty_value); + } return; + } std::vector<std::string> active_services; std::vector<std::string> inactive_services; for (size_t i = 0; i < arraysize(ordered_types); ++i) { @@ -454,7 +487,30 @@ void FakeShillManagerClient::SortManagerServices() { for (size_t i = 0; i < inactive_services.size(); ++i) service_list->AppendString(inactive_services[i]); - CallNotifyObserversPropertyChanged(shill::kServicesProperty, 0); + CallNotifyObserversPropertyChanged(shill::kServicesProperty); + + // Set the first active service as the Default service. + std::string new_default_service; + if (!active_services.empty()) { + ShillServiceClient::TestInterface* service_client = + DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface(); + std::string service_path = active_services[0]; + const base::DictionaryValue* properties = + service_client->GetServiceProperties(service_path); + if (!properties) { + LOG(ERROR) << "Properties not found for service: " << service_path; + } else { + std::string state; + properties->GetString(shill::kStateProperty, &state); + if (IsConnectedState(state)) + new_default_service = service_path; + } + } + if (default_service_ != new_default_service) { + default_service_ = new_default_service; + base::StringValue default_service_value(default_service_); + SetManagerProperty(shill::kDefaultServiceProperty, default_service_value); + } } void FakeShillManagerClient::AddGeoNetwork( @@ -473,7 +529,7 @@ void FakeShillManagerClient::AddProfile(const std::string& profile_path) { const char* key = shill::kProfilesProperty; if (GetListProperty(key)->AppendIfNotPresent( new base::StringValue(profile_path))) { - CallNotifyObserversPropertyChanged(key, 0); + CallNotifyObserversPropertyChanged(key); } } @@ -485,7 +541,7 @@ void FakeShillManagerClient::AddServiceToWatchList( GetListProperty(shill::kServiceWatchListProperty)->Insert( 0, base::Value::CreateStringValue(service_path)); CallNotifyObserversPropertyChanged( - shill::kServiceWatchListProperty, 0); + shill::kServiceWatchListProperty); } void FakeShillManagerClient::PassStubProperties( @@ -508,22 +564,16 @@ void FakeShillManagerClient::PassStubGeoNetworks( } void FakeShillManagerClient::CallNotifyObserversPropertyChanged( - const std::string& property, - int delay_ms) { + const std::string& property) { // Avoid unnecessary delayed task if we have no observers (e.g. during // initial setup). if (!observer_list_.might_have_observers()) return; - if (!CommandLine::ForCurrentProcess()->HasSwitch( - chromeos::switches::kEnableStubInteractive)) { - delay_ms = 0; - } - base::MessageLoop::current()->PostDelayedTask( + base::MessageLoop::current()->PostTask( FROM_HERE, base::Bind(&FakeShillManagerClient::NotifyObserversPropertyChanged, weak_ptr_factory_.GetWeakPtr(), - property), - base::TimeDelta::FromMilliseconds(delay_ms)); + property)); } void FakeShillManagerClient::NotifyObserversPropertyChanged( @@ -594,11 +644,11 @@ void FakeShillManagerClient::SetTechnologyEnabled( else enabled_list->Remove(base::StringValue(type), NULL); CallNotifyObserversPropertyChanged( - shill::kEnabledTechnologiesProperty, 0 /* already delayed */); + shill::kEnabledTechnologiesProperty); base::MessageLoop::current()->PostTask(FROM_HERE, callback); // May affect available services - CallNotifyObserversPropertyChanged(shill::kServicesProperty, 0); - CallNotifyObserversPropertyChanged(shill::kServiceWatchListProperty, 0); + CallNotifyObserversPropertyChanged(shill::kServicesProperty); + CallNotifyObserversPropertyChanged(shill::kServiceWatchListProperty); } base::ListValue* FakeShillManagerClient::GetEnabledServiceList( @@ -638,8 +688,8 @@ void FakeShillManagerClient::ScanCompleted(const std::string& device_path, shill::kScanningProperty, base::FundamentalValue(false)); } - CallNotifyObserversPropertyChanged(shill::kServicesProperty, 0); - CallNotifyObserversPropertyChanged(shill::kServiceWatchListProperty, 0); + CallNotifyObserversPropertyChanged(shill::kServicesProperty); + CallNotifyObserversPropertyChanged(shill::kServiceWatchListProperty); base::MessageLoop::current()->PostTask(FROM_HERE, callback); } diff --git a/chromeos/dbus/fake_shill_manager_client.h b/chromeos/dbus/fake_shill_manager_client.h index 65f449e..cbcc404 100644 --- a/chromeos/dbus/fake_shill_manager_client.h +++ b/chromeos/dbus/fake_shill_manager_client.h @@ -17,8 +17,8 @@ namespace chromeos { // A fake implementation of ShillManagerClient. This works in close coordination // with FakeShillServiceClient. FakeShillDeviceClient, and // FakeShillProfileClient, and is not intended to be used independently. -class CHROMEOS_EXPORT FakeShillManagerClient : - public ShillManagerClient, +class CHROMEOS_EXPORT FakeShillManagerClient + : public ShillManagerClient, public ShillManagerClient::TestInterface { public: FakeShillManagerClient(); @@ -91,11 +91,15 @@ class CHROMEOS_EXPORT FakeShillManagerClient : const base::DictionaryValue& network) OVERRIDE; virtual void AddProfile(const std::string& profile_path) OVERRIDE; virtual void ClearProperties() OVERRIDE; + virtual void SetManagerProperty(const std::string& key, + const base::Value& value) OVERRIDE; virtual void AddManagerService(const std::string& service_path, bool add_to_visible_list, bool add_to_watch_list) OVERRIDE; virtual void RemoveManagerService(const std::string& service_path) OVERRIDE; virtual void ClearManagerServices() OVERRIDE; + virtual void ServiceStateChanged(const std::string& service_path, + const std::string& state) OVERRIDE; virtual void SortManagerServices() OVERRIDE; private: @@ -103,8 +107,7 @@ class CHROMEOS_EXPORT FakeShillManagerClient : void SetDefaultProperties(); void PassStubProperties(const DictionaryValueCallback& callback) const; void PassStubGeoNetworks(const DictionaryValueCallback& callback) const; - void CallNotifyObserversPropertyChanged(const std::string& property, - int delay_ms); + void CallNotifyObserversPropertyChanged(const std::string& property); void NotifyObserversPropertyChanged(const std::string& property); base::ListValue* GetListProperty(const std::string& property); bool TechnologyEnabled(const std::string& type) const; @@ -126,6 +129,9 @@ class CHROMEOS_EXPORT FakeShillManagerClient : // invalidate its weak pointers before any other members are destroyed. base::WeakPtrFactory<FakeShillManagerClient> weak_ptr_factory_; + // Track the default service for signaling Manager.DefaultService. + std::string default_service_; + DISALLOW_COPY_AND_ASSIGN(FakeShillManagerClient); }; diff --git a/chromeos/dbus/fake_shill_service_client.cc b/chromeos/dbus/fake_shill_service_client.cc index 8987c18..ed47b28 100644 --- a/chromeos/dbus/fake_shill_service_client.cc +++ b/chromeos/dbus/fake_shill_service_client.cc @@ -39,6 +39,11 @@ void PassStubServiceProperties( callback.Run(call_status, *properties); } +void CallSortManagerServices() { + DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface()-> + SortManagerServices(); +} + } // namespace FakeShillServiceClient::FakeShillServiceClient() : weak_ptr_factory_(this) { @@ -358,10 +363,14 @@ void FakeShillServiceClient::AddServiceWithIPConfig( properties->SetWithoutPathExpansion( shill::kStateProperty, base::Value::CreateStringValue(state)); - if (!ipconfig_path.empty()) + if (!ipconfig_path.empty()) { properties->SetWithoutPathExpansion( shill::kIPConfigProperty, base::Value::CreateStringValue(ipconfig_path)); + } + + DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface()-> + SortManagerServices(); } void FakeShillServiceClient::RemoveService(const std::string& service_path) { @@ -401,12 +410,22 @@ bool FakeShillServiceClient::SetServiceProperty(const std::string& service_path, dict->MergeDictionary(&new_properties); + // Notify the Manager if the state changed (affects DefaultService). if (property == shill::kStateProperty) { - // When State changes the sort order of Services may change. + std::string state; + value.GetAsString(&state); DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface()-> - SortManagerServices(); + ServiceStateChanged(service_path, state); + } + + // If the State changes, the sort order of Services may change and the + // DefaultService property may change. + if (property == shill::kStateProperty) { + base::MessageLoop::current()->PostTask( + FROM_HERE, base::Bind(&CallSortManagerServices)); } + // Notifiy Chrome of the property change. base::MessageLoop::current()->PostTask( FROM_HERE, base::Bind(&FakeShillServiceClient::NotifyObserversPropertyChanged, diff --git a/chromeos/dbus/fake_shill_service_client.h b/chromeos/dbus/fake_shill_service_client.h index 91000e4..1f1f60d 100644 --- a/chromeos/dbus/fake_shill_service_client.h +++ b/chromeos/dbus/fake_shill_service_client.h @@ -18,8 +18,8 @@ namespace chromeos { // A fake implementation of ShillServiceClient. This works in close coordination // with FakeShillManagerClient and is not intended to be used independently. -class CHROMEOS_EXPORT FakeShillServiceClient : - public ShillServiceClient, +class CHROMEOS_EXPORT FakeShillServiceClient + : public ShillServiceClient, public ShillServiceClient::TestInterface { public: FakeShillServiceClient(); diff --git a/chromeos/dbus/shill_manager_client.h b/chromeos/dbus/shill_manager_client.h index a128b46..90373ba 100644 --- a/chromeos/dbus/shill_manager_client.h +++ b/chromeos/dbus/shill_manager_client.h @@ -57,6 +57,10 @@ class CHROMEOS_EXPORT ShillManagerClient : public DBusClient { // Used to reset all properties; does not notify observers. virtual void ClearProperties() = 0; + // Set manager property. + virtual void SetManagerProperty(const std::string& key, + const base::Value& value) = 0; + // Add/Remove/ClearService should only be called from ShillServiceClient. virtual void AddManagerService(const std::string& service_path, bool add_to_visible_list, @@ -64,8 +68,15 @@ class CHROMEOS_EXPORT ShillManagerClient : public DBusClient { virtual void RemoveManagerService(const std::string& service_path) = 0; virtual void ClearManagerServices() = 0; - // Called by ShillServiceClient when a service's State property changes. - // Services are sorted first by Active vs. Inactive State, then by Type. + // Called by ShillServiceClient when a service's State property changes, + // before notifying observers. Sets the DefaultService property to empty + // if the state changes to a non-connected state. + virtual void ServiceStateChanged(const std::string& service_path, + const std::string& state) = 0; + + // Called by ShillServiceClient when a service's State property changes, + // after notifying observers. Services are sorted first by Active or + // Inactive State, then by Type. virtual void SortManagerServices() = 0; protected: diff --git a/chromeos/network/network_state_handler.cc b/chromeos/network/network_state_handler.cc index f3a9339..c0912714 100644 --- a/chromeos/network/network_state_handler.cc +++ b/chromeos/network/network_state_handler.cc @@ -180,13 +180,9 @@ const NetworkState* NetworkStateHandler::GetNetworkState( } const NetworkState* NetworkStateHandler::DefaultNetwork() const { - if (network_list_.empty()) + if (default_network_path_.empty()) return NULL; - const NetworkState* network = network_list_.front()->AsNetworkState(); - DCHECK(network); - if (!network->update_received() || !network->IsConnectedState()) - return NULL; - return network; + return GetNetworkState(default_network_path_); } const FavoriteState* NetworkStateHandler::DefaultFavoriteNetwork() const { @@ -538,7 +534,8 @@ void NetworkStateHandler::UpdateNetworkStateProperties( // Signal connection state changed after all properties have been updated. if (ConnectionStateChanged(network, prev_connection_state)) OnNetworkConnectionStateChanged(network); - NetworkPropertiesUpdated(network); + NET_LOG_EVENT("NetworkPropertiesUpdated", GetManagedStateLogName(network)); + NotifyNetworkPropertiesUpdated(network); } } @@ -580,10 +577,13 @@ void NetworkStateHandler::UpdateNetworkServiceProperty( if (key != shill::kSignalStrengthProperty && key != shill::kWifiFrequencyListProperty && (key != shill::kDeviceProperty || value_str != "/")) { + std::string log_event = "NetworkPropertyUpdated"; // Trigger a default network update for interesting changes only. - if (network->path() == default_network_path_) - OnDefaultNetworkChanged(); - // Log interesting event. + if (network->path() == default_network_path_) { + NotifyDefaultNetworkChanged(network); + log_event = "Default" + log_event; + } + // Log event. std::string detail = network->name() + "." + key; detail += " = " + network_event_log::ValueAsString(value); network_event_log::LogLevel log_level; @@ -592,12 +592,12 @@ void NetworkStateHandler::UpdateNetworkServiceProperty( } else { log_level = network_event_log::LOG_LEVEL_EVENT; } - NET_LOG_LEVEL(log_level, "NetworkPropertyUpdated", detail); + NET_LOG_LEVEL(log_level, log_event, detail); } } // All property updates signal 'NetworkPropertiesUpdated'. - NetworkPropertiesUpdated(network); + NotifyNetworkPropertiesUpdated(network); // If added to a Profile, request a full update so that a FavoriteState // gets created. @@ -657,9 +657,6 @@ void NetworkStateHandler::ManagedStateListChanged( base::StringPrintf("Size:%" PRIuS, network_list_.size())); FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_, NetworkListChanged()); - // The list order may have changed, so check if the default network changed. - if (CheckDefaultNetworkChanged()) - OnDefaultNetworkChanged(); // Update UMA stats. UMA_HISTOGRAM_COUNTS_100("Networks.Visible", network_list_.size()); } else if (type == ManagedState::MANAGED_TYPE_FAVORITE) { @@ -689,6 +686,34 @@ void NetworkStateHandler::ManagedStateListChanged( } } +void NetworkStateHandler::DefaultNetworkServiceChanged( + const std::string& service_path) { + // Shill uses '/' for empty service path values; check explicitly for that. + const char* kEmptyServicePath = "/"; + if (service_path == kEmptyServicePath) + default_network_path_.clear(); + else + default_network_path_ = service_path; + NET_LOG_EVENT("DefaultNetworkServiceChanged:", default_network_path_); + const NetworkState* network = NULL; + if (!default_network_path_.empty()) { + network = GetNetworkState(default_network_path_); + if (!network) { + // If NetworkState is not available yet, do not notify observers here, + // they will be notified when the state is received. + NET_LOG_DEBUG("Default NetworkState not available", + default_network_path_); + return; + } + } + if (network && !network->IsConnectedState()) { + NET_LOG_ERROR( + "DefaultNetwork is not connected: " + network->connection_state(), + network->path()); + } + NotifyDefaultNetworkChanged(network); +} + //------------------------------------------------------------------------------ // Private methods @@ -745,35 +770,30 @@ NetworkStateHandler::ManagedStateList* NetworkStateHandler::GetManagedList( void NetworkStateHandler::OnNetworkConnectionStateChanged( NetworkState* network) { DCHECK(network); - NET_LOG_EVENT("NetworkConnectionStateChanged", base::StringPrintf( - "%s:%s", GetManagedStateLogName(network).c_str(), - network->connection_state().c_str())); + std::string event = "NetworkConnectionStateChanged"; + if (network->path() == default_network_path_) { + event = "Default" + event; + if (!network->IsConnectedState()) { + NET_LOG_ERROR( + "DefaultNetwork is not connected: " + network->connection_state(), + network->path()); + } + } + NET_LOG_EVENT(event + ": " + network->connection_state(), + GetManagedStateLogName(network)); FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_, NetworkConnectionStateChanged(network)); - if (CheckDefaultNetworkChanged() || network->path() == default_network_path_) - OnDefaultNetworkChanged(); -} - -bool NetworkStateHandler::CheckDefaultNetworkChanged() { - std::string new_default_network_path; - const NetworkState* new_default_network = DefaultNetwork(); - if (new_default_network) - new_default_network_path = new_default_network->path(); - if (new_default_network_path == default_network_path_) - return false; - default_network_path_ = new_default_network_path; - return true; + if (network->path() == default_network_path_) + NotifyDefaultNetworkChanged(network); } -void NetworkStateHandler::OnDefaultNetworkChanged() { - const NetworkState* default_network = DefaultNetwork(); - NET_LOG_EVENT("DefaultNetworkChanged", - GetManagedStateLogName(default_network)); +void NetworkStateHandler::NotifyDefaultNetworkChanged( + const NetworkState* default_network) { FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_, DefaultNetworkChanged(default_network)); } -void NetworkStateHandler::NetworkPropertiesUpdated( +void NetworkStateHandler::NotifyNetworkPropertiesUpdated( const NetworkState* network) { FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_, NetworkPropertiesUpdated(network)); diff --git a/chromeos/network/network_state_handler.h b/chromeos/network/network_state_handler.h index 6f6d978..f2277ea 100644 --- a/chromeos/network/network_state_handler.h +++ b/chromeos/network/network_state_handler.h @@ -121,8 +121,9 @@ class CHROMEOS_EXPORT NetworkStateHandler // observe this class and implement NetworkPropertyChanged(). const NetworkState* GetNetworkState(const std::string& service_path) const; - // Returns the default connected network (which includes VPNs) or NULL. - // This is equivalent to ConnectedNetworkByType(kMatchTypeDefault). + // Returns the default network (which includes VPNs) based on the + // Shill Manager.DefaultNetwork property. Normally this is the same as + // ConnectedNetworkByType(kMatchTypeDefault), but the timing might differ. const NetworkState* DefaultNetwork() const; // Returns the FavoriteState associated to DefaultNetwork. Returns NULL if, @@ -281,6 +282,11 @@ class CHROMEOS_EXPORT NetworkStateHandler virtual void ManagedStateListChanged( ManagedState::ManagedType type) OVERRIDE; + // Called when the default network service changes. Sets default_network_path_ + // and notifies listeners. + virtual void DefaultNetworkServiceChanged( + const std::string& service_path) OVERRIDE; + // Called after construction. Called explicitly by tests after adding // test observers. void InitShillPropertyHandler(); @@ -314,15 +320,11 @@ class CHROMEOS_EXPORT NetworkStateHandler // Helper function to notify observers. Calls CheckDefaultNetworkChanged(). void OnNetworkConnectionStateChanged(NetworkState* network); - // If the default network changed returns true and sets - // |default_network_path_|. - bool CheckDefaultNetworkChanged(); - - // Logs an event and notifies observers. - void OnDefaultNetworkChanged(); + // Notifies observers when the default network or its properties change. + void NotifyDefaultNetworkChanged(const NetworkState* default_network); // Notifies observers about changes to |network|. - void NetworkPropertiesUpdated(const NetworkState* network); + void NotifyNetworkPropertiesUpdated(const NetworkState* network); // Called whenever Device.Scanning state transitions to false. void ScanCompleted(const std::string& type); diff --git a/chromeos/network/network_state_handler_unittest.cc b/chromeos/network/network_state_handler_unittest.cc index 76ccc49..cdbc4bf 100644 --- a/chromeos/network/network_state_handler_unittest.cc +++ b/chromeos/network/network_state_handler_unittest.cc @@ -73,7 +73,9 @@ class TestObserver : public chromeos::NetworkStateHandlerObserver { ++default_network_change_count_; default_network_ = network ? network->path() : ""; default_network_connection_state_ = - network ? network->connection_state() : ""; + network ? network->connection_state() : ""; + DVLOG(1) << "DefaultNetworkChanged: " << default_network_ + << " State: " << default_network_connection_state_; } virtual void NetworkConnectionStateChanged( @@ -92,6 +94,10 @@ class TestObserver : public chromeos::NetworkStateHandlerObserver { size_t default_network_change_count() { return default_network_change_count_; } + void reset_network_change_count() { + DVLOG(1) << "ResetNetworkChangeCount"; + default_network_change_count_ = 0; + } std::string default_network() { return default_network_; } std::string default_network_connection_state() { return default_network_connection_state_; @@ -132,7 +138,8 @@ namespace chromeos { class NetworkStateHandlerTest : public testing::Test { public: - NetworkStateHandlerTest() {} + NetworkStateHandlerTest() + : device_test_(NULL), manager_test_(NULL), service_test_(NULL) {} virtual ~NetworkStateHandlerTest() {} virtual void SetUp() OVERRIDE { @@ -160,40 +167,58 @@ class NetworkStateHandlerTest : public testing::Test { protected: void SetupDefaultShillState() { message_loop_.RunUntilIdle(); // Process any pending updates - ShillDeviceClient::TestInterface* device_test = + device_test_ = DBusThreadManager::Get()->GetShillDeviceClient()->GetTestInterface(); - device_test->ClearDevices(); - device_test->AddDevice("/device/stub_wifi_device1", - shill::kTypeWifi, "stub_wifi_device1"); - device_test->AddDevice("/device/stub_cellular_device1", - shill::kTypeCellular, "stub_cellular_device1"); - - ShillServiceClient::TestInterface* service_test = + ASSERT_TRUE(device_test_); + device_test_->ClearDevices(); + device_test_->AddDevice( + "/device/stub_wifi_device1", shill::kTypeWifi, "stub_wifi_device1"); + device_test_->AddDevice("/device/stub_cellular_device1", + shill::kTypeCellular, + "stub_cellular_device1"); + + manager_test_ = + DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface(); + ASSERT_TRUE(manager_test_); + + service_test_ = DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface(); - service_test->ClearServices(); + ASSERT_TRUE(service_test_); + service_test_->ClearServices(); const bool add_to_visible = true; const bool add_to_watchlist = true; - service_test->AddService(kShillManagerClientStubDefaultService, - kShillManagerClientStubDefaultService, - shill::kTypeEthernet, shill::kStateOnline, - add_to_visible, add_to_watchlist); - service_test->AddService(kShillManagerClientStubDefaultWireless, - kShillManagerClientStubDefaultWireless, - shill::kTypeWifi, shill::kStateOnline, - add_to_visible, add_to_watchlist); - service_test->AddService(kShillManagerClientStubWireless2, - kShillManagerClientStubWireless2, - shill::kTypeWifi, shill::kStateIdle, - add_to_visible, add_to_watchlist); - service_test->AddService(kShillManagerClientStubCellular, - kShillManagerClientStubCellular, - shill::kTypeCellular, shill::kStateIdle, - add_to_visible, add_to_watchlist); + service_test_->AddService(kShillManagerClientStubDefaultService, + kShillManagerClientStubDefaultService, + shill::kTypeEthernet, + shill::kStateOnline, + add_to_visible, + add_to_watchlist); + service_test_->AddService(kShillManagerClientStubDefaultWireless, + kShillManagerClientStubDefaultWireless, + shill::kTypeWifi, + shill::kStateOnline, + add_to_visible, + add_to_watchlist); + service_test_->AddService(kShillManagerClientStubWireless2, + kShillManagerClientStubWireless2, + shill::kTypeWifi, + shill::kStateIdle, + add_to_visible, + add_to_watchlist); + service_test_->AddService(kShillManagerClientStubCellular, + kShillManagerClientStubCellular, + shill::kTypeCellular, + shill::kStateIdle, + add_to_visible, + add_to_watchlist); } base::MessageLoopForUI message_loop_; scoped_ptr<NetworkStateHandler> network_state_handler_; scoped_ptr<TestObserver> test_observer_; + ShillDeviceClient::TestInterface* device_test_; + ShillManagerClient::TestInterface* manager_test_; + ShillServiceClient::TestInterface* service_test_; private: DISALLOW_COPY_AND_ASSIGN(NetworkStateHandlerTest); @@ -259,27 +284,25 @@ TEST_F(NetworkStateHandlerTest, TechnologyChanged) { } TEST_F(NetworkStateHandlerTest, TechnologyState) { - ShillManagerClient::TestInterface* manager_test = - DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface(); - manager_test->RemoveTechnology(shill::kTypeWimax); + manager_test_->RemoveTechnology(shill::kTypeWimax); message_loop_.RunUntilIdle(); EXPECT_EQ( NetworkStateHandler::TECHNOLOGY_UNAVAILABLE, network_state_handler_->GetTechnologyState(NetworkTypePattern::Wimax())); - manager_test->AddTechnology(shill::kTypeWimax, false); + manager_test_->AddTechnology(shill::kTypeWimax, false); message_loop_.RunUntilIdle(); EXPECT_EQ( NetworkStateHandler::TECHNOLOGY_AVAILABLE, network_state_handler_->GetTechnologyState(NetworkTypePattern::Wimax())); - manager_test->SetTechnologyInitializing(shill::kTypeWimax, true); + manager_test_->SetTechnologyInitializing(shill::kTypeWimax, true); message_loop_.RunUntilIdle(); EXPECT_EQ( NetworkStateHandler::TECHNOLOGY_UNINITIALIZED, network_state_handler_->GetTechnologyState(NetworkTypePattern::Wimax())); - manager_test->SetTechnologyInitializing(shill::kTypeWimax, false); + manager_test_->SetTechnologyInitializing(shill::kTypeWimax, false); network_state_handler_->SetTechnologyEnabled( NetworkTypePattern::Wimax(), true, network_handler::ErrorCallback()); message_loop_.RunUntilIdle(); @@ -287,7 +310,7 @@ TEST_F(NetworkStateHandlerTest, TechnologyState) { NetworkStateHandler::TECHNOLOGY_ENABLED, network_state_handler_->GetTechnologyState(NetworkTypePattern::Wimax())); - manager_test->RemoveTechnology(shill::kTypeWimax); + manager_test_->RemoveTechnology(shill::kTypeWimax); message_loop_.RunUntilIdle(); EXPECT_EQ( NetworkStateHandler::TECHNOLOGY_UNAVAILABLE, @@ -332,11 +355,9 @@ TEST_F(NetworkStateHandlerTest, FavoriteState) { TEST_F(NetworkStateHandlerTest, NetworkConnectionStateChanged) { // Change a network state. - ShillServiceClient::TestInterface* service_test = - DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface(); const std::string eth1 = kShillManagerClientStubDefaultService; base::StringValue connection_state_idle_value(shill::kStateIdle); - service_test->SetServiceProperty(eth1, shill::kStateProperty, + service_test_->SetServiceProperty(eth1, shill::kStateProperty, connection_state_idle_value); message_loop_.RunUntilIdle(); EXPECT_EQ(shill::kStateIdle, @@ -344,51 +365,104 @@ TEST_F(NetworkStateHandlerTest, NetworkConnectionStateChanged) { EXPECT_EQ(2, test_observer_->ConnectionStateChangesForService(eth1)); // Confirm that changing the connection state to the same value does *not* // signal the observer. - service_test->SetServiceProperty(eth1, shill::kStateProperty, + service_test_->SetServiceProperty(eth1, shill::kStateProperty, connection_state_idle_value); message_loop_.RunUntilIdle(); EXPECT_EQ(2, test_observer_->ConnectionStateChangesForService(eth1)); } -TEST_F(NetworkStateHandlerTest, DefaultServiceChanged) { - ShillManagerClient::TestInterface* manager_test = - DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface(); - ASSERT_TRUE(manager_test); - ShillServiceClient::TestInterface* service_test = - DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface(); - ASSERT_TRUE(service_test); - - // Change the default network by changing the state of eth1 to Idle which - // should re-sort Manager.Services. +TEST_F(NetworkStateHandlerTest, DefaultServiceDisconnected) { const std::string eth1 = kShillManagerClientStubDefaultService; const std::string wifi1 = kShillManagerClientStubDefaultWireless; + + // Disconnect ethernet. + test_observer_->reset_network_change_count(); base::StringValue connection_state_idle_value(shill::kStateIdle); - service_test->SetServiceProperty(eth1, shill::kStateProperty, - connection_state_idle_value); + service_test_->SetServiceProperty(eth1, shill::kStateProperty, + connection_state_idle_value); message_loop_.RunUntilIdle(); + // Expect two changes: first when eth1 becomes disconnected, second when + // wifi1 becomes the default. + EXPECT_EQ(2u, test_observer_->default_network_change_count()); EXPECT_EQ(wifi1, test_observer_->default_network()); - EXPECT_EQ(shill::kStateOnline, + + // Disconnect wifi. + test_observer_->reset_network_change_count(); + service_test_->SetServiceProperty(wifi1, shill::kStateProperty, + connection_state_idle_value); + message_loop_.RunUntilIdle(); + EXPECT_EQ(1u, test_observer_->default_network_change_count()); + EXPECT_EQ("", test_observer_->default_network()); +} + +TEST_F(NetworkStateHandlerTest, DefaultServiceConnected) { + const std::string eth1 = kShillManagerClientStubDefaultService; + const std::string wifi1 = kShillManagerClientStubDefaultWireless; + + // Disconnect ethernet and wifi. + base::StringValue connection_state_idle_value(shill::kStateIdle); + service_test_->SetServiceProperty(eth1, shill::kStateProperty, + connection_state_idle_value); + service_test_->SetServiceProperty(wifi1, shill::kStateProperty, + connection_state_idle_value); + message_loop_.RunUntilIdle(); + EXPECT_EQ(std::string(), test_observer_->default_network()); + + // Connect ethernet, should become the default network. + test_observer_->reset_network_change_count(); + base::StringValue connection_state_ready_value(shill::kStateReady); + service_test_->SetServiceProperty(eth1, shill::kStateProperty, + connection_state_ready_value); + message_loop_.RunUntilIdle(); + EXPECT_EQ(eth1, test_observer_->default_network()); + EXPECT_EQ(shill::kStateReady, test_observer_->default_network_connection_state()); - // We should have seen 2 default network updates - for the default - // service change, and for the state change. - EXPECT_EQ(2u, test_observer_->default_network_change_count()); + EXPECT_EQ(1u, test_observer_->default_network_change_count()); +} + +TEST_F(NetworkStateHandlerTest, DefaultServiceChanged) { + const std::string eth1 = kShillManagerClientStubDefaultService; + // The default service should be eth1. + EXPECT_EQ(eth1, test_observer_->default_network()); + + // Change the default network by changing Manager.DefaultService. + test_observer_->reset_network_change_count(); + const std::string wifi1 = kShillManagerClientStubDefaultWireless; + base::StringValue wifi1_value(wifi1); + manager_test_->SetManagerProperty( + shill::kDefaultServiceProperty, wifi1_value); + message_loop_.RunUntilIdle(); + EXPECT_EQ(wifi1, test_observer_->default_network()); + EXPECT_EQ(1u, test_observer_->default_network_change_count()); + + // Change the state of the default network. + test_observer_->reset_network_change_count(); + base::StringValue connection_state_ready_value(shill::kStateReady); + service_test_->SetServiceProperty(wifi1, shill::kStateProperty, + connection_state_ready_value); + message_loop_.RunUntilIdle(); + EXPECT_EQ(shill::kStateReady, + test_observer_->default_network_connection_state()); + EXPECT_EQ(1u, test_observer_->default_network_change_count()); // Updating a property on the default network should trigger // a default network change. + test_observer_->reset_network_change_count(); DBusThreadManager::Get()->GetShillServiceClient()->SetProperty( dbus::ObjectPath(wifi1), shill::kSecurityProperty, base::StringValue("TestSecurity"), base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction)); message_loop_.RunUntilIdle(); - EXPECT_EQ(3u, test_observer_->default_network_change_count()); + EXPECT_EQ(1u, test_observer_->default_network_change_count()); // No default network updates for signal strength changes. + test_observer_->reset_network_change_count(); DBusThreadManager::Get()->GetShillServiceClient()->SetProperty( dbus::ObjectPath(wifi1), shill::kSignalStrengthProperty, base::FundamentalValue(32), base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction)); message_loop_.RunUntilIdle(); - EXPECT_EQ(3u, test_observer_->default_network_change_count()); + EXPECT_EQ(0u, test_observer_->default_network_change_count()); } TEST_F(NetworkStateHandlerTest, RequestUpdate) { diff --git a/chromeos/network/shill_property_handler.cc b/chromeos/network/shill_property_handler.cc index 7ece2f6..5a4bca0 100644 --- a/chromeos/network/shill_property_handler.cc +++ b/chromeos/network/shill_property_handler.cc @@ -287,7 +287,11 @@ void ShillPropertyHandler::CheckPendingStateListUpdates( void ShillPropertyHandler::ManagerPropertyChanged(const std::string& key, const base::Value& value) { - if (key == shill::kServicesProperty) { + if (key == shill::kDefaultServiceProperty) { + std::string service_path; + value.GetAsString(&service_path); + listener_->DefaultNetworkServiceChanged(service_path); + } else if (key == shill::kServicesProperty) { const base::ListValue* vlist = GetListValue(key, value); if (vlist) { listener_->UpdateManagedList(ManagedState::MANAGED_TYPE_NETWORK, *vlist); diff --git a/chromeos/network/shill_property_handler.h b/chromeos/network/shill_property_handler.h index 2b96a8c..e484e98 100644 --- a/chromeos/network/shill_property_handler.h +++ b/chromeos/network/shill_property_handler.h @@ -83,6 +83,10 @@ class CHROMEOS_EXPORT ShillPropertyHandler // UpdateManagedStateProperties has been called for each new entry. virtual void ManagedStateListChanged(ManagedState::ManagedType type) = 0; + // Called when the default network service changes. + virtual void DefaultNetworkServiceChanged( + const std::string& service_path) = 0; + protected: virtual ~Listener() {} }; diff --git a/chromeos/network/shill_property_handler_unittest.cc b/chromeos/network/shill_property_handler_unittest.cc index 431cccf..a287012 100644 --- a/chromeos/network/shill_property_handler_unittest.cc +++ b/chromeos/network/shill_property_handler_unittest.cc @@ -82,6 +82,10 @@ class TestListener : public internal::ShillPropertyHandler::Listener { AddStateListUpdate(GetTypeString(type)); } + virtual void DefaultNetworkServiceChanged( + const std::string& service_path) OVERRIDE { + } + std::vector<std::string>& entries(const std::string& type) { return entries_[type]; } @@ -93,6 +97,7 @@ class TestListener : public internal::ShillPropertyHandler::Listener { return initial_property_updates_[type]; } int list_updates(const std::string& type) { return list_updates_[type]; } + void reset_list_updates() { list_updates_.clear(); } int technology_list_updates() { return technology_list_updates_; } int errors() { return errors_; } @@ -332,22 +337,23 @@ TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerTechnologyChanged) { } TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerDevicePropertyChanged) { - EXPECT_EQ(1, listener_->list_updates(shill::kDevicesProperty)); const size_t kNumShillManagerClientStubImplDevices = 2; EXPECT_EQ(kNumShillManagerClientStubImplDevices, listener_->entries(shill::kDevicesProperty).size()); // Add a device. + listener_->reset_list_updates(); const std::string kTestDevicePath("test_wifi_device1"); AddDevice(shill::kTypeWifi, kTestDevicePath); message_loop_.RunUntilIdle(); - EXPECT_EQ(2, listener_->list_updates(shill::kDevicesProperty)); + EXPECT_EQ(1, listener_->list_updates(shill::kDevicesProperty)); EXPECT_EQ(kNumShillManagerClientStubImplDevices + 1, listener_->entries(shill::kDevicesProperty).size()); // Device changes are not observed. // Remove a device + listener_->reset_list_updates(); RemoveDevice(kTestDevicePath); message_loop_.RunUntilIdle(); - EXPECT_EQ(3, listener_->list_updates(shill::kDevicesProperty)); + EXPECT_EQ(1, listener_->list_updates(shill::kDevicesProperty)); EXPECT_EQ(kNumShillManagerClientStubImplDevices, listener_->entries(shill::kDevicesProperty).size()); @@ -355,17 +361,17 @@ TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerDevicePropertyChanged) { } TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerServicePropertyChanged) { - EXPECT_EQ(1, listener_->list_updates(shill::kServicesProperty)); const size_t kNumShillManagerClientStubImplServices = 4; EXPECT_EQ(kNumShillManagerClientStubImplServices, listener_->entries(shill::kServicesProperty).size()); // Add an unwatched service. + listener_->reset_list_updates(); const std::string kTestServicePath("test_wifi_service1"); AddService(shill::kTypeWifi, kTestServicePath, shill::kStateIdle, false); message_loop_.RunUntilIdle(); // Watched and unwatched services trigger a service list update. - EXPECT_EQ(2, listener_->list_updates(shill::kServicesProperty)); + EXPECT_EQ(1, listener_->list_updates(shill::kServicesProperty)); EXPECT_EQ(kNumShillManagerClientStubImplServices + 1, listener_->entries(shill::kServicesProperty).size()); // Service receives an initial property update. @@ -384,10 +390,11 @@ TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerServicePropertyChanged) { shill::kServicesProperty)[kTestServicePath]); // Add the existing service to the watch list. + listener_->reset_list_updates(); AddService(shill::kTypeWifi, kTestServicePath, shill::kStateIdle, true); message_loop_.RunUntilIdle(); // Service list update should be received when watch list changes. - EXPECT_EQ(2, listener_->list_updates(shill::kServicesProperty)); + EXPECT_EQ(1, listener_->list_updates(shill::kServicesProperty)); // Number of services shouldn't change. EXPECT_EQ(kNumShillManagerClientStubImplServices + 1, listener_->entries(shill::kServicesProperty).size()); @@ -404,9 +411,10 @@ TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerServicePropertyChanged) { shill::kServicesProperty)[kTestServicePath]); // Remove a service + listener_->reset_list_updates(); RemoveService(kTestServicePath); message_loop_.RunUntilIdle(); - EXPECT_EQ(3, listener_->list_updates(shill::kServicesProperty)); + EXPECT_EQ(1, listener_->list_updates(shill::kServicesProperty)); EXPECT_EQ(kNumShillManagerClientStubImplServices, listener_->entries(shill::kServicesProperty).size()); @@ -472,19 +480,19 @@ TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerIPConfigPropertyChanged) { } TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerServiceCompleteList) { - // Initial list updates. - EXPECT_EQ(1, listener_->list_updates(shill::kServicesProperty)); - EXPECT_EQ(1, listener_->list_updates(shill::kServiceCompleteListProperty)); + // Add a new entry to the profile only (triggers a Services update). + const std::string kTestServicePath1("stub_wifi_profile_only1"); + AddServiceToProfile(shill::kTypeWifi, kTestServicePath1, false); + message_loop_.RunUntilIdle(); - // Add a new entry to the profile only; should trigger a single list update + // Update the Manager properties. This should trigger a single list update // for both Services and ServiceCompleteList, and a single property update // for ServiceCompleteList. - const std::string kTestServicePath1("stub_wifi_profile_only1"); - AddServiceToProfile(shill::kTypeWifi, kTestServicePath1, false); + listener_->reset_list_updates(); shill_property_handler_->UpdateManagerProperties(); message_loop_.RunUntilIdle(); - EXPECT_EQ(2, listener_->list_updates(shill::kServicesProperty)); - EXPECT_EQ(2, listener_->list_updates(shill::kServiceCompleteListProperty)); + EXPECT_EQ(1, listener_->list_updates(shill::kServicesProperty)); + EXPECT_EQ(1, listener_->list_updates(shill::kServiceCompleteListProperty)); EXPECT_EQ(0, listener_->initial_property_updates( shill::kServicesProperty)[kTestServicePath1]); EXPECT_EQ(1, listener_->initial_property_updates( @@ -499,12 +507,13 @@ TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerServiceCompleteList) { // trigger tow property updates for Services (one when the Profile propety // changes, and one for the Request) and one ServiceCompleteList change for // the Request. + listener_->reset_list_updates(); const std::string kTestServicePath2("stub_wifi_profile_only2"); AddServiceToProfile(shill::kTypeWifi, kTestServicePath2, true); shill_property_handler_->UpdateManagerProperties(); message_loop_.RunUntilIdle(); - EXPECT_EQ(3, listener_->list_updates(shill::kServicesProperty)); - EXPECT_EQ(3, listener_->list_updates(shill::kServiceCompleteListProperty)); + EXPECT_EQ(1, listener_->list_updates(shill::kServicesProperty)); + EXPECT_EQ(1, listener_->list_updates(shill::kServiceCompleteListProperty)); EXPECT_EQ(1, listener_->initial_property_updates( shill::kServicesProperty)[kTestServicePath2]); EXPECT_EQ(1, listener_->initial_property_updates( |