Skip to content

Commit

Permalink
[CrOS Hotspot] Restore WiFi status when hotspot if off
Browse files Browse the repository at this point in the history 
This CL restores WiFi status to previous status when hotspot is turned
off.

Bug: b/295213726
Change-Id: Icced517c3aca533c0d5d18f8ba7cd000cd39fbc4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4766456
Reviewed-by: Nikhil Nayunigari <nikhilcn@google.com>
Commit-Queue: Jason Zhang <jiajunz@google.com>
Cr-Commit-Position: refs/heads/main@{#1185463}
  • Loading branch information
Jason Zhang authored and Chromium LUCI CQ committed Aug 19, 2023
1 parent 2b13273 commit 5d4c142
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 13 deletions.
51 changes: 41 additions & 10 deletions chromeos/ash/components/network/hotspot_controller.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ HotspotController::~HotspotController() {
if (technology_state_controller_) {
technology_state_controller_->set_hotspot_operation_delegate(nullptr);
}
if (hotspot_state_handler_ && hotspot_state_handler_->HasObserver(this)) {
hotspot_state_handler_->RemoveObserver(this);
}
}

void HotspotController::Init(
Expand All @@ -41,6 +44,7 @@ void HotspotController::Init(
hotspot_capabilities_provider_ = hotspot_capabilities_provider;
hotspot_feature_usage_metrics_ = hotspot_feature_usage_metrics;
hotspot_state_handler_ = hotspot_state_handler;
hotspot_state_handler_->AddObserver(this);
technology_state_controller_ = technology_state_controller;
technology_state_controller_->set_hotspot_operation_delegate(this);
}
Expand Down Expand Up @@ -161,7 +165,7 @@ void HotspotController::OnPrepareEnableHotspotCompleted(bool prepare_success,
}
NET_LOG(EVENT) << "Prepare enable hotspot completed, success: "
<< prepare_success << ", wifi turned off " << wifi_turned_off;
current_enable_request_->wifi_turned_off = wifi_turned_off;
wifi_turned_off_ = wifi_turned_off;
if (!prepare_success) {
CompleteEnableRequest(
hotspot_config::mojom::HotspotControlResult::kDisableWifiFailed);
Expand Down Expand Up @@ -225,12 +229,9 @@ void HotspotController::CompleteEnableRequest(
HotspotMetricsHelper::RecordSetTetheringEnabledResult(
/*enabled=*/true, result);

NET_LOG(EVENT)
<< "Complete SetTetheringEnabled request, enabled: true, result: "
<< result;
NET_LOG(EVENT) << "Complete enable tethering request, result: " << result;

if (current_enable_request_->wifi_turned_off &&
result != HotspotControlResult::kSuccess &&
if (wifi_turned_off_ && result != HotspotControlResult::kSuccess &&
!current_enable_request_->abort) {
// Turn Wifi back on if failed to enable hotspot.
technology_state_controller_->SetTechnologiesEnabled(
Expand All @@ -239,7 +240,7 @@ void HotspotController::CompleteEnableRequest(
}

if (result == HotspotControlResult::kSuccess) {
NotifyHotspotTurnedOn(current_enable_request_->wifi_turned_off);
NotifyHotspotTurnedOn(wifi_turned_off_);
}
std::move(current_enable_request_->callback).Run(result);
current_enable_request_.reset();
Expand All @@ -253,9 +254,9 @@ void HotspotController::CompleteDisableRequest(
HotspotMetricsHelper::RecordSetTetheringEnabledResult(
/*enabled=*/false, result);

NET_LOG(EVENT)
<< "Complete SetTetheringEnabled request, enabled: false, result: "
<< result;
NET_LOG(EVENT) << "Complete disable tethering request, result: " << result
<< ", disable reason: "
<< current_disable_request_->disable_reason.value();

if (result == HotspotControlResult::kSuccess) {
NotifyHotspotTurnedOff(current_disable_request_->disable_reason.value());
Expand Down Expand Up @@ -306,6 +307,36 @@ void HotspotController::OnPrepareEnableWifiCompleted(
std::move(callback).Run(/*prepare_success=*/false);
}

void HotspotController::OnHotspotStatusChanged() {
if (!wifi_turned_off_) {
return;
}

hotspot_config::mojom::HotspotState hotspot_state =
hotspot_state_handler_->GetHotspotState();
if (hotspot_state != hotspot_config::mojom::HotspotState::kDisabled) {
return;
}

absl::optional<hotspot_config::mojom::DisableReason> disable_reason =
hotspot_state_handler_->GetDisableReason();
if (disable_reason &&
*disable_reason == hotspot_config::mojom::DisableReason::kRestart) {
// No need to turn WiFi back on since the hotspot will restart immediately.
return;
}

if (disable_reason) {
NET_LOG(EVENT)
<< "Turning Wifi back on because hotspot is turned off due to "
<< *disable_reason;
}
technology_state_controller_->SetTechnologiesEnabled(
NetworkTypePattern::WiFi(), /*enabled=*/true,
network_handler::ErrorCallback());
wifi_turned_off_ = false;
}

void HotspotController::OnDisableHotspotCompleteForRestart(
hotspot_config::mojom::HotspotControlResult disable_result) {
if (disable_result ==
Expand Down
13 changes: 10 additions & 3 deletions chromeos/ash/components/network/hotspot_controller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ class HotspotFeatureUsageMetrics;
// Enable or disable requests executed as they come in but are ignored if there
// is already a pending request.
class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotController
: public TechnologyStateController::HotspotOperationDelegate {
: public TechnologyStateController::HotspotOperationDelegate,
public HotspotStateHandler::Observer {
public:
class Observer : public base::CheckedObserver {
public:
Expand All @@ -44,7 +45,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotController
HotspotController();
HotspotController(const HotspotController&) = delete;
HotspotController& operator=(const HotspotController&) = delete;
virtual ~HotspotController();
~HotspotController() override;

void Init(HotspotCapabilitiesProvider* hotspot_capabilities_provider,
HotspotFeatureUsageMetrics* hotspot_feature_usage_metrics,
Expand Down Expand Up @@ -87,7 +88,6 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotController
~HotspotControlRequest();

bool enabled;
bool wifi_turned_off = false;
bool abort = false;
// Set for disable requests and will be nullopt for enable requests.
absl::optional<hotspot_config::mojom::DisableReason> disable_reason;
Expand All @@ -101,6 +101,9 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotController
void PrepareEnableWifi(
base::OnceCallback<void(bool prepare_success)> callback) override;

// HotspotStateHandler::Observer:
void OnHotspotStatusChanged() override;

void CheckTetheringReadiness();
void OnCheckTetheringReadiness(
HotspotCapabilitiesProvider::CheckTetheringReadinessResult result);
Expand Down Expand Up @@ -132,6 +135,10 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotController
std::unique_ptr<HotspotControlRequest> current_disable_request_;

bool allow_hotspot_ = true;
// Store whether the WiFi was turned off due to the start of hotspot. Need to
// restore the WiFi status once hotspot is turned off.
bool wifi_turned_off_ = false;

raw_ptr<HotspotCapabilitiesProvider, ExperimentalAsh>
hotspot_capabilities_provider_ = nullptr;
raw_ptr<HotspotFeatureUsageMetrics, ExperimentalAsh>
Expand Down
37 changes: 37 additions & 0 deletions chromeos/ash/components/network/hotspot_controller_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ class HotspotControllerTest : public ::testing::Test {
hotspot_config::mojom::HotspotControlResult return_result;
hotspot_controller_->EnableHotspot(base::BindLambdaForTesting(
[&](hotspot_config::mojom::HotspotControlResult result) {
if (result == hotspot_config::mojom::HotspotControlResult::kSuccess) {
SetHotspotStateInShill(shill::kTetheringStateActive);
}
return_result = result;
run_loop.Quit();
}));
Expand All @@ -172,6 +175,10 @@ class HotspotControllerTest : public ::testing::Test {
hotspot_controller_->DisableHotspot(
base::BindLambdaForTesting(
[&](hotspot_config::mojom::HotspotControlResult result) {
if (result ==
hotspot_config::mojom::HotspotControlResult::kSuccess) {
SetHotspotStateInShill(shill::kTetheringStateIdle);
}
return_result = result;
run_loop.Quit();
}),
Expand Down Expand Up @@ -480,4 +487,34 @@ TEST_F(HotspotControllerTest, RestartHotspotIfActive) {
observer_.last_disable_reason());
}

TEST_F(HotspotControllerTest, RestoreWiFiStatus) {
SetValidTetheringCapabilities();
AddActiveCellularServivce();
// Verify Wifi is on before turning on hotspot.
EXPECT_EQ(
NetworkStateHandler::TECHNOLOGY_ENABLED,
network_state_test_helper_.network_state_handler()->GetTechnologyState(
NetworkTypePattern::WiFi()));

network_state_test_helper_.manager_test()->SetSimulateTetheringEnableResult(
FakeShillSimulatedResult::kSuccess, shill::kTetheringEnableResultSuccess);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(hotspot_config::mojom::HotspotControlResult::kSuccess,
EnableHotspot(/*abort=*/false));

// Verifies that Wifi will be turned off.
EXPECT_EQ(
NetworkStateHandler::TECHNOLOGY_AVAILABLE,
network_state_test_helper_.network_state_handler()->GetTechnologyState(
NetworkTypePattern::WiFi()));

SetHotspotStateInShill(shill::kTetheringStateIdle);
base::RunLoop().RunUntilIdle();
// Verifies that Wifi will be turned back on.
EXPECT_EQ(
NetworkStateHandler::TECHNOLOGY_ENABLED,
network_state_test_helper_.network_state_handler()->GetTechnologyState(
NetworkTypePattern::WiFi()));
}

} // namespace ash

0 comments on commit 5d4c142

Please sign in to comment.