Skip to content

Commit

Permalink
[CrOS Hotspot] Implement set policy allowed
Browse files Browse the repository at this point in the history
This CL implements the method to set hotspot policy allowed. It will
disable current hotspot if not allowed and update the allow status
accordingly.

Bug: b/239477916
Change-Id: I223fbec99f3ef19be01a507879cc54c6b18758f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4163625
Reviewed-by: Regan Hsu <hsuregan@chromium.org>
Commit-Queue: Jason Zhang <jiajunz@google.com>
Cr-Commit-Position: refs/heads/main@{#1098115}
  • Loading branch information
Jason Zhang authored and Chromium LUCI CQ committed Jan 27, 2023
1 parent 596d15b commit 0e3789b
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 13 deletions.
20 changes: 19 additions & 1 deletion chromeos/ash/components/network/hotspot_capabilities_provider.cc
Expand Up @@ -234,7 +234,9 @@ void HotspotCapabilitiesProvider::OnCheckReadinessSuccess(
using HotspotAllowStatus = hotspot_config::mojom::HotspotAllowStatus;

if (result == shill::kTetheringReadinessReady) {
SetHotspotAllowStatus(HotspotAllowStatus::kAllowed);
SetHotspotAllowStatus(policy_allow_hotspot_
? HotspotAllowStatus::kAllowed
: HotspotAllowStatus::kDisallowedByPolicy);
std::move(callback).Run(CheckTetheringReadinessResult::kReady);
return;
}
Expand Down Expand Up @@ -280,4 +282,20 @@ void HotspotCapabilitiesProvider::NotifyHotspotCapabilitiesChanged() {
}
}

void HotspotCapabilitiesProvider::SetPolicyAllowed(bool allowed) {
policy_allow_hotspot_ = allowed;
if (!policy_allow_hotspot_ &&
hotspot_capabilities_.allow_status ==
hotspot_config::mojom::HotspotAllowStatus::kAllowed) {
SetHotspotAllowStatus(
hotspot_config::mojom::HotspotAllowStatus::kDisallowedByPolicy);
return;
}
if (policy_allow_hotspot_ &&
hotspot_capabilities_.allow_status ==
hotspot_config::mojom::HotspotAllowStatus::kDisallowedByPolicy) {
SetHotspotAllowStatus(hotspot_config::mojom::HotspotAllowStatus::kAllowed);
}
}

} // namespace ash
Expand Up @@ -84,6 +84,8 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotCapabilitiesProvider
// necessary. |callback| is called with check readiness result.
void CheckTetheringReadiness(CheckTetheringReadinessCallback callback);

void SetPolicyAllowed(bool allowed);

void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);
bool HasObserver(Observer* observer) const;
Expand Down Expand Up @@ -122,6 +124,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotCapabilitiesProvider
HotspotCapabilities hotspot_capabilities_{
hotspot_config::mojom::HotspotAllowStatus::kDisallowedNoCellularUpstream};

bool policy_allow_hotspot_ = true;
NetworkStateHandler* network_state_handler_ = nullptr;
base::ScopedObservation<NetworkStateHandler, NetworkStateHandlerObserver>
network_state_handler_observer_{this};
Expand Down
Expand Up @@ -192,6 +192,18 @@ TEST_F(HotspotCapabilitiesProviderTest, GetHotspotCapabilities) {
hotspot_config::mojom::HotspotAllowStatus::kAllowed,
hotspot_capabilities_provider_->GetHotspotCapabilities().allow_status);
EXPECT_EQ(5u, observer_.hotspot_capabilities_changed_count());

hotspot_capabilities_provider_->SetPolicyAllowed(/*allowed=*/false);
EXPECT_EQ(
hotspot_config::mojom::HotspotAllowStatus::kDisallowedByPolicy,
hotspot_capabilities_provider_->GetHotspotCapabilities().allow_status);
EXPECT_EQ(6u, observer_.hotspot_capabilities_changed_count());

