Commit 23a2216b authored by Kyle Horimoto's avatar Kyle Horimoto Committed by Commit Bot

[CrOS Tether] Notify default network changes for Ethernet -> Tether.

Previously, NetworkStateHandler contained the following bug:
  (1) Connect Ethernet network (the default network).
  (2) Connect Tether network (Ethernet is still default).
  (3) Disconnect Ethernet network (Tether should be the default).
  (4) NSH notifies that the new default is null, which is incorrect.

This issue was caused by an early return in
NetworkStateHandler::DefaultNetworkServiceChanged() which prevented the
default network change event from being propagated.

This CL fixes an issue which prevents ARC++ apps from utilizing a Tether
connection, since ARC++ relies on default network change events to
forward network connections to Android apps.

Bug: 766933, 672263
Change-Id: Ib4824bfef1d3e7faa86b631a7cc87870c818ab42
Reviewed-on: https://chromium-review.googlesource.com/809535
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522299}
parent 4920476d
...@@ -1410,8 +1410,8 @@ void NetworkStateHandler::DefaultNetworkServiceChanged( ...@@ -1410,8 +1410,8 @@ void NetworkStateHandler::DefaultNetworkServiceChanged(
// If the new default network from Shill's point of view is a Wi-Fi // 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 // network which corresponds to a hotspot for a Tether network, set the
// default network to be the associated Tether network instead. // default network to be the associated Tether network instead.
default_network_path_ = network->tether_guid(); network = GetNetworkStateFromGuid(network->tether_guid());
return; default_network_path_ = network->path();
} }
} }
if (network && !network->IsConnectedState()) { if (network && !network->IsConnectedState()) {
......
...@@ -1332,6 +1332,82 @@ TEST_F(NetworkStateHandlerTest, ...@@ -1332,6 +1332,82 @@ TEST_F(NetworkStateHandlerTest,
test_observer_->default_network_connection_state()); test_observer_->default_network_connection_state());
} }
TEST_F(NetworkStateHandlerTest,
EthernetIsDefaultNetwork_ThenTetherConnects_ThenEthernetDisconnects) {
network_state_handler_->SetTetherTechnologyState(
NetworkStateHandler::TECHNOLOGY_ENABLED);
// The ethernet corresponding to |eth1| starts out connected, then ends up
// becoming disconnected.
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(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));
// No default network changes should have occurred, since the Ethernet
// network should still be considered the default network.
EXPECT_EQ(0u, test_observer_->default_network_change_count());
// Disconnect from the Ethernet network.
service_test_->SetServiceProperty(eth1, shill::kStateProperty,
base::Value(shill::kStateIdle));
base::RunLoop().RunUntilIdle();
// The Tether network should now be the default network. However, there should
// have been two updates to the default network: one which changed it from
// |eth1| to null, then one from null to |kTetherGuid1"|.
EXPECT_EQ(2u, test_observer_->default_network_change_count());
EXPECT_EQ(kTetherGuid1, test_observer_->default_network());
EXPECT_EQ(shill::kStateOnline,
test_observer_->default_network_connection_state());
}
TEST_F(NetworkStateHandlerTest, TEST_F(NetworkStateHandlerTest,
SetTetherNetworkStateConnectionState_NoDefaultThenTetherThenEthernet) { SetTetherNetworkStateConnectionState_NoDefaultThenTetherThenEthernet) {
network_state_handler_->SetTetherTechnologyState( network_state_handler_->SetTetherTechnologyState(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment