Use GUID in captive portal API instead of service path

Also changes the fake implementations and other tests to explicitly specify guids.

BUG=none
For components/wifi/fake_wifi_service.cc:
TBR=mef@chromium.org

Review URL: https://codereview.chromium.org/377063003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282995 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 82 insertions, 45 deletions

diff --git a/chromeos/dbus/fake_shill_manager_client.cc b/chromeos/dbus/fake_shill_manager_client.cc
index 226a778..71d2dc0 100644
--- a/chromeos/dbus/fake_shill_manager_client.cc
+++ b/chromeos/dbus/fake_shill_manager_client.cc
@@ -109,7 +109,7 @@ const char* kNetworkDisabled = "disabled";
 }  // namespace
 
 // static
-const char FakeShillManagerClient::kFakeEthernetNetworkPath[] = "/service/eth1";
+const char FakeShillManagerClient::kFakeEthernetNetworkGuid[] = "eth1_guid";
 
 FakeShillManagerClient::FakeShillManagerClient()
     : interactive_delay_(0),
@@ -267,7 +267,8 @@ void FakeShillManagerClient::ConfigureService(
                                            guid /* guid */,
                                            guid /* name */,
                                            type,
-                                           shill::kStateIdle, ipconfig_path,
+                                           shill::kStateIdle,
+                                           ipconfig_path,
                                            true /* visible */);
     existing_properties = service_client->GetServiceProperties(service_path);
   }
