WebUI: QuickSettings: Portal network shows signin on click, update text

This CL updates the quick settings to show the portal sign in when a
network list item is clicked that is in a portal state. Along with this,
the message is updated depending on if the network is in a portal state
or not.

This CL also updated the NetworkRowClickedAction to include a metric for
opening the portal signin from the list of networks in the quick
settings.

Screenshots:
1. No Internet or Portal Suspected subtext:
https://screenshot.googleplex.com/xCD29YTZLoTpKEn.png
2. Portal or Proxy Auth subtext:
https://screenshot.googleplex.com/6Y3oPsHKmKvNqYQ.png
3. Clicking on Portal subtext network list item:
https://screenshot.googleplex.com/6PmRxrpKHF7qDwi

BUG=b:244353766
TEST=ash_unittests --gtest_filter="*NetworkDetailedViewController*",
ash_unittests --gtest_filter="*NetworkListNetworkItemView*"

Change-Id: Ifa9823a0e71c4929f165233dd6bcf13051ee9853
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3930732
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Andre Le <leandre@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Michael Rygiel <michaelrygiel@google.com>
Cr-Commit-Position: refs/heads/main@{#1055816}

ash/ash_strings.grd

@@ -2243,6 +2243,12 @@ Connect your device to power.
       <message name="IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTING" desc="The label used when a network connection is connecting. [CHAR_LIMIT=14]">
         Connecting...
       </message>
+      <message name="IDS_ASH_STATUS_TRAY_NETWORK_STATUS_SIGNIN" desc="The label used when a network connection is behind a captive portal and requires signin.">
+        Sign in to network
+      </message>
+      <message name="IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED_NO_INTERNET" desc="The label used when a network connection is connected but has no internet.">
+        Connected, no internet
+      </message>
       <message name="IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CLICK_TO_ACTIVATE" desc="The label used in quick settings to indicate an unactivated cellular network that can be clicked to launch activation through setup flow. [CHAR_LIMIT=29]">
         Click to activate
       </message>

ash/ash_strings_grd/IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED_NO_INTERNET.png.sha1

@@ -0,0 +1 @@
+19e50f480eedf134a5797d5898be49e162a4d7b4

ash/ash_strings_grd/IDS_ASH_STATUS_TRAY_NETWORK_STATUS_SIGNIN.png.sha1

@@ -0,0 +1 @@
+cfe8d5de077d41934797f2c5ed75d0cdf594422a

ash/system/network/network_detailed_view_controller.cc

@@ -39,6 +39,7 @@ using ::chromeos::network_config::mojom::DeviceStateType;
 using ::chromeos::network_config::mojom::NetworkStateProperties;
 using ::chromeos::network_config::mojom::NetworkStatePropertiesPtr;
 using ::chromeos::network_config::mojom::NetworkType;
+using ::chromeos::network_config::mojom::PortalState;
 
 void LogUserNetworkEvent(const NetworkStateProperties& network) {
   auto* const logger = ml::UserSettingsEventLogger::Get();
@@ -71,6 +72,11 @@ bool NetworkTypeIsConfigurable(NetworkType type) {
   return false;
 }
 
+bool IsNetworkBehindPortalOrProxy(PortalState portalState) {
+  return portalState == PortalState::kPortal ||
+         portalState == PortalState::kProxyAuthRequired;
+}
+
 bool IsNetworkConnectable(const NetworkStatePropertiesPtr& network_properties) {
   // The network must not already be connected to be able to be connected to.
   if (network_properties->connection_state !=
@@ -166,6 +172,17 @@ void NetworkDetailedViewController::OnNetworkListItemSelected(
       return;
     }
 
+    // If the captive portal UI flag is enabled, the network is connected, and
+    // the network is in a portal or proxy state, the user is shown the portal
+    // signin.
+    if (features::IsCaptivePortalUI2022Enabled() &&
+        chromeos::network_config::StateIsConnected(network->connection_state) &&
+        IsNetworkBehindPortalOrProxy(network->portal_state)) {
+      RecordNetworkRowClickedAction(NetworkRowClickedAction::kOpenPortalSignin);
+      NetworkConnect::Get()->ShowPortalSignin(network->guid);
+      return;
+    }
+
     if (IsNetworkConnectable(network)) {
       base::RecordAction(
           UserMetricsAction("StatusArea_Network_ConnectConfigured"));

ash/system/network/network_detailed_view_controller_unittest.cc

@@ -34,10 +34,15 @@ using ::chromeos::network_config::mojom::ActivationStateType;
 using ::chromeos::network_config::mojom::ConnectionStateType;
 using ::chromeos::network_config::mojom::NetworkStatePropertiesPtr;
 using ::chromeos::network_config::mojom::NetworkType;
+using ::chromeos::network_config::mojom::PortalState;
 
 const std::string kCellular = "cellular";
 constexpr char kCellularDevicePath[] = "/device/cellular_device";
+
 constexpr char kWifi[] = "Wifi";
+constexpr char kServicePatternWiFi[] = R"({
+    "GUID": "%s", "Type": "wifi", "State": "%s", "Strength": 100,
+            "Connectable": true})";
 
 constexpr char kTetherName[] = "tether";
 constexpr char kTetherGuid[] = "tetherNetworkGuid";
@@ -74,10 +79,17 @@ class NetworkConnectTestDelegate : public NetworkConnect::Delegate {
   }
   void ShowMobileSetupDialog(const std::string& network_id) override {}
   void ShowCarrierAccountDetail(const std::string& network_id) override {}
-  void ShowPortalSignin(const std::string& network_id) override {}
+  void ShowPortalSignin(const std::string& network_id) override {
+    portal_signin_guid_ = network_id;
+  }
   void ShowNetworkConnectError(const std::string& error_name,
                                const std::string& network_id) override {}
   void ShowMobileActivationError(const std::string& network_id) override {}
+
+  const std::string& portal_signin_guid() const { return portal_signin_guid_; }
+
+ private:
+  std::string portal_signin_guid_;
 };
 
 }  // namespace
@@ -94,17 +106,6 @@ class NetworkDetailedViewControllerTest : public AshTestBase {
     NetworkHandler::Initialize();
     base::RunLoop().RunUntilIdle();
 
-    // Creating a service here, since we would be testing that wifi,
-    // networks which can be connected to are actually connected to. This
-    // checks that NetworkConnect eventually connects us to the
-    // network.
-    wifi_service_path_ =
-        network_state_helper()->ConfigureService(base::StringPrintf(
-            R"({"GUID": "%s", "Type": "wifi",
-            "State": "idle", "Strength": 100,
-            "Connectable": true})",
-            kWifi));
-
     network_connect_delegate_ = std::make_unique<NetworkConnectTestDelegate>();
     NetworkConnect::Initialize(network_connect_delegate_.get());
     AshTestBase::SetUp();
