Improve functionality of FakeShillProfileClient

Note: The fake shill client behaviors are tested by the higher level
tests that use them. They may be reaching a complexity level that merits
explicit testing, but it's not clear whether or not that would actually
be a win.

These changes are being added to support https://codereview.chromium.org/275543005/.

BUG=none
R=pneubeck@chromium.org

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

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

9 files changed, 217 insertions, 67 deletions

diff --git a/chromeos/dbus/fake_shill_manager_client.cc b/chromeos/dbus/fake_shill_manager_client.cc
index 452f16d..117f850 100644
--- a/chromeos/dbus/fake_shill_manager_client.cc
+++ b/chromeos/dbus/fake_shill_manager_client.cc
@@ -250,7 +250,10 @@ void FakeShillManagerClient::ConfigureService(
   if (!existing_properties) {
     // Add a new service to the service client stub because none exists, yet.
     // This calls AddManagerService.
-    service_client->AddServiceWithIPConfig(service_path, guid, type,
+    service_client->AddServiceWithIPConfig(service_path,
+                                           guid /* guid */,
+                                           guid /* name */,
+                                           type,
                                            shill::kStateIdle, ipconfig_path,
                                            true /* visible */,
                                            true /* watch */);
diff --git a/chromeos/dbus/fake_shill_profile_client.cc b/chromeos/dbus/fake_shill_profile_client.cc
index d0ff0f6..302eebce 100644
--- a/chromeos/dbus/fake_shill_profile_client.cc
+++ b/chromeos/dbus/fake_shill_profile_client.cc
@@ -22,8 +22,8 @@
 namespace chromeos {
 
 struct FakeShillProfileClient::ProfileProperties {
-  base::DictionaryValue entries;
-  base::DictionaryValue properties;
+  base::DictionaryValue entries;  // Dictionary of Service Dictionaries
+  base::DictionaryValue properties;  // Dictionary of Profile properties
 };
 
 namespace {
@@ -143,8 +143,7 @@ void FakeShillProfileClient::AddEntry(const std::string& profile_path,
   ProfileProperties* profile = GetProfile(dbus::ObjectPath(profile_path),
                                           ErrorCallback());
   DCHECK(profile);
-  profile->entries.SetWithoutPathExpansion(entry_path,
-                                           properties.DeepCopy());
+  profile->entries.SetWithoutPathExpansion(entry_path, properties.DeepCopy());
   DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface()->
       AddManagerService(entry_path, false /* visible */, false /* watch */);
 }
@@ -154,10 +153,37 @@ bool FakeShillProfileClient::AddService(const std::string& profile_path,
   ProfileProperties* profile = GetProfile(dbus::ObjectPath(profile_path),
                                           ErrorCallback());
   if (!profile) {
-    LOG(ERROR) << "No matching profile: " << profile_path;
+    LOG(ERROR) << "AddService: No matching profile: " << profile_path;
     return false;
   }
+  if (profile->entries.HasKey(service_path)) {
+    LOG(ERROR) << "AddService: Profile: " << profile_path
+               << " already contains Service: " << service_path;
+    return false;
+  }
+  return AddOrUpdateServiceImpl(profile_path, service_path, profile);
+}
 
+bool FakeShillProfileClient::UpdateService(const std::string& profile_path,
+                                           const std::string& service_path) {
+  ProfileProperties* profile = GetProfile(dbus::ObjectPath(profile_path),
+                                          ErrorCallback());
+  if (!profile) {
+    LOG(ERROR) << "UpdateService: No matching profile: " << profile_path;
+    return false;
+  }
+  if (!profile->entries.HasKey(service_path)) {
+    LOG(ERROR) << "UpdateService: Profile: " << profile_path
+               << " does not contain Service: " << service_path;
+    return false;
+  }
+  return AddOrUpdateServiceImpl(profile_path, service_path, profile);
+}
+
+bool FakeShillProfileClient::AddOrUpdateServiceImpl(
+    const std::string& profile_path,
+    const std::string& service_path,
+    ProfileProperties* profile) {
   ShillServiceClient::TestInterface* service_test =
       DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface();
   const base::DictionaryValue* service_properties =
@@ -169,16 +195,17 @@ bool FakeShillProfileClient::AddService(const std::string& profile_path,
   std::string service_profile_path;
   service_properties->GetStringWithoutPathExpansion(shill::kProfileProperty,
                                                     &service_profile_path);
-  if (!service_profile_path.empty() && service_profile_path != profile_path) {
+  if (service_profile_path.empty()) {
+    base::StringValue profile_path_value(profile_path);
+    service_test->SetServiceProperty(service_path,
+                                     shill::kProfileProperty,
+                                     profile_path_value);
+  } else if (service_profile_path != profile_path) {
     LOG(ERROR) << "Service has non matching profile path: "
                << service_profile_path;
     return false;
   }
 
-  base::StringValue profile_path_value(profile_path);
-  service_test->SetServiceProperty(service_path,
-                                   shill::kProfileProperty,
-                                   profile_path_value);
   profile->entries.SetWithoutPathExpansion(service_path,
                                            service_properties->DeepCopy());
   return true;
@@ -192,6 +219,29 @@ void FakeShillProfileClient::GetProfilePaths(
   }
 }
 
+bool FakeShillProfileClient::GetService(const std::string& service_path,
+                                        std::string* profile_path,
+                                        base::DictionaryValue* properties) {
+  properties->Clear();
+  for (ProfileMap::const_iterator iter = profiles_.begin();
+       iter != profiles_.end(); ++iter) {
+    const ProfileProperties* profile = iter->second;
+    const base::DictionaryValue* entry;
+    if (!profile->entries.GetDictionaryWithoutPathExpansion(
+            service_path, &entry)) {
+      continue;
+    }
+    *profile_path = iter->first;
+    properties->MergeDictionary(entry);
+    return true;
+  }
+  return false;
+}
+
+void FakeShillProfileClient::ClearProfiles() {
+  STLDeleteValues(&profiles_);
+}
+
 FakeShillProfileClient::ProfileProperties* FakeShillProfileClient::GetProfile(
     const dbus::ObjectPath& profile_path,
     const ErrorCallback& error_callback) {
diff --git a/chromeos/dbus/fake_shill_profile_client.h b/chromeos/dbus/fake_shill_profile_client.h
index 9353f50..65c8417 100644
--- a/chromeos/dbus/fake_shill_profile_client.h
+++ b/chromeos/dbus/fake_shill_profile_client.h
@@ -53,12 +53,22 @@ class CHROMEOS_EXPORT FakeShillProfileClient :
                         const base::DictionaryValue& properties) OVERRIDE;
   virtual bool AddService(const std::string& profile_path,
                           const std::string& service_path) OVERRIDE;
+  virtual bool UpdateService(const std::string& profile_path,
+                             const std::string& service_path) OVERRIDE;
   virtual void GetProfilePaths(std::vector<std::string>* profiles) OVERRIDE;
+  virtual bool GetService(const std::string& service_path,
+                          std::string* profile_path,
+                          base::DictionaryValue* properties) OVERRIDE;
+  virtual void ClearProfiles() OVERRIDE;
 
  private:
   struct ProfileProperties;
   typedef std::map<std::string, ProfileProperties*> ProfileMap;
 
+  bool AddOrUpdateServiceImpl(const std::string& profile_path,
+                              const std::string& service_path,
+                              ProfileProperties* profile);
+
   ProfileProperties* GetProfile(const dbus::ObjectPath& profile_path,
                                 const ErrorCallback& error_callback);
 
diff --git a/chromeos/dbus/fake_shill_service_client.cc b/chromeos/dbus/fake_shill_service_client.cc
index ef4ffe6..da3a810 100644
--- a/chromeos/dbus/fake_shill_service_client.cc
+++ b/chromeos/dbus/fake_shill_service_client.cc
@@ -13,6 +13,7 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/shill_device_client.h"
 #include "chromeos/dbus/shill_manager_client.h"
+#include "chromeos/dbus/shill_profile_client.h"
 #include "chromeos/dbus/shill_property_changed_observer.h"
 #include "chromeos/network/shill_property_util.h"
 #include "dbus/bus.h"
@@ -309,12 +310,14 @@ void FakeShillServiceClient::AddService(const std::string& service_path,
                                         const std::string& state,
                                         bool add_to_visible_list,
                                         bool add_to_watch_list) {
-  AddServiceWithIPConfig(service_path, name, type, state, "",
+  AddServiceWithIPConfig(service_path, "" /* guid */, name,
+                         type, state, "" /* ipconfig_path */,
                          add_to_visible_list, add_to_watch_list);
 }
 
 void FakeShillServiceClient::AddServiceWithIPConfig(
     const std::string& service_path,
+    const std::string& guid,
     const std::string& name,
     const std::string& type,
     const std::string& state,
@@ -330,27 +333,59 @@ void FakeShillServiceClient::AddServiceWithIPConfig(
   base::DictionaryValue* properties =
       GetModifiableServiceProperties(service_path, true);
   connect_behavior_.erase(service_path);
+
+  std::string profile_path;
+  base::DictionaryValue profile_properties;
+  if (DBusThreadManager::Get()->GetShillProfileClient()->GetTestInterface()->
+      GetService(service_path, &profile_path, &profile_properties)) {
+    properties->SetWithoutPathExpansion(
+        shill::kProfileProperty,
+        new base::StringValue(profile_path));
+  }
+
+  // If |guid| is provided, set Service.GUID to that. Otherwise if a GUID is
+  // stored in a profile entry, use that. Otherwise leave it blank. Shill does
+  // not enforce a valid guid, we do that at the NetworkStateHandler layer.
+  std::string guid_to_set = guid;
+  if (guid_to_set.empty()) {
+    profile_properties.GetStringWithoutPathExpansion(
+        shill::kGuidProperty, &guid_to_set);
+  }
+  if (!guid_to_set.empty()) {
+    properties->SetWithoutPathExpansion(shill::kGuidProperty,
+                                        new base::StringValue(guid_to_set));
+  }
   shill_property_util::SetSSID(name, properties);
   properties->SetWithoutPathExpansion(
       shill::kNameProperty,
-      base::Value::CreateStringValue(name));
+      new base::StringValue(name));
   properties->SetWithoutPathExpansion(
       shill::kDeviceProperty,
-      base::Value::CreateStringValue(device_path));
+      new base::StringValue(device_path));
   properties->SetWithoutPathExpansion(
       shill::kTypeProperty,
-      base::Value::CreateStringValue(type));
+      new base::StringValue(type));
   properties->SetWithoutPathExpansion(
       shill::kStateProperty,
-      base::Value::CreateStringValue(state));
+      new base::StringValue(state));
   if (!ipconfig_path.empty()) {
     properties->SetWithoutPathExpansion(
         shill::kIPConfigProperty,
-        base::Value::CreateStringValue(ipconfig_path));
+        new base::StringValue(ipconfig_path));
+  }
+  if (type == shill::kTypeWifi) {
+    properties->SetWithoutPathExpansion(
+        shill::kSecurityProperty,
+        new base::StringValue(shill::kSecurityNone));
   }
 
   DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface()->
       SortManagerServices();
+
+  if (!profile_path.empty()) {
+    DBusThreadManager::Get()->GetShillProfileClient()->GetTestInterface()->
+        UpdateService(profile_path, service_path);
+  }
 }
 
 void FakeShillServiceClient::RemoveService(const std::string& service_path) {
@@ -390,6 +425,24 @@ bool FakeShillServiceClient::SetServiceProperty(const std::string& service_path,
 
   dict->MergeDictionary(&new_properties);
 
+  // Add or update the profile entry.
+  if (property == shill::kProfileProperty) {
+    std::string profile_path;
+    if (value.GetAsString(&profile_path)) {
+      DBusThreadManager::Get()->GetShillProfileClient()->GetTestInterface()->
+          AddService(profile_path, service_path);
+    } else {
+      LOG(ERROR) << "Profile value is not a String!";
+    }
+  } else {
+    std::string profile_path;
+    if (dict->GetStringWithoutPathExpansion(
+            shill::kProfileProperty, &profile_path) && !profile_path.empty()) {
+      DBusThreadManager::Get()->GetShillProfileClient()->GetTestInterface()->
+          UpdateService(profile_path, service_path);
+    }
+  }
+
   // Notify the Manager if the state changed (affects DefaultService).
   if (property == shill::kStateProperty) {
     std::string state;
@@ -503,7 +556,7 @@ void FakeShillServiceClient::SetOtherServicesOffline(
       continue;
     properties->SetWithoutPathExpansion(
         shill::kStateProperty,
-        base::Value::CreateStringValue(shill::kStateIdle));
+        new base::StringValue(shill::kStateIdle));
   }
 }
 
diff --git a/chromeos/dbus/fake_shill_service_client.h b/chromeos/dbus/fake_shill_service_client.h
index 1f1f60d..65e932e 100644
--- a/chromeos/dbus/fake_shill_service_client.h
+++ b/chromeos/dbus/fake_shill_service_client.h
@@ -83,6 +83,7 @@ class CHROMEOS_EXPORT FakeShillServiceClient
                           bool add_to_visible_list,
                           bool add_to_watch_list) OVERRIDE;
   virtual void AddServiceWithIPConfig(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_profile_client.h b/chromeos/dbus/shill_profile_client.h
index b237fb0..9c0f925 100644
--- a/chromeos/dbus/shill_profile_client.h
+++ b/chromeos/dbus/shill_profile_client.h
@@ -58,14 +58,31 @@ class CHROMEOS_EXPORT ShillProfileClient : public DBusClient {
                           const base::DictionaryValue& properties) = 0;
 
     // Adds a service to the profile, copying properties from the
-    // ShillServiceClient entry (which must be present). Also sets the Profile
-    // property of the service in ShillServiceClient.
+    // ShillServiceClient entry matching |service_path|. Returns false if no
+    // Service entry exists or if a Profile entry already exists. Also sets
+    // the Profile property of the service in ShillServiceClient.
     virtual bool AddService(const std::string& profile_path,
                             const std::string& service_path) = 0;
 
+    // Copies properties from the ShillServiceClient entry matching
+    // |service_path| to the profile entry matching |profile_path|. Returns
+    // false if no Service entry exits or if no Profile entry exists.
+    virtual bool UpdateService(const std::string& profile_path,
+                               const std::string& service_path) = 0;
+
     // Sets |profiles| to the current list of profile paths.
     virtual void GetProfilePaths(std::vector<std::string>* profiles) = 0;
 
+    // Sets |properties| to the entry for |service_path|, sets |profile_path|
+    // to the path of the profile with the entry, and returns true if the
+    // service exists in any profile.
+    virtual bool GetService(const std::string& service_path,
+                            std::string* profile_path,
+                            base::DictionaryValue* properties) = 0;
+
+    // Remove all profile entries.
+    virtual void ClearProfiles() = 0;
+
    protected:
     virtual ~TestInterface() {}
   };
diff --git a/chromeos/dbus/shill_service_client.h b/chromeos/dbus/shill_service_client.h
index 4fa4922..5e2e6de 100644
--- a/chromeos/dbus/shill_service_client.h
+++ b/chromeos/dbus/shill_service_client.h
@@ -51,6 +51,7 @@ class CHROMEOS_EXPORT ShillServiceClient : public DBusClient {
                             bool add_to_visible_list,
                             bool add_to_watch_list) = 0;
     virtual void AddServiceWithIPConfig(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_state_handler_unittest.cc b/chromeos/network/network_state_handler_unittest.cc
index c0d6325..ca167d4 100644
--- a/chromeos/network/network_state_handler_unittest.cc
+++ b/chromeos/network/network_state_handler_unittest.cc
@@ -32,8 +32,8 @@ void ErrorCallbackFunction(const std::string& error_name,
 }
 
 const std::string kShillManagerClientStubDefaultService = "eth1";
-const std::string kShillManagerClientStubDefaultWireless = "wifi1";
-const std::string kShillManagerClientStubWireless2 = "wifi2";
+const std::string kShillManagerClientStubDefaultWifi = "wifi1";
+const std::string kShillManagerClientStubWifi2 = "wifi2";
 const std::string kShillManagerClientStubCellular = "cellular1";
 
 using chromeos::NetworkState;
@@ -139,7 +139,10 @@ namespace chromeos {
 class NetworkStateHandlerTest : public testing::Test {
  public:
   NetworkStateHandlerTest()
-      : device_test_(NULL), manager_test_(NULL), service_test_(NULL) {}
+      : device_test_(NULL),
+        manager_test_(NULL),
+        profile_test_(NULL),
+        service_test_(NULL) {}
   virtual ~NetworkStateHandlerTest() {}
 
   virtual void SetUp() OVERRIDE {
@@ -165,6 +168,15 @@ class NetworkStateHandlerTest : public testing::Test {
   }
 
  protected:
+  void AddService(const std::string& service_path,
+                  const std::string& name,
+                  const std::string& type,
+                  const std::string& state) {
+    service_test_->AddService(service_path, name, type, state,
+                              true /* add_to_visible */,
+                              true /* add_to_watchlist */);
+  }
+
   void SetupDefaultShillState() {
     message_loop_.RunUntilIdle();  // Process any pending updates
     device_test_ =
@@ -181,36 +193,37 @@ class NetworkStateHandlerTest : public testing::Test {
         DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface();
     ASSERT_TRUE(manager_test_);
 
+    profile_test_ =
+        DBusThreadManager::Get()->GetShillProfileClient()->GetTestInterface();
+    ASSERT_TRUE(profile_test_);
+    profile_test_->ClearProfiles();
+
     service_test_ =
         DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface();
     ASSERT_TRUE(service_test_);
     service_test_->ClearServices();
-    const bool add_to_visible = true;
-    const bool add_to_watchlist = true;
-    service_test_->AddService(kShillManagerClientStubDefaultService,
-                              kShillManagerClientStubDefaultService,
-                              shill::kTypeEthernet,
-                              shill::kStateOnline,
-                              add_to_visible,
-                              add_to_watchlist);
-    service_test_->AddService(kShillManagerClientStubDefaultWireless,
-                              kShillManagerClientStubDefaultWireless,
-                              shill::kTypeWifi,
-                              shill::kStateOnline,
-                              add_to_visible,
-                              add_to_watchlist);
-    service_test_->AddService(kShillManagerClientStubWireless2,
-                              kShillManagerClientStubWireless2,
-                              shill::kTypeWifi,
-                              shill::kStateIdle,
-                              add_to_visible,
-                              add_to_watchlist);
-    service_test_->AddService(kShillManagerClientStubCellular,
-                              kShillManagerClientStubCellular,
-                              shill::kTypeCellular,
-                              shill::kStateIdle,
-                              add_to_visible,
-                              add_to_watchlist);
+    AddService(kShillManagerClientStubDefaultService,
+               kShillManagerClientStubDefaultService,
+               shill::kTypeEthernet,
+               shill::kStateOnline);
+    AddService(kShillManagerClientStubDefaultWifi,
+               kShillManagerClientStubDefaultWifi,
+               shill::kTypeWifi,
+               shill::kStateOnline);
+    AddService(kShillManagerClientStubWifi2,
+               kShillManagerClientStubWifi2,
+               shill::kTypeWifi,
+               shill::kStateIdle);
+    AddService(kShillManagerClientStubCellular,
+               kShillManagerClientStubCellular,
+               shill::kTypeCellular,
+               shill::kStateIdle);
+  }
+
+  void UpdateManagerProperties() {
+    message_loop_.RunUntilIdle();
+    network_state_handler_->UpdateManagerProperties();
+    message_loop_.RunUntilIdle();
   }
 
   base::MessageLoopForUI message_loop_;
@@ -218,6 +231,7 @@ class NetworkStateHandlerTest : public testing::Test {
   scoped_ptr<TestObserver> test_observer_;
   ShillDeviceClient::TestInterface* device_test_;
   ShillManagerClient::TestInterface* manager_test_;
+  ShillProfileClient::TestInterface* profile_test_;
   ShillServiceClient::TestInterface* service_test_;
 
  private:
@@ -238,9 +252,9 @@ TEST_F(NetworkStateHandlerTest, NetworkStateHandlerStub) {
   EXPECT_EQ(kShillManagerClientStubDefaultService,
             network_state_handler_->ConnectedNetworkByType(
                 NetworkTypePattern::Ethernet())->path());
-  EXPECT_EQ(kShillManagerClientStubDefaultWireless,
+  EXPECT_EQ(kShillManagerClientStubDefaultWifi,
             network_state_handler_->ConnectedNetworkByType(
-                NetworkTypePattern::Wireless())->path());
+                NetworkTypePattern::WiFi())->path());
   EXPECT_EQ(kShillManagerClientStubCellular,
             network_state_handler_->FirstNetworkByType(
                 NetworkTypePattern::Mobile())->path());
@@ -345,13 +359,11 @@ TEST_F(NetworkStateHandlerTest, ServicePropertyChanged) {
 
 TEST_F(NetworkStateHandlerTest, FavoriteState) {
   // Set the profile entry of a service
-  const std::string wifi1 = kShillManagerClientStubDefaultWireless;
-  ShillProfileClient::TestInterface* profile_test =
-      DBusThreadManager::Get()->GetShillProfileClient()->GetTestInterface();
-  EXPECT_TRUE(profile_test->AddService("/profile/default", wifi1));
-  message_loop_.RunUntilIdle();
-  network_state_handler_->UpdateManagerProperties();
-  message_loop_.RunUntilIdle();
+  const std::string profile = "/profile/profile1";
+  const std::string wifi1 = kShillManagerClientStubDefaultWifi;
+  profile_test_->AddProfile(profile, "" /* userhash */);
+  EXPECT_TRUE(profile_test_->AddService(profile, wifi1));
+  UpdateManagerProperties();
   EXPECT_EQ(1u, test_observer_->favorite_count());
 }
 
@@ -375,7 +387,7 @@ TEST_F(NetworkStateHandlerTest, NetworkConnectionStateChanged) {
 
 TEST_F(NetworkStateHandlerTest, DefaultServiceDisconnected) {
   const std::string eth1 = kShillManagerClientStubDefaultService;
-  const std::string wifi1 = kShillManagerClientStubDefaultWireless;
+  const std::string wifi1 = kShillManagerClientStubDefaultWifi;
 
   // Disconnect ethernet.
   test_observer_->reset_network_change_count();
@@ -399,7 +411,7 @@ TEST_F(NetworkStateHandlerTest, DefaultServiceDisconnected) {
 
 TEST_F(NetworkStateHandlerTest, DefaultServiceConnected) {
   const std::string eth1 = kShillManagerClientStubDefaultService;
-  const std::string wifi1 = kShillManagerClientStubDefaultWireless;
+  const std::string wifi1 = kShillManagerClientStubDefaultWifi;
 
   // Disconnect ethernet and wifi.
   base::StringValue connection_state_idle_value(shill::kStateIdle);
@@ -429,7 +441,7 @@ TEST_F(NetworkStateHandlerTest, DefaultServiceChanged) {
 
   // Change the default network by changing Manager.DefaultService.
   test_observer_->reset_network_change_count();
-  const std::string wifi1 = kShillManagerClientStubDefaultWireless;
+  const std::string wifi1 = kShillManagerClientStubDefaultWifi;
   base::StringValue wifi1_value(wifi1);
   manager_test_->SetManagerProperty(
       shill::kDefaultServiceProperty, wifi1_value);
@@ -468,14 +480,13 @@ TEST_F(NetworkStateHandlerTest, DefaultServiceChanged) {
 }
 
 TEST_F(NetworkStateHandlerTest, RequestUpdate) {
-  // Request an update for kShillManagerClientStubDefaultWireless.
+  // Request an update for kShillManagerClientStubDefaultWifi.
   EXPECT_EQ(1, test_observer_->PropertyUpdatesForService(
-      kShillManagerClientStubDefaultWireless));
+      kShillManagerClientStubDefaultWifi));
   network_state_handler_->RequestUpdateForNetwork(
-      kShillManagerClientStubDefaultWireless);
+      kShillManagerClientStubDefaultWifi);
   message_loop_.RunUntilIdle();
   EXPECT_EQ(2, test_observer_->PropertyUpdatesForService(
-      kShillManagerClientStubDefaultWireless));
+      kShillManagerClientStubDefaultWifi));
 }
-
 }  // namespace chromeos
diff --git a/chromeos/network/shill_property_handler_unittest.cc b/chromeos/network/shill_property_handler_unittest.cc
index 7655d84..313e161 100644
--- a/chromeos/network/shill_property_handler_unittest.cc
+++ b/chromeos/network/shill_property_handler_unittest.cc
@@ -229,7 +229,11 @@ class ShillPropertyHandlerTest : public testing::Test {
                               const std::string& ipconfig_path,
                               bool add_to_watch_list) {
     ASSERT_TRUE(IsValidType(type));
-    service_test_->AddServiceWithIPConfig(id, id, type, state,
+    service_test_->AddServiceWithIPConfig(id, /* service_path */
+                                          "" /* guid */,
+                                          id /* name */,
+                                          type,
+                                          state,
                                           ipconfig_path,
                                           true /* visible */,
                                           add_to_watch_list);