Chrome OS: Use Manager.DefaultService for Default Network

chromeos/network/network_state_handler.cc

 // Copyright (c) 2012 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/network_state_handler.h"
 
 #include "base/bind.h"
 #include "base/format_macros.h"
 #include "base/location.h"
 #include "base/logging.h"
@@ skipping 162 matching lines @@
 
 const NetworkState* NetworkStateHandler::GetNetworkState(
     const std::string& service_path) const {
   const NetworkState* network = GetModifiableNetworkState(service_path);
   if (network && !network->update_received())
     return NULL;
   return network;
 }
 
 const NetworkState* NetworkStateHandler::DefaultNetwork() const {
-  if (network_list_.empty())
+  if (default_network_path_.empty())

[mukesh agrawal, 2014/02/25 02:08:41]

Not sure about this check. When there is no connected network, shill will send a
path of "/". (It can't send an empty string, because the value is encoded as a
D-Bus path type, and path types aren't allowed to be empty.)

[stevenjb, 2014/02/25 21:31:39]

Hmm, that's unfortunate, but I can add a test for that where we set
default_network_path_. The code will work as-is since GetNetworkState() will
return NULL if the path doesn't have an associated service, but for debugging,
etc, it would be better to use an empty string for no default network.

     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 {
   const NetworkState* default_network = DefaultNetwork();
   if (!default_network)
     return NULL;
   const FavoriteState* default_favorite =
       GetFavoriteState(default_network->path());
   DCHECK(default_favorite);
   DCHECK(default_favorite->update_received());
@@ skipping 331 matching lines @@
        !iter.IsAtEnd(); iter.Advance()) {
     if (network->PropertyChanged(iter.key(), iter.value()))
       network_property_updated = true;
   }
   network_property_updated |= network->InitialPropertiesReceived(properties);
   // Notify observers of NetworkState changes.
   if (network_property_updated || network->update_requested()) {
     // 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);
   }
 }
 
 void NetworkStateHandler::UpdateNetworkServiceProperty(
     const std::string& service_path,
     const std::string& key,
     const base::Value& value) {
   // Update any associated FavoriteState.
   ManagedState* favorite =
       GetModifiableManagedState(&favorite_list_, service_path);
@@ skipping 21 matching lines @@
   } else {
     std::string value_str;
     value.GetAsString(&value_str);
     // Some property changes are noisy and not interesting:
     // * Wifi SignalStrength
     // * WifiFrequencyList updates
     // * Device property changes to "/" (occurs before a service is removed)
     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;
       if (key == shill::kErrorProperty || key == shill::kErrorDetailsProperty) {
         log_level = network_event_log::LOG_LEVEL_ERROR;
       } 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.
   if (prev_profile_path.empty() && !network->profile_path().empty())
     RequestUpdateForNetwork(service_path);
 }
 
 void NetworkStateHandler::UpdateDeviceProperty(const std::string& device_path,
                                                const std::string& key,
                                                const base::Value& value) {
@@ skipping 39 matching lines @@
 }
 
 void NetworkStateHandler::ManagedStateListChanged(
     ManagedState::ManagedType type) {
   if (type == ManagedState::MANAGED_TYPE_NETWORK) {
     // Notify observers that the list of networks has changed.
     NET_LOG_EVENT("NetworkListChanged",
                   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) {
     NET_LOG_DEBUG("FavoriteListChanged",
                   base::StringPrintf("Size:%" PRIuS, favorite_list_.size()));
     // The FavoriteState list only changes when the NetworkState list changes,
     // so no need to signal observers here again.
 
     // Update UMA stats.
     size_t shared = 0, unshared = 0;
     for (ManagedStateList::iterator iter = favorite_list_.begin();
          iter != favorite_list_.end(); ++iter) {
       FavoriteState* favorite = (*iter)->AsFavoriteState();
       if (!favorite->is_favorite())
         continue;
       if (favorite->IsPrivate())
         ++unshared;
       else
         ++shared;
     }
     UMA_HISTOGRAM_COUNTS_100("Networks.RememberedShared", shared);
     UMA_HISTOGRAM_COUNTS_100("Networks.RememberedUnshared", unshared);
   } else if (type == ManagedState::MANAGED_TYPE_DEVICE) {
     NotifyDeviceListChanged();
   } else {
     NOTREACHED();
   }
 }
 
+void NetworkStateHandler::DefaultNetworkServiceChanged(
+    const std::string& service_path) {
+  default_network_path_ = service_path;
+  NET_LOG_EVENT("DefaultNetworkServiceChanged", service_path);
+  const NetworkState* network = NULL;
+  if (!service_path.empty()) {

[mukesh agrawal, 2014/02/25 02:08:41]

Same here (might need to check equality against '/' instead of .empty()).

[stevenjb, 2014/02/25 21:31:39]

I will add the check for "/" where we set default_network_path_.

+    network = GetNetworkState(service_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", service_path);
+      return;
+    }
+  }
+  if (network && !network->IsConnectedState()) {
+    NET_LOG_ERROR(
+        "DefaultNetwork is not connected: " + network->connection_state(),
+        network->path());
+  }
+  NotifyDefaultNetworkChanged(network);
+}
+
 //------------------------------------------------------------------------------
 // Private methods
 
 void NetworkStateHandler::NotifyDeviceListChanged() {
   NET_LOG_DEBUG("NotifyDeviceListChanged",
                 base::StringPrintf("Size:%" PRIuS, device_list_.size()));
   FOR_EACH_OBSERVER(NetworkStateHandlerObserver, observers_,
                     DeviceListChanged());
 }
 
@@ skipping 36 matching lines @@
     case ManagedState::MANAGED_TYPE_DEVICE:
       return &device_list_;
   }
   NOTREACHED();
   return NULL;
 }
 
 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();
+  if (network->path() == default_network_path_)
+    NotifyDefaultNetworkChanged(network);
 }
 
-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;
-}
-
-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));
 }
 
 void NetworkStateHandler::ScanCompleted(const std::string& type) {
   size_t num_callbacks = scan_complete_callbacks_.count(type);
   NET_LOG_EVENT("ScanCompleted",
                 base::StringPrintf("%s:%" PRIuS, type.c_str(), num_callbacks));
   if (num_callbacks == 0)
@@ skipping 24 matching lines @@
   }
 
   if (type.MatchesType(shill::kTypeCellular))
     return shill::kTypeCellular;
 
   NOTREACHED();
   return std::string();
 }
 
 }  // namespace chromeos