@@ -226,6 +227,11 @@ class NetworkDetailedViewControllerTest : public AshTestBase {
         kTetherGuid, kWifiServiceGuid);
   }
 
+  void AddWifiService(std::string state) {
+    wifi_service_path_ = network_state_helper()->ConfigureService(
+        base::StringPrintf(kServicePatternWiFi, kWifi, state.c_str()));
+  }
+
   void SetBluetoothAdapterState(BluetoothSystemState system_state) {
     bluetooth_config_test_helper()
         ->fake_adapter_state_controller()
@@ -239,6 +245,11 @@ class NetworkDetailedViewControllerTest : public AshTestBase {
         ->GetAdapterState();
   }
 
+  const std::string& portal_signin_guid() const {
+    return network_connect_delegate_->portal_signin_guid();
+  }
+  base::test::ScopedFeatureList feature_list_;
+
  private:
   NetworkStateHandler* network_state_handler() {
     return network_state_helper()->network_state_handler();
@@ -252,7 +263,6 @@ class NetworkDetailedViewControllerTest : public AshTestBase {
     return ash_test_helper()->bluetooth_config_test_helper();
   }
 
-  base::test::ScopedFeatureList feature_list_;
   std::unique_ptr<chromeos::network_config::CrosNetworkConfigTestHelper>
       network_config_helper_;
   std::unique_ptr<NetworkConnectTestDelegate> network_connect_delegate_;
@@ -391,6 +401,7 @@ TEST_F(NetworkDetailedViewControllerTest, WifiNetworkListItemSelected) {
       NetworkRowClickedAction::kConnectToNetwork,
       /*count=*/0u, /*total_count=*/0u);
 
+  AddWifiService(shill::kStateIdle);
   // Clicking on an already connected network opens settings page.
   // Since this network is already connected, selecting this network
   // in network list vew should result in no change in NetworkState of
@@ -597,4 +608,34 @@ TEST_F(NetworkDetailedViewControllerTest, MobileToggleClicked) {
             GetTechnologyState(NetworkTypePattern::Tether()));
 }
 
+TEST_F(NetworkDetailedViewControllerTest,
+       PortalNetworkListItemSelectedWithFlagEnabled) {
+  feature_list_.Reset();
+  feature_list_.InitWithFeatures(
+      /*enabled_features=*/{features::kCaptivePortalUI2022,
+                            features::kQuickSettingsNetworkRevamp},
+      /*disabled_features=*/{});
+
+  AddWifiService(shill::kStateRedirectFound);
+
+  NetworkStatePropertiesPtr wifi_network = CreateStandaloneNetworkProperties(
+      kWifi, NetworkType::kWiFi, ConnectionStateType::kPortal);
+  wifi_network->portal_state = PortalState::kPortal;
+
+  SelectNetworkListItem(wifi_network);
+
+  // Wait for Network to be connected to.
+  base::RunLoop().RunUntilIdle();
+
+  // Verify that guid is set from ShowPortalSignin.
+  EXPECT_EQ(portal_signin_guid(), kWifi);
+  EXPECT_EQ(0, GetSystemTrayClient()->show_network_settings_count());
+  EXPECT_EQ(0, GetSystemTrayClient()->show_sim_unlock_settings_count());
+  EXPECT_EQ(shill::kStateRedirectFound, GetWifiNetworkState());
+
+  CheckRowClickedActionHistogramBuckets(
+      NetworkRowClickedAction::kOpenPortalSignin,
+      /*count=*/1u, /*total_count=*/1u);
+}
+
 }  // namespace ash

