Skip to content

Commit

Permalink
[CrOS Hotspot] Disable hotspot before enable WiFi
Browse files Browse the repository at this point in the history 
This CL adds the functionality to disable active hotspot before enabling
Wifi.

Bug: b/239477916
Change-Id: I4b7198f894efbbd759df92b704f778ca30fa8e23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4236513
Commit-Queue: Jason Zhang <jiajunz@google.com>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1105699}
  • Loading branch information
Jason Zhang authored and Chromium LUCI CQ committed Feb 15, 2023
1 parent 204e33b commit 18a4ecd
Show file tree
Hide file tree
Showing 7 changed files with 223 additions and 7 deletions.
35 changes: 33 additions & 2 deletions chromeos/ash/components/network/hotspot_controller.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,20 @@ HotspotController::HotspotControlRequest::~HotspotControlRequest() = default;

HotspotController::HotspotController() = default;

HotspotController::~HotspotController() = default;
HotspotController::~HotspotController() {
if (technology_state_controller_) {
technology_state_controller_->set_hotspot_operation_delegate(nullptr);
}
}

void HotspotController::Init(
HotspotCapabilitiesProvider* hotspot_capabilities_provider,
HotspotStateHandler* hotspot_state_handler) {
HotspotStateHandler* hotspot_state_handler,
TechnologyStateController* technology_state_controller) {
hotspot_capabilities_provider_ = hotspot_capabilities_provider;
hotspot_state_handler_ = hotspot_state_handler;
technology_state_controller_ = technology_state_controller;
technology_state_controller_->set_hotspot_operation_delegate(this);
}

void HotspotController::EnableHotspot(HotspotControlCallback callback) {
Expand Down Expand Up @@ -137,4 +144,28 @@ void HotspotController::SetPolicyAllowHotspot(bool allow_hotspot) {
}
}

