Skip to content

Commit

Permalink
[Cros Network] Fix issue where managed networks are clicked
Browse files Browse the repository at this point in the history
Before CL when a network is disabled by policy, the network row would
still be clickable. This CL ensures that the network row would be
disabled and not clickable if disabled by policy.

This CL cherry pick changes here https://crrev.com/c/4123170 into
M110 branch

(cherry picked from commit 87a7331)

Bug: b/262792658
Test: Updated unittest, deployed to device and verified also ran cq
Change-Id: Iaefda73ec448f231204c0b49fe396aea10e65dc8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4123170
Reviewed-by: Gordon Seto <gordonseto@google.com>
Commit-Queue: Theo Johnson-kanu <tjohnsonkanu@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1086558}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4123035
Cr-Commit-Position: refs/branch-heads/5481@{#73}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
Theo Johnson-Kanu authored and Chromium LUCI CQ committed Dec 27, 2022
1 parent 12da232 commit daf4dc8
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 39 deletions.
39 changes: 0 additions & 39 deletions ash/system/network/network_list_network_item_view.cc
Expand Up @@ -93,20 +93,6 @@ ActivationStateType GetNetworkActivationState(
return ActivationStateType::kUnknown;
}

bool IsNetworkInhibited(const NetworkStatePropertiesPtr& network_properties) {
if (!NetworkTypeMatchesType(network_properties->type,
NetworkType::kCellular)) {
return false;
}

const chromeos::network_config::mojom::DeviceStateProperties*
cellular_device =
Shell::Get()->system_tray_model()->network_state_model()->GetDevice(
NetworkType::kCellular);

return cellular_device && IsInhibited(cellular_device);
}

bool IsCellularNetworkSimLocked(
const NetworkStatePropertiesPtr& network_properties) {
DCHECK(
Expand Down Expand Up @@ -157,31 +143,6 @@ bool IsNetworkConnectable(const NetworkStatePropertiesPtr& network_properties) {
return false;
}

bool IsNetworkDisabled(const NetworkStatePropertiesPtr& network_properties) {
if (!NetworkTypeMatchesType(network_properties->type,
NetworkType::kCellular)) {
return false;
}

const CellularStateProperties* cellular =
network_properties->type_state->get_cellular().get();

if (!Shell::Get()->session_controller()->IsActiveUserSessionStarted() &&
cellular->sim_locked) {
return true;
}

if (cellular->activation_state == ActivationStateType::kActivating) {
return true;
}

if (IsNetworkInhibited(network_properties)) {
return true;
}

return network_properties->prohibited_by_policy;
}

bool IsWifiNetworkSecured(const NetworkStatePropertiesPtr& network_properties) {
DCHECK(NetworkTypeMatchesType(network_properties->type, NetworkType::kWiFi));
return network_properties->type_state->get_wifi()->security !=
Expand Down
1 change: 1 addition & 0 deletions ash/system/network/network_list_view_controller_impl.cc
Expand Up @@ -798,6 +798,7 @@ size_t NetworkListViewControllerImpl::CreateItemViewsIfMissingAndReorder(
network_view->UpdateViewForNetwork(network);
network_detailed_network_view()->network_list()->ReorderChildView(
network_view, index);
network_view->SetEnabled(!IsNetworkDisabled(network));

// Only emit ethernet metric each time we show Ethernet section
// for the first time. We use |has_reordered_a_network| to determine
Expand Down
37 changes: 37 additions & 0 deletions ash/system/network/network_list_view_controller_unittest.cc
Expand Up @@ -599,6 +599,17 @@ class NetworkListViewControllerTest : public AshTestBase,
id, type, connection_state, kSignalStrength);
}

bool GetNetworkListItemIsEnabled(NetworkType type, size_t index) {
EXPECT_STREQ(network_list()->children().at(index)->GetClassName(),
kNetworkListNetworkItemView);

NetworkListNetworkItemView* network =
static_cast<NetworkListNetworkItemView*>(
network_list()->children().at(index));

return network->GetEnabled();
}

void SetBluetoothAdapterState(BluetoothSystemState system_state) {
bluetooth_config_test_helper()
->fake_adapter_state_controller()
Expand Down Expand Up @@ -1390,4 +1401,30 @@ TEST_P(NetworkListViewControllerTest, NetworkScanning) {
EXPECT_EQ(initial_tether_count + 1u, GetTetherScanCount());
}

TEST_P(NetworkListViewControllerTest, NetworkItemIsEnabled) {
AddEuicc();
SetupCellular();
ASSERT_THAT(GetMobileSubHeader(), NotNull());

std::vector<NetworkStatePropertiesPtr> networks;

NetworkStatePropertiesPtr cellular_network =
CreateStandaloneNetworkProperties(kCellularName, NetworkType::kCellular,
ConnectionStateType::kConnected);
cellular_network->prohibited_by_policy = false;
networks.push_back(std::move(cellular_network));
UpdateNetworkList(networks);

CheckNetworkListItem(NetworkType::kCellular, /*index=*/1u, kCellularName);
EXPECT_TRUE(GetNetworkListItemIsEnabled(NetworkType::kCellular, 1u));

networks.front()->prohibited_by_policy = true;
UpdateNetworkList(networks);
EXPECT_FALSE(GetNetworkListItemIsEnabled(NetworkType::kCellular, 1u));

networks.front()->prohibited_by_policy = false;
UpdateNetworkList(networks);
EXPECT_TRUE(GetNetworkListItemIsEnabled(NetworkType::kCellular, 1u));
}

} // namespace ash
56 changes: 56 additions & 0 deletions ash/system/network/network_utils.cc
Expand Up @@ -5,9 +5,14 @@
#include "ash/system/network/network_utils.h"

#include "ash/constants/ash_features.h"
#include "ash/session/session_controller_impl.h"
#include "ash/shell.h"
#include "ash/strings/grit/ash_strings.h"
#include "ash/system/model/system_tray_model.h"
#include "ash/system/network/tray_network_state_model.h"
#include "base/metrics/histogram_functions.h"
#include "base/strings/strcat.h"
#include "chromeos/services/network_config/public/cpp/cros_network_config_util.h"
#include "ui/base/l10n/l10n_util.h"

namespace ash {
Expand Down Expand Up @@ -83,4 +88,55 @@ absl::optional<std::u16string> GetPortalStateSubtext(
}
}

bool IsNetworkDisabled(
const chromeos::network_config::mojom::NetworkStatePropertiesPtr&
network_properties) {
if (network_properties->prohibited_by_policy) {
return true;
}

if (!chromeos::network_config::NetworkTypeMatchesType(
network_properties->type,
chromeos::network_config::mojom::NetworkType::kCellular)) {
return false;
}

const chromeos::network_config::mojom::CellularStateProperties* cellular =
network_properties->type_state->get_cellular().get();

if (!Shell::Get()->session_controller()->IsActiveUserSessionStarted() &&
cellular->sim_locked) {
return true;
}

if (cellular->activation_state ==
chromeos::network_config::mojom::ActivationStateType::kActivating) {
return true;
}

if (IsNetworkInhibited(network_properties)) {
return true;
}

return false;
}

bool IsNetworkInhibited(
const chromeos::network_config::mojom::NetworkStatePropertiesPtr&
network_properties) {
if (!chromeos::network_config::NetworkTypeMatchesType(
network_properties->type,
chromeos::network_config::mojom::NetworkType::kCellular)) {
return false;
}

const chromeos::network_config::mojom::DeviceStateProperties*
cellular_device =
Shell::Get()->system_tray_model()->network_state_model()->GetDevice(
chromeos::network_config::mojom::NetworkType::kCellular);

return cellular_device &&
chromeos::network_config::IsInhibited(cellular_device);
}

} // namespace ash
10 changes: 10 additions & 0 deletions ash/system/network/network_utils.h
Expand Up @@ -46,6 +46,16 @@ ASH_EXPORT void RecordNetworkTypeToggled(
ASH_EXPORT absl::optional<std::u16string> GetPortalStateSubtext(
const chromeos::network_config::mojom::PortalState& portal_state);

// Returns true if current network row is disabled.
ASH_EXPORT bool IsNetworkDisabled(
const chromeos::network_config::mojom::NetworkStatePropertiesPtr&
network_properties);

// Returns true if current network is a cellular network and is inhibited.
ASH_EXPORT bool IsNetworkInhibited(
const chromeos::network_config::mojom::NetworkStatePropertiesPtr&
network_properties);

} // namespace ash

#endif // ASH_SYSTEM_NETWORK_NETWORK_UTILS_H_

0 comments on commit daf4dc8

Please sign in to comment.