hotspot_capabilities_provider_->SetPolicyAllowed(/*allowed=*/true);
EXPECT_EQ(
hotspot_config::mojom::HotspotAllowStatus::kAllowed,
hotspot_capabilities_provider_->GetHotspotCapabilities().allow_status);
EXPECT_EQ(7u, observer_.hotspot_capabilities_changed_count());
}

TEST_F(HotspotCapabilitiesProviderTest, CheckTetheringReadiness) {
Expand Down
16 changes: 15 additions & 1 deletion chromeos/ash/components/network/hotspot_controller.cc
Expand Up @@ -24,8 +24,10 @@ HotspotController::HotspotController() = default;
HotspotController::~HotspotController() = default;

void HotspotController::Init(
HotspotCapabilitiesProvider* hotspot_capabilities_provider) {
HotspotCapabilitiesProvider* hotspot_capabilities_provider,
HotspotStateHandler* hotspot_state_handler) {
hotspot_capabilities_provider_ = hotspot_capabilities_provider;
hotspot_state_handler_ = hotspot_state_handler;
}

void HotspotController::EnableHotspot(HotspotControlCallback callback) {
Expand Down Expand Up @@ -123,4 +125,16 @@ void HotspotController::CompleteCurrentRequest(
ProcessRequestQueue();
}

void HotspotController::SetPolicyAllowHotspot(bool allow_hotspot) {
if (allow_hotspot_ == allow_hotspot) {
return;
}

hotspot_capabilities_provider_->SetPolicyAllowed(allow_hotspot);
if (!allow_hotspot && hotspot_state_handler_->GetHotspotState() !=
hotspot_config::mojom::HotspotState::kDisabled) {
DisableHotspot(base::DoNothing());
}
}

} // namespace ash
10 changes: 9 additions & 1 deletion chromeos/ash/components/network/hotspot_controller.h
Expand Up @@ -11,6 +11,7 @@
#include "base/containers/queue.h"
#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/services/hotspot_config/public/mojom/cros_hotspot_config.mojom-forward.h"

namespace ash {
Expand All @@ -31,7 +32,8 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotController {
HotspotController& operator=(const HotspotController&) = delete;
~HotspotController();

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

// Return callback for the EnableHotspot or DisableHotspot method.
using HotspotControlCallback = base::OnceCallback<void(
Expand All @@ -43,6 +45,9 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotController {
void EnableHotspot(HotspotControlCallback callback);
void DisableHotspot(HotspotControlCallback callback);

// Set whether Hotspot should be allowed/disallowed by policy.
void SetPolicyAllowHotspot(bool allow_hotspot);

private:
// Represents hotspot enable or disable control request parameters. Requests
// are queued and processed one at a time.
Expand All @@ -69,7 +74,10 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotController {

std::unique_ptr<HotspotControlRequest> current_request_;
base::queue<std::unique_ptr<HotspotControlRequest>> queued_requests_;
bool allow_hotspot_ = true;
HotspotCapabilitiesProvider* hotspot_capabilities_provider_ = nullptr;
HotspotStateHandler* hotspot_state_handler_ = nullptr;

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

Expand Down
Expand Up @@ -11,6 +11,7 @@
#include "chromeos/ash/components/dbus/shill/shill_clients.h"
#include "chromeos/ash/components/dbus/shill/shill_manager_client.h"
#include "chromeos/ash/components/network/hotspot_capabilities_provider.h"
#include "chromeos/ash/components/network/hotspot_state_handler.h"
#include "chromeos/ash/components/network/network_state_handler.h"
#include "chromeos/ash/components/network/network_state_test_helper.h"
#include "chromeos/ash/services/hotspot_config/public/mojom/cros_hotspot_config.mojom.h"
Expand All @@ -35,8 +36,11 @@ class HotspotControllerTest : public ::testing::Test {
std::make_unique<HotspotCapabilitiesProvider>();
hotspot_capabilities_provider_->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_controller_->Init(hotspot_capabilities_provider_.get(),
hotspot_state_handler_.get());
SetReadinessCheckResultReady();
}

Expand All @@ -45,6 +49,7 @@ class HotspotControllerTest : public ::testing::Test {
network_state_test_helper_.ClearServices();
hotspot_controller_.reset();
hotspot_capabilities_provider_.reset();
hotspot_state_handler_.reset();
}

void SetValidTetheringCapabilities() {
Expand Down Expand Up @@ -132,6 +137,7 @@ class HotspotControllerTest : public ::testing::Test {
base::test::TaskEnvironment::TimeSource::MOCK_TIME};
std::unique_ptr<HotspotController> hotspot_controller_;
std::unique_ptr<HotspotCapabilitiesProvider> hotspot_capabilities_provider_;
std::unique_ptr<HotspotStateHandler> hotspot_state_handler_;
NetworkStateTestHelper network_state_test_helper_{
/*use_default_devices_and_services=*/false};
};
Expand Down
4 changes: 0 additions & 4 deletions chromeos/ash/components/network/hotspot_state_handler.cc
Expand Up @@ -227,10 +227,6 @@ void HotspotStateHandler::UpdateHotspotStatus(const base::Value::Dict& status) {
NotifyHotspotStatusChanged();
}

void HotspotStateHandler::SetPolicyAllowHotspot(bool allow) {
// TODO (jiajunz)
}

void HotspotStateHandler::NotifyHotspotStatusChanged() {
for (auto& observer : observer_list_)
observer.OnHotspotStatusChanged();
Expand Down
2 changes: 0 additions & 2 deletions chromeos/ash/components/network/hotspot_state_handler.h
Expand Up @@ -61,8 +61,6 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) HotspotStateHandler
// the success result of SetHotspotConfig operation.
void SetHotspotConfig(hotspot_config::mojom::HotspotConfigPtr config,
SetHotspotConfigCallback callback);
// Set whether Hotspot should be allowed/disallowed by policy.
void SetPolicyAllowHotspot(bool allow);

void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);
Expand Down
5 changes: 3 additions & 2 deletions chromeos/ash/components/network/network_handler.cc
Expand Up @@ -77,8 +77,8 @@ NetworkHandler::NetworkHandler()
}
if (ash::features::IsHotspotEnabled()) {
hotspot_capabilities_provider_.reset(new HotspotCapabilitiesProvider());
hotspot_controller_.reset(new HotspotController());
hotspot_state_handler_.reset(new HotspotStateHandler());
hotspot_controller_.reset(new HotspotController());
}
if (NetworkCertLoader::IsInitialized()) {
network_cert_migrator_.reset(new NetworkCertMigrator());
Expand Down Expand Up @@ -142,8 +142,9 @@ void NetworkHandler::Init() {
hotspot_allowed_flag_handler_->Init();
if (ash::features::IsHotspotEnabled()) {
hotspot_capabilities_provider_->Init(network_state_handler_.get());
hotspot_controller_->Init(hotspot_capabilities_provider_.get());
hotspot_state_handler_->Init();
hotspot_controller_->Init(hotspot_capabilities_provider_.get(),
hotspot_state_handler_.get());
}
managed_cellular_pref_handler_->Init(network_state_handler_.get());
esim_policy_login_metrics_logger_->Init(
Expand Down
2 changes: 1 addition & 1 deletion chromeos/ash/components/network/network_handler.h
Expand Up @@ -160,8 +160,8 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkHandler {
std::unique_ptr<HiddenNetworkHandler> hidden_network_handler_;
std::unique_ptr<HotspotAllowedFlagHandler> hotspot_allowed_flag_handler_;
std::unique_ptr<HotspotCapabilitiesProvider> hotspot_capabilities_provider_;
std::unique_ptr<HotspotController> hotspot_controller_;
std::unique_ptr<HotspotStateHandler> hotspot_state_handler_;
std::unique_ptr<HotspotController> hotspot_controller_;
std::unique_ptr<ESimPolicyLoginMetricsLogger>
esim_policy_login_metrics_logger_;
std::unique_ptr<HiddenNetworkMetricsHelper> hidden_network_metrics_helper_;
Expand Down

0 comments on commit 0e3789b

Please sign in to comment.