void HotspotController::PrepareEnableWifi(
base::OnceCallback<void(bool prepare_success)> callback) {
if (hotspot_state_handler_->GetHotspotState() ==
hotspot_config::mojom::HotspotState::kEnabled ||
hotspot_state_handler_->GetHotspotState() ==
hotspot_config::mojom::HotspotState::kEnabling) {
DisableHotspot(
base::BindOnce(&HotspotController::OnPrepareEnableWifiCompleted,
weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
return;
}
std::move(callback).Run(/*prepare_success=*/true);
}

void HotspotController::OnPrepareEnableWifiCompleted(
base::OnceCallback<void(bool prepare_success)> callback,
hotspot_config::mojom::HotspotControlResult control_result) {
if (control_result == hotspot_config::mojom::HotspotControlResult::kSuccess) {
std::move(callback).Run(/*prepare_success=*/true);
return;
}
std::move(callback).Run(/*prepare_success=*/false);
}

} // namespace ash
19 changes: 16 additions & 3 deletions chromeos/ash/components/network/hotspot_controller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/memory/weak_ptr.h"
#include "chromeos/ash/components/network/hotspot_capabilities_provider.h"
#include "chromeos/ash/components/network/hotspot_state_handler.h"
#include "chromeos/ash/components/network/technology_state_controller.h"
#include "chromeos/ash/services/hotspot_config/public/mojom/cros_hotspot_config.mojom-forward.h"

namespace ash {
Expand All @@ -25,15 +26,17 @@ namespace ash {
//
// Enable or disable requests are queued and executes one request at a time in
// order.
class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotController {
class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotController
: public TechnologyStateController::HotspotOperationDelegate {
public:
HotspotController();
HotspotController(const HotspotController&) = delete;
HotspotController& operator=(const HotspotController&) = delete;
~HotspotController();
virtual ~HotspotController();

void Init(HotspotCapabilitiesProvider* hotspot_capabilities_provider,
HotspotStateHandler* hotspot_state_handler);
HotspotStateHandler* hotspot_state_handler,
TechnologyStateController* technolog_state_controller);

// Return callback for the EnableHotspot or DisableHotspot method.
using HotspotControlCallback = base::OnceCallback<void(
Expand All @@ -49,6 +52,8 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotController {
void SetPolicyAllowHotspot(bool allow_hotspot);

private:
friend class HotspotControllerTest;

// Represents hotspot enable or disable control request parameters. Requests
// are queued and processed one at a time.
struct HotspotControlRequest {
Expand All @@ -61,6 +66,10 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotController {
HotspotControlCallback callback;
};

// TechnologyStateController::HotspotOperationDelegate:
void PrepareEnableWifi(
base::OnceCallback<void(bool prepare_success)> callback) override;

void ProcessRequestQueue();
void CheckTetheringReadiness();
void OnCheckTetheringReadiness(
Expand All @@ -69,6 +78,9 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotController {
void OnSetTetheringEnabledSuccess(const std::string& result);
void OnSetTetheringEnabledFailure(const std::string& error_name,
const std::string& error_message);
void OnPrepareEnableWifiCompleted(
base::OnceCallback<void(bool success)> callback,
hotspot_config::mojom::HotspotControlResult control_result);
void CompleteCurrentRequest(
hotspot_config::mojom::HotspotControlResult result);

Expand All @@ -77,6 +89,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotController {
bool allow_hotspot_ = true;
HotspotCapabilitiesProvider* hotspot_capabilities_provider_ = nullptr;
HotspotStateHandler* hotspot_state_handler_ = nullptr;
TechnologyStateController* technology_state_controller_ = nullptr;

base::WeakPtrFactory<HotspotController> weak_ptr_factory_{this};
};
Expand Down
37 changes: 36 additions & 1 deletion chromeos/ash/components/network/hotspot_controller_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,16 @@ class HotspotControllerTest : public ::testing::Test {
std::make_unique<HotspotCapabilitiesProvider>();
hotspot_capabilities_provider_->Init(
network_state_test_helper_.network_state_handler());
technology_state_controller_ =
std::make_unique<TechnologyStateController>();
technology_state_controller_->Init(
network_state_test_helper_.network_state_handler());
hotspot_state_handler_ = std::make_unique<HotspotStateHandler>();
hotspot_state_handler_->Init();
hotspot_controller_ = std::make_unique<HotspotController>();
hotspot_controller_->Init(hotspot_capabilities_provider_.get(),
hotspot_state_handler_.get());
hotspot_state_handler_.get(),
technology_state_controller_.get());
SetReadinessCheckResultReady();
}

Expand All @@ -50,6 +55,7 @@ class HotspotControllerTest : public ::testing::Test {
hotspot_controller_.reset();
hotspot_capabilities_provider_.reset();
hotspot_state_handler_.reset();
technology_state_controller_.reset();
}

void SetValidTetheringCapabilities() {
Expand Down Expand Up @@ -115,6 +121,18 @@ class HotspotControllerTest : public ::testing::Test {
return return_result;
}

bool PrepareEnableWifi() {
base::RunLoop run_loop;
bool prepare_success;
hotspot_controller_->PrepareEnableWifi(
base::BindLambdaForTesting([&](bool result) {
prepare_success = result;
run_loop.QuitClosure();
}));
run_loop.RunUntilIdle();
return prepare_success;
}

void EnableAndDisableHotspot(
hotspot_config::mojom::HotspotControlResult& enable_result,
hotspot_config::mojom::HotspotControlResult& disable_result) {
Expand All @@ -138,6 +156,7 @@ class HotspotControllerTest : public ::testing::Test {
std::unique_ptr<HotspotController> hotspot_controller_;
std::unique_ptr<HotspotCapabilitiesProvider> hotspot_capabilities_provider_;
std::unique_ptr<HotspotStateHandler> hotspot_state_handler_;
std::unique_ptr<TechnologyStateController> technology_state_controller_;
NetworkStateTestHelper network_state_test_helper_{
/*use_default_devices_and_services=*/false};
};
Expand Down Expand Up @@ -218,4 +237,20 @@ TEST_F(HotspotControllerTest, QueuedRequests) {
disable_result);
}

TEST_F(HotspotControllerTest, PrepareEnableWifi) {
network_state_test_helper_.manager_test()->SetSimulateTetheringEnableResult(
FakeShillSimulatedResult::kSuccess, shill::kTetheringEnableResultSuccess);
base::Value::Dict status_dict;
status_dict.Set(shill::kTetheringStatusStateProperty,
shill::kTetheringStateActive);
network_state_test_helper_.manager_test()->SetManagerProperty(
shill::kTetheringStatusProperty, base::Value(status_dict.Clone()));
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(PrepareEnableWifi());

network_state_test_helper_.manager_test()->SetSimulateTetheringEnableResult(
FakeShillSimulatedResult::kSuccess, kShillNetworkingFailure);
EXPECT_FALSE(PrepareEnableWifi());
}

} // namespace ash
3 changes: 2 additions & 1 deletion chromeos/ash/components/network/network_handler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@ void NetworkHandler::Init() {
hotspot_capabilities_provider_->Init(network_state_handler_.get());
hotspot_state_handler_->Init();
hotspot_controller_->Init(hotspot_capabilities_provider_.get(),
hotspot_state_handler_.get());
hotspot_state_handler_.get(),
technology_state_controller_.get());
}
managed_cellular_pref_handler_->Init(network_state_handler_.get());
esim_policy_login_metrics_logger_->Init(
Expand Down
37 changes: 37 additions & 0 deletions chromeos/ash/components/network/technology_state_controller.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,16 @@

#include "chromeos/ash/components/network/technology_state_controller.h"

#include "ash/constants/ash_features.h"
#include "chromeos/ash/components/network/network_event_log.h"
#include "chromeos/ash/components/network/network_state_handler.h"

namespace ash {

// static
const char TechnologyStateController::kErrorDisableHotspot[] =
"disable-hotspot-failed";

TechnologyStateController::TechnologyStateController() = default;

TechnologyStateController::~TechnologyStateController() = default;
Expand All @@ -21,8 +27,39 @@ void TechnologyStateController::SetTechnologiesEnabled(
const NetworkTypePattern& type,
bool enabled,
network_handler::ErrorCallback error_callback) {
if (!hotspot_operation_delegate_ && ash::features::IsHotspotEnabled()) {
NET_LOG(ERROR) << "hotspot operation delegate is null while hotspot flag is"
<< " on.";
network_handler::RunErrorCallback(std::move(error_callback),
kErrorDisableHotspot);
return;
}

if (ash::features::IsHotspotEnabled() && enabled &&
type.MatchesPattern(NetworkTypePattern::WiFi())) {
hotspot_operation_delegate_->PrepareEnableWifi(base::BindOnce(
&TechnologyStateController::OnPrepareEnableWifiCompleted,
weak_ptr_factory_.GetWeakPtr(), type, std::move(error_callback)));
return;
}

network_state_handler_->SetTechnologiesEnabled(type, enabled,
std::move(error_callback));
}

void TechnologyStateController::OnPrepareEnableWifiCompleted(
const NetworkTypePattern& type,
network_handler::ErrorCallback error_callback,
bool success) {
DCHECK(ash::features::IsHotspotEnabled());

if (success) {
network_state_handler_->SetTechnologiesEnabled(type, /*enabled=*/true,
std::move(error_callback));
return;
}
network_handler::RunErrorCallback(std::move(error_callback),
kErrorDisableHotspot);
}

} // namespace ash
26 changes: 26 additions & 0 deletions chromeos/ash/components/network/technology_state_controller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define CHROMEOS_ASH_COMPONENTS_NETWORK_TECHNOLOGY_STATE_CONTROLLER_H_

#include "base/component_export.h"
#include "base/memory/weak_ptr.h"

#include "chromeos/ash/components/network/network_handler.h"
#include "chromeos/ash/components/network/network_handler_callbacks.h"
Expand All @@ -20,6 +21,18 @@ class NetworkStateHandler;
// concurrency issues by disabling one before enabling the other.
class COMPONENT_EXPORT(CHROMEOS_NETWORK) TechnologyStateController {
public:
class HotspotOperationDelegate {
public:
// Prepare for enable Wifi technology by disabling hotspot if active.
// Calls |callback| when the preparation is completed.
virtual void PrepareEnableWifi(
base::OnceCallback<void(bool prepare_success)> callback) = 0;
};

// Constant for |error_name| from network_handler::ErrorCallback. This error
// name indicated failed to disable hotspot before enable Wifi.
static const char kErrorDisableHotspot[];

TechnologyStateController();
TechnologyStateController(const TechnologyStateController&) = delete;
TechnologyStateController& operator=(const TechnologyStateController&) =
Expand All @@ -34,8 +47,21 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) TechnologyStateController {
bool enabled,
network_handler::ErrorCallback error_callback);

void set_hotspot_operation_delegate(
HotspotOperationDelegate* hotspot_operation_delegate) {
hotspot_operation_delegate_ = hotspot_operation_delegate;
}

private:
NetworkStateHandler* network_state_handler_ = nullptr;
HotspotOperationDelegate* hotspot_operation_delegate_ = nullptr;

void OnPrepareEnableWifiCompleted(
const NetworkTypePattern& type,
network_handler::ErrorCallback error_callback,
bool success);

base::WeakPtrFactory<TechnologyStateController> weak_ptr_factory_{this};
};

} // namespace ash
Expand Down

0 comments on commit 18a4ecd

Please sign in to comment.