[Instant Hotspot] Replace kMobile with kCellular in QS view controller

Replaces instances of kMobile with kCellular in QS view controller if
kInstantHotspot feature flag is enabled. Also reverts CL that removed
tether networks from mobile section: crrev.com/c/4706048.

Tested on cellular DUT with feature flags on/off and connected tether
device.

Screenshot(feature flag disabled):
https://screenshot.googleplex.com/4pYxjn6D2vWCAUL
Screenshot(feature flag enabled): https://screenshot.googleplex.com/3AXhvtutMFCybz9

Bug: b/301288153
Change-Id: I4549825cf9eb17a076a98abb3e6f07b993d913e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4879618
Auto-Submit: Rudransh Dikshit <rudranshd@google.com>
Reviewed-by: Chad Duffin <chadduffin@chromium.org>
Commit-Queue: Rudransh Dikshit <rudranshd@google.com>
Cr-Commit-Position: refs/heads/main@{#1206572}

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);
 }

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();

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));

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>(

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_;

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);

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",
   ]

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 ||