Merge FavoriteState into NetworkState

chromeos/network/shill_property_handler.cc

Index: chromeos/network/shill_property_handler.cc
diff --git a/chromeos/network/shill_property_handler.cc b/chromeos/network/shill_property_handler.cc
index 7996ac07e06df8d633044deb6c0a3efea358ae5c..64375f04d944a68aba8e5a5bd4768727b11239ef 100644
--- a/chromeos/network/shill_property_handler.cc
+++ b/chromeos/network/shill_property_handler.cc
@@ -61,9 +61,11 @@ class ShillPropertyObserver : public ShillPropertyChangedObserver {
         path_(path),
         handler_(handler) {
     if (type_ == ManagedState::MANAGED_TYPE_NETWORK) {
+      VLOG(2) << "ShillPropertyObserver: Network: " << path;
       DBusThreadManager::Get()->GetShillServiceClient()->
           AddPropertyChangedObserver(dbus::ObjectPath(path_), this);
     } else if (type_ == ManagedState::MANAGED_TYPE_DEVICE) {
+      VLOG(2) << "ShillPropertyObserver: Device: " << path;
       DBusThreadManager::Get()->GetShillDeviceClient()->
           AddPropertyChangedObserver(dbus::ObjectPath(path_), this);
     } else {
@@ -102,7 +104,8 @@ class ShillPropertyObserver : public ShillPropertyChangedObserver {
 
 ShillPropertyHandler::ShillPropertyHandler(Listener* listener)
     : listener_(listener),
-      shill_manager_(DBusThreadManager::Get()->GetShillManagerClient()) {
+      shill_manager_(DBusThreadManager::Get()->GetShillManagerClient()),
+      new_observed_networks_(0) {
 }
 
 ShillPropertyHandler::~ShillPropertyHandler() {
@@ -122,6 +125,7 @@ void ShillPropertyHandler::Init() {
 
 void ShillPropertyHandler::UpdateManagerProperties() {
   NET_LOG_EVENT("UpdateManagerProperties", "");
+  new_observed_networks_ = 0;
   shill_manager_->GetProperties(
       base::Bind(&ShillPropertyHandler::ManagerPropertiesCallback,
                  AsWeakPtr()));
@@ -206,10 +210,10 @@ void ShillPropertyHandler::RequestProperties(ManagedState::ManagedType type,
   if (pending_updates_[type].find(path) != pending_updates_[type].end())
     return;  // Update already requested.
 
-  NET_LOG_DEBUG("Request Properties", path);
+  NET_LOG_DEBUG("Request Properties: " + ManagedState::TypeToString(type),
+                path);
   pending_updates_[type].insert(path);
-  if (type == ManagedState::MANAGED_TYPE_NETWORK ||
-      type == ManagedState::MANAGED_TYPE_FAVORITE) {
+  if (type == ManagedState::MANAGED_TYPE_NETWORK) {
     DBusThreadManager::Get()->GetShillServiceClient()->GetProperties(
         dbus::ObjectPath(path),
         base::Bind(&ShillPropertyHandler::GetPropertiesCallback,
@@ -254,15 +258,14 @@ void ShillPropertyHandler::ManagerPropertiesCallback(
     else
       ManagerPropertyChanged(iter.key(), iter.value());
   }
-  // Update Services which can safely assume other properties have been set.
-  if (update_service_value)
-    ManagerPropertyChanged(shill::kServicesProperty, *update_service_value);
-  // Update ServiceCompleteList which skips entries that have already been
-  // requested for Services.
+  // Update Service lists after other Manager properties. Update
+  // ServiceCompleteList first so that Services (visible) entries already exist.
   if (update_service_complete_value) {
     ManagerPropertyChanged(shill::kServiceCompleteListProperty,
                            *update_service_complete_value);
   }
+  if (update_service_value)
+    ManagerPropertyChanged(shill::kServicesProperty, *update_service_value);
 
   CheckPendingStateListUpdates("");
 }
@@ -270,25 +273,23 @@ void ShillPropertyHandler::ManagerPropertiesCallback(
 void ShillPropertyHandler::CheckPendingStateListUpdates(
     const std::string& key) {
   // Once there are no pending updates, signal the state list changed callbacks.
-  if ((key.empty() || key == shill::kServicesProperty) &&
+  if ((key.empty() || key == shill::kServiceCompleteListProperty) &&
       pending_updates_[ManagedState::MANAGED_TYPE_NETWORK].size() == 0) {
     listener_->ManagedStateListChanged(ManagedState::MANAGED_TYPE_NETWORK);
   }
-  // Both Network update requests and Favorite update requests will affect
-  // the list of favorites, so wait for both to complete.
-  if ((key.empty() || key == shill::kServiceCompleteListProperty) &&
-      pending_updates_[ManagedState::MANAGED_TYPE_NETWORK].size() == 0 &&
-      pending_updates_[ManagedState::MANAGED_TYPE_FAVORITE].size() == 0) {
-    listener_->ManagedStateListChanged(ManagedState::MANAGED_TYPE_FAVORITE);
-  }
   if ((key.empty() || key == shill::kDevicesProperty) &&
       pending_updates_[ManagedState::MANAGED_TYPE_DEVICE].size() == 0) {
     listener_->ManagedStateListChanged(ManagedState::MANAGED_TYPE_DEVICE);
   }
+  // If we have added new observed (visible) networks, request an update, since
+  // Shill does not push changes to ServiceCompleteList.
+  if (key == shill::kServicesProperty && new_observed_networks_ > 0)
+    UpdateManagerProperties();  // clears new_observed_networks_
 }
 
 void ShillPropertyHandler::ManagerPropertyChanged(const std::string& key,
                                                   const base::Value& value) {
+  NET_LOG_DEBUG("ManagerPropertyChanged", key);
   if (key == shill::kDefaultServiceProperty) {
     std::string service_path;
     value.GetAsString(&service_path);
@@ -296,8 +297,11 @@ void ShillPropertyHandler::ManagerPropertyChanged(const std::string& key,
   } else if (key == shill::kServicesProperty) {
     const base::ListValue* vlist = GetListValue(key, value);
     if (vlist) {
-      listener_->UpdateManagedList(ManagedState::MANAGED_TYPE_NETWORK, *vlist);
-      UpdateProperties(ManagedState::MANAGED_TYPE_NETWORK, *vlist);
+      listener_->UpdateManagedNetworks(*vlist, false /* complete_list */);
+      // NOTE: Only call UpdateManagedNetworks for kServiceCompleteListProperty.
+      // This property only determines which networks are "visible", it does
+      // not affect the contents of the managed list.
+
       // 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
@@ -307,13 +311,15 @@ void ShillPropertyHandler::ManagerPropertyChanged(const std::string& key,
   } else if (key == shill::kServiceCompleteListProperty) {
     const base::ListValue* vlist = GetListValue(key, value);
     if (vlist) {
-      listener_->UpdateManagedList(ManagedState::MANAGED_TYPE_FAVORITE, *vlist);
-      UpdateProperties(ManagedState::MANAGED_TYPE_FAVORITE, *vlist);
+      listener_->UpdateManagedNetworks(*vlist, true /* complete_list */);
+      UpdateProperties(ManagedState::MANAGED_TYPE_NETWORK, *vlist);
     }
+  } else if (key == shill::kServiceWatchListProperty) {
+    // Currently we ignore the watch list.
   } else if (key == shill::kDevicesProperty) {
     const base::ListValue* vlist = GetListValue(key, value);
     if (vlist) {
-      listener_->UpdateManagedList(ManagedState::MANAGED_TYPE_DEVICE, *vlist);
+      listener_->UpdateManagedDevices(*vlist);
       UpdateProperties(ManagedState::MANAGED_TYPE_DEVICE, *vlist);
       UpdateObserved(ManagedState::MANAGED_TYPE_DEVICE, *vlist);
     }
@@ -343,34 +349,23 @@ void ShillPropertyHandler::ManagerPropertyChanged(const std::string& key,
 void ShillPropertyHandler::UpdateProperties(ManagedState::ManagedType type,
                                             const base::ListValue& entries) {
   std::set<std::string>& requested_updates = requested_updates_[type];
-  std::set<std::string>& requested_service_updates =
-      requested_updates_[ManagedState::MANAGED_TYPE_NETWORK];  // For favorites
   std::set<std::string> new_requested_updates;
-  NET_LOG_DEBUG(
-      base::StringPrintf("UpdateProperties: %" PRIuS, entries.GetSize()),
-      ManagedState::TypeToString(type));
+  NET_LOG_DEBUG("UpdateProperties: " + ManagedState::TypeToString(type),
+                base::StringPrintf("%" PRIuS, entries.GetSize()));
   for (base::ListValue::const_iterator iter = entries.begin();
        iter != entries.end(); ++iter) {
     std::string path;
     (*iter)->GetAsString(&path);
     if (path.empty())
       continue;
-    // Only request properties once. Favorites that are visible will be updated
-    // when the Network entry is updated. Since 'Services' is always processed
-    // before ServiceCompleteList, only Favorites that are not visible will be
-    // requested here, and GetPropertiesCallback() will only get called with
-    // type == FAVORITE for non-visible Favorites.
-    if (type == ManagedState::MANAGED_TYPE_FAVORITE &&
-        requested_service_updates.count(path) > 0) {
-      continue;
-    }
 
     // We add a special case for devices here to work around an issue in shill
     // that prevents it from sending property changed signals for cellular
     // devices (see crbug.com/321854).
     if (type == ManagedState::MANAGED_TYPE_DEVICE ||
-        requested_updates.find(path) == requested_updates.end())
+        requested_updates.find(path) == requested_updates.end()) {
       RequestProperties(type, path);
+    }
     new_requested_updates.insert(path);
   }
   requested_updates.swap(new_requested_updates);
@@ -378,8 +373,6 @@ void ShillPropertyHandler::UpdateProperties(ManagedState::ManagedType type,
 
 void ShillPropertyHandler::UpdateObserved(ManagedState::ManagedType type,
                                           const base::ListValue& entries) {
-  DCHECK(type == ManagedState::MANAGED_TYPE_NETWORK ||
-         type == ManagedState::MANAGED_TYPE_DEVICE);
   ShillPropertyObserverMap& observer_map =
       (type == ManagedState::MANAGED_TYPE_NETWORK)
       ? observed_networks_ : observed_devices_;
@@ -398,6 +391,8 @@ void ShillPropertyHandler::UpdateObserved(ManagedState::ManagedType type,
       new_observed[path] = new ShillPropertyObserver(
           type, path, base::Bind(
               &ShillPropertyHandler::PropertyChangedCallback, AsWeakPtr()));
+      if (type == ManagedState::MANAGED_TYPE_NETWORK)
+        ++new_observed_networks_;
     }
     observer_map.erase(path);
     // Limit the number of observed services.
@@ -488,14 +483,6 @@ void ShillPropertyHandler::GetPropertiesCallback(
                   base::StringPrintf("%s: %d", path.c_str(), call_status));
     return;
   }
-  // Update Favorite properties for networks in the Services list. Call this
-  // for all networks, regardless of whether or not Profile is set, because
-  // we track all networks in the Favorites list (even if they aren't saved
-  // in a Profile). See notes in UpdateProperties() and favorite_state.h.
-  if (type == ManagedState::MANAGED_TYPE_NETWORK) {
-    listener_->UpdateManagedStateProperties(
-        ManagedState::MANAGED_TYPE_FAVORITE, path, properties);
-  }
   listener_->UpdateManagedStateProperties(type, path, properties);
 
   if (type == ManagedState::MANAGED_TYPE_NETWORK) {
@@ -511,15 +498,8 @@ void ShillPropertyHandler::GetPropertiesCallback(
   }
 
   // Notify the listener only when all updates for that type have completed.
-  if (pending_updates_[type].size() == 0) {
+  if (pending_updates_[type].size() == 0)
     listener_->ManagedStateListChanged(type);
-    // Notify that Favorites have changed when notifying for Networks if there
-    // are no additional Favorite updates pending.
-    if (type == ManagedState::MANAGED_TYPE_NETWORK &&
-        pending_updates_[ManagedState::MANAGED_TYPE_FAVORITE].size() == 0) {
-      listener_->ManagedStateListChanged(ManagedState::MANAGED_TYPE_FAVORITE);
-    }
-  }
 }
 
 void ShillPropertyHandler::PropertyChangedCallback(