ash/system/network/network_list_network_item_view.cc

@@ -48,6 +48,7 @@ using chromeos::network_config::mojom::NetworkStateProperties;
 using chromeos::network_config::mojom::NetworkStatePropertiesPtr;
 using chromeos::network_config::mojom::NetworkType;
 using chromeos::network_config::mojom::OncSource;
+using chromeos::network_config::mojom::PortalState;
 using chromeos::network_config::mojom::ProxyMode;
 using chromeos::network_config::mojom::SecurityType;
 
@@ -342,11 +343,39 @@ void NetworkListNetworkItemView::SetupCellularSubtext() {
 }
 
 void NetworkListNetworkItemView::SetupNetworkSubtext() {
-  if (StateIsConnected(network_properties()->connection_state)) {
+  if (network_properties()->connection_state ==
-    SetupConnectedScrollListItem(this);
+      ConnectionStateType::kConnecting) {
-  } else if (network_properties_.get()->connection_state ==
-             ConnectionStateType::kConnecting) {
     SetupConnectingScrollListItem(this);
+    return;
+  }
+
+  if (!StateIsConnected(network_properties()->connection_state)) {
+    return;
+  }
+
+  if (!ash::features::IsCaptivePortalUI2022Enabled()) {
+    SetupConnectedScrollListItem(this);
+    return;
+  }
+
+  switch (network_properties()->portal_state) {
+    // Portal state is portal or proxy auth, setup signin subtext.
+    case PortalState::kPortal:
+    case PortalState::kProxyAuthRequired:
+      SetWarningSubText(this, l10n_util::GetStringUTF16(
+                                  IDS_ASH_STATUS_TRAY_NETWORK_STATUS_SIGNIN));
+      return;
+    // Portal state is portal-suspected or no internet, setup no internet
+    // subtext.
+    case PortalState::kPortalSuspected:
+    case PortalState::kNoInternet:
+      SetWarningSubText(
+          this, l10n_util::GetStringUTF16(
+                    IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED_NO_INTERNET));
+      return;
+    default:
+      SetupConnectedScrollListItem(this);
+      return;
   }
 }
 

ash/system/network/network_list_network_item_view_unittest.cc

@@ -17,6 +17,7 @@
 #include "ash/system/network/network_icon.h"
 #include "ash/system/network/network_info.h"
 #include "ash/test/ash_test_base.h"
+#include "base/bind.h"
 #include "base/i18n/number_formatting.h"
 #include "base/test/scoped_feature_list.h"
 #include "chromeos/services/network_config/public/cpp/cros_network_config_test_helper.h"
@@ -42,6 +43,7 @@ using chromeos::network_config::mojom::ConnectionStateType;
 using chromeos::network_config::mojom::NetworkStatePropertiesPtr;
 using chromeos::network_config::mojom::NetworkType;
 using chromeos::network_config::mojom::OncSource;
+using chromeos::network_config::mojom::PortalState;
 using chromeos::network_config::mojom::SecurityType;
 
 const std::string kWiFiName = "WiFi";
@@ -142,7 +144,7 @@ class NetworkListNetworkItemViewTest : public AshTestBase {
     return network_list_network_item_view_;
   }
 
- private:
+ protected:
   void SetUpDefaultNetworkDevices() {
     network_state_helper()->ClearDevices();
     network_state_helper()->AddDevice(kCellularDevicePath, shill::kTypeCellular,
@@ -270,6 +272,98 @@ TEST_F(NetworkListNetworkItemViewTest, HasCorrectCellularSublabel) {
             network_list_network_item_view()->sub_text_label()->GetText());
 }
 
+TEST_F(NetworkListNetworkItemViewTest, HasCorrectPortalSublabel) {
+  EXPECT_FALSE(network_list_network_item_view()->sub_text_label());
+
+  NetworkStatePropertiesPtr wifi_network = CreateStandaloneNetworkProperties(
+      kWiFiName, NetworkType::kWiFi, ConnectionStateType::kPortal);
+  wifi_network->portal_state = PortalState::kPortal;
+
+  UpdateViewForNetwork(wifi_network);
+  EXPECT_TRUE(network_list_network_item_view()->sub_text_label());
+  EXPECT_EQ(
+      l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED),
+      network_list_network_item_view()->sub_text_label()->GetText());
+}
+
+TEST_F(NetworkListNetworkItemViewTest, HasCorrectPortalSublabelWithFlag) {
+  feature_list_.Reset();
+  feature_list_.InitWithFeatures(
+      /*enabled_features=*/{features::kCaptivePortalUI2022,
+                            features::kQuickSettingsNetworkRevamp},
+      /*disabled_features=*/{});
+  EXPECT_FALSE(network_list_network_item_view()->sub_text_label());
+
+  NetworkStatePropertiesPtr wifi_network = CreateStandaloneNetworkProperties(
+      kWiFiName, NetworkType::kWiFi, ConnectionStateType::kPortal);
+  wifi_network->portal_state = PortalState::kPortal;
+
+  UpdateViewForNetwork(wifi_network);
+  EXPECT_TRUE(network_list_network_item_view()->sub_text_label());
+  EXPECT_EQ(
+      l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_STATUS_SIGNIN),
+      network_list_network_item_view()->sub_text_label()->GetText());
+}
+
+TEST_F(NetworkListNetworkItemViewTest, HasCorrectProxyAuthSublabelWithFlag) {
+  feature_list_.Reset();
+  feature_list_.InitWithFeatures(
+      /*enabled_features=*/{features::kCaptivePortalUI2022,
+                            features::kQuickSettingsNetworkRevamp},
+      /*disabled_features=*/{});
+  EXPECT_FALSE(network_list_network_item_view()->sub_text_label());
+
+  NetworkStatePropertiesPtr wifi_network = CreateStandaloneNetworkProperties(
+      kWiFiName, NetworkType::kWiFi, ConnectionStateType::kPortal);
+  wifi_network->portal_state = PortalState::kProxyAuthRequired;
+
+  UpdateViewForNetwork(wifi_network);
+  EXPECT_TRUE(network_list_network_item_view()->sub_text_label());
+  EXPECT_EQ(
+      l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_STATUS_SIGNIN),
+      network_list_network_item_view()->sub_text_label()->GetText());
+}
+
+TEST_F(NetworkListNetworkItemViewTest,
+       HasCorrectPortalSuspectedSublabelWithFlag) {
+  feature_list_.Reset();
+  feature_list_.InitWithFeatures(
+      /*enabled_features=*/{features::kCaptivePortalUI2022,
+                            features::kQuickSettingsNetworkRevamp},
+      /*disabled_features=*/{});
+  EXPECT_FALSE(network_list_network_item_view()->sub_text_label());
+
+  NetworkStatePropertiesPtr wifi_network = CreateStandaloneNetworkProperties(
+      kWiFiName, NetworkType::kWiFi, ConnectionStateType::kPortal);
+  wifi_network->portal_state = PortalState::kPortalSuspected;
+
+  UpdateViewForNetwork(wifi_network);
+  EXPECT_TRUE(network_list_network_item_view()->sub_text_label());
+  EXPECT_EQ(l10n_util::GetStringUTF16(
+                IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED_NO_INTERNET),
+            network_list_network_item_view()->sub_text_label()->GetText());
+}
+
+TEST_F(NetworkListNetworkItemViewTest,
+       HasCorrectNoConnectivitySublabelWithFlag) {
+  feature_list_.Reset();
+  feature_list_.InitWithFeatures(
+      /*enabled_features=*/{features::kCaptivePortalUI2022,
+                            features::kQuickSettingsNetworkRevamp},
+      /*disabled_features=*/{});
+  EXPECT_FALSE(network_list_network_item_view()->sub_text_label());
+
+  NetworkStatePropertiesPtr wifi_network = CreateStandaloneNetworkProperties(
+      kWiFiName, NetworkType::kWiFi, ConnectionStateType::kPortal);
+  wifi_network->portal_state = PortalState::kNoInternet;
+
+  UpdateViewForNetwork(wifi_network);
+  EXPECT_TRUE(network_list_network_item_view()->sub_text_label());
+  EXPECT_EQ(l10n_util::GetStringUTF16(
+                IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED_NO_INTERNET),
+            network_list_network_item_view()->sub_text_label()->GetText());
+}
+
 TEST_F(NetworkListNetworkItemViewTest, NotifiesListenerWhenClicked) {
   EXPECT_FALSE(LastClickedNetworkListItem());
   LeftClickOn(network_list_network_item_view());

ash/system/network/network_utils.h

@@ -17,7 +17,8 @@ enum class NetworkRowClickedAction {
   kConnectToNetwork = 0,
   kOpenNetworkSettingsPage = 1,
   kOpenSimUnlockDialog = 2,
-  kMaxValue = kOpenSimUnlockDialog
+  kOpenPortalSignin = 3,
+  kMaxValue = kOpenPortalSignin
 };
 
 // This enum is tied directly to a UMA enum |DetailedViewSection| defined in

ash/system/tray/tray_utils.cc

@@ -66,6 +66,16 @@ void SetupConnectingScrollListItem(HoverHighlightView* view) {
       l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTING));
 }
 