@@ -617,7 +618,10 @@ void FakeShillManagerClient::SetupDefaultEnvironment() {
     devices->SetDeviceProperty("/device/eth1",
                                shill::kIPConfigsProperty,
                                eth_ip_configs);
-    services->AddService(kFakeEthernetNetworkPath, "eth1",
+    const std::string kFakeEthernetNetworkPath = "/service/eth1";
+    services->AddService(kFakeEthernetNetworkPath,
+                         kFakeEthernetNetworkGuid,
+                         "eth1" /* name */,
                          shill::kTypeEthernet,
                          state,
                          add_to_visible);
@@ -646,7 +650,8 @@ void FakeShillManagerClient::SetupDefaultEnvironment() {
 
     const std::string kWifi1Path = "/service/wifi1";
     services->AddService(kWifi1Path,
-                         "wifi1",
+                         "wifi1_guid",
+                         "wifi1" /* name */,
                          shill::kTypeWifi,
                          state,
                          add_to_visible);
@@ -660,7 +665,8 @@ void FakeShillManagerClient::SetupDefaultEnvironment() {
 
     const std::string kWifi2Path = "/service/wifi2";
     services->AddService(kWifi2Path,
-                         "wifi2_PSK",
+                         "wifi2_PSK_guid",
+                         "wifi2_PSK" /* name */,
                          shill::kTypeWifi,
                          shill::kStateIdle,
                          add_to_visible);
@@ -676,7 +682,8 @@ void FakeShillManagerClient::SetupDefaultEnvironment() {
     if (portaled) {
       const std::string kPortaledWifiPath = "/service/portaled_wifi";
       services->AddService(kPortaledWifiPath,
-                           "Portaled Wifi",
+                           "portaled_wifi_guid",
+                           "Portaled Wifi" /* name */,
                            shill::kTypeWifi,
                            shill::kStatePortal,
                            add_to_visible);
@@ -701,7 +708,8 @@ void FakeShillManagerClient::SetupDefaultEnvironment() {
         "/device/wimax1", shill::kTypeWimax, "stub_wimax_device1");
 
     services->AddService("/service/wimax1",
-                         "wimax1",
+                         "wimax1_guid",
+                         "wimax1" /* name */,
                          shill::kTypeWimax,
                          state,
                          add_to_visible);
@@ -726,7 +734,8 @@ void FakeShillManagerClient::SetupDefaultEnvironment() {
                                base::StringValue(shill::kCarrierSprint));
 
     services->AddService("/service/cellular1",
-                         "cellular1",
+                         "cellular1_guid",
+                         "cellular1" /* name */,
                          shill::kTypeCellular,
                          state,
                          add_to_visible);
@@ -768,7 +777,8 @@ void FakeShillManagerClient::SetupDefaultEnvironment() {
     provider_properties.SetString(shill::kHostProperty, "vpn_host");
 
     services->AddService("/service/vpn1",
-                         "vpn1",
+                         "vpn1_guid",
+                         "vpn1" /* name */,
                          shill::kTypeVPN,
                          state,
                          add_to_visible);
@@ -777,7 +787,8 @@ void FakeShillManagerClient::SetupDefaultEnvironment() {
     profiles->AddService(shared_profile, "/service/vpn1");
 
     services->AddService("/service/vpn2",
-                         "vpn2",
+                         "vpn2_guid",
+                         "vpn2" /* name */,
                          shill::kTypeVPN,
                          shill::kStateIdle,
                          add_to_visible);
diff --git a/chromeos/dbus/fake_shill_manager_client.h b/chromeos/dbus/fake_shill_manager_client.h
index c3bd618..4a94199 100644
--- a/chromeos/dbus/fake_shill_manager_client.h
+++ b/chromeos/dbus/fake_shill_manager_client.h
@@ -106,7 +106,7 @@ class CHROMEOS_EXPORT FakeShillManagerClient
       const std::string& service_path) OVERRIDE;
 
   // Constants used for testing.
-  static const char kFakeEthernetNetworkPath[];
+  static const char kFakeEthernetNetworkGuid[];
 
  private:
   void SetDefaultProperties();
diff --git a/chromeos/dbus/fake_shill_service_client.cc b/chromeos/dbus/fake_shill_service_client.cc
index 605f372..9f4bbc3 100644
--- a/chromeos/dbus/fake_shill_service_client.cc
+++ b/chromeos/dbus/fake_shill_service_client.cc
@@ -305,11 +305,12 @@ ShillServiceClient::TestInterface* FakeShillServiceClient::GetTestInterface() {
 // ShillServiceClient::TestInterface overrides.
 
 void FakeShillServiceClient::AddService(const std::string& service_path,
+                                        const std::string& guid,
                                         const std::string& name,
                                         const std::string& type,
                                         const std::string& state,
                                         bool visible) {
-  AddServiceWithIPConfig(service_path, "" /* guid */, name,
+  AddServiceWithIPConfig(service_path, guid, name,
                          type, state, "" /* ipconfig_path */,
                          visible);
 }
@@ -602,6 +603,7 @@ void FakeShillServiceClient::ContinueConnect(
   if (ContainsKey(connect_behavior_, service_path)) {
     const base::Closure& custom_connect_behavior =
         connect_behavior_[service_path];
+    VLOG(1) << "Running custom connect behavior for " << service_path;
     custom_connect_behavior.Run();
     return;
   }
@@ -625,6 +627,7 @@ void FakeShillServiceClient::ContinueConnect(
             base::StringValue(shill::kErrorBadPassphrase)));
   } else {
     // Set Online.
+    VLOG(1) << "Setting state to Online " << service_path;
     SetServiceProperty(service_path,
                        shill::kStateProperty,
                        base::StringValue(shill::kStateOnline));
diff --git a/chromeos/dbus/fake_shill_service_client.h b/chromeos/dbus/fake_shill_service_client.h
index dfacec7..b2692a4 100644
--- a/chromeos/dbus/fake_shill_service_client.h
+++ b/chromeos/dbus/fake_shill_service_client.h
@@ -77,6 +77,7 @@ class CHROMEOS_EXPORT FakeShillServiceClient
 
   // ShillServiceClient::TestInterface overrides.
   virtual void AddService(const std::string& service_path,
+                          const std::string& guid,
                           const std::string& name,
                           const std::string& type,
                           const std::string& state,
diff --git a/chromeos/dbus/shill_service_client.h b/chromeos/dbus/shill_service_client.h
index e8b950f..a2e8950 100644
--- a/chromeos/dbus/shill_service_client.h
+++ b/chromeos/dbus/shill_service_client.h
@@ -45,6 +45,7 @@ class CHROMEOS_EXPORT ShillServiceClient : public DBusClient {
    public:
     // Adds a Service to the Manager and Service stubs.
     virtual void AddService(const std::string& service_path,
+                            const std::string& guid,
                             const std::string& name,
                             const std::string& type,
                             const std::string& state,
diff --git a/chromeos/network/network_cert_migrator_unittest.cc b/chromeos/network/network_cert_migrator_unittest.cc
index 0dcc68d..1482ec3 100644
--- a/chromeos/network/network_cert_migrator_unittest.cc
+++ b/chromeos/network/network_cert_migrator_unittest.cc
@@ -104,13 +104,19 @@ class NetworkCertMigratorTest : public testing::Test {
     network_cert_migrator_->Init(network_state_handler_.get());
   }
 
+  void AddService(const std::string& network_id,
+                  const std::string& type,
+                  const std::string& state) {
+    service_test_->AddService(network_id /* service_path */,
+                              network_id /* guid */,
+                              network_id /* name */,
+                              type,
+                              state,
+                              true /* add_to_visible */);
+  }
+
   void SetupWifiWithNss() {
-    const bool add_to_visible = true;
-    service_test_->AddService(kWifiStub,
-                              kWifiStub,
-                              shill::kTypeWifi,
-                              shill::kStateOnline,
-                              add_to_visible);
+    AddService(kWifiStub, shill::kTypeWifi, shill::kStateOnline);
     service_test_->SetServiceProperty(kWifiStub,
                                       shill::kEapCaCertNssProperty,
                                       base::StringValue(kNSSNickname));
@@ -131,12 +137,7 @@ class NetworkCertMigratorTest : public testing::Test {
   }
 
   void SetupVpnWithNss(bool open_vpn) {
-    const bool add_to_visible = true;
-    service_test_->AddService(kVPNStub,
-                              kVPNStub,
-                              shill::kTypeVPN,
-                              shill::kStateIdle,
-                              add_to_visible);
+    AddService(kVPNStub, shill::kTypeVPN, shill::kStateIdle);
     base::DictionaryValue provider;
     const char* nss_property = open_vpn ? shill::kOpenVPNCaCertNSSProperty
                                         : shill::kL2tpIpsecCaCertNssProperty;
diff --git a/chromeos/network/network_state_handler_unittest.cc b/chromeos/network/network_state_handler_unittest.cc
index c0e3ab7..619a284 100644
--- a/chromeos/network/network_state_handler_unittest.cc
+++ b/chromeos/network/network_state_handler_unittest.cc
@@ -176,10 +176,11 @@ class NetworkStateHandlerTest : public testing::Test {
 
  protected:
   void AddService(const std::string& service_path,
+                  const std::string& guid,
                   const std::string& name,
                   const std::string& type,
                   const std::string& state) {
-    service_test_->AddService(service_path, name, type, state,
+    service_test_->AddService(service_path, guid, name, type, state,
                               true /* add_to_visible */);
   }
 
@@ -209,18 +210,22 @@ class NetworkStateHandlerTest : public testing::Test {
     ASSERT_TRUE(service_test_);
     service_test_->ClearServices();
     AddService(kShillManagerClientStubDefaultService,
+               "eth1_guid",
                "eth1",
                shill::kTypeEthernet,
                shill::kStateOnline);
     AddService(kShillManagerClientStubDefaultWifi,
+               "wifi1_guid",
                "wifi1",
                shill::kTypeWifi,
                shill::kStateOnline);
     AddService(kShillManagerClientStubWifi2,
+               "wifi2_guid",
                "wifi2",
                shill::kTypeWifi,
                shill::kStateIdle);
     AddService(kShillManagerClientStubCellular,
+               "cellular1_guid",
                "cellular1",
                shill::kTypeCellular,
                shill::kStateIdle);
@@ -281,8 +286,11 @@ TEST_F(NetworkStateHandlerTest, GetNetworkList) {
   // Add a non-visible network to the profile.
   const std::string profile = "/profile/profile1";
   const std::string wifi_favorite_path = "/service/wifi_faviorite";
-  service_test_->AddService(wifi_favorite_path, "wifi_faviorite",
-                            shill::kTypeWifi, shill::kStateIdle,
+  service_test_->AddService(wifi_favorite_path,
+                            "wifi_faviorite_guid",
+                            "wifi_faviorite",
+                            shill::kTypeWifi,
+                            shill::kStateIdle,
                             false /* add_to_visible */);
   profile_test_->AddProfile(profile, "" /* userhash */);
   EXPECT_TRUE(profile_test_->AddService(profile, wifi_favorite_path));
@@ -362,8 +370,11 @@ TEST_F(NetworkStateHandlerTest, GetVisibleNetworks) {
   // Add a non-visible network to the profile.
   const std::string profile = "/profile/profile1";
   const std::string wifi_favorite_path = "/service/wifi_faviorite";
-  service_test_->AddService(wifi_favorite_path, "wifi_faviorite",
-                            shill::kTypeWifi, shill::kStateIdle,
+  service_test_->AddService(wifi_favorite_path,
+                            "wifi_faviorite_guid",
+                            "wifi_faviorite",
+                            shill::kTypeWifi,
+                            shill::kStateIdle,
                             false /* add_to_visible */);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(kNumShillManagerClientStubImplServices + 1,
@@ -634,18 +645,13 @@ TEST_F(NetworkStateHandlerTest, RequestUpdate) {
 TEST_F(NetworkStateHandlerTest, NetworkGuidInProfile) {
   const std::string profile = "/profile/profile1";
   const std::string wifi_path = "/service/wifi_with_guid";
-  const std::string wifi_guid = "WIFI_GUID";
+  const std::string wifi_guid = "wifi_guid";
+  const std::string wifi_name = "WifiWithGuid";
   const bool is_service_configured = true;
 
   // Add a network to the default Profile with a specified GUID.
-  service_test_->AddServiceWithIPConfig(
-      wifi_path,
-      wifi_guid,
-      wifi_path  /* name */,
-      shill::kTypeWifi,
-      shill::kStateOnline,
-      "" /* ipconfig_path */,
-      true /* add_to_visible */);
+  AddService(wifi_path, wifi_guid, wifi_name,
+             shill::kTypeWifi, shill::kStateOnline);
   profile_test_->AddProfile(profile, "" /* userhash */);
   EXPECT_TRUE(profile_test_->AddService(profile, wifi_path));
   UpdateManagerProperties();
@@ -664,7 +670,8 @@ TEST_F(NetworkStateHandlerTest, NetworkGuidInProfile) {
 
   // Add the service (simulating a network coming back in range) and verify that
   // the NetworkState was created with the same GUID.
-  AddService(wifi_path, wifi_path, shill::kTypeWifi, shill::kStateOnline);
+  AddService(wifi_path, "" /* guid */, wifi_name,
+             shill::kTypeWifi, shill::kStateOnline);
   UpdateManagerProperties();
   network = network_state_handler_->GetNetworkStateFromServicePath(
       wifi_path, is_service_configured);
@@ -674,10 +681,12 @@ TEST_F(NetworkStateHandlerTest, NetworkGuidInProfile) {
 
 TEST_F(NetworkStateHandlerTest, NetworkGuidNotInProfile) {
   const std::string wifi_path = "/service/wifi_with_guid";
+  const std::string wifi_name = "WifiWithGuid";
   const bool is_service_configured = false;
 
-  // Add a network without adding it to a profile.
-  AddService(wifi_path, wifi_path, shill::kTypeWifi, shill::kStateOnline);
+  // Add a network without specifying a GUID or adding it to a profile.
+  AddService(wifi_path, "" /* guid */, wifi_name,
+             shill::kTypeWifi, shill::kStateOnline);
   UpdateManagerProperties();
 
   // Verify that a NetworkState exists with an assigned GUID.
@@ -695,7 +704,8 @@ TEST_F(NetworkStateHandlerTest, NetworkGuidNotInProfile) {
 
   // Add the service (simulating a network coming back in range) and verify that
   // the NetworkState was created with the same GUID.
-  AddService(wifi_path, wifi_path, shill::kTypeWifi, shill::kStateOnline);
+  AddService(wifi_path, "" /* guid */, wifi_name,
+             shill::kTypeWifi, shill::kStateOnline);
   UpdateManagerProperties();
   network = network_state_handler_->GetNetworkStateFromServicePath(
       wifi_path, is_service_configured);
diff --git a/chromeos/network/shill_property_handler_unittest.cc b/chromeos/network/shill_property_handler_unittest.cc
index 341b9ca..e807531 100644
--- a/chromeos/network/shill_property_handler_unittest.cc
+++ b/chromeos/network/shill_property_handler_unittest.cc
@@ -216,7 +216,12 @@ class ShillPropertyHandlerTest : public testing::Test {
                   const std::string& state) {
     VLOG(2) << "AddService: " << type << ": " << id << ": " << state;
     ASSERT_TRUE(IsValidType(type));
-    service_test_->AddService(id, id, type, state, true /* visible */);
+    service_test_->AddService(id /* service_path */,
+                              id /* guid */,
+                              id /* name */,
+                              type,
+                              state,
+                              true /* visible */);
   }
 
   void AddServiceWithIPConfig(const std::string& type,
@@ -225,7 +230,7 @@ class ShillPropertyHandlerTest : public testing::Test {
                               const std::string& ipconfig_path) {
     ASSERT_TRUE(IsValidType(type));
     service_test_->AddServiceWithIPConfig(id, /* service_path */
-                                          "" /* guid */,
+                                          id /* guid */,
                                           id /* name */,
                                           type,
                                           state,
@@ -236,7 +241,12 @@ class ShillPropertyHandlerTest : public testing::Test {
   void AddServiceToProfile(const std::string& type,
                            const std::string& id,
                            bool visible) {
-    service_test_->AddService(id, id, type, shill::kStateIdle, visible);
+    service_test_->AddService(id /* service_path */,
+                              id /* guid */,
+                              id /* name */,
+                              type,
+                              shill::kStateIdle,
+                              visible);
     std::vector<std::string> profiles;
     profile_test_->GetProfilePaths(&profiles);
     ASSERT_TRUE(profiles.size() > 0);