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.

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-Commit-Position: refs/heads/main@{#1086558}
  • Loading branch information
Theo Johnson-Kanu authored and Chromium LUCI CQ committed Dec 22, 2022
1 parent 5b3c7c1 commit 87a7331
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 39 deletions.
39 changes: 0 additions & 39 deletions ash/system/network/network_list_network_item_view.cc
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,7 @@ size_t NetworkListViewControllerImpl::CreateItemViewsIfMissingAndReorder(
network_view->UpdateViewForNetwork(network);
network_detailed_network_view()->GetNetworkList(type)->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
55 changes: 55 additions & 0 deletions ash/system/network/network_list_view_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,17 @@ class NetworkListViewControllerTest : public AshTestBase,
}
}

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

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

return network->GetEnabled();
}

void SetBluetoothAdapterState(BluetoothSystemState system_state) {
bluetooth_config_test_helper()
->fake_adapter_state_controller()
Expand Down Expand Up @@ -1256,4 +1267,48 @@ TEST_P(NetworkListViewControllerTest, NetworkScanning) {
cros_network()->GetScanCount(NetworkType::kTether));
}

TEST_P(NetworkListViewControllerTest, NetworkItemIsEnabled) {
auto properties =
chromeos::network_config::mojom::DeviceStateProperties::New();
properties->type = NetworkType::kCellular;
properties->device_state = DeviceStateType::kEnabled;
properties->sim_infos = CellularSIMInfos(kCellularTestIccid, kTestBaseEid);

cros_network()->SetDeviceProperties(properties.Clone());
ASSERT_THAT(GetMobileSubHeader(), NotNull());
EXPECT_TRUE(GetAddEsimButton()->GetEnabled());

cros_network()->AddNetworkAndDevice(
CrosNetworkConfigTestHelper::CreateStandaloneNetworkProperties(
kCellularName, NetworkType::kCellular,
ConnectionStateType::kConnected));

if (IsQsRevampEnabled()) {
CheckNetworkListItem(NetworkType::kCellular, /*index=*/0u, kCellularName);
EXPECT_TRUE(GetNetworkListItemIsEnabled(NetworkType::kCellular, 0u));
} else {
CheckNetworkListItem(NetworkType::kCellular, /*index=*/1u, kCellularName);
EXPECT_TRUE(GetNetworkListItemIsEnabled(NetworkType::kCellular, 1u));
}

// Inhibit cellular device.
properties->inhibit_reason = InhibitReason::kResettingEuiccMemory;
cros_network()->SetDeviceProperties(properties.Clone());

if (IsQsRevampEnabled()) {
EXPECT_FALSE(GetNetworkListItemIsEnabled(NetworkType::kCellular, 0u));
} else {
EXPECT_FALSE(GetNetworkListItemIsEnabled(NetworkType::kCellular, 1u));
}

// Uninhibit the device.
properties->inhibit_reason = InhibitReason::kNotInhibited;
cros_network()->SetDeviceProperties(properties.Clone());
if (IsQsRevampEnabled()) {
EXPECT_TRUE(GetNetworkListItemIsEnabled(NetworkType::kCellular, 0u));
} else {
EXPECT_TRUE(GetNetworkListItemIsEnabled(NetworkType::kCellular, 1u));
}
}

} // namespace ash
56 changes: 56 additions & 0 deletions ash/system/network/network_utils.cc
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 87a7331

Please sign in to comment.