+void SetWarningSubText(HoverHighlightView* view, std::u16string subtext) {
+  DCHECK(view->is_populated());
+
+  view->SetSubText(subtext);
+  view->sub_text_label()->SetAutoColorReadabilityEnabled(false);
+  view->sub_text_label()->SetEnabledColor(
+      AshColorProvider::Get()->GetContentLayerColor(
+          AshColorProvider::ContentLayerType::kTextColorWarning));
+}
+
 SkColor TrayIconColor(session_manager::SessionState session_state) {
   if (!features::IsDarkLightModeEnabled() &&
       session_state == session_manager::SessionState::OOBE) {

ash/system/tray/tray_utils.h

@@ -37,6 +37,9 @@ void SetupConnectedScrollListItem(HoverHighlightView* view,
 // accessibility label.
 void SetupConnectingScrollListItem(HoverHighlightView* view);
 
+// Add `subtext` with warning color to `view`.
+void SetWarningSubText(HoverHighlightView* view, std::u16string subtext);
+
 // Gets the current tray icon color for the given session state.
 SkColor TrayIconColor(session_manager::SessionState session_state);
 

tools/metrics/histograms/enums.xml

@@ -71153,6 +71153,7 @@ Called by update_net_trust_anchors.py.-->
   <int value="0" label="Connect to Network"/>
   <int value="1" label="Open Network settings page"/>
   <int value="2" label="Open SIM unlock dialog"/>
+  <int value="3" label="Open portal signin"/>
 </enum>
 
 <enum name="NetworkSandboxState">