Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(26)

Unified Diff: chromeos/network/network_state_handler.cc

Issue 2819993002: [CrOS Tether] Add the notion of a tether DeviceState. (Closed)
Patch Set: hansberry@ comments. Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chromeos/network/network_state_handler.cc
diff --git a/chromeos/network/network_state_handler.cc b/chromeos/network/network_state_handler.cc
index d1fe75c1ebad0dac0bd732bcc7960573fac3f965..8f4265d1a1c4f5624147157fbae68e8d04f18ceb 100644
--- a/chromeos/network/network_state_handler.cc
+++ b/chromeos/network/network_state_handler.cc
@@ -126,9 +126,7 @@ NetworkStateHandler::TechnologyState NetworkStateHandler::GetTechnologyState(
std::string technology = GetTechnologyForType(type);
if (technology == kTypeTether) {
- bool is_tether_enabled = base::CommandLine::ForCurrentProcess()->HasSwitch(
- chromeos::switches::kEnableTether);
- return is_tether_enabled ? TECHNOLOGY_ENABLED : TECHNOLOGY_UNAVAILABLE;
+ return tether_technology_state_;
}
TechnologyState state;
@@ -154,6 +152,14 @@ void NetworkStateHandler::SetTechnologyEnabled(
const network_handler::ErrorCallback& error_callback) {
std::vector<std::string> technologies = GetTechnologiesForType(type);
for (const std::string& technology : technologies) {
+ if (technology == kTypeTether) {
+ // Tether does not exist in Shill, so set |tether_technology_state_| and
+ // skip the below interactions with |shill_property_handler_|.
+ tether_technology_state_ =
+ enabled ? TECHNOLOGY_ENABLED : TECHNOLOGY_AVAILABLE;
stevenjb 2017/04/20 16:49:41 We need to ensure that this transition is valid: i
Kyle Horimoto 2017/04/20 19:38:46 Done.
+ continue;
+ }
+
if (!shill_property_handler_->IsTechnologyAvailable(technology))
continue;
NET_LOG_USER("SetTechnologyEnabled",
@@ -165,11 +171,53 @@ void NetworkStateHandler::SetTechnologyEnabled(
NotifyDeviceListChanged();
}
+void NetworkStateHandler::SetTetherTechnologyState(
+ TechnologyState technology_state) {
+ if (tether_technology_state_ == technology_state)
+ return;
+
+ tether_technology_state_ = technology_state;
stevenjb 2017/04/20 18:13:45 So, to go with my comments in UpdateManagedList(),
Kyle Horimoto 2017/04/20 19:38:46 Done.
+
+ // Signal Device/Technology state changed.
+ NotifyDeviceListChanged();
+}
+
+void NetworkStateHandler::SetTetherScanState(bool is_scanning) {
+ DeviceState* tether_device_state =
+ GetModifiableDeviceState(kTetherDevicePath);
+ DCHECK(tether_device_state);
+
+ bool was_scanning = tether_device_state->scanning();
+ tether_device_state->set_scanning(is_scanning);
+
+ if (was_scanning && !is_scanning) {
+ // If a scan was in progress but has completed, notify observers.
+ NotifyScanCompleted(tether_device_state);
+ }
+}
+
void NetworkStateHandler::SetProhibitedTechnologies(
const std::vector<std::string>& prohibited_technologies,
const network_handler::ErrorCallback& error_callback) {
- shill_property_handler_->SetProhibitedTechnologies(prohibited_technologies,
- error_callback);
+ // Make a copy of |prohibited_technologies| since the list may be edited
+ // within this function.
+ std::vector<std::string> prohibited_technologies_copy =
+ prohibited_technologies;
+
+ for (auto it = prohibited_technologies_copy.begin();
+ it < prohibited_technologies_copy.end(); ++it) {
+ if (*it == kTypeTether) {
+ // If Tether networks are prohibited, set |tether_technology_state_| and
+ // remove |kTypeTether| from the list before passing it to
+ // |shill_property_handler_| below. Shill does not have a concept of
+ // Tether networks, so it cannot prohibit that technology type.
+ tether_technology_state_ = TECHNOLOGY_PROHIBITED;
+ it = prohibited_technologies_copy.erase(it);
+ }
+ }
+
+ shill_property_handler_->SetProhibitedTechnologies(
+ prohibited_technologies_copy, error_callback);
// Signal Device/Technology state changed.
NotifyDeviceListChanged();
}
@@ -300,7 +348,7 @@ const NetworkState* NetworkStateHandler::FirstNetworkByType(
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
+ // 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()) {
@@ -323,9 +371,14 @@ const NetworkState* NetworkStateHandler::FirstNetworkByType(
std::string NetworkStateHandler::FormattedHardwareAddressForType(
const NetworkTypePattern& type) const {
const NetworkState* network = ConnectedNetworkByType(type);
+ if (network && network->type() == kTypeTether) {
+ // If this is a Tether network, get the MAC address corresponding to that
+ // network instead.
+ network = GetNetworkStateFromGuid(network->tether_guid());
+ }
const DeviceState* device = network ? GetDeviceState(network->device_path())
: GetDeviceStateByType(type);
- if (!device)
+ if (!device || device->mac_address().empty())
return std::string();
return network_util::FormattedMacAddress(device->mac_address());
}
@@ -361,7 +414,7 @@ void NetworkStateHandler::GetNetworkListByType(const NetworkTypePattern& type,
if (type.Equals(NetworkTypePattern::Tether()) ||
(limit != 0 && count >= limit)) {
- // If only searching for tether networks, there is no need to continue
+ // 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;
@@ -431,6 +484,7 @@ void NetworkStateHandler::AddTetherNetworkState(const std::string& guid,
DCHECK(!guid.empty());
DCHECK(battery_percentage >= 0 && battery_percentage <= 100);
DCHECK(signal_strength >= 0 && signal_strength <= 100);
+ DCHECK(tether_technology_state_ == TECHNOLOGY_ENABLED);
stevenjb 2017/04/20 16:49:41 This probably shouldn't be a DCHECK. I can imagine
Kyle Horimoto 2017/04/20 19:38:46 Done.
// If the network already exists, do nothing.
if (GetNetworkStateFromGuid(guid)) {
@@ -463,6 +517,8 @@ bool NetworkStateHandler::UpdateTetherNetworkProperties(
const std::string& carrier,
int battery_percentage,
int signal_strength) {
+ DCHECK(tether_technology_state_ == TECHNOLOGY_ENABLED);
stevenjb 2017/04/20 16:49:41 ditto
Kyle Horimoto 2017/04/20 19:38:46 Done.
+
NetworkState* tether_network_state = GetModifiableNetworkStateFromGuid(guid);
if (!tether_network_state)
return false;
@@ -494,6 +550,8 @@ void NetworkStateHandler::RemoveTetherNetworkState(const std::string& guid) {
bool NetworkStateHandler::AssociateTetherNetworkStateWithWifiNetwork(
const std::string& tether_network_guid,
const std::string& wifi_network_guid) {
+ DCHECK(tether_technology_state_ == TECHNOLOGY_ENABLED);
stevenjb 2017/04/20 16:49:41 ditto
Kyle Horimoto 2017/04/20 19:38:46 Done.
+
NetworkState* tether_network =
GetModifiableNetworkStateFromGuid(tether_network_guid);
if (!tether_network) {
@@ -525,14 +583,14 @@ bool NetworkStateHandler::AssociateTetherNetworkStateWithWifiNetwork(
void NetworkStateHandler::SetTetherNetworkStateDisconnected(
const std::string& guid) {
- // TODO(khorimoto): Remove the tether network as the default network, and
+ // TODO(khorimoto): Remove the Tether network as the default network, and
// send a connection status change.
SetTetherNetworkStateConnectionState(guid, shill::kStateDisconnect);
}
void NetworkStateHandler::SetTetherNetworkStateConnecting(
const std::string& guid) {
- // TODO(khorimoto): Set the tether network as the default network, and send
+ // TODO(khorimoto): Set the Tether network as the default network, and send
// a connection status change.
SetTetherNetworkStateConnectionState(guid, shill::kStateConfiguration);
}
@@ -693,6 +751,17 @@ void NetworkStateHandler::UpdateManagedList(ManagedState::ManagedType type,
ManagedStateList* managed_list = GetManagedList(type);
NET_LOG_DEBUG("UpdateManagedList: " + ManagedState::TypeToString(type),
base::StringPrintf("%" PRIuS, entries.GetSize()));
+
+ // Create a copy of |entries| since it may be edited below.
+ base::ListValue entries_copy(entries);
+ if (type == ManagedState::ManagedType::MANAGED_TYPE_DEVICE) {
+ // If a device list update is received from Shill, the list supplied will
+ // never contain an entry for Tether networks since Shill does not recognize
+ // Tether networks as a device type. Before processing the list, add an
+ // entry for the tether device type with the constant tether device path.
+ entries_copy.Append(base::MakeUnique<base::Value>(kTetherDevicePath));
+ }
stevenjb 2017/04/20 16:49:41 This seems unnecessary, and placing the code at 79
Kyle Horimoto 2017/04/20 19:38:46 Done.
+
// Create a map of existing entries. Assumes all entries in |managed_list|
// are unique.
std::map<std::string, std::unique_ptr<ManagedState>> managed_map;
@@ -705,7 +774,7 @@ void NetworkStateHandler::UpdateManagedList(ManagedState::ManagedType type,
managed_list->clear();
// Updates managed_list and request updates for new entries.
std::set<std::string> list_entries;
- for (auto& iter : entries) {
+ for (auto& iter : entries_copy) {
std::string path;
iter.GetAsString(&path);
if (path.empty() || path == shill::kFlimflamServicePath) {
@@ -718,7 +787,17 @@ void NetworkStateHandler::UpdateManagedList(ManagedState::ManagedType type,
NET_LOG_ERROR("Duplicate entry in list", path);
continue;
}
- managed_list->push_back(ManagedState::Create(type, path));
+ std::unique_ptr<ManagedState> state = ManagedState::Create(type, path);
+ if (type == ManagedState::ManagedType::MANAGED_TYPE_DEVICE &&
+ path == kTetherDevicePath) {
+ // If adding the tether DeviceState, set appropriate properties since
+ // it will never receive a properties update from Shill.
+ state->set_update_received();
+ state->set_update_requested(false);
+ state->set_name(kTetherDeviceName);
+ state->set_type(kTypeTether);
+ }
+ managed_list->push_back(std::move(state));
} else {
managed_list->push_back(std::move(found->second));
managed_map.erase(found);
@@ -971,7 +1050,7 @@ void NetworkStateHandler::ManagedStateListChanged(
}
}
-// TODO(khorimoto): Add sorting for the tether network list as well.
+// 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
@@ -1275,6 +1354,8 @@ std::vector<std::string> NetworkStateHandler::GetTechnologiesForType(
technologies.emplace_back(shill::kTypeBluetooth);
if (type.MatchesType(shill::kTypeVPN))
technologies.emplace_back(shill::kTypeVPN);
+ if (type.MatchesType(kTypeTether))
+ technologies.emplace_back(kTypeTether);
CHECK_GT(technologies.size(), 0ul);
return technologies;

Powered by Google App Engine
This is Rietveld 408576698