diff --git a/ash/constants/ash_features.cc b/ash/constants/ash_features.cc index aa513dc1788a3..6c57276911c86 100644 --- a/ash/constants/ash_features.cc +++ b/ash/constants/ash_features.cc @@ -3489,6 +3489,11 @@ bool IsHotspotEnabled() { return base::FeatureList::IsEnabled(kHotspot); } +bool IsInstantHotspotRebrandEnabled() { + return base::FeatureList::IsEnabled(kInstantHotspotRebrand) && + base::FeatureList::IsEnabled(kQsRevamp); +} + bool IsScreenSaverDurationEnabled() { return base::FeatureList::IsEnabled(kScreenSaverDuration); } diff --git a/ash/constants/ash_features.h b/ash/constants/ash_features.h index 1eee435fd473f..16354a07bde83 100644 --- a/ash/constants/ash_features.h +++ b/ash/constants/ash_features.h @@ -1004,6 +1004,7 @@ COMPONENT_EXPORT(ASH_CONSTANTS) bool IsHomeButtonQuickAppAccessEnabled(); COMPONENT_EXPORT(ASH_CONSTANTS) bool IsHomeButtonWithTextEnabled(); COMPONENT_EXPORT(ASH_CONSTANTS) bool IsHostnameSettingEnabled(); COMPONENT_EXPORT(ASH_CONSTANTS) bool IsHotspotEnabled(); +COMPONENT_EXPORT(ASH_CONSTANTS) bool IsInstantHotspotRebrandEnabled(); COMPONENT_EXPORT(ASH_CONSTANTS) bool IsInputNoiseCancellationUiEnabled(); COMPONENT_EXPORT(ASH_CONSTANTS) bool IsInstantTetheringBackgroundAdvertisingSupported(); diff --git a/ash/system/network/network_detailed_network_view_impl.cc b/ash/system/network/network_detailed_network_view_impl.cc index 76c5e51a49310..bb1412d805052 100644 --- a/ash/system/network/network_detailed_network_view_impl.cc +++ b/ash/system/network/network_detailed_network_view_impl.cc @@ -40,13 +40,16 @@ using chromeos::network_config::mojom::InhibitReason; constexpr auto kMainContainerMargins = gfx::Insets::TLBR(2, 0, 0, 0); constexpr auto kTopContainerBorder = gfx::Insets::TLBR(4, 0, 4, 4); -// The following getter methods should only be used for `NetworkType::kWiFi` and -// `NetworkType::kMobile` types otherwise a crash will occur. +// The following getter methods should only be used for `NetworkType::kWiFi`, +// `NetworkType::kMobile`, or `NetworkType::kCellular` types otherwise a crash +// will occur. std::u16string GetLabelForWifiAndMobileNetwork(NetworkType type) { switch (type) { case NetworkType::kWiFi: return l10n_util::GetStringUTF16( IDS_ASH_QUICK_SETTINGS_JOIN_WIFI_NETWORK); + case NetworkType::kCellular: + [[fallthrough]]; case NetworkType::kMobile: return l10n_util::GetStringUTF16(IDS_ASH_QUICK_SETTINGS_ADD_ESIM); default: @@ -58,6 +61,8 @@ std::u16string GetTooltipForWifiAndMobileNetwork(NetworkType type) { switch (type) { case NetworkType::kWiFi: return l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_OTHER_WIFI); + case NetworkType::kCellular: + [[fallthrough]]; case NetworkType::kMobile: return l10n_util::GetStringUTF16(GetAddESimTooltipMessageId()); default: @@ -69,6 +74,8 @@ int GetViewIDForWifiAndMobileNetwork(NetworkType type) { switch (type) { case NetworkType::kWiFi: return VIEW_ID_JOIN_WIFI_NETWORK_ENTRY; + case NetworkType::kCellular: + [[fallthrough]]; case NetworkType::kMobile: return VIEW_ID_ADD_ESIM_ENTRY; default: @@ -123,7 +130,8 @@ NetworkListNetworkItemView* NetworkDetailedNetworkViewImpl::AddNetworkListItem( HoverHighlightView* NetworkDetailedNetworkViewImpl::AddConfigureNetworkEntry( NetworkType type) { - CHECK(type == NetworkType::kWiFi || type == NetworkType::kMobile); + CHECK(type == NetworkType::kWiFi || type == NetworkType::kMobile || + type == NetworkType::kCellular); HoverHighlightView* entry = GetNetworkList(type)->AddChildView( std::make_unique<HoverHighlightView>(/*listener=*/this)); entry->SetID(GetViewIDForWifiAndMobileNetwork(type)); diff --git a/ash/system/network/network_list_view_controller_impl.cc b/ash/system/network/network_list_view_controller_impl.cc index d5a8ad8d5818f..12a7d697a9341 100644 --- a/ash/system/network/network_list_view_controller_impl.cc +++ b/ash/system/network/network_list_view_controller_impl.cc @@ -24,6 +24,7 @@ #include "ash/system/network/tray_network_state_model.h" #include "ash/system/tray/tray_info_label.h" #include "ash/system/tray/tri_view.h" +#include "base/feature_list.h" #include "base/timer/timer.h" #include "chromeos/ash/components/dbus/hermes/hermes_manager_client.h" #include "chromeos/constants/chromeos_features.h" @@ -130,6 +131,13 @@ bool IsCellularSimLocked() { !cellular_device->sim_lock_status->lock_type.empty(); } +NetworkType GetMobileSectionNetworkType() { + if (features::IsInstantHotspotRebrandEnabled()) { + return NetworkType::kCellular; + } + return NetworkType::kMobile; +} + } // namespace NetworkListViewControllerImpl::NetworkListViewControllerImpl( @@ -259,20 +267,20 @@ void NetworkListViewControllerImpl::OnGetNetworkStateList( size_t mobile_item_index = 0; mobile_item_index = CreateItemViewsIfMissingAndReorder( - NetworkType::kMobile, mobile_item_index, networks, + GetMobileSectionNetworkType(), mobile_item_index, networks, &previous_network_views); // Add mobile status message to NetworkDetailedNetworkView's // `mobile_network_list_view_` if it exist. if (mobile_status_message_) { network_detailed_network_view() - ->GetNetworkList(NetworkType::kMobile) + ->GetNetworkList(GetMobileSectionNetworkType()) ->ReorderChildView(mobile_status_message_, mobile_item_index++); } if (ShouldAddESimEntry()) { mobile_item_index = CreateConfigureNetworkEntry( - &add_esim_entry_, NetworkType::kMobile, mobile_item_index); + &add_esim_entry_, GetMobileSectionNetworkType(), mobile_item_index); } else { RemoveAndResetViewIfExists(&add_esim_entry_); } @@ -280,17 +288,18 @@ void NetworkListViewControllerImpl::OnGetNetworkStateList( network_detailed_network_view()->ReorderMobileListView(index++); } else { network_detailed_network_view() - ->GetNetworkList(NetworkType::kMobile) + ->GetNetworkList(GetMobileSectionNetworkType()) ->ReorderChildView(mobile_header_view_, index++); - index = CreateItemViewsIfMissingAndReorder( - NetworkType::kMobile, index, networks, &previous_network_views); + index = CreateItemViewsIfMissingAndReorder(GetMobileSectionNetworkType(), + index, networks, + &previous_network_views); // Add mobile status message to NetworkDetailedNetworkView scroll list if // it exist. if (mobile_status_message_) { network_detailed_network_view() - ->GetNetworkList(NetworkType::kMobile) + ->GetNetworkList(GetMobileSectionNetworkType()) ->ReorderChildView(mobile_status_message_, index++); } } @@ -388,15 +397,18 @@ void NetworkListViewControllerImpl::OnGetNetworkStateList( void NetworkListViewControllerImpl::UpdateNetworkTypeExistence( const std::vector<NetworkStatePropertiesPtr>& networks) { - has_mobile_networks_ = false; + has_cellular_networks_ = false; has_wifi_networks_ = false; + has_tether_networks_ = false; connected_vpn_guid_ = std::string(); for (auto& network : networks) { - if (NetworkTypeMatchesType(network->type, NetworkType::kMobile)) { - has_mobile_networks_ = true; + if (NetworkTypeMatchesType(network->type, NetworkType::kCellular)) { + has_cellular_networks_ = true; } else if (NetworkTypeMatchesType(network->type, NetworkType::kWiFi)) { has_wifi_networks_ = true; + } else if (NetworkTypeMatchesType(network->type, NetworkType::kTether)) { + has_tether_networks_ = true; } else if (NetworkTypeMatchesType(network->type, NetworkType::kVPN) && StateIsConnected(network->connection_state)) { connected_vpn_guid_ = network->guid; @@ -820,7 +832,9 @@ void NetworkListViewControllerImpl::UpdateMobileToggleAndSetStatusMessage() { network_detailed_network_view()->UpdateMobileStatus(cellular_enabled); if (cellular_enabled) { - if (has_mobile_networks_) { + if (has_cellular_networks_ || + (has_tether_networks_ && + !features::IsInstantHotspotRebrandEnabled())) { RemoveAndResetViewIfExists(&mobile_status_message_); return; } @@ -877,7 +891,7 @@ void NetworkListViewControllerImpl::UpdateMobileToggleAndSetStatusMessage() { /*animate_toggle=*/true); network_detailed_network_view()->UpdateMobileStatus(tether_enabled); - if (tether_enabled && !has_mobile_networks_) { + if (tether_enabled && !has_tether_networks_ && !has_cellular_networks_) { CreateInfoLabelIfMissingAndUpdate( IDS_ASH_STATUS_TRAY_NO_MOBILE_DEVICES_FOUND, &mobile_status_message_); return; @@ -906,7 +920,7 @@ void NetworkListViewControllerImpl::CreateInfoLabelIfMissingAndUpdate( info->SetID(static_cast<int>( NetworkListViewControllerViewChildId::kMobileStatusMessage)); *info_label_ptr = network_detailed_network_view() - ->GetNetworkList(NetworkType::kMobile) + ->GetNetworkList(GetMobileSectionNetworkType()) ->AddChildView(std::move(info)); } else if (info_label_ptr == &wifi_status_message_) { info->SetID(static_cast<int>( diff --git a/ash/system/network/network_list_view_controller_impl.h b/ash/system/network/network_list_view_controller_impl.h index f72dd2df93b83..77de67a8a08e5 100644 --- a/ash/system/network/network_list_view_controller_impl.h +++ b/ash/system/network/network_list_view_controller_impl.h @@ -265,8 +265,9 @@ class ASH_EXPORT NetworkListViewControllerImpl // for: #addr-of RAW_PTR_EXCLUSION HoverHighlightView* add_esim_entry_ = nullptr; - bool has_mobile_networks_; + bool has_cellular_networks_; bool has_wifi_networks_; + bool has_tether_networks_; bool is_mobile_network_enabled_; bool is_wifi_enabled_; std::string connected_vpn_guid_; diff --git a/ash/system/network/network_list_view_controller_unittest.cc b/ash/system/network/network_list_view_controller_unittest.cc index 700b4bc821d5b..e1ad6153acf16 100644 --- a/ash/system/network/network_list_view_controller_unittest.cc +++ b/ash/system/network/network_list_view_controller_unittest.cc @@ -172,8 +172,9 @@ class FakeNetworkDetailedNetworkViewDelegate } // namespace -class NetworkListViewControllerTest : public AshTestBase, - public testing::WithParamInterface<bool> { +class NetworkListViewControllerTest + : public AshTestBase, + public testing::WithParamInterface<std::tuple<bool, bool>> { public: NetworkListViewControllerTest() : AshTestBase(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {} @@ -183,11 +184,20 @@ class NetworkListViewControllerTest : public AshTestBase, ~NetworkListViewControllerTest() override = default; void SetUp() override { + std::vector<base::test::FeatureRef> enabled_features; + std::vector<base::test::FeatureRef> disabled_features; if (IsQsRevampEnabled()) { - feature_list_.InitAndEnableFeature(features::kQsRevamp); + enabled_features.push_back(features::kQsRevamp); } else { - feature_list_.InitAndDisableFeature(features::kQsRevamp); + disabled_features.push_back(features::kQsRevamp); } + if (IsInstantHotspotRebrandEnabled()) { + enabled_features.push_back(features::kInstantHotspotRebrand); + } else { + disabled_features.push_back(features::kInstantHotspotRebrand); + } + feature_list_.InitWithFeatures(enabled_features, disabled_features); + AshTestBase::SetUp(); cros_network_ = std::make_unique<FakeCrosNetworkConfig>(); @@ -215,7 +225,9 @@ class NetworkListViewControllerTest : public AshTestBase, network_detailed_network_view_); } - bool IsQsRevampEnabled() { return GetParam(); } + bool IsQsRevampEnabled() { return std::get<0>(GetParam()); } + + bool IsInstantHotspotRebrandEnabled() { return std::get<1>(GetParam()); } void TearDown() override { network_list_view_controller_impl_.reset(); @@ -554,9 +566,9 @@ class NetworkListViewControllerTest : public AshTestBase, network_list_view_controller_impl_; }; -INSTANTIATE_TEST_SUITE_P(QsRevamp, +INSTANTIATE_TEST_SUITE_P(All, NetworkListViewControllerTest, - testing::Bool() /* IsQsRevampEnabled() */); + testing::Combine(testing::Bool(), testing::Bool())); TEST_P(NetworkListViewControllerTest, MobileDataSectionIsShown) { EXPECT_THAT(GetMobileSubHeader(), IsNull()); @@ -872,9 +884,19 @@ TEST_P(NetworkListViewControllerTest, HasCorrectMobileNetworkList) { CrosNetworkConfigTestHelper::CreateStandaloneNetworkProperties( kTetherName, NetworkType::kTether, ConnectionStateType::kConnected)); - CheckNetworkListOrdering(/*ethernet_network_count=*/0, - /*mobile_network_count=*/1, - /*wifi_network_count=*/0); + if (IsInstantHotspotRebrandEnabled() && IsQsRevampEnabled()) { + CheckNetworkListOrdering(/*ethernet_network_count=*/0, + /*mobile_network_count=*/0, + /*wifi_network_count=*/0); + } else { + CheckNetworkListOrdering(/*ethernet_network_count=*/0, + /*mobile_network_count=*/1, + /*wifi_network_count=*/0); + } + + if (IsInstantHotspotRebrandEnabled()) { + return; + } if (IsQsRevampEnabled()) { CheckNetworkListItem(NetworkType::kTether, /*index=*/0u, /*guid=*/kTetherName); diff --git a/chromeos/services/network_config/public/cpp/BUILD.gn b/chromeos/services/network_config/public/cpp/BUILD.gn index 63e23b48f7247..2b66185585cde 100644 --- a/chromeos/services/network_config/public/cpp/BUILD.gn +++ b/chromeos/services/network_config/public/cpp/BUILD.gn @@ -14,7 +14,6 @@ source_set("cpp") { public_deps = [ "//chromeos/services/network_config/public/mojom" ] deps = [ - "//ash/constants", "//components/device_event_log:device_event_log", "//components/onc", ] diff --git a/chromeos/services/network_config/public/cpp/cros_network_config_util.cc b/chromeos/services/network_config/public/cpp/cros_network_config_util.cc index 3c2f1f36db85d..d52def178485c 100644 --- a/chromeos/services/network_config/public/cpp/cros_network_config_util.cc +++ b/chromeos/services/network_config/public/cpp/cros_network_config_util.cc @@ -3,7 +3,6 @@ // found in the LICENSE file. #include "chromeos/services/network_config/public/cpp/cros_network_config_util.h" -#include "ash/constants/ash_features.h" #include "components/device_event_log/device_event_log.h" #include "components/onc/onc_constants.h" @@ -265,12 +264,8 @@ bool NetworkTypeMatchesType(mojom::NetworkType network_type, case mojom::NetworkType::kAll: return true; case mojom::NetworkType::kMobile: - if (base::FeatureList::IsEnabled(ash::features::kInstantHotspotRebrand)) { - return network_type == mojom::NetworkType::kCellular; - } else { - return network_type == mojom::NetworkType::kCellular || - network_type == mojom::NetworkType::kTether; - } + return network_type == mojom::NetworkType::kCellular || + network_type == mojom::NetworkType::kTether; case mojom::NetworkType::kWireless: return network_type == mojom::NetworkType::kCellular || network_type == mojom::NetworkType::kTether ||