Observe property updates for all Network Services

BUG=239200
R=pneubeck@chromium.org

Review URL: https://codereview.chromium.org/14813021

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@200024 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 15 insertions, 14 deletions

diff --git a/chromeos/network/shill_property_handler.cc b/chromeos/network/shill_property_handler.cc
index e6efd65..968b9bc 100644
--- a/chromeos/network/shill_property_handler.cc
+++ b/chromeos/network/shill_property_handler.cc
@@ -205,9 +205,9 @@ void ShillPropertyHandler::OnPropertyChanged(const std::string& key,
                                              const base::Value& value) {
   if (ManagerPropertyChanged(key, value))
     listener_->ManagerPropertyChanged();
-  // If the service watch or device list changed and there are no pending
+  // If the service or device list changed and there are no pending
   // updates, signal the state list changed callback.
-  if ((key == flimflam::kServiceWatchListProperty) &&
+  if ((key == flimflam::kServicesProperty) &&
       pending_updates_[ManagedState::MANAGED_TYPE_NETWORK].size() == 0) {
     listener_->ManagedStateListChanged(ManagedState::MANAGED_TYPE_NETWORK);
   }
@@ -258,11 +258,12 @@ bool ShillPropertyHandler::ManagerPropertyChanged(const std::string& key,
   bool notify_manager_changed = false;
   if (key == flimflam::kServicesProperty) {
     const base::ListValue* vlist = GetListValue(key, value);
-    if (vlist)
-      listener_->UpdateManagedList(ManagedState::MANAGED_TYPE_NETWORK, *vlist);
-  } else if (key == flimflam::kServiceWatchListProperty) {
-    const base::ListValue* vlist = GetListValue(key, value);
     if (vlist) {
+      listener_->UpdateManagedList(ManagedState::MANAGED_TYPE_NETWORK, *vlist);
+      // UpdateObserved used to use kServiceWatchListProperty for TYPE_NETWORK,
+      // however that prevents us from receiving Strength updates from inactive
+      // networks. The overhead for observing all services is not unreasonable
+      // (and we limit the max number of observed services to kMaxObserved).
       UpdateObserved(ManagedState::MANAGED_TYPE_NETWORK, *vlist);
     }
   } else if (key == flimflam::kDevicesProperty) {
diff --git a/chromeos/network/shill_property_handler_unittest.cc b/chromeos/network/shill_property_handler_unittest.cc
index 34e685a..c41bbdb 100644
--- a/chromeos/network/shill_property_handler_unittest.cc
+++ b/chromeos/network/shill_property_handler_unittest.cc
@@ -324,10 +324,13 @@ TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerServicePropertyChanged) {
              flimflam::kStateIdle, false);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(1, listener_->manager_updates());  // No new manager updates.
-  // Only watched services trigger a service list update.
-  EXPECT_EQ(1, listener_->list_updates(flimflam::kServicesProperty));
+  // Watched and unwatched services trigger a service list update.
+  EXPECT_EQ(2, listener_->list_updates(flimflam::kServicesProperty));
   EXPECT_EQ(kNumShillManagerClientStubImplServices + 1,
             listener_->entries(flimflam::kServicesProperty).size());
+  // Service receives an initial property update.
+  EXPECT_EQ(1, listener_->
+            property_updates(flimflam::kServicesProperty)[kTestServicePath]);
   // Change a property.
   base::FundamentalValue scan_interval(3);
   DBusThreadManager::Get()->GetShillServiceClient()->SetProperty(
@@ -336,8 +339,8 @@ TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerServicePropertyChanged) {
       scan_interval,
       base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction));
   message_loop_.RunUntilIdle();
-  // Property change should NOT trigger an update.
-  EXPECT_EQ(0, listener_->
+  // Property change triggers an update.
+  EXPECT_EQ(2, listener_->
             property_updates(flimflam::kServicesProperty)[kTestServicePath]);
 
   // Add the existing service to the watch list.
@@ -349,9 +352,6 @@ TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerServicePropertyChanged) {
   // Number of services shouldn't change.
   EXPECT_EQ(kNumShillManagerClientStubImplServices + 1,
             listener_->entries(flimflam::kServicesProperty).size());
-  // Property update should be received when watched service is added.
-  EXPECT_EQ(1, listener_->
-            property_updates(flimflam::kServicesProperty)[kTestServicePath]);
 
   // Change a property.
   DBusThreadManager::Get()->GetShillServiceClient()->SetProperty(
@@ -361,7 +361,7 @@ TEST_F(ShillPropertyHandlerTest, ShillPropertyHandlerServicePropertyChanged) {
       base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction));
   message_loop_.RunUntilIdle();
   // Property change should trigger another update.
-  EXPECT_EQ(2, listener_->
+  EXPECT_EQ(3, listener_->
             property_updates(flimflam::kServicesProperty)[kTestServicePath]);
 
   // Remove a service