CrOS Network UI: Fix configure logic

This is a fix for logic introduced in:
https://chromium-review.googlesource.com/c/chromium/src/+/2350315

It also includes a unit test that would have caught the regression.

BUG=1112814

Change-Id: I057f01f29892278231f36e6abb5334692e8ea401
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2378459Reviewed-by: Azeem Arshad <azeemarshad@chromium.org>
Reviewed-by: Jon Mann <jonmann@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802368}

chromeos/network/network_connect.cc

@@ -124,17 +124,16 @@ void NetworkConnectImpl::HandleUnconfiguredNetwork(
   }
 
   if (network->type() == shill::kTypeWifi) {
-    // If the network does not require a password, do not show the dialog since
-    // there is nothing to configure. Likewise, if the network is the underlying
-    // Wi-Fi hotspot for a Tether network, do not show the dialog since the
-    // Tether component handles this case itself.
-    if (!network->IsSecure() && network->tether_guid().empty())
+    // If the network requires a password and is not the underlying Wi-Fi
+    // hotspot for a Tether network, show the configure dialog.
+    if (network->IsSecure() && network->tether_guid().empty())
       delegate_->ShowNetworkConfigure(network_id);
     return;
   }
 
   if (network->type() == shill::kTypeVPN) {
-    // Third-party VPNs handle configuration UI themselves.
+    // Show the configure dialog for non third-party VPNs (which provide their
+    // own configuration UI).
     if (network->GetVpnProviderType() != shill::kProviderThirdPartyVpn)
       delegate_->ShowNetworkConfigure(network_id);
     return;

chromeos/network/network_connect_unittest.cc

@@ -36,6 +36,9 @@ namespace {
 const char kWiFi1ServicePath[] = "/service/wifi1";
 const char kWiFi1Guid[] = "wifi1_guid";
 
+const char kWiFiUnconfiguredServicePath[] = "/service/wifi_unconfigured";
+const char kWiFiUnconfiguredGuid[] = "wifi_unconfigured_guid";
+
 const char kCellular1DevicePath[] = "/device/stub_cellular_device1";
 const char kCellular1ServicePath[] = "/service/cellular1";
 const char kCellular1Guid[] = "cellular1_guid";
@@ -144,6 +147,21 @@ class NetworkConnectTest : public testing::Test {
     service_test_->SetServiceProperty(
         kWiFi1ServicePath, shill::kPassphraseProperty, base::Value("password"));
 
+    // Create an unconfigured wifi network.
+    service_test_->AddService(kWiFiUnconfiguredServicePath,
+                              kWiFiUnconfiguredGuid, "wifi_unconfigured",
+                              shill::kTypeWifi, shill::kStateIdle,
+                              add_to_visible);
+    service_test_->SetServiceProperty(kWiFiUnconfiguredServicePath,
+                                      shill::kSecurityClassProperty,
+                                      base::Value(shill::kSecurityWep));
+    service_test_->SetServiceProperty(kWiFiUnconfiguredServicePath,
+                                      shill::kConnectableProperty,
+                                      base::Value(false));
+    service_test_->SetServiceProperty(kWiFiUnconfiguredServicePath,
+                                      shill::kErrorProperty,
+                                      base::Value("bad-password"));
+
     // Create a cellular network.
     service_test_->AddService(kCellular1ServicePath, kCellular1Guid,
                               "cellular1", shill::kTypeCellular,
@@ -188,6 +206,14 @@ TEST_F(NetworkConnectTest, ConnectToNetworkId_NoConfiguration) {
   NetworkConnect::Get()->ConnectToNetworkId("bad guid");
 }
 
+TEST_F(NetworkConnectTest, ConnectToNetworkId_Unconfigured) {
+  EXPECT_CALL(*mock_delegate_, ShowNetworkConfigure(_)).Times(1);
+  EXPECT_CALL(*mock_delegate_, ShowNetworkConnectError(_, _)).Times(0);
+
+  NetworkConnect::Get()->ConnectToNetworkId(kWiFiUnconfiguredGuid);
+  base::RunLoop().RunUntilIdle();
+}
+
 TEST_F(NetworkConnectTest, ConfigureAndConnectToNetwork_NoConfiguration) {
   EXPECT_CALL(*mock_delegate_,
               ShowNetworkConnectError(NetworkConnectionHandler::kErrorNotFound,