[Instant Tethering] Request WiFi scan on tether
In order for our new tether network to be properly
connectable, we need to have a WiFi scan start AFTER
our tether network has been configured. Without this
scan, the network stack does not recognize the tether
network as connectable.
This was manually verified by starting a tether session,
manually triggering a WiFi scan via opening the Quick
Settings network menu, and seeing a successful tether
connection AFTER the scan completes. Without triggering
the scan, the network will not connect successfully.
BUG=b/234057522
Change-Id: Ie6d59ca8f7523f2e5a3fc56dec4b013d16676c43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4437652
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Jon Mann <jonmann@chromium.org>
Commit-Queue: Jack Shira <jackshira@google.com>
Cr-Commit-Position: refs/heads/main@{#1133897}
diff --git a/chromeos/ash/components/network/network_state_handler.cc b/chromeos/ash/components/network/network_state_handler.cc
index 6a8123c..5e0a049 100644
--- a/chromeos/ash/components/network/network_state_handler.cc
+++ b/chromeos/ash/components/network/network_state_handler.cc
@@ -1019,10 +1019,13 @@
const std::string& guid) {
// Being connected implies that AssociateTetherNetworkStateWithWifiNetwork()
// was already called, so ensure that the association is still intact.
+ // TODO(b/278966899): Promote this to a CHECK.
DCHECK(GetNetworkStateFromGuid(GetNetworkStateFromGuid(guid)->tether_guid())
->tether_guid() == guid);
// At this point, there should be a default network set.
+ // TODO(b/279047073): We can hit this due to a race between
+ // `SetTetherNetworkStateConnected` and `DefaultNetworkServiceChange`.
DCHECK(!default_network_path_.empty());
SetTetherNetworkStateConnectionState(guid, shill::kStateOnline);
diff --git a/chromeos/ash/components/tether/wifi_hotspot_connector.cc b/chromeos/ash/components/tether/wifi_hotspot_connector.cc
index 9464bcc..44e020d4 100644
--- a/chromeos/ash/components/tether/wifi_hotspot_connector.cc
+++ b/chromeos/ash/components/tether/wifi_hotspot_connector.cc
@@ -105,6 +105,10 @@
}
}
+void WifiHotspotConnector::RequestWifiScan() {
+ network_state_handler_->RequestScan(NetworkTypePattern::WiFi());
+}
+
void WifiHotspotConnector::OnEnableWifiError(const std::string& error_name) {
is_waiting_for_wifi_to_enable_ = false;
PA_LOG(ERROR) << "Failed to enable Wi-Fi: " << error_name;
@@ -122,6 +126,11 @@
return;
}
+ if (!has_requested_wifi_scan_) {
+ has_requested_wifi_scan_ = true;
+ RequestWifiScan();
+ }
+
if (network->IsConnectedState()) {
// The network has connected, so complete the connection attempt. Because
// this is a NetworkStateHandlerObserver callback, complete the attempt in
@@ -135,6 +144,12 @@
}
if (network->connectable() && !has_initiated_connection_to_current_network_) {
+ // Set |has_initiated_connection_to_current_network_| to true to ensure that
+ // this code path is only run once per connection attempt. Without this
+ // field, the association and connection code below would be re-run multiple
+ // times for one network.
+ has_initiated_connection_to_current_network_ = true;
+
// The network is connectable, so initiate a connection attempt. Because
// this is a NetworkStateHandlerObserver callback, complete the attempt in
// a new task to ensure that NetworkStateHandler is not modified while it is
@@ -181,12 +196,6 @@
return;
}
- // Set |has_initiated_connection_to_current_network_| to true to ensure that
- // this code path is only run once per connection attempt. Without this
- // field, the association and connection code below would be re-run multiple
- // times for one network.
- has_initiated_connection_to_current_network_ = true;
-
// If the network is now connectable, associate it with a Tether network
// ASAP so that the correct icon will be displayed in the tray while the
// network is connecting.
@@ -234,6 +243,7 @@
wifi_network_guid_.clear();
has_initiated_connection_to_current_network_ = false;
is_waiting_for_wifi_to_enable_ = false;
+ has_requested_wifi_scan_ = false;
timer_->Stop();
diff --git a/chromeos/ash/components/tether/wifi_hotspot_connector.h b/chromeos/ash/components/tether/wifi_hotspot_connector.h
index 52bde66..26f9bf1 100644
--- a/chromeos/ash/components/tether/wifi_hotspot_connector.h
+++ b/chromeos/ash/components/tether/wifi_hotspot_connector.h
@@ -71,6 +71,7 @@
void InitiateConnectionToCurrentNetwork();
void CompleteActiveConnectionAttempt(bool success);
void CreateWifiConfiguration();
+ void RequestWifiScan();
base::Value::Dict CreateWifiPropertyDictionary(const std::string& ssid,
const std::string& password);
void OnConnectionTimeout();
@@ -93,6 +94,7 @@
std::string tether_network_guid_;
std::string wifi_network_guid_;
WifiConnectionCallback callback_;
+ bool has_requested_wifi_scan_ = false;
bool is_waiting_for_wifi_to_enable_ = false;
bool has_initiated_connection_to_current_network_ = false;
base::Time connection_attempt_start_time_;
diff --git a/chromeos/ash/components/tether/wifi_hotspot_connector_unittest.cc b/chromeos/ash/components/tether/wifi_hotspot_connector_unittest.cc
index af8a2df9..70322a8 100644
--- a/chromeos/ash/components/tether/wifi_hotspot_connector_unittest.cc
+++ b/chromeos/ash/components/tether/wifi_hotspot_connector_unittest.cc
@@ -20,7 +20,9 @@
#include "chromeos/ash/components/network/network_connect.h"
#include "chromeos/ash/components/network/network_state.h"
#include "chromeos/ash/components/network/network_state_handler.h"
+#include "chromeos/ash/components/network/network_state_handler_observer.h"
#include "chromeos/ash/components/network/network_state_test_helper.h"
+#include "chromeos/ash/components/network/network_type_pattern.h"
#include "chromeos/ash/components/network/shill_property_util.h"
#include "chromeos/ash/components/network/technology_state_controller.h"
#include "testing/gmock/include/gmock/gmock.h"
@@ -249,6 +251,22 @@
bool is_running_in_test_task_runner_ = false;
};
+ class TestNetworkStateHandlerObserver : public NetworkStateHandlerObserver {
+ public:
+ bool DidTriggerScan() { return did_trigger_scan_; }
+ void Reset() { did_trigger_scan_ = false; }
+
+ // NetworkStateHandlerObserver:
+ void ScanRequested(const NetworkTypePattern& type) override {
+ if (type.MatchesPattern(NetworkTypePattern::WiFi())) {
+ did_trigger_scan_ = true;
+ }
+ }
+
+ private:
+ bool did_trigger_scan_ = false;
+ };
+
WifiHotspotConnectorTest() = default;
WifiHotspotConnectorTest(const WifiHotspotConnectorTest&) = delete;
@@ -289,6 +307,7 @@
test_task_runner_ = base::MakeRefCounted<base::TestSimpleTaskRunner>();
wifi_hotspot_connector_->SetTestDoubles(base::WrapUnique(mock_timer_),
&test_clock_, test_task_runner_);
+ helper_.network_state_handler()->AddObserver(&scan_observer_);
}
void SetUpShillState() {
@@ -298,7 +317,11 @@
CreateConfigurationJsonString(std::string(kOtherWifiServiceGuid)));
}
- void TearDown() override { wifi_hotspot_connector_.reset(); }
+ void TearDown() override {
+ wifi_hotspot_connector_.reset();
+ helper_.network_state_handler()->RemoveObserver(&scan_observer_);
+ scan_observer_.Reset();
+ }
void VerifyTimerSet() {
EXPECT_TRUE(mock_timer_->IsRunning());
@@ -372,6 +395,10 @@
connection_callback_responses_.push_back(wifi_guid);
}
+ void VerifyWifiScanRequested() {
+ EXPECT_TRUE(scan_observer_.DidTriggerScan());
+ }
+
NetworkStateHandler* network_state_handler() {
return helper_.network_state_handler();
}
@@ -385,6 +412,7 @@
std::string other_wifi_service_path_;
std::vector<std::string> connection_callback_responses_;
+ TestNetworkStateHandlerObserver scan_observer_;
base::MockOneShotTimer* mock_timer_;
base::SimpleTestClock test_clock_;
@@ -407,6 +435,9 @@
VerifyPskConfiguration(test_network_connect_->GetLastConfiguration(),
kSsid, kPassword, &guid_unused));
+ // A Wi-Fi scan occurs after the network is configured.
+ VerifyWifiScanRequested();
+
// Network does not become connectable.
EXPECT_TRUE(test_network_connect_->network_id_to_connect().empty());
@@ -431,6 +462,9 @@
VerifyPskConfiguration(test_network_connect_->GetLastConfiguration(),
kSsid, kPassword, &wifi_guid));
+ // A Wi-Fi scan occurs after the network is configured.
+ VerifyWifiScanRequested();
+
// Another network becomes connectable. This should not cause the connection
// to start.
NotifyConnectable(other_wifi_service_path_);
@@ -462,6 +496,9 @@
VerifyPskConfiguration(test_network_connect_->GetLastConfiguration(),
kSsid, kPassword, &wifi_guid));
+ // A Wi-Fi scan occurs after the network is configured.
+ VerifyWifiScanRequested();
+
// Network becomes connectable.
NotifyConnectable(test_network_connect_->last_service_path_created());
VerifyTetherAndWifiNetworkAssociation(
@@ -492,6 +529,9 @@
VerifyPskConfiguration(test_network_connect_->GetLastConfiguration(),
kSsid, kPassword, &wifi_guid));
+ // A Wi-Fi scan occurs after the network is configured.
+ VerifyWifiScanRequested();
+
// Network becomes connectable.
NotifyConnectable(test_network_connect_->last_service_path_created());
VerifyTetherAndWifiNetworkAssociation(
@@ -519,6 +559,9 @@
VerifyPskConfiguration(test_network_connect_->GetLastConfiguration(),
kSsid, kPassword, &wifi_guid));
+ // A Wi-Fi scan occurs after the network is configured.
+ VerifyWifiScanRequested();
+
// Network becomes connectable.
NotifyConnectable(test_network_connect_->last_service_path_created());
VerifyTetherAndWifiNetworkAssociation(
@@ -548,6 +591,9 @@
EXPECT_TRUE(VerifyOpenConfiguration(
test_network_connect_->GetLastConfiguration(), kSsid, &wifi_guid));
+ // A Wi-Fi scan occurs after the network is configured.
+ VerifyWifiScanRequested();
+
// Network becomes connectable.
NotifyConnectable(test_network_connect_->last_service_path_created());
VerifyTetherAndWifiNetworkAssociation(
@@ -598,6 +644,10 @@
EXPECT_TRUE(
VerifyPskConfiguration(test_network_connect_->GetLastConfiguration(),
"ssid2", "password2", &wifi_guid_2));
+
+ // A Wi-Fi scan occurs after the network is configured.
+ VerifyWifiScanRequested();
+
std::string service_path_2 =
test_network_connect_->last_service_path_created();
EXPECT_FALSE(service_path_2.empty());
@@ -656,6 +706,9 @@
test_network_connect_->last_service_path_created();
EXPECT_FALSE(service_path_1.empty());
+ // A Wi-Fi scan occurs after the network is configured.
+ VerifyWifiScanRequested();
+
// First network becomes connectable.
NotifyConnectable(service_path_1);
VerifyTetherAndWifiNetworkAssociation(
@@ -741,6 +794,9 @@
VerifyPskConfiguration(test_network_connect_->GetLastConfiguration(),
kSsid, kPassword, &wifi_guid));
+ // A Wi-Fi scan occurs after the network is configured.
+ VerifyWifiScanRequested();
+
test_clock_.Advance(kConnectionToHotspotTime);
// Network becomes connectable.
@@ -799,6 +855,9 @@
VerifyPskConfiguration(test_network_connect_->GetLastConfiguration(),
kSsid, kPassword, &wifi_guid));
+ // A Wi-Fi scan occurs after the network is configured.
+ VerifyWifiScanRequested();
+
// Network becomes connectable.
NotifyConnectable(test_network_connect_->last_service_path_created());
VerifyTetherAndWifiNetworkAssociation(
@@ -849,6 +908,9 @@
// timed out before Wi-Fi was successfully enabled.
EXPECT_TRUE(test_network_connect_->GetLastConfiguration().empty());
+ // No Wi-Fi scan should have occurred either.
+ EXPECT_FALSE(scan_observer_.DidTriggerScan());
+
VerifyConnectionToHotspotDurationRecorded(false /* expected */);
EXPECT_EQ(0u, test_network_connect_->num_disconnection_attempts());
}
