Replace add eSim button with add eSim entry for QsRevamp

This CL replaced the add eSim button in the mobile header with add eSim
entry at the bottom of the mobile section. This entry row will show up
only under the conditions that the previous eSim button is enabled and
visible. If there's no mobile network set up and the eSim entry is
visible, the "No mobile devices set up" message will not show while the
add eSim entry is showing. This CL also modifies the unit tests to be
compatible with this new entry.

Screenshot:
https://screenshot.googleplex.com/7K6iLLH9fVXTgmk

Bug: b:266761290
Change-Id: I91a0a19db479fa0d48c0638ae81948d12454efed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4711948
Commit-Queue: Sylvie Liu <sylvieliu@chromium.org>
Reviewed-by: Jiaming Cheng <jiamingc@chromium.org>
Reviewed-by: Chad Duffin <chadduffin@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1183765}

ash/ash_strings.grd

@@ -2709,6 +2709,9 @@ Turning on battery saver to increase your battery life.
       <message name="IDS_ASH_QUICK_SETTINGS_JOIN_WIFI_NETWORK" desc="The label used in the network detailed page for the Join Wi-Fi network entry">
         Join Wi-Fi network
       </message>
+      <message name="IDS_ASH_QUICK_SETTINGS_ADD_ESIM" desc="The label used in the network detailed page for the Add eSIM network entry">
+        Add eSIM
+      </message>
       <message name="IDS_ASH_QUICK_SETTINGS_NETWORK_DISABLED" desc="Network disabled label in the quick settings bubble network subpage." meaning="Network is disabled.">
         Off
       </message>

ash/ash_strings_grd/IDS_ASH_QUICK_SETTINGS_ADD_ESIM.png.sha1

@@ -0,0 +1 @@
+df78ff2c03fadd041b0a32bdafbc3ee7abaf432a

ash/public/cpp/ash_view_ids.h

