[CrOS Tether] Update NetworkState to include tether properties and integrate into NetworkStateHandl…

chromeos/network/network_state_handler.cc

Index: chromeos/network/network_state_handler.cc
diff --git a/chromeos/network/network_state_handler.cc b/chromeos/network/network_state_handler.cc
index 620ae84f4872738cf701f42994016c6f30db9d66..421f7a622a6de40d2384067083c9f406dad909a3 100644
--- a/chromeos/network/network_state_handler.cc
+++ b/chromeos/network/network_state_handler.cc
@@ -13,7 +13,6 @@
 #include "base/json/json_string_value_serializer.h"
 #include "base/json/json_writer.h"
 #include "base/location.h"
-#include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_number_conversions.h"
@@ -223,6 +222,18 @@ const NetworkState* NetworkStateHandler::DefaultNetwork() const {
 
 const NetworkState* NetworkStateHandler::ConnectedNetworkByType(
     const NetworkTypePattern& type) const {
+  // If a tether network is connected, return that network before checking other
+  // network types.
+  if (type.MatchesPattern(NetworkTypePattern::Tether())) {
+    for (auto iter = tether_network_list_.begin();
+         iter != tether_network_list_.end(); ++iter) {
+      const NetworkState* network = (*iter)->AsNetworkState();
+      DCHECK(network);
+      if (network->IsConnectedState())
+        return network;
+    }
+  }
+
   // Active networks are always listed first by Shill so no need to sort.
   for (auto iter = network_list_.begin(); iter != network_list_.end(); ++iter) {
     const NetworkState* network = (*iter)->AsNetworkState();
@@ -239,6 +250,18 @@ const NetworkState* NetworkStateHandler::ConnectedNetworkByType(
 
 const NetworkState* NetworkStateHandler::ConnectingNetworkByType(
     const NetworkTypePattern& type) const {
+  // If a tether network is connecting, return that network before checking
+  // other network types.
+  if (type.MatchesPattern(NetworkTypePattern::Tether())) {
+    for (auto iter = tether_network_list_.begin();
+         iter != tether_network_list_.end(); ++iter) {
+      const NetworkState* network = (*iter)->AsNetworkState();
+      DCHECK(network);
+      if (network->IsConnectingState())
+        return network;
+    }
+  }
+
   // Active networks are always listed first by Shill so no need to sort.
   for (auto iter = network_list_.begin(); iter != network_list_.end(); ++iter) {
     const NetworkState* network = (*iter)->AsNetworkState();
@@ -257,6 +280,14 @@ const NetworkState* NetworkStateHandler::FirstNetworkByType(
     const NetworkTypePattern& type) {
   if (!network_list_sorted_)
     SortNetworkList();  // Sort to ensure visible networks are listed first.
+
+  // If |type| matches tether networks and at least one tether network is
+  // present, return the first network (since it has been sorted already).
+  if (type.MatchesPattern(NetworkTypePattern::Tether()) &&
+      !tether_network_list_.empty()) {
+    return tether_network_list_[0]->AsNetworkState();
+  }
+
   for (auto iter = network_list_.begin(); iter != network_list_.end(); ++iter) {
     const NetworkState* network = (*iter)->AsNetworkState();
     DCHECK(network);
@@ -298,10 +329,25 @@ void NetworkStateHandler::GetNetworkListByType(const NetworkTypePattern& type,
                                                NetworkStateList* list) {
   DCHECK(list);
   list->clear();
-  int count = 0;
+
   // Sort the network list if necessary.
   if (!network_list_sorted_)
     SortNetworkList();
+
+  if (type.MatchesPattern(NetworkTypePattern::Tether())) {
+    GetTetherNetworkList(limit, list);
+  }
+
+  int count = list->size();
+
+  if (type.Equals(NetworkTypePattern::Tether()) ||
+      (limit != 0 && count >= limit)) {
+    // If only searching for tether networks, there is no need to continue
+    // searching through other network types; likewise, if the limit has already
+    // been reached, there is no need to continue searching.
+    return;
+  }
+
   for (auto iter = network_list_.begin(); iter != network_list_.end(); ++iter) {
     const NetworkState* network = (*iter)->AsNetworkState();
     DCHECK(network);
@@ -354,8 +400,13 @@ const NetworkState* NetworkStateHandler::GetNetworkStateFromGuid(
 }
 
 void NetworkStateHandler::AddTetherNetworkState(const std::string& guid,
-                                                const std::string& name) {
+                                                const std::string& name,
+                                                const std::string& carrier,
+                                                int battery_percentage,
+                                                int signal_strength) {
   DCHECK(!guid.empty());
+  DCHECK(battery_percentage >= 0 && battery_percentage <= 100);
+  DCHECK(signal_strength >= 0 && signal_strength <= 100);

[Ryan Hansberry, 2017/04/18 16:08:55]

Signal strength can be between 0 and 4 inclusive.

[Kyle Horimoto, 2017/04/18 17:15:23]

Not on Chrome OS.

 
   // If the network already exists, do nothing.
   if (GetNetworkStateFromGuid(guid)) {
@@ -364,19 +415,43 @@ void NetworkStateHandler::AddTetherNetworkState(const std::string& guid,
     return;
   }
 
+  // Use the GUID as the network's service path.
   std::unique_ptr<NetworkState> tether_network_state =
-      base::MakeUnique<NetworkState>(guid);
+      base::MakeUnique<NetworkState>(guid /* path */);
 
+  // Tether networks are always connectable
+  tether_network_state->connectable_ = true;

[Ryan Hansberry, 2017/04/18 16:08:55]

Should probably create a set_connectable().

[Kyle Horimoto, 2017/04/18 17:15:23]

Done.

   tether_network_state->set_name(name);
   tether_network_state->set_type(kTypeTether);
   tether_network_state->SetGuid(guid);
   tether_network_state->set_visible(true);
   tether_network_state->set_update_received();
+  tether_network_state->set_update_requested(false);
+  tether_network_state->SetCarrier(carrier);
+  tether_network_state->SetBatteryPercentage(battery_percentage);
+  tether_network_state->SetTetherSignalStrength(signal_strength);
 
   tether_network_list_.push_back(std::move(tether_network_state));
   NotifyNetworkListChanged();
 }
 
+bool NetworkStateHandler::UpdateTetherNetworkProperties(
+    const std::string& guid,
+    const std::string& carrier,
+    int battery_percentage,
+    int signal_strength) {
+  NetworkState* tether_network_state = GetModifiableNetworkStateFromGuid(guid);
+  if (!tether_network_state)
+    return false;
+
+  tether_network_state->SetCarrier(carrier);
+  tether_network_state->SetBatteryPercentage(battery_percentage);
+  tether_network_state->SetTetherSignalStrength(signal_strength);
+
+  NotifyNetworkListChanged();
+  return true;
+}
+
 void NetworkStateHandler::RemoveTetherNetworkState(const std::string& guid) {
   for (auto iter = tether_network_list_.begin();
        iter != tether_network_list_.end(); ++iter) {
@@ -427,16 +502,24 @@ bool NetworkStateHandler::AssociateTetherNetworkStateWithWifiNetwork(
 
 void NetworkStateHandler::SetTetherNetworkStateDisconnected(
     const std::string& guid) {
+  // TODO(khorimoto): Change default network? Notify of connection change?

[Ryan Hansberry, 2017/04/18 16:08:55]

Will the Tether component be the only caller of this method? If so, there
shouldn't be any need for NSH to do anything; Tether will.

[Kyle Horimoto, 2017/04/18 17:15:23]

That's not true. There's no public method for changing default_network_path_ or
causing a DefaultNetworkChanged() callback to be invoked on observers.

[Ryan Hansberry, 2017/04/18 23:56:17]

Do we need to change default_network_path_? In any case, it seems any updating
could be done in NotifyNetworkListChanged() or the methods it calls?

[Kyle Horimoto, 2017/04/19 00:28:17]

Yes, we want to change the default network path so that the system is updated
that we are connecting to a different network. I'm going to do this as part of a
follow-up CL.

   SetTetherNetworkStateConnectionState(guid, shill::kStateDisconnect);
 }
 
 void NetworkStateHandler::SetTetherNetworkStateConnecting(
     const std::string& guid) {
+  // TODO(khorimoto): Change default network? Notify of connection change?
   SetTetherNetworkStateConnectionState(guid, shill::kStateConfiguration);
 }
 
 void NetworkStateHandler::SetTetherNetworkStateConnected(
     const std::string& guid) {
+  // Being connected implies that AssociateTetherNetworkStateWithWifiNetwork()
+  // was already called, so ensure that the association is still intact.
+  DCHECK(GetNetworkStateFromGuid(GetNetworkStateFromGuid(guid)->tether_guid())
+             ->tether_guid() == guid);
+
+  // TODO(khorimoto): Change default network? Notify of connection change?
   SetTetherNetworkStateConnectionState(guid, shill::kStateOnline);
 }
 
@@ -450,11 +533,7 @@ void NetworkStateHandler::SetTetherNetworkStateConnectionState(
     return;
   }
 
-  if (!NetworkTypePattern::Tether().MatchesType(tether_network->type())) {
-    NET_LOG(ERROR) << "SetTetherNetworkStateConnectionState: network "
-                   << "is not a Tether network: " << guid;
-    return;
-  }
+  DCHECK(NetworkTypePattern::Tether().MatchesType(tether_network->type()));
 
   tether_network->set_connection_state(connection_state);
   NotifyNetworkListChanged();
@@ -575,7 +654,7 @@ void NetworkStateHandler::SetLastErrorForTest(const std::string& service_path,
                                               const std::string& error) {
   NetworkState* network_state = GetModifiableNetworkState(service_path);
   if (!network_state) {
-    LOG(ERROR) << "No matching NetworkState for: " << service_path;
+    NET_LOG(ERROR) << "No matching NetworkState for: " << service_path;
     return;
   }
   network_state->last_error_ = error;
@@ -867,6 +946,7 @@ void NetworkStateHandler::ManagedStateListChanged(
   }
 }
 
+// TODO(khorimoto): Add sorting for the tether network list as well.
 void NetworkStateHandler::SortNetworkList() {
   // Note: usually active networks will precede inactive networks, however
   // this may briefly be untrue during state transitions (e.g. a network may