Skip to content

Commit

Permalink
[CrOS Tether] Set the Tether network to be the "default" network.
Browse files Browse the repository at this point in the history 
The default network is set once the device starts connecting, and a "default network changed" handler is invoked on observers when the default network becomes connected or disconnected.

BUG=672263

Review-Url: https://codereview.chromium.org/2937783002
Cr-Commit-Position: refs/heads/master@{#479769}
  • Loading branch information
khorimoto authored and Commit Bot committed Jun 15, 2017
1 parent a5f1895 commit ac4f519
Show file tree
Hide file tree
Showing 2 changed files with 263 additions and 21 deletions.
38 changes: 36 additions & 2 deletions chromeos/network/network_state_handler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -690,13 +690,24 @@ bool NetworkStateHandler::AssociateTetherNetworkStateWithWifiNetwork(

void NetworkStateHandler::SetTetherNetworkStateDisconnected(
const std::string& guid) {
// TODO(khorimoto): Remove the Tether network as the default network.
SetTetherNetworkStateConnectionState(guid, shill::kStateDisconnect);
}

void NetworkStateHandler::SetTetherNetworkStateConnecting(
const std::string& guid) {
// TODO(khorimoto): Set the Tether network as the default network.
// The default network should only be set if there currently is no default
// network. Otherwise, the default network should not change unless the
// connection completes successfully and the newly-connected network is
// prioritized higher than the existing default network. Note that, in
// general, a connected Ethernet network is still considered the default
// network even if a Tether or Wi-Fi network becomes connected.
if (default_network_path_.empty()) {
NET_LOG(EVENT) << "Connecting to Tether network when there is currently no "
<< "default network; setting as new default network. GUID: "
<< guid;
default_network_path_ = guid;
}

SetTetherNetworkStateConnectionState(guid, shill::kStateConfiguration);
}

Expand All @@ -707,6 +718,9 @@ void NetworkStateHandler::SetTetherNetworkStateConnected(
DCHECK(GetNetworkStateFromGuid(GetNetworkStateFromGuid(guid)->tether_guid())
->tether_guid() == guid);

// At this point, there should be a default network set.
DCHECK(!default_network_path_.empty());

SetTetherNetworkStateConnectionState(guid, shill::kStateOnline);
}

Expand Down Expand Up @@ -1259,6 +1273,17 @@ void NetworkStateHandler::DefaultNetworkServiceChanged(
if (new_service_path == default_network_path_)
return;

if (new_service_path.empty()) {
// If Shill reports that there is no longer a default network but there is
// still an active Tether connection corresponding to the default network,
// return early without changing |default_network_path_|. Observers will be
// notified of the default network change due to a subsequent call to
// SetTetherNetworkStateDisconnected().
const NetworkState* old_default_network = DefaultNetwork();
if (old_default_network && old_default_network->type() == kTypeTether)
return;
}

default_network_path_ = service_path;
NET_LOG_EVENT("DefaultNetworkServiceChanged:", default_network_path_);
const NetworkState* network = nullptr;
Expand All @@ -1271,6 +1296,15 @@ void NetworkStateHandler::DefaultNetworkServiceChanged(
<< default_network_path_;
return;
}
if (!network->tether_guid().empty()) {
DCHECK(network->type() == shill::kTypeWifi);

// If the new default network from Shill's point of view is a Wi-Fi
// network which corresponds to a hotspot for a Tether network, set the
// default network to be the associated Tether network instead.
default_network_path_ = network->tether_guid();
return;
}
}
if (network && !network->IsConnectedState()) {
if (network->IsConnectingState()) {
Expand Down
246 changes: 227 additions & 19 deletions chromeos/network/network_state_handler_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -969,60 +969,268 @@ TEST_F(NetworkStateHandlerTest, TetherNetworkStateAssociation_NoTetherNetwork) {
kTetherGuid1, kWifiGuid1));
}

TEST_F(NetworkStateHandlerTest, SetTetherNetworkStateConnectionState) {
TEST_F(NetworkStateHandlerTest,
SetTetherNetworkStateConnectionState_NoDefaultNetworkToStart) {
network_state_handler_->SetTetherTechnologyState(
NetworkStateHandler::TECHNOLOGY_ENABLED);

// Disconnect Ethernet and Wi-Fi so that there is no default network. For the
// purpose of this test, the default Wi-Fi network will serve as the Tether
// network's underlying Wi-Fi hotspot.
const std::string eth1 = kShillManagerClientStubDefaultService;
const std::string wifi1 = kShillManagerClientStubDefaultWifi;
service_test_->SetServiceProperty(eth1, shill::kStateProperty,
base::Value(shill::kStateIdle));
service_test_->SetServiceProperty(wifi1, shill::kStateProperty,
base::Value(shill::kStateIdle));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(std::string(), test_observer_->default_network());

// Simulate a host scan, and reset the change counts for the connection flow.
network_state_handler_->AddTetherNetworkState(
kTetherGuid1, kTetherName1, kTetherCarrier1, kTetherBatteryPercentage1,
kTetherSignalStrength1, kTetherHasConnectedToHost1);
test_observer_->reset_change_counts();
test_observer_->reset_updates();

// Add corresponding Wi-Fi network.
const std::string profile = "/profile/profile1";
const std::string wifi_path = "/service/wifi_with_guid";
AddService(wifi_path, kWifiGuid1, kWifiName1, shill::kTypeWifi,
shill::kStateOnline);
profile_test_->AddProfile(profile, "" /* userhash */);
EXPECT_TRUE(profile_test_->AddService(profile, wifi_path));
UpdateManagerProperties();
// Preconditions.
EXPECT_EQ(0, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(0, test_observer_->PropertyUpdatesForService(kTetherGuid1));
EXPECT_EQ(0u, test_observer_->default_network_change_count());
EXPECT_EQ("", test_observer_->default_network());
EXPECT_EQ("", test_observer_->default_network_connection_state());
EXPECT_EQ(nullptr, network_state_handler_->DefaultNetwork());

// Set the Tether network state to "connecting." This is expected to be called
// before the connection to the underlying hotspot Wi-Fi network begins.
const NetworkState* tether_network =
network_state_handler_->GetNetworkStateFromGuid(kTetherGuid1);
network_state_handler_->SetTetherNetworkStateConnecting(kTetherGuid1);
EXPECT_TRUE(tether_network->IsConnectingState());
EXPECT_EQ(1, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(1, test_observer_->PropertyUpdatesForService(kTetherGuid1));
EXPECT_EQ(0u, test_observer_->default_network_change_count());
EXPECT_EQ(tether_network, network_state_handler_->DefaultNetwork());

// Associate Tether and Wi-Fi networks.
network_state_handler_->AssociateTetherNetworkStateWithWifiNetwork(
kTetherGuid1, kWifiGuid1);
kTetherGuid1, "wifi1_guid");
EXPECT_EQ(1, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(2, test_observer_->PropertyUpdatesForService(kTetherGuid1));

// Connect to the underlying Wi-Fi network. The default network should not
// change yet.
service_test_->SetServiceProperty(wifi1, shill::kStateProperty,
base::Value(shill::kStateOnline));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(0u, test_observer_->default_network_change_count());

// Now, set the Tether network state to "connected." This should result in a
// default network change event.
network_state_handler_->SetTetherNetworkStateConnected(kTetherGuid1);
EXPECT_TRUE(tether_network->IsConnectedState());
EXPECT_EQ(2, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(3, test_observer_->PropertyUpdatesForService(kTetherGuid1));
EXPECT_EQ(1u, test_observer_->default_network_change_count());
EXPECT_EQ(kTetherGuid1, test_observer_->default_network());
EXPECT_EQ(shill::kStateOnline,
test_observer_->default_network_connection_state());
EXPECT_EQ(tether_network, network_state_handler_->DefaultNetwork());

// Disconnect from the underlying Wi-Fi network. The default network should
// not change yet.
service_test_->SetServiceProperty(wifi1, shill::kStateProperty,
base::Value(shill::kStateIdle));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1u, test_observer_->default_network_change_count());

// Now, set the Tether network state to "disconnected." This should result in
// a default network change event.
network_state_handler_->SetTetherNetworkStateDisconnected(kTetherGuid1);
EXPECT_FALSE(tether_network->IsConnectingState());
EXPECT_FALSE(tether_network->IsConnectedState());
EXPECT_EQ(3, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(4, test_observer_->PropertyUpdatesForService(kTetherGuid1));
EXPECT_EQ(2u, test_observer_->default_network_change_count());
EXPECT_EQ("", test_observer_->default_network());
EXPECT_EQ("", test_observer_->default_network_connection_state());
EXPECT_EQ(nullptr, network_state_handler_->DefaultNetwork());
}

TEST_F(NetworkStateHandlerTest,
SetTetherNetworkStateConnectionState_EthernetIsDefaultNetwork) {
network_state_handler_->SetTetherTechnologyState(
NetworkStateHandler::TECHNOLOGY_ENABLED);

// The ethernet corresponding to |eth1| will be left connected this entire
// test. It should be expected to remain the default network during the Tether
// connection.
const std::string eth1 = kShillManagerClientStubDefaultService;

// Disconnect the Wi-Fi network, which will serve as the underlying connection
// for the Tether network under test.
const std::string wifi1 = kShillManagerClientStubDefaultWifi;
service_test_->SetServiceProperty(wifi1, shill::kStateProperty,
base::Value(shill::kStateIdle));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(eth1, test_observer_->default_network());

// Simulate a host scan, and reset the change counts for the connection flow.
network_state_handler_->AddTetherNetworkState(
kTetherGuid1, kTetherName1, kTetherCarrier1, kTetherBatteryPercentage1,
kTetherSignalStrength1, kTetherHasConnectedToHost1);
test_observer_->reset_change_counts();
test_observer_->reset_updates();

// Preconditions.
EXPECT_EQ(0, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(1, test_observer_->PropertyUpdatesForService(kTetherGuid1));
EXPECT_EQ(0, test_observer_->PropertyUpdatesForService(kTetherGuid1));
EXPECT_EQ(0u, test_observer_->default_network_change_count());
EXPECT_EQ(eth1, test_observer_->default_network());
EXPECT_EQ(shill::kStateOnline,
test_observer_->default_network_connection_state());

// Set the Tether network state to "connecting." This is expected to be called
// before the connection to the underlying hotspot Wi-Fi network begins.
const NetworkState* tether_network =
network_state_handler_->GetNetworkStateFromGuid(kTetherGuid1);
network_state_handler_->SetTetherNetworkStateConnecting(kTetherGuid1);
EXPECT_TRUE(tether_network->IsConnectingState());
EXPECT_EQ(1, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(1, test_observer_->PropertyUpdatesForService(kTetherGuid1));

// Associate Tether and Wi-Fi networks.
network_state_handler_->AssociateTetherNetworkStateWithWifiNetwork(
kTetherGuid1, "wifi1_guid");
EXPECT_EQ(1, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(2, test_observer_->PropertyUpdatesForService(kTetherGuid1));

// Connect to the underlying Wi-Fi network.
service_test_->SetServiceProperty(wifi1, shill::kStateProperty,
base::Value(shill::kStateOnline));
base::RunLoop().RunUntilIdle();

// Now, set the Tether network state to "connected."
network_state_handler_->SetTetherNetworkStateConnected(kTetherGuid1);
EXPECT_TRUE(tether_network->IsConnectedState());
EXPECT_EQ(2, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(3, test_observer_->PropertyUpdatesForService(kTetherGuid1));

// Disconnect from the underlying Wi-Fi network.
service_test_->SetServiceProperty(wifi1, shill::kStateProperty,
base::Value(shill::kStateIdle));
base::RunLoop().RunUntilIdle();

// Now, set the Tether network state to "disconnected."
network_state_handler_->SetTetherNetworkStateDisconnected(kTetherGuid1);
EXPECT_FALSE(tether_network->IsConnectingState());
EXPECT_FALSE(tether_network->IsConnectedState());
EXPECT_EQ(3, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(4, test_observer_->PropertyUpdatesForService(kTetherGuid1));

// The Ethernet network should still be the default network, and no changes
// should have occurred during this test.
EXPECT_EQ(0u, test_observer_->default_network_change_count());
EXPECT_EQ(eth1, test_observer_->default_network());
EXPECT_EQ(shill::kStateOnline,
test_observer_->default_network_connection_state());
}

TEST_F(NetworkStateHandlerTest,
SetTetherNetworkStateConnectionState_NoDefaultThenTetherThenEthernet) {
network_state_handler_->SetTetherTechnologyState(
NetworkStateHandler::TECHNOLOGY_ENABLED);

// Disconnect Ethernet and Wi-Fi so that there is no default network. For the
// purpose of this test, the default Wi-Fi network will serve as the Tether
// network's underlying Wi-Fi hotspot.
const std::string eth1 = kShillManagerClientStubDefaultService;
const std::string wifi1 = kShillManagerClientStubDefaultWifi;
service_test_->SetServiceProperty(eth1, shill::kStateProperty,
base::Value(shill::kStateIdle));
service_test_->SetServiceProperty(wifi1, shill::kStateProperty,
base::Value(shill::kStateIdle));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(std::string(), test_observer_->default_network());

// Simulate a host scan, and reset the change counts for the connection flow.
network_state_handler_->AddTetherNetworkState(
kTetherGuid1, kTetherName1, kTetherCarrier1, kTetherBatteryPercentage1,
kTetherSignalStrength1, kTetherHasConnectedToHost1);
test_observer_->reset_change_counts();
test_observer_->reset_updates();

// Preconditions.
EXPECT_EQ(0, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(0, test_observer_->PropertyUpdatesForService(kTetherGuid1));
EXPECT_EQ(0u, test_observer_->default_network_change_count());
EXPECT_EQ("", test_observer_->default_network());
EXPECT_EQ("", test_observer_->default_network_connection_state());
EXPECT_EQ(nullptr, network_state_handler_->DefaultNetwork());

// Set the Tether network state to "connecting." This is expected to be called
// before the connection to the underlying hotspot Wi-Fi network begins.
const NetworkState* tether_network =
network_state_handler_->GetNetworkStateFromGuid(kTetherGuid1);
network_state_handler_->SetTetherNetworkStateConnecting(kTetherGuid1);
EXPECT_TRUE(tether_network->IsConnectingState());
EXPECT_EQ(1, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(1, test_observer_->PropertyUpdatesForService(kTetherGuid1));
EXPECT_EQ(0u, test_observer_->default_network_change_count());
EXPECT_EQ(tether_network, network_state_handler_->DefaultNetwork());

// Associate Tether and Wi-Fi networks.
network_state_handler_->AssociateTetherNetworkStateWithWifiNetwork(
kTetherGuid1, "wifi1_guid");
EXPECT_EQ(1, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(2, test_observer_->PropertyUpdatesForService(kTetherGuid1));

// Connect to the underlying Wi-Fi network. The default network should not
// change yet.
service_test_->SetServiceProperty(wifi1, shill::kStateProperty,
base::Value(shill::kStateOnline));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(0u, test_observer_->default_network_change_count());

// Now, set the Tether network state to "connected." This should result in a
// default network change event.
network_state_handler_->SetTetherNetworkStateConnected(kTetherGuid1);
EXPECT_TRUE(tether_network->IsConnectedState());

EXPECT_EQ(2, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(3, test_observer_->PropertyUpdatesForService(kTetherGuid1));
EXPECT_EQ(1u, test_observer_->default_network_change_count());
EXPECT_EQ(kTetherGuid1, test_observer_->default_network());
EXPECT_EQ(shill::kStateOnline,
test_observer_->default_network_connection_state());
EXPECT_EQ(tether_network, network_state_handler_->DefaultNetwork());

network_state_handler_->SetTetherNetworkStateConnecting(kTetherGuid1);
EXPECT_TRUE(tether_network->IsReconnecting());
// Now, connect the Ethernet network. This should be the new default network.
service_test_->SetServiceProperty(eth1, shill::kStateProperty,
base::Value(shill::kStateOnline));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(2u, test_observer_->default_network_change_count());
EXPECT_EQ(eth1, test_observer_->default_network());
EXPECT_EQ(shill::kStateOnline,
test_observer_->default_network_connection_state());

EXPECT_EQ(3, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(4, test_observer_->PropertyUpdatesForService(kTetherGuid1));
// Disconnect from the underlying Wi-Fi network. The default network should
// still be the Ethernet network.
service_test_->SetServiceProperty(wifi1, shill::kStateProperty,
base::Value(shill::kStateIdle));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(2u, test_observer_->default_network_change_count());

// Now, set the Tether network state to "disconnected." The default network
// should still be the Ethernet network.
network_state_handler_->SetTetherNetworkStateDisconnected(kTetherGuid1);
EXPECT_FALSE(tether_network->IsConnectingState());
EXPECT_FALSE(tether_network->IsConnectedState());

EXPECT_EQ(4, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(5, test_observer_->PropertyUpdatesForService(kTetherGuid1));
EXPECT_EQ(3, test_observer_->ConnectionStateChangesForService(kTetherGuid1));
EXPECT_EQ(4, test_observer_->PropertyUpdatesForService(kTetherGuid1));
EXPECT_EQ(2u, test_observer_->default_network_change_count());
EXPECT_EQ(eth1, test_observer_->default_network());
EXPECT_EQ(shill::kStateOnline,
test_observer_->default_network_connection_state());
}

TEST_F(NetworkStateHandlerTest, NetworkConnectionStateChanged) {
Expand Down

0 comments on commit ac4f519

Please sign in to comment.