@@ -26,6 +26,8 @@ enum ViewID {
 
   // Accessibility feature pod button in main view.
   VIEW_ID_ACCESSIBILITY_TRAY_ITEM,
+  // The entry to add esim in the quick settings network subpage.
+  VIEW_ID_ADD_ESIM_ENTRY,
   // System tray AddUserButton in UserChooserView.
   VIEW_ID_ADD_USER_BUTTON,
   VIEW_ID_BLUETOOTH_DEFAULT_VIEW,
@@ -55,7 +57,7 @@ enum ViewID {
   VIEW_ID_IME_TITLE_VIEW,
 
   // The entry to add wifi network in the quick settings network subpage.
-  VIEW_ID_JOIN_NETWORK_ENTRY,
+  VIEW_ID_JOIN_WIFI_NETWORK_ENTRY,
 
   VIEW_ID_MEDIA_TRAY_VIEW,
 

ash/system/network/fake_network_detailed_network_view.cc

@@ -59,7 +59,8 @@ FakeNetworkDetailedNetworkView::AddWifiSectionHeader() {
   return network_list_->AddChildView(std::move(wifi_header_view));
 }
 
-HoverHighlightView* FakeNetworkDetailedNetworkView::AddJoinNetworkEntry() {
+HoverHighlightView* FakeNetworkDetailedNetworkView::AddConfigureNetworkEntry(
+    NetworkType type) {
   return nullptr;
 }
 

ash/system/network/fake_network_detailed_network_view.h

@@ -49,7 +49,7 @@ class ASH_EXPORT FakeNetworkDetailedNetworkView
   NetworkListNetworkItemView* AddNetworkListItem(
       chromeos::network_config::mojom::NetworkType type) override;
   NetworkListWifiHeaderView* AddWifiSectionHeader() override;
-  HoverHighlightView* AddJoinNetworkEntry() override;
+  HoverHighlightView* AddConfigureNetworkEntry(NetworkType type) override;
   NetworkListMobileHeaderView* AddMobileSectionHeader() override;
   void UpdateScanningBarVisibility(bool visible) override;
   void ReorderFirstListView(size_t index) override {}

ash/system/network/network_detailed_network_view.h

@@ -17,6 +17,10 @@
 
 namespace ash {
 
+namespace {
+using chromeos::network_config::mojom::NetworkType;
+}  // namespace
+
 class DetailedViewDelegate;
 
 // This class defines both the interface used to interact with the
@@ -69,13 +73,14 @@ class ASH_EXPORT NetworkDetailedNetworkView {
   // Creates, adds and returns a new network list item. The client is
   // expected to use the returned pointer for removing and rearranging
   // the list item.
-  virtual NetworkListNetworkItemView* AddNetworkListItem(
-      chromeos::network_config::mojom::NetworkType type) = 0;
+  virtual NetworkListNetworkItemView* AddNetworkListItem(NetworkType type) = 0;
 
-  // Creates, adds and returns a `HoverHighlightView`, which is the "Join WIFI
-  // network" entry. The client is expected to use the returned pointer for
+  // Creates, adds and returns a `HoverHighlightView`, which is the "Join Wifi
+  // network" entry for the Wifi section if `NetworkType::kWiFi` is passed in or
+  // the "Add eSIM" entry for the Mobile data section if `NetworkType::kMobile`
+  // is passed in. The client is expected to use the returned pointer for
   // removing and rearranging this entry.
-  virtual HoverHighlightView* AddJoinNetworkEntry() = 0;
+  virtual HoverHighlightView* AddConfigureNetworkEntry(NetworkType type) = 0;
 
   // Creates, adds and returns a Wifi sticky sub-header to the end of the
   // network list. The client is expected to use the returned pointer for
@@ -91,8 +96,7 @@ class ASH_EXPORT NetworkDetailedNetworkView {
   virtual void UpdateScanningBarVisibility(bool visible) = 0;
 
   // Returns the network list.
-  virtual views::View* GetNetworkList(
-      chromeos::network_config::mojom::NetworkType type) = 0;
+  virtual views::View* GetNetworkList(NetworkType type) = 0;
 
   // Reorders the container or list view based on the index.
   virtual void ReorderFirstListView(size_t index) = 0;

ash/system/network/network_detailed_network_view_impl.cc

@@ -13,6 +13,7 @@
 #include "ash/style/ash_color_id.h"
 #include "ash/style/rounded_container.h"
 #include "ash/style/typography.h"
+#include "ash/system/model/system_tray_model.h"
 #include "ash/system/network/network_detailed_view.h"
 #include "ash/system/network/network_list_mobile_header_view_impl.h"
 #include "ash/system/network/network_list_network_item_view.h"
@@ -33,10 +34,46 @@
 
 namespace ash {
 namespace {
-using chromeos::network_config::mojom::NetworkType;
+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.
+std::u16string GetLabelForWifiAndMobileNetwork(NetworkType type) {
+  switch (type) {
+    case NetworkType::kWiFi:
+      return l10n_util::GetStringUTF16(
+          IDS_ASH_QUICK_SETTINGS_JOIN_WIFI_NETWORK);
+    case NetworkType::kMobile:
+      return l10n_util::GetStringUTF16(IDS_ASH_QUICK_SETTINGS_ADD_ESIM);
+    default:
+      NOTREACHED_NORETURN();
+  }
+}
+
+std::u16string GetTooltipForWifiAndMobileNetwork(NetworkType type) {
+  switch (type) {
+    case NetworkType::kWiFi:
+      return l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_OTHER_WIFI);
+    case NetworkType::kMobile:
+      return l10n_util::GetStringUTF16(GetAddESimTooltipMessageId());
+    default:
+      NOTREACHED_NORETURN();
+  }
+}
+
+int GetViewIDForWifiAndMobileNetwork(NetworkType type) {
+  switch (type) {
+    case NetworkType::kWiFi:
+      return VIEW_ID_JOIN_WIFI_NETWORK_ENTRY;
+    case NetworkType::kMobile:
+      return VIEW_ID_ADD_ESIM_ENTRY;
+    default:
+      NOTREACHED_NORETURN();
+  }
+}
 }  // namespace
 
 NetworkDetailedNetworkViewImpl::NetworkDetailedNetworkViewImpl(
@@ -83,19 +120,19 @@ NetworkListNetworkItemView* NetworkDetailedNetworkViewImpl::AddNetworkListItem(
       std::make_unique<NetworkListNetworkItemView>(/*listener=*/this));
 }
 
-HoverHighlightView* NetworkDetailedNetworkViewImpl::AddJoinNetworkEntry() {
-  HoverHighlightView* entry =
-      GetNetworkList(NetworkType::kWiFi)
-          ->AddChildView(
-              std::make_unique<HoverHighlightView>(/*listener=*/this));
-  entry->SetID(VIEW_ID_JOIN_NETWORK_ENTRY);
+HoverHighlightView* NetworkDetailedNetworkViewImpl::AddConfigureNetworkEntry(
+    NetworkType type) {
+  CHECK(type == NetworkType::kWiFi || type == NetworkType::kMobile);
+  HoverHighlightView* entry = GetNetworkList(type)->AddChildView(
+      std::make_unique<HoverHighlightView>(/*listener=*/this));
+  entry->SetID(GetViewIDForWifiAndMobileNetwork(type));
+  entry->SetTooltipText(GetTooltipForWifiAndMobileNetwork(type));
 
   auto image_view = std::make_unique<views::ImageView>();
   image_view->SetImage(ui::ImageModel::FromVectorIcon(
       kSystemMenuPlusIcon, cros_tokens::kCrosSysPrimary));
-  entry->AddViewAndLabel(
-      std::move(image_view),
-      l10n_util::GetStringUTF16(IDS_ASH_QUICK_SETTINGS_JOIN_WIFI_NETWORK));
+  entry->AddViewAndLabel(std::move(image_view),
+                         GetLabelForWifiAndMobileNetwork(type));
   views::Label* label = entry->text_label();
   label->SetEnabledColorId(cros_tokens::kCrosSysPrimary);
   TypographyProvider::Get()->StyleLabel(ash::TypographyToken::kCrosButton2,

ash/system/network/network_detailed_network_view_impl.h

@@ -43,14 +43,12 @@ class ASH_EXPORT NetworkDetailedNetworkViewImpl
   // NetworkDetailedNetworkView:
   void NotifyNetworkListChanged() override;
   views::View* GetAsView() override;
-  NetworkListNetworkItemView* AddNetworkListItem(
-      chromeos::network_config::mojom::NetworkType type) override;
-  HoverHighlightView* AddJoinNetworkEntry() override;
+  NetworkListNetworkItemView* AddNetworkListItem(NetworkType type) override;
+  HoverHighlightView* AddConfigureNetworkEntry(NetworkType type) override;
   NetworkListMobileHeaderView* AddMobileSectionHeader() override;
   NetworkListWifiHeaderView* AddWifiSectionHeader() override;
   void UpdateScanningBarVisibility(bool visible) override;
-  views::View* GetNetworkList(
-      chromeos::network_config::mojom::NetworkType type) override;
+  views::View* GetNetworkList(NetworkType type) override;
   void ReorderFirstListView(size_t index) override;
   void ReorderNetworkTopContainer(size_t index) override;
   void ReorderNetworkListView(size_t index) override;

ash/system/network/network_detailed_network_view_pixeltest.cc

@@ -129,6 +129,7 @@ TEST_F(NetworkDetailedNetworkViewPixelTest, Basics) {
   ASSERT_TRUE(scroll_contents);
   ASSERT_TRUE(scroll_contents->children().empty());
   ChildAddedWaiter(scroll_contents).Wait();
+  ASSERT_FALSE(scroll_contents->children().empty());
 
   // Compare pixels.
   EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(

ash/system/network/network_detailed_view.cc

@@ -51,13 +51,21 @@ void NetworkDetailedView::HandleViewClicked(views::View* view) {
     return;
   }
 
-  if (view->GetID() == VIEW_ID_JOIN_NETWORK_ENTRY) {
+  if (view->GetID() == VIEW_ID_JOIN_WIFI_NETWORK_ENTRY) {
     base::RecordAction(
         base::UserMetricsAction("QS_Subpage_Network_JoinNetwork"));
     Shell::Get()->system_tray_model()->client()->ShowNetworkCreate(
         onc::network_type::kWiFi);
     return;
   }
+
+  if (view->GetID() == VIEW_ID_ADD_ESIM_ENTRY) {
+    base::RecordAction(base::UserMetricsAction("QS_Subpage_Network_AddESim"));
+    Shell::Get()->system_tray_model()->client()->ShowNetworkCreate(
+        ::onc::network_type::kCellular);
+    return;
+  }
+
   delegate()->OnNetworkListItemSelected(
       static_cast<NetworkListItemView*>(view)->network_properties());
 }

ash/system/network/network_list_mobile_header_view_impl.cc

@@ -14,6 +14,7 @@
 #include "ash/system/model/system_tray_model.h"
 #include "ash/system/network/network_list_mobile_header_view.h"
 #include "ash/system/network/network_list_network_header_view.h"
+#include "ash/system/network/network_utils.h"
 #include "ash/system/network/tray_network_state_model.h"
 #include "ash/system/tray/hover_highlight_view.h"
 #include "ash/system/tray/tray_popup_utils.h"
@@ -29,43 +30,6 @@
 
 namespace ash {
 
-namespace {
-
-using chromeos::network_config::mojom::DeviceStateProperties;
-using chromeos::network_config::mojom::NetworkType;
-
-int GetAddESimTooltipMessageId() {
-  const DeviceStateProperties* cellular_device =
-      Shell::Get()->system_tray_model()->network_state_model()->GetDevice(
-          NetworkType::kCellular);
-
-  DCHECK(cellular_device);
-
-  switch (cellular_device->inhibit_reason) {
-    case chromeos::network_config::mojom::InhibitReason::kInstallingProfile:
-      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_INSTALLING_PROFILE;
-    case chromeos::network_config::mojom::InhibitReason::kRenamingProfile:
-      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_RENAMING_PROFILE;
-    case chromeos::network_config::mojom::InhibitReason::kRemovingProfile:
-      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_REMOVING_PROFILE;
-    case chromeos::network_config::mojom::InhibitReason::kConnectingToProfile:
-      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_CONNECTING_TO_PROFILE;
-    case chromeos::network_config::mojom::InhibitReason::kRefreshingProfileList:
-      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_REFRESHING_PROFILE_LIST;
-    case chromeos::network_config::mojom::InhibitReason::kNotInhibited:
-      return IDS_ASH_STATUS_TRAY_ADD_CELLULAR_LABEL;
-    case chromeos::network_config::mojom::InhibitReason::kResettingEuiccMemory:
-      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_RESETTING_ESIM;
-    case chromeos::network_config::mojom::InhibitReason::kDisablingProfile:
-      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_DISABLING_PROFILE;
-    case chromeos::network_config::mojom::InhibitReason::
-        kRequestingAvailableProfiles:
-      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_REQUESTING_AVAILABLE_PROFILES;
-  }
-}
-
-}  // namespace
-
 NetworkListMobileHeaderViewImpl::NetworkListMobileHeaderViewImpl(
     NetworkListNetworkHeaderView::Delegate* delegate)
     : NetworkListMobileHeaderView(delegate) {
@@ -75,6 +39,10 @@ NetworkListMobileHeaderViewImpl::NetworkListMobileHeaderViewImpl(
 NetworkListMobileHeaderViewImpl::~NetworkListMobileHeaderViewImpl() = default;
 
 void NetworkListMobileHeaderViewImpl::AddExtraButtons() {
+  if (features::IsQsRevampEnabled()) {
+    return;
+  }
+
   // The button navigates to Settings, only add it if this can occur.
   if (!TrayPopupUtils::CanOpenWebUISettings()) {
     return;
@@ -90,12 +58,8 @@ void NetworkListMobileHeaderViewImpl::AddExtraButtons() {
       /*has_border=*/false);
   add_esim_button.get()->SetID(kAddESimButtonId);
   add_esim_button_ = add_esim_button.get();
-  if (features::IsQsRevampEnabled()) {
-    entry_row()->AddAdditionalRightView(add_esim_button.release());
-  } else {
-    container()->AddViewAt(TriView::Container::END, add_esim_button.release(),
-                           /*index=*/0);
-  }
+  container()->AddViewAt(TriView::Container::END, add_esim_button.release(),
+                         /*index=*/0);
 }
 
 void NetworkListMobileHeaderViewImpl::SetToggleState(bool enabled,

ash/system/network/network_list_mobile_header_view_unittest.cc

@@ -181,6 +181,10 @@ TEST_P(NetworkListMobileHeaderViewTest, HeaderLabel) {
 }
 
 TEST_P(NetworkListMobileHeaderViewTest, AddEsimButtonStates) {
+  // QsRevamped `NetworkListHeaderView` doesn't have a `add_esim_button`.
+  if (IsQsRevampEnabled()) {
+    return;
+  }
   Init();
   IconButton* add_esim_button = GetAddEsimButton();
   ASSERT_NE(nullptr, add_esim_button);
@@ -199,6 +203,10 @@ TEST_P(NetworkListMobileHeaderViewTest, AddEsimButtonStates) {
 }
 
 TEST_P(NetworkListMobileHeaderViewTest, CellularInhibitState) {
+  // QsRevamped `NetworkListHeaderView` doesn't have a `add_esim_button`.
+  if (IsQsRevampEnabled()) {
+    return;
+  }
   Init();
 
   IconButton* add_esim_button = GetAddEsimButton();
@@ -254,6 +262,11 @@ TEST_P(NetworkListMobileHeaderViewTest, CellularInhibitState) {
 }
 
 TEST_P(NetworkListMobileHeaderViewTest, EnabledButtonNotAdded) {
+  // QsRevamped `NetworkListHeaderView` doesn't have a `add_esim_button`.
+  if (IsQsRevampEnabled()) {
+    return;
+  }
+
   // Add eSim button should not be added if the screen is locked.
   GetSessionControllerClient()->SetSessionState(
       session_manager::SessionState::LOCKED);

ash/system/network/network_list_view_controller_impl.cc

@@ -269,6 +269,14 @@ void NetworkListViewControllerImpl::OnGetNetworkStateList(
             ->GetNetworkList(NetworkType::kMobile)
             ->ReorderChildView(mobile_status_message_, mobile_item_index++);
       }
+
+      if (ShouldAddESimEntry()) {
+        mobile_item_index = CreateConfigureNetworkEntry(
+            &add_esim_entry_, NetworkType::kMobile, mobile_item_index);
+      } else {
+        RemoveAndResetViewIfExists(&add_esim_entry_);
+      }
+
       network_detailed_network_view()->ReorderMobileListView(index++);
     } else {
       network_detailed_network_view()
@@ -341,7 +349,8 @@ void NetworkListViewControllerImpl::OnGetNetworkStateList(
       RemoveAndResetViewIfExists(&unknown_header_);
     }
     if (is_wifi_enabled_) {
-      network_item_index = CreateJoinWifiEntry(network_item_index);
+      network_item_index = CreateConfigureNetworkEntry(
+          &join_wifi_entry_, NetworkType::kWiFi, network_item_index);
     } else {
       RemoveAndResetViewIfExists(&join_wifi_entry_);
     }
@@ -473,6 +482,23 @@ void NetworkListViewControllerImpl::MaybeShowConnectionWarningManagedIcon(
   }
 }
 
+bool NetworkListViewControllerImpl::ShouldAddESimEntry() const {
+  const bool is_add_esim_enabled =
+      is_mobile_network_enabled_ && !IsCellularDeviceInhibited();
+
+  bool is_add_esim_visible = IsESimSupported();
+  const GlobalPolicy* global_policy = model()->global_policy();
+
+  // Adding new cellular networks is disallowed when only policy cellular
+  // networks are allowed by admin.
+  if (!global_policy || global_policy->allow_only_policy_cellular_networks) {
+    is_add_esim_visible = false;
+  }
+  // The entry navigates to Settings, only add it if this can occur.
+  return is_add_esim_enabled && is_add_esim_visible &&
+         TrayPopupUtils::CanOpenWebUISettings();
+}
+
 void NetworkListViewControllerImpl::OnGetManagedPropertiesResult(
     const std::string& guid,
     ManagedPropertiesPtr properties) {
@@ -648,15 +674,18 @@ size_t NetworkListViewControllerImpl::CreateWifiGroupHeader(
   return index;
 }
 
-size_t NetworkListViewControllerImpl::CreateJoinWifiEntry(size_t index) {
-  if (join_wifi_entry_) {
-    network_detailed_network_view()
-        ->GetNetworkList(NetworkType::kWiFi)
-        ->ReorderChildView(join_wifi_entry_, index++);
+size_t NetworkListViewControllerImpl::CreateConfigureNetworkEntry(
+    HoverHighlightView** configure_network_entry_ptr,
+    NetworkType type,
+    size_t index) {
+  if (*configure_network_entry_ptr) {
+    network_detailed_network_view()->GetNetworkList(type)->ReorderChildView(
+        *configure_network_entry_ptr, index++);
     return index;
   }
 
-  join_wifi_entry_ = network_detailed_network_view()->AddJoinNetworkEntry();
+  *configure_network_entry_ptr =
+      network_detailed_network_view()->AddConfigureNetworkEntry(type);
   return index++;
 }
 
@@ -796,6 +825,12 @@ void NetworkListViewControllerImpl::UpdateMobileToggleAndSetStatusMessage() {
         return;
       }
 
+      // For QsRevamp: if `add_esim_entry_` is added, don't show the no mobile
+      // network label.
+      if (features::IsQsRevampEnabled() && ShouldAddESimEntry()) {
+        RemoveAndResetViewIfExists(&mobile_status_message_);
+        return;
+      }
       CreateInfoLabelIfMissingAndUpdate(IDS_ASH_STATUS_TRAY_NO_MOBILE_NETWORKS,
                                         &mobile_status_message_);
       return;

ash/system/network/network_list_view_controller_impl.h

@@ -53,7 +53,7 @@ class ASH_EXPORT NetworkListViewControllerImpl
   ~NetworkListViewControllerImpl() override;
 
  protected:
-  TrayNetworkStateModel* model() { return model_; }
+  TrayNetworkStateModel* model() const { return model_; }
 
   NetworkDetailedNetworkView* network_detailed_network_view() {
     return network_detailed_network_view_;
@@ -125,8 +125,16 @@ class ASH_EXPORT NetworkListViewControllerImpl
   // `unknown_header_`.
   size_t CreateWifiGroupHeader(size_t index, const bool is_known);
 
-  // Creates and adds the join wifi entry at the bottom of the wifi networks.
-  size_t CreateJoinWifiEntry(size_t index);
+  // Creates and adds the "+ <network>" entry at the bottom of the wifi
+  // networks (for `NetworkType::kWiFi`) or mobile networks (for
+  // `NetworkType::kMobile`) based on the value of `type`.
+  // `plus_network_entry_ptr` is the pointer to the "+ <network>" entry, and
+  // `index` is increased by 1 to indicate the order of this view so that this
+  // view can be reordered later if necessary.
+  size_t CreateConfigureNetworkEntry(
+      HoverHighlightView** plus_network_entry_ptr,
+      NetworkType type,
+      size_t index);
 
   // Updates Mobile data section, updates add eSIM button states and
   // calls UpdateMobileToggleAndSetStatusMessage().
@@ -197,6 +205,9 @@ class ASH_EXPORT NetworkListViewControllerImpl
   // if the default network has a proxy configured or if a VPN is active.
   void MaybeShowConnectionWarningManagedIcon(bool using_proxy);
 
+  // For QsRevamp: whether to add eSim entry or not.
+  bool ShouldAddESimEntry() const;
+
   raw_ptr<TrayNetworkStateModel, ExperimentalAsh> model_;
 
   mojo::Remote<bluetooth_config::mojom::CrosBluetoothConfig>
@@ -250,6 +261,9 @@ class ASH_EXPORT NetworkListViewControllerImpl
   // This field is not a raw_ptr<> because it was filtered by the rewriter
   // for: #addr-of
   RAW_PTR_EXCLUSION HoverHighlightView* join_wifi_entry_ = nullptr;
+  // This field is not a raw_ptr<> because it was filtered by the rewriter
+  // for: #addr-of
+  RAW_PTR_EXCLUSION HoverHighlightView* add_esim_entry_ = nullptr;
 
   bool has_mobile_networks_;
   bool has_wifi_networks_;

ash/system/network/network_list_view_controller_unittest.cc

@@ -268,7 +268,11 @@ class NetworkListViewControllerTest : public AshTestBase,
   }
 
   HoverHighlightView* GetAddWifiEntry() {
-    return FindViewById<HoverHighlightView*>(VIEW_ID_JOIN_NETWORK_ENTRY);
+    return FindViewById<HoverHighlightView*>(VIEW_ID_JOIN_WIFI_NETWORK_ENTRY);
+  }
+
+  HoverHighlightView* GetAddESimEntry() {
+    return FindViewById<HoverHighlightView*>(VIEW_ID_ADD_ESIM_ENTRY);
   }
 
   NetworkListMobileHeaderView* GetMobileSubHeader() {
@@ -665,45 +669,126 @@ TEST_P(NetworkListViewControllerTest, MobileSectionHeaderAddEsimButtonStates) {
   properties->device_state = DeviceStateType::kEnabled;
   cros_network()->SetDeviceProperties(properties.Clone());
 
-  ASSERT_THAT(GetMobileSubHeader(), NotNull());
-  EXPECT_TRUE(GetAddEsimButton()->GetEnabled());
-
   // Since no Euicc was added, this means device is not eSIM capable, do not
-  // show add eSIM button.
-  EXPECT_FALSE(GetAddEsimButton()->GetVisible());
+  // show add eSIM button(`kQsRevamp` disabled) or the add eSIM
+  // entry(`kQsRevamp` enabled).
+  if (IsQsRevampEnabled()) {
+    EXPECT_THAT(GetAddESimEntry(), IsNull());
+  } else {
+    ASSERT_THAT(GetMobileSubHeader(), NotNull());
+    EXPECT_TRUE(GetAddEsimButton()->GetEnabled());
+    EXPECT_FALSE(GetAddEsimButton()->GetVisible());
+  }
 
   cros_network()->ClearNetworksAndDevices();
 
   properties->sim_infos = CellularSIMInfos(kCellularTestIccid, kTestBaseEid);
   cros_network()->SetDeviceProperties(properties.Clone());
 
-  EXPECT_TRUE(GetAddEsimButton()->GetVisible());
+  if (IsQsRevampEnabled()) {
+    // If there is no network and add eSIM entry should be shown, the mobile
+    // status message shouldn't been shown.
+    EXPECT_TRUE(GetAddESimEntry()->GetVisible());
+    EXPECT_EQ(GetAddESimEntry()->GetTooltipText(),
+              l10n_util::GetStringUTF16(GetAddESimTooltipMessageId()));
+    ASSERT_THAT(GetMobileStatusMessage(), IsNull());
+  } else {
+    EXPECT_TRUE(GetAddEsimButton()->GetVisible());
+    ASSERT_THAT(GetMobileStatusMessage(), NotNull());
+  }
   EXPECT_THAT(GetMobileSeparator(), IsNull());
-  ASSERT_THAT(GetMobileStatusMessage(), NotNull());
 
   // Add eSIM button is not enabled when inhibited.
   properties->inhibit_reason = InhibitReason::kResettingEuiccMemory;
   cros_network()->SetDeviceProperties(properties.Clone());
 
-  EXPECT_FALSE(GetAddEsimButton()->GetEnabled());
-  EXPECT_TRUE(GetAddEsimButton()->GetVisible());
+  if (IsQsRevampEnabled()) {
+    EXPECT_THAT(GetAddESimEntry(), IsNull());
+  } else {
+    EXPECT_FALSE(GetAddEsimButton()->GetEnabled());
+    EXPECT_TRUE(GetAddEsimButton()->GetVisible());
+  }
 
   // Uninhibit the device.
   properties->inhibit_reason = InhibitReason::kNotInhibited;
   cros_network()->SetDeviceProperties(properties.Clone());
 
-  EXPECT_TRUE(GetAddEsimButton()->GetEnabled());
-  EXPECT_TRUE(GetAddEsimButton()->GetVisible());
+  if (IsQsRevampEnabled()) {
+    ASSERT_THAT(GetAddESimEntry(), NotNull());
+    EXPECT_TRUE(GetAddESimEntry()->GetVisible());
+    EXPECT_EQ(GetAddESimEntry()->GetTooltipText(),
+              l10n_util::GetStringUTF16(GetAddESimTooltipMessageId()));
+  } else {
+    EXPECT_TRUE(GetAddEsimButton()->GetEnabled());
+    EXPECT_TRUE(GetAddEsimButton()->GetVisible());
+  }
 
   // When no Mobile networks are available and eSIM policy is set to allow only
   // cellular devices which means adding a new eSIM is disallowed by enterprise
-  // policy, add eSIM button is not displayed.
+  // policy, add eSIM button or entry is not displayed.
   cros_network()->SetGlobalPolicy(
       /*allow_only_policy_cellular_networks=*/true,
       /*dns_queries_monitored=*/false,
       /*report_xdr_events_enabled=*/false);
+  if (IsQsRevampEnabled()) {
+    EXPECT_THAT(GetAddESimEntry(), IsNull());
+  } else {
+    EXPECT_FALSE(GetAddEsimButton()->GetVisible());
+  }
+}
 
-  EXPECT_FALSE(GetAddEsimButton()->GetVisible());
+TEST_P(NetworkListViewControllerTest,
+       MobileSectionListAddEsimEntryNotAddedWhenLocked) {
+  // Pre-revamped add esim button is tested in
+  // `NetworkListMobileHeaderViewTest`.
+  if (!IsQsRevampEnabled()) {
+    return;
+  }
+
+  EXPECT_THAT(GetMobileSubHeader(), IsNull());
+  EXPECT_THAT(GetMobileStatusMessage(), IsNull());
+
+  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(GetAddESimEntry(), NotNull());
+  EXPECT_TRUE(GetAddESimEntry()->GetVisible());
+
+  // In the locked session, the add esim entry should not be added.
+  GetSessionControllerClient()->SetSessionState(
+      session_manager::SessionState::LOCKED);
+  cros_network()->SetDeviceProperties(properties.Clone());
+  EXPECT_THAT(GetAddESimEntry(), IsNull());
+}
+
+TEST_P(NetworkListViewControllerTest, AddESimEntryUMAMetrics) {
+  // Doesn't apply to the pre-revamped Qs bubble.
+  if (!IsQsRevampEnabled()) {
+    return;
+  }
+
+  EXPECT_THAT(GetMobileSubHeader(), IsNull());
+  EXPECT_THAT(GetMobileStatusMessage(), IsNull());
+
+  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());
+
+  // Makes sure the add esim entry is visible.
+  ASSERT_THAT(GetAddESimEntry(), NotNull());
+  EXPECT_TRUE(GetAddESimEntry()->GetVisible());
+
+  base::UserActionTester user_action_tester;
+  EXPECT_EQ(0, user_action_tester.GetActionCount("QS_Subpage_Network_AddESim"));
+  LeftClickOn(GetAddESimEntry());
+  EXPECT_EQ(1, user_action_tester.GetActionCount("QS_Subpage_Network_AddESim"));
 }
 
 TEST_P(NetworkListViewControllerTest, HasCorrectMobileNetworkList) {
@@ -905,6 +990,8 @@ TEST_P(NetworkListViewControllerTest, HasCorrectWifiNetworkList) {
     CheckNetworkListItem(NetworkType::kWiFi, /*index=*/1u,
                          /*guid=*/kWifiName);
     EXPECT_TRUE(GetAddWifiEntry()->GetVisible());
+    EXPECT_EQ(GetAddWifiEntry()->GetTooltipText(),
+              l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_OTHER_WIFI));
   } else {
     // Wifi list item be at index 4 after Mobile header, Mobile network
     // item, Wifi separator and header.
@@ -1036,10 +1123,16 @@ TEST_P(NetworkListViewControllerTest,
   properties->device_state = DeviceStateType::kEnabled;
   cros_network()->SetDeviceProperties(properties.Clone());
 
-  ASSERT_THAT(GetMobileStatusMessage(), NotNull());
   ASSERT_THAT(GetMobileSubHeader(), NotNull());
-  EXPECT_EQ(l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NO_MOBILE_NETWORKS),
-            GetMobileStatusMessage()->label()->GetText());
+  // For QsRevamp: if add eSim entry is shown, no mobile network message should
+  // no show.
+  if (IsQsRevampEnabled() && GetAddESimEntry()) {
+    ASSERT_THAT(GetMobileStatusMessage(), IsNull());
+  } else {
+    ASSERT_THAT(GetMobileStatusMessage(), NotNull());
+    EXPECT_EQ(l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NO_MOBILE_NETWORKS),
+              GetMobileStatusMessage()->label()->GetText());
+  }
   CheckMobileToggleButtonStatus(/*enabled=*/true, /*toggled_on=*/true);
   if (IsQsRevampEnabled()) {
     EXPECT_TRUE(network_list(NetworkType::kMobile)->GetVisible());
@@ -1058,9 +1151,14 @@ TEST_P(NetworkListViewControllerTest,
   cros_network()->ClearNetworksAndDevices();
   cros_network()->SetDeviceProperties(properties.Clone());
 
-  ASSERT_THAT(GetMobileStatusMessage(), NotNull());
-  EXPECT_EQ(l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NO_MOBILE_NETWORKS),
-            GetMobileStatusMessage()->label()->GetText());
+  if (IsQsRevampEnabled() && GetAddESimEntry()) {
+    ASSERT_THAT(GetMobileStatusMessage(), IsNull());
+  } else {
+    ASSERT_THAT(GetMobileStatusMessage(), NotNull());
+    EXPECT_EQ(l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NO_MOBILE_NETWORKS),
+              GetMobileStatusMessage()->label()->GetText());
+  }
+
   if (IsQsRevampEnabled()) {
     EXPECT_TRUE(GetQsMobileToggleButton()->GetVisible());
     EXPECT_TRUE(network_list(NetworkType::kMobile)->GetVisible());
@@ -1079,9 +1177,13 @@ TEST_P(NetworkListViewControllerTest,
   cros_network()->SetDeviceProperties(properties.Clone());
 
   // Message is shown when uninhibited.
-  ASSERT_THAT(GetMobileStatusMessage(), NotNull());
-  EXPECT_EQ(l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NO_MOBILE_NETWORKS),
-            GetMobileStatusMessage()->label()->GetText());
+  if (IsQsRevampEnabled() && GetAddESimEntry()) {
+    ASSERT_THAT(GetMobileStatusMessage(), IsNull());
+  } else {
+    ASSERT_THAT(GetMobileStatusMessage(), NotNull());
+    EXPECT_EQ(l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NO_MOBILE_NETWORKS),
+              GetMobileStatusMessage()->label()->GetText());
+  }
   CheckMobileToggleButtonStatus(/*enabled=*/true, /*toggled_on=*/true);
   if (IsQsRevampEnabled()) {
     EXPECT_TRUE(network_list(NetworkType::kMobile)->GetVisible());
@@ -1526,7 +1628,11 @@ TEST_P(NetworkListViewControllerTest, NetworkItemIsEnabled) {
 
   cros_network()->SetDeviceProperties(properties.Clone());
   ASSERT_THAT(GetMobileSubHeader(), NotNull());
-  EXPECT_TRUE(GetAddEsimButton()->GetEnabled());
+  if (IsQsRevampEnabled()) {
+    EXPECT_TRUE(GetAddESimEntry()->GetVisible());
+  } else {
+    EXPECT_TRUE(GetAddEsimButton()->GetVisible());
+  }
 
   cros_network()->AddNetworkAndDevice(
       CrosNetworkConfigTestHelper::CreateStandaloneNetworkProperties(

ash/system/network/network_utils.cc

@@ -11,6 +11,7 @@
 #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/notreached.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"
@@ -19,22 +20,24 @@ namespace ash {
 
 namespace {
 
-std::string GetNetworkTypeName(
-    chromeos::network_config::mojom::NetworkType network_type) {
+using chromeos::network_config::mojom::DeviceStateProperties;
+using chromeos::network_config::mojom::InhibitReason;
+using chromeos::network_config::mojom::NetworkType;
+
+std::string GetNetworkTypeName(NetworkType network_type) {
   switch (network_type) {
-    case chromeos::network_config::mojom::NetworkType::kCellular:
+    case NetworkType::kCellular:
       [[fallthrough]];
-    case chromeos::network_config::mojom::NetworkType::kTether:
+    case NetworkType::kTether:
       [[fallthrough]];
-    case chromeos::network_config::mojom::NetworkType::kMobile:
+    case NetworkType::kMobile:
       return "Mobile";
-    case chromeos::network_config::mojom::NetworkType::kWiFi:
+    case NetworkType::kWiFi:
       return "WiFi";
     default:
       // A network type of other is unexpected, and no success
       // metric for it exists.
-      NOTREACHED();
-      return "";
+      NOTREACHED_NORETURN();
   }
 }
 
@@ -53,6 +56,35 @@ int GetStringIdForNetworkDetailedViewTitleRow(
   }
 }
 
+int GetAddESimTooltipMessageId() {
+  const DeviceStateProperties* cellular_device =
+      Shell::Get()->system_tray_model()->network_state_model()->GetDevice(
+          NetworkType::kCellular);
+
+  CHECK(cellular_device);
+
+  switch (cellular_device->inhibit_reason) {
+    case InhibitReason::kInstallingProfile:
+      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_INSTALLING_PROFILE;
+    case InhibitReason::kRenamingProfile:
+      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_RENAMING_PROFILE;
+    case InhibitReason::kRemovingProfile:
+      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_REMOVING_PROFILE;
+    case InhibitReason::kConnectingToProfile:
+      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_CONNECTING_TO_PROFILE;
+    case InhibitReason::kRefreshingProfileList:
+      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_REFRESHING_PROFILE_LIST;
+    case InhibitReason::kNotInhibited:
+      return IDS_ASH_STATUS_TRAY_ADD_CELLULAR_LABEL;
+    case InhibitReason::kResettingEuiccMemory:
+      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_RESETTING_ESIM;
+    case InhibitReason::kDisablingProfile:
+      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_DISABLING_PROFILE;
+    case InhibitReason::kRequestingAvailableProfiles:
+      return IDS_ASH_STATUS_TRAY_INHIBITED_CELLULAR_REQUESTING_AVAILABLE_PROFILES;
+  }
+}
+
 void RecordNetworkRowClickedAction(NetworkRowClickedAction action) {
   base::UmaHistogramEnumeration("ChromeOS.SystemTray.Network.RowClickedAction",
                                 action);
@@ -63,9 +95,7 @@ void RecordDetailedViewSection(DetailedViewSection section) {
                                 section);
 }
 
-void RecordNetworkTypeToggled(
-    chromeos::network_config::mojom::NetworkType network_type,
-    bool new_state) {
+void RecordNetworkTypeToggled(NetworkType network_type, bool new_state) {
   const std::string network_name = GetNetworkTypeName(network_type);
 
   DCHECK(!network_name.empty());
@@ -106,8 +136,7 @@ bool IsNetworkDisabled(
   }
 
   if (!chromeos::network_config::NetworkTypeMatchesType(
-          network_properties->type,
-          chromeos::network_config::mojom::NetworkType::kCellular)) {
+          network_properties->type, NetworkType::kCellular)) {
     return false;
   }
 
@@ -142,15 +171,13 @@ bool IsNetworkInhibited(
     const chromeos::network_config::mojom::NetworkStatePropertiesPtr&
         network_properties) {
   if (!chromeos::network_config::NetworkTypeMatchesType(
-          network_properties->type,
-          chromeos::network_config::mojom::NetworkType::kCellular)) {
+          network_properties->type, 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);
+  const DeviceStateProperties* cellular_device =
+      Shell::Get()->system_tray_model()->network_state_model()->GetDevice(
+          NetworkType::kCellular);
 
   return cellular_device &&
          chromeos::network_config::IsInhibited(cellular_device);

ash/system/network/network_utils.h

@@ -43,6 +43,10 @@ ASH_EXPORT void RecordNetworkTypeToggled(
     chromeos::network_config::mojom::NetworkType network_type,
     bool new_state);
 
+// Returns the add esim button (`kQsRevamp` disabled) or add esim entry
+// (`kQsRevamp` enabled) tooltip message id.
+ASH_EXPORT int GetAddESimTooltipMessageId();
+
 // Returns the subtext to display for a connected network in a portal state.
 // This is used in the network menu, the tooltip, and for a11y.
 ASH_EXPORT absl::optional<std::u16string> GetPortalStateSubtext(

tools/metrics/actions/actions.xml

@@ -28556,6 +28556,15 @@ should be able to be added at any place in this file.
   </description>
 </action>
 
+<action name="QS_Subpage_Network_AddESim">
+  <owner>sylvieliu@google.com</owner>
+  <owner>cros-status-area-eng@google.com</owner>
+  <description>
+    User clicked on the Add ESim network entry in the quick settings network
+    subpage on ChromeOS.
+  </description>
+</action>
+
 <action name="QS_Subpage_Network_JoinNetwork">
   <owner>jiamingc@google.com</owner>
   <owner>cros-status-area-eng@google.com</owner>