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

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 <stddef.h>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/format_macros.h"
 #include "base/guid.h"
 #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"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/network/device_state.h"
 #include "chromeos/network/network_event_log.h"
 #include "chromeos/network/network_state.h"
@@ skipping 189 matching lines @@
 }
 
 const NetworkState* NetworkStateHandler::DefaultNetwork() const {
   if (default_network_path_.empty())
     return nullptr;
   return GetNetworkState(default_network_path_);
 }
 
 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();
     DCHECK(network);
     if (!network->update_received())
       continue;
     if (!network->IsConnectedState())
       break;  // Connected networks are listed first.
     if (network->Matches(type))
       return network;
   }
   return nullptr;
 }
 
 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();
     DCHECK(network);
     if (!network->update_received() || network->IsConnectedState())
       continue;
     if (!network->IsConnectingState())
       break;  // Connected and connecting networks are listed first.
     if (network->Matches(type))
       return network;
   }
   return nullptr;
 }
 
 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);
     if (!network->update_received())
       continue;
     if (!network->visible())
       break;
     if (network->Matches(type))
       return network;
   }
@@ skipping 14 matching lines @@
     const NetworkTypePattern& type,
     NetworkStateList* list) {
   GetNetworkListByType(type, false /* configured_only */,
                        true /* visible_only */, 0 /* no limit */, list);
 }
 
 void NetworkStateHandler::GetVisibleNetworkList(NetworkStateList* list) {
   GetVisibleNetworkListByType(NetworkTypePattern::Default(), list);
 }
 
 void NetworkStateHandler::GetNetworkListByType(const NetworkTypePattern& type,

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

This now makes GetTetherNetworkList pointless. Is it necessary to change the
behavior of GetNetworkListByType like this? If so, remove GetTetherNetworkList
(stevenjb@ will also have input on this because we decided initially not to
change the behavior of GetNetworkListByType and instead create
GetTetherNetworkList).

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

The implementation of the chrome.networkingPrivate.getNetworks() API function
calls GetNetworkListByType() to return networks for the API. It should not be
the responsibility of the API to call multiple NSH functions to fetch networks -
this should be encapsulated by NSH.

That being said, I think that we should maintain the separate
GetTetherNetworkList() since it reduces the complexity of
GetNetworkListByType(). We can move it to be a private function if you would
prefer, though. Thoughts?

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

I would prefer if GetTetherNetworkList() were private.

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

Done.

[Kyle Horimoto, 2017/04/19 02:05:46]

Actually, this function is still called by ash. Changed it back to public.

[Ryan Hansberry, 2017/04/19 17:49:07]

Do you mean that GetTetherNetworkList() is called by ash? In that case those
calls should be replaced by GetNetworkListByType calls.

[Kyle Horimoto, 2017/04/19 19:23:28]

Done.

                                                bool configured_only,
                                                bool visible_only,
                                                int limit,
                                                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);
     if (!network->update_received() || !network->Matches(type))
       continue;
     if (configured_only && !network->IsInProfile())
       continue;
     if (visible_only && !network->visible())
       continue;
     list->push_back(network);
@@ skipping 32 matching lines @@
   return network;
 }
 
 const NetworkState* NetworkStateHandler::GetNetworkStateFromGuid(
     const std::string& guid) const {
   DCHECK(!guid.empty());
   return GetModifiableNetworkStateFromGuid(guid);
 }
 
 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)) {
     NET_LOG(ERROR) << "AddTetherNetworkState: " << name
                    << " called with existing guid:" << 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) {
     if (iter->get()->AsNetworkState()->guid() == guid) {
       NetworkState* wifi_network = GetModifiableNetworkStateFromGuid(
           iter->get()->AsNetworkState()->tether_guid());
       if (wifi_network)
         wifi_network->set_tether_guid(std::string());
 
       tether_network_list_.erase(iter);
@@ skipping 30 matching lines @@
   }
 
   tether_network->set_tether_guid(wifi_network_guid);
   wifi_network->set_tether_guid(tether_network_guid);
   NotifyNetworkListChanged();
   return true;
 }
 
 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);
 }
 
 void NetworkStateHandler::SetTetherNetworkStateConnectionState(
     const std::string& guid,
     const std::string& connection_state) {
   NetworkState* tether_network = GetModifiableNetworkStateFromGuid(guid);
   if (!tether_network) {
     NET_LOG(ERROR) << "SetTetherNetworkStateConnectionState: Tether network "
                    << "not found: " << guid;
     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();
 }
 
 void NetworkStateHandler::GetDeviceList(DeviceStateList* list) const {
   GetDeviceListByType(NetworkTypePattern::Default(), list);
 }
 
 void NetworkStateHandler::GetDeviceListByType(const NetworkTypePattern& type,
@@ skipping 100 matching lines @@
                       service_path.c_str()));
     return nullptr;
   }
   return list.front();
 }
 
 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;
 }
 
 //------------------------------------------------------------------------------
 // ShillPropertyHandler::Delegate overrides
 
 void NetworkStateHandler::UpdateManagedList(ManagedState::ManagedType type,
                                             const base::ListValue& entries) {
@@ skipping 271 matching lines @@
         devices += ", ";
       devices += (*iter)->name();
     }
     NET_LOG_EVENT("DeviceList", devices);
     NotifyDeviceListChanged();
   } else {
     NOTREACHED();
   }
 }
 
+// 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
   // transition to idle before the list is updated).
   ManagedStateList active, non_wifi_visible, wifi_visible, hidden, new_networks;
   for (ManagedStateList::iterator iter = network_list_.begin();
        iter != network_list_.end(); ++iter) {
     NetworkState* network = (*iter)->AsNetworkState();
     if (!network->update_received()) {
       new_networks.push_back(std::move(*iter));
@@ skipping 289 matching lines @@
   if (type.MatchesType(shill::kTypeBluetooth))
     technologies.emplace_back(shill::kTypeBluetooth);
   if (type.MatchesType(shill::kTypeVPN))
     technologies.emplace_back(shill::kTypeVPN);
 
   CHECK_GT(technologies.size(), 0ul);
   return technologies;
 }
 
 }  // namespace chromeos