shill: wifi: Load hidden services from storage

When a device or profile comes into existence, the device
will search the profile for hidden services and instantiate
services if they do not exist.  These services will not
be visible in the RPC service list until they either appear
in scan or are actively being connected.

Side effects:
  * Manager now loads the devices with profile data.
  * Manager now respects the "powered" attribute loaded by
    devices from the profile to determine whether or not
    to call Start() on them.
  * Key files can be searched for groups with a certain
    key.  This will be useful when we start supporting GUIDs.
  * On service registration go backward (from top of stack
    downward) through the list of profiles searching for
    configuration instead of forwards.
  * Move the update of the "Services" property of the manager
    to a more centralized spot in SortServices.  This way,
    when the service order changes (or anything else that
    affects the service list) this RPC property will update.
  * Hidden services are not scanned for if they are in the
    ephemeral profile -- it means that whatever profile was
    supporting them does not exist anymore.
  * WiFi services have the unenviable task of also decoding
    storage identifiers in order to glean the address, mode
    and security parameters.

BUG=chromium-os:22073,chromium-os:22074
TEST=New unit tests, Manual on real hardware.
Note: I could not connect to a hidden network, but this
is because we're not setting the ApScan parameter on
wpa_supplicant so on connect it is just doing broadcast
scans.  However if I seed the profile with with a record
containing a hidden SSID, shill will force a scan for
the hidden network, which will allow it to connect.

Change-Id: I97a5b5f6db7c6e6d2aabf212c5c2984ce7f4daaa
Reviewed-on: https://gerrit.chromium.org/gerrit/11558
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Commit-Ready: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/device.h b/device.h
index 4f3d3c0..4e0f26a 100644
--- a/device.h
+++ b/device.h
@@ -77,6 +77,7 @@
   const std::string &link_name() const { return link_name_; }
   int interface_index() const { return interface_index_; }
   const ConnectionRefPtr &connection() const { return connection_; }
+  bool powered() const { return powered_; }
 
   const std::string &FriendlyName() const;
 
@@ -90,7 +91,12 @@
 
   EventDispatcher *dispatcher() const { return dispatcher_; }
 
-  bool Load(StoreInterface *storage);
+  // Load configuration for the device from |storage|.  This may include
+  // instantiating non-visible services for which configuration has been
+  // stored.
+  virtual bool Load(StoreInterface *storage);
+
+  // Save configuration for the device to |storage|.
   virtual bool Save(StoreInterface *storage);
 
   void set_dhcp_provider(DHCPProvider *provider) { dhcp_provider_ = provider; }
@@ -103,6 +109,7 @@
   FRIEND_TEST(DeviceTest, Save);
   FRIEND_TEST(DeviceTest, SelectedService);
   FRIEND_TEST(DeviceTest, Stop);
+  FRIEND_TEST(ManagerTest, DeviceRegistrationAndStart);
   FRIEND_TEST(WiFiMainTest, Connect);
 
   // If there's an IP configuration in |ipconfig_|, releases the IP address and
@@ -139,6 +146,7 @@
   // Property getters reserved for subclasses
   ControlInterface *control_interface() const { return control_interface_; }
   Manager *manager() const { return manager_; }
+  bool running() const { return running_; }
 
  private:
   friend class DeviceAdaptorInterface;
@@ -162,14 +170,14 @@
   std::string GetRpcConnectionIdentifier();
 
   // Properties
-  bool powered_;  // TODO(pstew): Is this what |running_| is for?
+  bool powered_;  // indicates whether the device is configured to operate
   bool reconnect_;
   const std::string hardware_address_;
 
   PropertyStore store_;
 
   const int interface_index_;
-  bool running_;
+  bool running_;  // indicates whether the device is actually in operation
   const std::string link_name_;
   const std::string unique_id_;
   ControlInterface *control_interface_;
diff --git a/glib.cc b/glib.cc
index b06cfb5..101ff7d 100644
--- a/glib.cc
+++ b/glib.cc
@@ -108,6 +108,13 @@
   return g_key_file_has_group(key_file, group_name);
 }
 
+gboolean GLib::KeyFileHasKey(GKeyFile *key_file,
+                             const gchar *group_name,
+                             const gchar *key,
+                             GError **error) {
+  return g_key_file_has_key(key_file, group_name, key, error);
+}
+
 GKeyFile *GLib::KeyFileNew() {
   return g_key_file_new();
 }
diff --git a/glib.h b/glib.h
index ee30cc9..673ba63 100644
--- a/glib.h
+++ b/glib.h
@@ -69,6 +69,11 @@
   // g_key_file_has_group
   virtual gboolean KeyFileHasGroup(GKeyFile *key_file,
                                    const gchar *group_name);
+  // g_key_file_has_key
+  virtual gboolean KeyFileHasKey(GKeyFile *key_file,
+                                 const gchar *group_name,
+                                 const gchar *key,
+                                 GError **error);
   // g_key_file_load_from_file
   virtual gboolean KeyFileLoadFromFile(GKeyFile *key_file,
                                        const gchar *file,
diff --git a/key_file_store.cc b/key_file_store.cc
index a83d151..9fc2d19 100644
--- a/key_file_store.cc
+++ b/key_file_store.cc
@@ -95,7 +95,7 @@
 set<string> KeyFileStore::GetGroups() {
   CHECK(key_file_);
   gsize length = 0;
-  gchar **groups = g_key_file_get_groups(key_file_, &length);
+  gchar **groups = glib_->KeyFileGetGroups(key_file_, &length);
   if (!groups) {
     LOG(ERROR) << "Unable to obtain groups.";
     return set<string>();
@@ -105,6 +105,19 @@
   return group_set;
 }
 
+// Returns a set so that caller can easily test whether a particular group
+// is contained within this collection.
+set<string> KeyFileStore::GetGroupsWithKey(const string &key) {
+  set<string> groups = GetGroups();
+  set<string> groups_with_key;
+  for (set<string>::iterator it = groups.begin(); it != groups.end(); ++it) {
+    if (glib_->KeyFileHasKey(key_file_, (*it).c_str(), key.c_str(), NULL)) {
+      groups_with_key.insert(*it);
+    }
+  }
+  return groups_with_key;
+}
+
 bool KeyFileStore::ContainsGroup(const string &group) {
   CHECK(key_file_);
   return glib_->KeyFileHasGroup(key_file_, group.c_str());
diff --git a/key_file_store.h b/key_file_store.h
index a293c0b..0400468 100644
--- a/key_file_store.h
+++ b/key_file_store.h
@@ -44,6 +44,7 @@
   // Inherited from StoreInterface.
   virtual bool Flush();
   virtual std::set<std::string> GetGroups();
+  virtual std::set<std::string> GetGroupsWithKey(const std::string &key);
   virtual bool ContainsGroup(const std::string &group);
   virtual bool DeleteKey(const std::string &group, const std::string &key);
   virtual bool DeleteGroup(const std::string &group);
diff --git a/key_file_store_unittest.cc b/key_file_store_unittest.cc
index 4733ba1..d9806d9 100644
--- a/key_file_store_unittest.cc
+++ b/key_file_store_unittest.cc
@@ -107,6 +107,36 @@
   ASSERT_TRUE(store_.Close());
 }
 
+TEST_F(KeyFileStoreTest, GetGroupsWithKey) {
+  static const char kGroupA[] = "g-a";
+  static const char kGroupB[] = "g-b";
+  static const char kGroupC[] = "g-c";
+  static const char kKeyA[] = "k-a";
+  static const char kKeyB[] = "k-b";
+  static const char kValue[] = "true";
+  WriteKeyFile(base::StringPrintf("[%s]\n"
+                                  "%s=%s\n"
+                                  "[%s]\n"
+                                  "%s=%s\n"
+                                  "%s=%s\n"
+                                  "[%s]\n"
+                                  "%s=%s\n",
+                                  kGroupA, kKeyA, kValue,
+                                  kGroupB, kKeyA, kValue, kKeyB, kValue,
+                                  kGroupC, kKeyB, kValue));
+  EXPECT_TRUE(store_.IsNonEmpty());
+  ASSERT_TRUE(store_.Open());
+  set<string> groups_a = store_.GetGroupsWithKey(kKeyA);
+  EXPECT_EQ(2, groups_a.size());
+  EXPECT_TRUE(ContainsKey(groups_a, kGroupA));
+  EXPECT_TRUE(ContainsKey(groups_a, kGroupB));
+  set<string> groups_b = store_.GetGroupsWithKey(kKeyB);
+  EXPECT_EQ(2, groups_b.size());
+  EXPECT_TRUE(ContainsKey(groups_b, kGroupB));
+  EXPECT_TRUE(ContainsKey(groups_b, kGroupC));
+  ASSERT_TRUE(store_.Close());
+}
+
 TEST_F(KeyFileStoreTest, ContainsGroup) {
   static const char kGroupA[] = "group-a";
   static const char kGroupB[] = "group-b";
diff --git a/manager.cc b/manager.cc
index b61f7aa..43d0341 100644
--- a/manager.cc
+++ b/manager.cc
@@ -148,7 +148,7 @@
   device_info_.Stop();
 }
 
-void Manager::CreateProfile(const std::string &name, Error *error) {
+void Manager::CreateProfile(const string &name, Error *error) {
   Profile::Identifier ident;
   if (!Profile::ParseIdentifier(name, &ident)) {
     Error::PopulateAndLog(error, Error::kInvalidArguments,
@@ -216,13 +216,17 @@
   profiles_.push_back(profile);
 
   // Offer each registered Service the opportunity to join this new Profile.
-  vector<ServiceRefPtr>::iterator it;
-  for (it = services_.begin(); it != services_.end(); ++it) {
+  for (vector<ServiceRefPtr>::iterator it = services_.begin();
+       it != services_.end(); ++it) {
     profile->ConfigureService(*it);
   }
 
-  // TODO(pstew): Now shop the Profile contents around to Devices which
-  // can create non-visible services.
+  // Shop the Profile contents around to Devices which can create
+  // non-visible services.
+  for (vector<DeviceRefPtr>::iterator it = devices_.begin();
+       it != devices_.end(); ++it) {
+    profile->ConfigureDevice(*it);
+  }
 }
 
 void Manager::PopProfileInternal() {
@@ -240,12 +244,13 @@
       }
       if (p_it == profiles_.rend()) {
         ephemeral_profile_->AdoptService(*s_it);
+        (*s_it)->Unload();
       }
     }
   }
 }
 
-void Manager::PopProfile(const std::string &name, Error *error) {
+void Manager::PopProfile(const string &name, Error *error) {
   Profile::Identifier ident;
   if (profiles_.empty()) {
     Error::PopulateAndLog(error, Error::kNotFound, "Profile stack is empty");
@@ -294,16 +299,25 @@
   }
   devices_.push_back(to_manage);
 
-  // TODO(pstew): Should check configuration
-  if (running_)
-    to_manage->Start();
-
-  // NB: Should we keep a ptr to default profile and only persist that here?
+  // We are applying device properties from the DefaultProfile, and adding
+  // the union of hidden services in all loaded profiles to the device.
   for (vector<ProfileRefPtr>::iterator it = profiles_.begin();
        it != profiles_.end();
        ++it) {
+    // Load device configuration, if any exists, as well as hidden services.
+    (*it)->ConfigureDevice(to_manage);
+
+    // Currently the only profile for which "Save" is implemented is the
+    // DefaultProfile.  It iterates over all Devices and stores their state.
+    // We perform the Save now in case the device we have just registered
+    // is new and needs to be added to the stored DefaultProfile.
     (*it)->Save();
   }
+
+  // In normal usage, running_ will always be true when we are here, however
+  // unit tests sometimes do things in otherwise invalid states.
+  if (running_ && to_manage->powered())
+    to_manage->Start();
 }
 
 void Manager::DeregisterDevice(const DeviceRefPtr &to_forget) {
@@ -323,8 +337,8 @@
   VLOG(2) << __func__ << to_manage->UniqueName();
 
   bool configured = false;
-  for (vector<ProfileRefPtr>::iterator it = profiles_.begin();
-       !configured && it != profiles_.end();
+  for (vector<ProfileRefPtr>::reverse_iterator it = profiles_.rbegin();
+       !configured && it != profiles_.rend();
        ++it) {
     configured = (*it)->ConfigureService(to_manage);
   }
@@ -340,13 +354,6 @@
   }
   services_.push_back(to_manage);
   SortServices();
-
-  vector<string> service_paths;
-  for (it = services_.begin(); it != services_.end(); ++it) {
-    service_paths.push_back((*it)->GetRpcIdentifier());
-  }
-  adaptor_->EmitRpcIdentifierArrayChanged(flimflam::kServicesProperty,
-                                          service_paths);
 }
 
 void Manager::DeregisterService(const ServiceRefPtr &to_forget) {
@@ -406,6 +413,16 @@
 
 void Manager::SortServices() {
   sort(services_.begin(), services_.end(), ServiceSorter(technology_order_));
+
+  vector<string> service_paths;
+  vector<ServiceRefPtr>::iterator it;
+  for (it = services_.begin(); it != services_.end(); ++it) {
+    if ((*it)->IsVisible()) {
+      service_paths.push_back((*it)->GetRpcIdentifier());
+    }
+  }
+  adaptor_->EmitRpcIdentifierArrayChanged(flimflam::kServicesProperty,
+                                          service_paths);
 }
 
 string Manager::CalculateState(Error */*error*/) {
@@ -460,7 +477,7 @@
 // called via RPC (e.g., from ManagerDBusAdaptor)
 WiFiServiceRefPtr Manager::GetWifiService(const KeyValueStore &args,
                                           Error *error) {
-  std::vector<DeviceRefPtr> wifi_devices;
+  vector<DeviceRefPtr> wifi_devices;
   FilterByTechnology(Technology::kWifi, &wifi_devices);
   if (wifi_devices.empty()) {
     error->Populate(Error::kInvalidArguments, kManagerErrorNoDevice);
diff --git a/manager.h b/manager.h
index 5df868c..c24f300 100644
--- a/manager.h
+++ b/manager.h
@@ -96,6 +96,7 @@
  private:
   friend class ManagerAdaptorInterface;
   friend class ManagerTest;
+  FRIEND_TEST(ManagerTest, DeviceRegistrationAndStart);
   FRIEND_TEST(ManagerTest, PushPopProfile);
   FRIEND_TEST(ManagerTest, SortServices);
 
diff --git a/manager_unittest.cc b/manager_unittest.cc
index c88897b..82bae32 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -173,6 +173,27 @@
   EXPECT_TRUE(IsDeviceRegistered(mock_devices_[2], Technology::kCellular));
 }
 
+TEST_F(ManagerTest, DeviceRegistrationAndStart) {
+  manager()->running_ = true;
+  mock_devices_[0]->powered_ = true;
+  mock_devices_[1]->powered_ = false;
+  EXPECT_CALL(*mock_devices_[0].get(), Start())
+      .Times(1);
+  EXPECT_CALL(*mock_devices_[1].get(), Start())
+      .Times(0);
+  manager()->RegisterDevice(mock_devices_[0]);
+  manager()->RegisterDevice(mock_devices_[1]);
+}
+
+TEST_F(ManagerTest, DeviceRegistrationWithProfile) {
+  MockProfile *profile = new MockProfile(control_interface(), manager(), "");
+  DeviceRefPtr device_ref(mock_devices_[0].get());
+  AdoptProfile(manager(), profile);  // Passes ownership.
+  EXPECT_CALL(*profile, ConfigureDevice(device_ref));
+  EXPECT_CALL(*profile, Save());
+  manager()->RegisterDevice(mock_devices_[0]);
+}
+
 TEST_F(ManagerTest, DeviceDeregistration) {
   ON_CALL(*mock_devices_[0].get(), TechnologyIs(Technology::kEthernet))
       .WillByDefault(Return(true));
diff --git a/mock_device.h b/mock_device.h
index f4e356e..b5053ee 100644
--- a/mock_device.h
+++ b/mock_device.h
@@ -29,6 +29,7 @@
   MOCK_METHOD1(Scan, void(Error *error));
   MOCK_CONST_METHOD1(TechnologyIs,
                      bool(const Technology::Identifier technology));
+  MOCK_METHOD1(Load, bool(StoreInterface*));
   MOCK_METHOD1(Save, bool(StoreInterface*));
 
  private:
diff --git a/mock_glib.h b/mock_glib.h
index 12a548a..90085a8 100644
--- a/mock_glib.h
+++ b/mock_glib.h
@@ -54,6 +54,10 @@
                                               GError **error));
   MOCK_METHOD2(KeyFileHasGroup, gboolean(GKeyFile *key_file,
                                          const gchar *group_name));
+  MOCK_METHOD4(KeyFileHasKey, gboolean(GKeyFile *key_file,
+                                       const gchar *group_name,
+                                       const gchar *key,
+                                       GError **error));
   MOCK_METHOD4(KeyFileLoadFromFile, gboolean(GKeyFile *key_file,
                                              const gchar *file,
                                              GKeyFileFlags flags,
diff --git a/mock_profile.h b/mock_profile.h
index d7cfe3a..f99006d 100644
--- a/mock_profile.h
+++ b/mock_profile.h
@@ -21,7 +21,9 @@
               const std::string &identifier);
   virtual ~MockProfile();
 
-  MOCK_METHOD1(GetStoragePath, bool(FilePath *));
+  MOCK_METHOD1(GetStoragePath, bool(FilePath *filepath));
+  MOCK_METHOD1(ConfigureService,  bool(const ServiceRefPtr &service));
+  MOCK_METHOD1(ConfigureDevice, bool(const DeviceRefPtr &device));
   MOCK_METHOD0(Save, bool());
 
  private:
diff --git a/mock_store.h b/mock_store.h
index 9220741..581d983 100644
--- a/mock_store.h
+++ b/mock_store.h
@@ -19,6 +19,8 @@
 
   MOCK_METHOD0(Flush, bool());
   MOCK_METHOD0(GetGroups, std::set<std::string>());
+  MOCK_METHOD1(GetGroupsWithKey,
+               std::set<std::string>(const std::string &key));
   MOCK_METHOD1(ContainsGroup, bool(const std::string &group));
   MOCK_METHOD2(DeleteKey,
                bool(const std::string &group, const std::string &key));
diff --git a/profile.cc b/profile.cc
index f922d39..4dd98cb 100644
--- a/profile.cc
+++ b/profile.cc
@@ -137,11 +137,15 @@
   return service->Load(storage_.get());
 }
 
+bool Profile::ConfigureDevice(const DeviceRefPtr &device) {
+  return device->Load(storage_.get());
+}
+
 bool Profile::ContainsService(const ServiceConstRefPtr &service) {
   return service->IsLoadableFrom(storage_.get());
 }
 
-bool Profile::IsValidIdentifierToken(const std::string &token) {
+bool Profile::IsValidIdentifierToken(const string &token) {
   if (token.empty()) {
     return false;
   }
diff --git a/profile.h b/profile.h
index 6d56ef3..269f429 100644
--- a/profile.h
+++ b/profile.h
@@ -84,7 +84,12 @@
   // Ask |service| if it can configure itself from the profile.  If it can,
   // change the service to point at this profile, ask |service| to perform
   // the configuration and return true.  If not, return false.
-  bool ConfigureService(const ServiceRefPtr &service);
+  virtual bool ConfigureService(const ServiceRefPtr &service);
+
+  // Allow the device to configure itself from this profile.  Returns
+  // true if the device succeeded in finding its configuration.  If not,
+  // return false.
+  virtual bool ConfigureDevice(const DeviceRefPtr &device);
 
   // Return whether |service| can configure itself from the profile.
   bool ContainsService(const ServiceConstRefPtr &service);
diff --git a/service.cc b/service.cc
index 2131ce6..8f45936 100644
--- a/service.cc
+++ b/service.cc
@@ -159,8 +159,7 @@
 
 Service::~Service() {}
 
-void Service::ActivateCellularModem(const std::string &/*carrier*/,
-                                    Error *error) {
+void Service::ActivateCellularModem(const string &/*carrier*/, Error *error) {
   const string kMessage = "Service doesn't support cellular modem activation.";
   LOG(ERROR) << kMessage;
   CHECK(error);
@@ -235,9 +234,20 @@
   // "APN"
   // "LastGoodAPN"
 
+  favorite_ = true;
+
   return true;
 }
 
+void Service::Unload() {
+  auto_connect_ = false;
+  favorite_ = false;
+  // TODO(pstew): Call a centralized function to purge all profile-set
+  // state.  This should be called both from Load() and Unload() since
+  // even in Load() profiles aren't cumulative -- they're exclusive.
+  // crosbug.com/22946
+}
+
 bool Service::Save(StoreInterface *storage) {
   const string id = GetStorageIdentifier();
 
diff --git a/service.h b/service.h
index bbc37ad..57c7ca8 100644
--- a/service.h
+++ b/service.h
@@ -133,9 +133,17 @@
   // Loads the service from persistent |storage|. Returns true on success.
   virtual bool Load(StoreInterface *storage);
 
+  // Indicate to service that it is no longer persisted to storage.  It
+  // should purge any stored profile state (e.g., credentials).
+  virtual void Unload();
+
   // Saves the service to persistent |storage|. Returns true on success.
   virtual bool Save(StoreInterface *storage);
 
+  // Returns true if the service RPC identifier should be part of the
+  // manager's advertised services list, false otherwise.
+  virtual bool IsVisible() const { return true; }
+
   bool auto_connect() const { return auto_connect_; }
   void set_auto_connect(bool connect) { auto_connect_ = connect; }
 
diff --git a/store_interface.h b/store_interface.h
index 24a6d37..9e20ce0 100644
--- a/store_interface.h
+++ b/store_interface.h
@@ -22,6 +22,9 @@
   // Returns a set of all groups contained in the store.
   virtual std::set<std::string> GetGroups() = 0;
 
+  // Returns the names of all groups that contain the named |key|.
+  virtual std::set<std::string> GetGroupsWithKey(const std::string &key) = 0;
+
   // Returns true if the store contains |group|, false otherwise.
   virtual bool ContainsGroup(const std::string &group) = 0;
 
diff --git a/wifi.cc b/wifi.cc
index 01d9613..ab9cc87 100644
--- a/wifi.cc
+++ b/wifi.cc
@@ -30,6 +30,7 @@
 #include "shill/manager.h"
 #include "shill/profile.h"
 #include "shill/proxy_factory.h"
+#include "shill/store_interface.h"
 #include "shill/supplicant_interface_proxy_interface.h"
 #include "shill/supplicant_process_proxy_interface.h"
 #include "shill/wifi_endpoint.h"
@@ -66,7 +67,7 @@
            EventDispatcher *dispatcher,
            Manager *manager,
            const string& link,
-           const std::string &address,
+           const string &address,
            int interface_index)
     : Device(control_interface,
              dispatcher,
@@ -107,7 +108,7 @@
       proxy_factory_->CreateSupplicantProcessProxy(
           wpa_supplicant::kDBusPath, wpa_supplicant::kDBusAddr));
   try {
-    std::map<string, DBus::Variant> create_interface_args;
+    map<string, DBus::Variant> create_interface_args;
     create_interface_args["Ifname"].writer().
         append_string(link_name().c_str());
     create_interface_args["Driver"].writer().
@@ -177,6 +178,11 @@
           << " ServiceMap entries.";
 }
 
+bool WiFi::Load(StoreInterface *storage) {
+  LoadHiddenServices(storage);
+  return Device::Load(storage);
+}
+
 void WiFi::Scan(Error */*error*/) {
   LOG(INFO) << __func__;
 
@@ -214,7 +220,7 @@
 
 void WiFi::BSSAdded(
     const ::DBus::Path &/*BSS*/,
-    const std::map<string, ::DBus::Variant> &properties) {
+    const map<string, ::DBus::Variant> &properties) {
   // TODO(quiche): Write test to verify correct behavior in the case
   // where we get multiple BSSAdded events for a single endpoint.
   // (Old Endpoint's refcount should fall to zero, and old Endpoint
@@ -262,7 +268,6 @@
   }
 
   supplicant_interface_proxy_->SelectNetwork(network_path);
-  // TODO(quiche): Add to favorite networks list?
 
   // SelectService here (instead of in LinkEvent, like Ethernet), so
   // that, if we fail to bring up L2, we can attribute failure correctly.
@@ -274,9 +279,9 @@
   pending_service_ = service;
 }
 
-WiFiServiceRefPtr WiFi::FindService(const std::vector<uint8_t> &ssid,
-                                    const std::string &mode,
-                                    const std::string &security) const {
+WiFiServiceRefPtr WiFi::FindService(const vector<uint8_t> &ssid,
+                                    const string &mode,
+                                    const string &security) const {
   for (vector<WiFiServiceRefPtr>::const_iterator it = services_.begin();
        it != services_.end();
        ++it) {
@@ -294,10 +299,11 @@
   for (vector<WiFiServiceRefPtr>::const_iterator it = services_.begin();
        it != services_.end();
        ++it) {
-    if ((*it)->hidden_ssid()) {
+    if ((*it)->hidden_ssid() && (*it)->favorite()) {
       hidden_ssids_set.insert((*it)->ssid());
     }
   }
+  VLOG(2) << "Found " << hidden_ssids_set.size() << " hidden services";
   ByteArrays hidden_ssids(hidden_ssids_set.begin(), hidden_ssids_set.end());
   if (!hidden_ssids.empty()) {
     // TODO(pstew): Devise a better method for time-sharing with SSIDs that do
@@ -315,6 +321,87 @@
   return hidden_ssids;
 }
 
+bool WiFi::LoadHiddenServices(StoreInterface *storage) {
+  bool created_hidden_service = false;
+  set<string> groups = storage->GetGroupsWithKey(flimflam::kWifiHiddenSsid);
+  for (set<string>::iterator it = groups.begin(); it != groups.end(); ++it) {
+    bool is_hidden = false;
+    if (!storage->GetBool(*it, flimflam::kWifiHiddenSsid, &is_hidden)) {
+      VLOG(2) << "Storage group " << *it << " returned by GetGroupsWithKey "
+              << "failed for GetBool(" << flimflam::kWifiHiddenSsid
+              << ") -- possible non-bool key";
+      continue;
+    }
+    if (!is_hidden) {
+      continue;
+    }
+    string ssid_hex;
+    vector<uint8_t> ssid_bytes;
+    if (!storage->GetString(*it, flimflam::kSSIDProperty, &ssid_hex) ||
+        !base::HexStringToBytes(ssid_hex, &ssid_bytes)) {
+      VLOG(2) << "Hidden network is missing/invalid \""
+              << flimflam::kSSIDProperty << "\" property";
+      continue;
+    }
+    string device_address;
+    string network_mode;
+    string security;
+    // It is gross that we have to do this, but the only place we can
+    // get information about the service is from its storage name.
+    if (!WiFiService::ParseStorageIdentifier(*it, &device_address,
+                                             &network_mode, &security) ||
+        device_address != address()) {
+      VLOG(2) << "Hidden network has unparsable storage identifier \""
+              << *it << "\"";
+      continue;
+    }
+    if (FindService(ssid_bytes, network_mode, security).get()) {
+      // If service already exists, we have nothing to do, since the
+      // service has already loaded its configuration from storage.
+      // This is guaranteed to happen in both cases where Load() is
+      // called on a device (via a ConfigureDevice() call on the
+      // profile):
+      //  - In RegisterDevice() the Device hasn't been started yet,
+      //    so it has no services registered, except for those
+      //    created by previous iterations of LoadHiddenService().
+      //    The latter can happen if another profile in the Manager's
+      //    stack defines the same ssid/mode/security.  Even this
+      //    case is okay, since even if the profiles differ
+      //    materially on configuration and credentials, the "right"
+      //    one will be configured in the course of the
+      //    RegisterService() call below.
+      // - In PushProfile(), all registered services have been
+      //   introduced to the profile via ConfigureService() prior
+      //   to calling ConfigureDevice() on the profile.
+      continue;
+    }
+    WiFiServiceRefPtr service(new WiFiService(control_interface(),
+                                              dispatcher(),
+                                              manager(),
+                                              this,
+                                              ssid_bytes,
+                                              network_mode,
+                                              security,
+                                              true));
+    services_.push_back(service);
+
+    // By registering the service, the rest of the configuration
+    // will be loaded from the profile into the service via ConfigureService().
+    manager()->RegisterService(service);
+
+    created_hidden_service = true;
+  }
+
+  // If we are idle and we created a service as a result of opening the
+  // profile, we should initiate a scan for our new hidden SSID.
+  if (running() && created_hidden_service &&
+      supplicant_state_ == wpa_supplicant::kInterfaceStateInactive) {
+    Scan(NULL);
+  }
+
+  return created_hidden_service;
+}
+
 void WiFi::ScanDoneTask() {
   LOG(INFO) << __func__;
 
@@ -360,7 +447,7 @@
 
 void WiFi::ScanTask() {
   VLOG(2) << "WiFi " << link_name() << " scan requested.";
-  std::map<string, DBus::Variant> scan_args;
+  map<string, DBus::Variant> scan_args;
   scan_args[wpa_supplicant::kPropertyScanType].writer().
       append_string(wpa_supplicant::kScanTypeActive);
 
diff --git a/wifi.h b/wifi.h
index fd1cedf..1dcc110 100644
--- a/wifi.h
+++ b/wifi.h
@@ -38,6 +38,7 @@
 
   virtual void Start();
   virtual void Stop();
+  virtual bool Load(StoreInterface *storage);
   virtual void Scan(Error *error);
   virtual bool TechnologyIs(const Technology::Identifier type) const;
   virtual void LinkEvent(unsigned int flags, unsigned int change);
@@ -60,8 +61,6 @@
 
  private:
   friend class WiFiMainTest;  // access to supplicant_*_proxy_, link_up_
-  FRIEND_TEST(WiFiMainTest, FindServiceWEP);
-  FRIEND_TEST(WiFiMainTest, FindServiceWPA);
   FRIEND_TEST(WiFiMainTest, InitialSupplicantState);  // kInterfaceStateUnknown
 
   typedef std::map<const std::string, WiFiEndpointRefPtr> EndpointMap;
@@ -81,6 +80,9 @@
                                 const std::string &mode,
                                 const std::string &security) const;
   ByteArrays GetHiddenSSIDList();
+  // Create services for hidden networks stored in |storage|.  Returns true
+  // if any were found, otherwise returns false.
+  bool LoadHiddenServices(StoreInterface *storage);
   void ScanDoneTask();
   void ScanTask();
   void StateChanged(const std::string &new_state);
diff --git a/wifi_service.cc b/wifi_service.cc
index 308a3e5..fdb6964 100644
--- a/wifi_service.cc
+++ b/wifi_service.cc
@@ -9,6 +9,7 @@
 #include <base/logging.h>
 #include <base/stringprintf.h>
 #include <base/string_number_conversions.h>
+#include <base/string_split.h>
 #include <base/string_util.h>
 #include <chromeos/dbus/service_constants.h>
 #include <dbus/dbus.h>
@@ -35,9 +36,9 @@
                          EventDispatcher *dispatcher,
                          Manager *manager,
                          const WiFiRefPtr &device,
-                         const std::vector<uint8_t> &ssid,
-                         const std::string &mode,
-                         const std::string &security,
+                         const vector<uint8_t> &ssid,
+                         const string &mode,
+                         const string &security,
                          bool hidden_ssid)
     : Service(control_interface, dispatcher, manager, flimflam::kTypeWifi),
       need_passphrase_(false),
@@ -112,7 +113,6 @@
 
 void WiFiService::Disconnect() {
   // TODO(quiche) RemoveNetwork from supplicant
-  // XXX remove from favorite networks list?
 }
 
 bool WiFiService::TechnologyIs(const Technology::Identifier type) const {
@@ -130,7 +130,7 @@
   return GetEAPKeyManagement();
 }
 
-const std::vector<uint8_t> &WiFiService::ssid() const {
+const vector<uint8_t> &WiFiService::ssid() const {
   return ssid_;
 }
 
@@ -154,6 +154,17 @@
       storage->ContainsGroup(GetSpecificStorageIdentifier());
 }
 
+bool WiFiService::IsVisible() const {
+  // TODO(quiche): Write a function that returns whether (or which)
+  // endpoints are associated with this service.  crosbug.com/22948
+  const bool is_visible_in_scan = true;
+
+  // WiFi Services should be displayed only if they are in range (have
+  // endpoints that have shown up in a scan) or if the service is actively
+  // being connected.
+  return is_visible_in_scan || IsConnected() || IsConnecting();
+}
+
 bool WiFiService::Load(StoreInterface *storage) {
   // First find out which storage identifier is available in priority order
   // of specific, generic.
@@ -418,6 +429,22 @@
   }
 }
 
+// static
+bool WiFiService::ParseStorageIdentifier(const string &storage_name,
+                                         string *address,
+                                         string *mode,
+                                         string *security) {
+  vector<string> wifi_parts;
+  base::SplitString(storage_name, '_', &wifi_parts);
+  if (wifi_parts.size() != 5 || wifi_parts[0] != flimflam::kTypeWifi) {
+    return false;
+  }
+  *address = wifi_parts[1];
+  *mode = wifi_parts[3];
+  *security = wifi_parts[4];
+  return true;
+}
+
 string WiFiService::GetGenericStorageIdentifier() const {
   return GetStorageIdentifierForSecurity(GetSecurityClass(security_));
 }
diff --git a/wifi_service.h b/wifi_service.h
index 2248b6c..e2d3cf6 100644
--- a/wifi_service.h
+++ b/wifi_service.h
@@ -40,6 +40,10 @@
 
   // wifi_<MAC>_<BSSID>_<mode_string>_<security_string>
   std::string GetStorageIdentifier() const;
+  static bool ParseStorageIdentifier(const std::string &storage_name,
+                                     std::string *address,
+                                     std::string *mode,
+                                     std::string *security);
 
   const std::string &mode() const;
   const std::string &key_management() const;
@@ -53,6 +57,7 @@
   virtual bool Load(StoreInterface *storage);
   virtual bool Save(StoreInterface *storage);
 
+  virtual bool IsVisible() const;
   bool IsSecurityMatch(const std::string &security) const;
   bool hidden_ssid() const { return hidden_ssid_; }
 
diff --git a/wifi_service_unittest.cc b/wifi_service_unittest.cc
index 8b366b9..4d618bd 100644
--- a/wifi_service_unittest.cc
+++ b/wifi_service_unittest.cc
@@ -372,4 +372,27 @@
                               false));
 }
 
+TEST_F(WiFiServiceTest, ParseStorageIdentifier) {
+  vector<uint8_t> ssid(5);
+  ssid.push_back(0xff);
+
+  WiFiServiceRefPtr service = new WiFiService(control_interface(),
+                                              dispatcher(),
+                                              manager(),
+                                              wifi(),
+                                              ssid,
+                                              flimflam::kModeManaged,
+                                              flimflam::kSecurityNone,
+                                              false);
+  const string storage_id = service->GetStorageIdentifier();
+  string address;
+  string mode;
+  string security;
+  EXPECT_TRUE(service->ParseStorageIdentifier(storage_id, &address, &mode,
+                                              &security));
+  EXPECT_EQ(StringToLowerASCII(string(fake_mac)), address);
+  EXPECT_EQ(flimflam::kModeManaged, mode);
+  EXPECT_EQ(flimflam::kSecurityNone, security);
+}
+
 }  // namespace shill
diff --git a/wifi_unittest.cc b/wifi_unittest.cc
index 5185e46..bdc16d2 100644
--- a/wifi_unittest.cc
+++ b/wifi_unittest.cc
@@ -10,11 +10,13 @@
 #include <linux/netlink.h>  // Needs typedefs from sys/socket.h.
 
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
 #include <base/memory/ref_counted.h>
 #include <base/memory/scoped_ptr.h>
+#include <base/stringprintf.h>
 #include <base/string_number_conversions.h>
 #include <base/string_util.h>
 #include <chromeos/dbus/service_constants.h>
@@ -32,6 +34,7 @@
 #include "shill/mock_dhcp_provider.h"
 #include "shill/mock_manager.h"
 #include "shill/mock_rtnl_handler.h"
+#include "shill/mock_store.h"
 #include "shill/mock_supplicant_interface_proxy.h"
 #include "shill/mock_supplicant_process_proxy.h"
 #include "shill/mock_wifi_service.h"
@@ -44,15 +47,20 @@
 #include "shill/wpa_supplicant.h"
 
 using std::map;
+using std::set;
 using std::string;
 using std::vector;
 using ::testing::_;
 using ::testing::AnyNumber;
 using ::testing::DefaultValue;
+using ::testing::DoAll;
 using ::testing::InSequence;
 using ::testing::Mock;
 using ::testing::NiceMock;
 using ::testing::Return;
+using ::testing::SetArgumentPointee;
+using ::testing::StrEq;
+using ::testing::StrictMock;
 using ::testing::Test;
 using ::testing::Throw;
 using ::testing::Values;
@@ -227,15 +235,7 @@
   void StopWiFi() {
     wifi_->Stop();
   }
-  void GetHiddenService(const char *service_type,
-                        const char *ssid,
-                        const char *mode,
-                        Error *result) {
-    WiFiServiceRefPtr service =
-        GetServiceInner(service_type, ssid, mode, NULL, NULL, true, result);
-    EXPECT_TRUE(service->hidden_ssid());
-  }
-  void GetOpenService(const char *service_type,
+ void GetOpenService(const char *service_type,
                       const char *ssid,
                       const char *mode,
                       Error *result) {
@@ -255,7 +255,7 @@
                                     const char *mode,
                                     const char *security,
                                     const char *passphrase,
-                                    bool hidden,
+                                    bool allow_hidden,
                                     Error *result) {
     map<string, ::DBus::Variant> args;
     // in general, we want to avoid D-Bus specific code for any RPCs
@@ -272,14 +272,38 @@
       args[flimflam::kSecurityProperty].writer().append_string(security);
     if (passphrase != NULL)
       args[flimflam::kPassphraseProperty].writer().append_string(passphrase);
-    if (hidden)
-      args[flimflam::kWifiHiddenSsid].writer().append_bool(true);
+    if (!allow_hidden)
+      args[flimflam::kWifiHiddenSsid].writer().append_bool(false);
 
     Error e;
     KeyValueStore args_kv;
     DBusAdaptor::ArgsToKeyValueStore(args, &args_kv, &e);
     return wifi_->GetService(args_kv, result);
   }
+  WiFiServiceRefPtr FindService(const vector<uint8_t> &ssid,
+                                const string &mode,
+                                const string &security) {
+    return wifi_->FindService(ssid, mode, security);
+  }
+  bool LoadHiddenServices(StoreInterface *storage) {
+    return wifi_->LoadHiddenServices(storage);
+  }
+  void SetupHiddenStorage(MockStore *storage, const string &ssid, string *id) {
+    const string hex_ssid = base::HexEncode(ssid.data(), ssid.size());
+    *id = StringToLowerASCII(base::StringPrintf("%s_%s_%s_%s_%s",
+                                                flimflam::kTypeWifi,
+                                                kDeviceAddress,
+                                                hex_ssid.c_str(),
+                                                flimflam::kModeManaged,
+                                                flimflam::kSecurityNone));
+    const char *groups[] = { id->c_str() };
+    EXPECT_CALL(*storage, GetGroupsWithKey(flimflam::kWifiHiddenSsid))
+        .WillRepeatedly(Return(set<string>(groups, groups + 1)));
+    EXPECT_CALL(*storage, GetBool(StrEq(*id), flimflam::kWifiHiddenSsid, _))
+        .WillRepeatedly(DoAll(SetArgumentPointee<2>(true), Return(true)));
+    EXPECT_CALL(*storage, GetString(StrEq(*id), flimflam::kSSIDProperty, _))
+        .WillRepeatedly(DoAll(SetArgumentPointee<2>(hex_ssid), Return(true)));
+  }
   MockManager *manager() {
     return &manager_;
   }
@@ -314,7 +338,7 @@
 };
 
 const char WiFiMainTest::kDeviceName[] = "wlan0";
-const char WiFiMainTest::kDeviceAddress[] = "00:01:02:03:04:05";
+const char WiFiMainTest::kDeviceAddress[] = "000102030405";
 const char WiFiMainTest::kNetworkModeAdHoc[] = "ad-hoc";
 const char WiFiMainTest::kNetworkModeInfrastructure[] = "infrastructure";
 
@@ -475,7 +499,7 @@
 
 TEST_F(WiFiMainTest, GetWifiServiceOpen) {
   Error e;
-  GetOpenService("wifi", "an_ssid", flimflam::kModeManaged, &e);
+  GetOpenService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged, &e);
   EXPECT_TRUE(e.IsSuccess());
 }
 
@@ -488,7 +512,7 @@
 
 TEST_F(WiFiMainTest, GetWifiServiceOpenNoSSID) {
   Error e;
-  GetOpenService("wifi", NULL, flimflam::kModeManaged, &e);
+  GetOpenService(flimflam::kTypeWifi, NULL, flimflam::kModeManaged, &e);
   EXPECT_EQ(Error::kInvalidArguments, e.type());
   EXPECT_EQ("must specify SSID", e.message());
 }
@@ -496,149 +520,151 @@
 TEST_F(WiFiMainTest, GetWifiServiceOpenLongSSID) {
   Error e;
   GetOpenService(
-      "wifi", "123456789012345678901234567890123", flimflam::kModeManaged, &e);
+      flimflam::kTypeWifi, "123456789012345678901234567890123",
+      flimflam::kModeManaged, &e);
   EXPECT_EQ(Error::kInvalidNetworkName, e.type());
   EXPECT_EQ("SSID is too long", e.message());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceOpenShortSSID) {
   Error e;
-  GetOpenService("wifi", "", flimflam::kModeManaged, &e);
+  GetOpenService(flimflam::kTypeWifi, "", flimflam::kModeManaged, &e);
   EXPECT_EQ(Error::kInvalidNetworkName, e.type());
   EXPECT_EQ("SSID is too short", e.message());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceOpenBadMode) {
   Error e;
-  GetOpenService("wifi", "an_ssid", "ad-hoc", &e);
+  GetOpenService(flimflam::kTypeWifi, "an_ssid", "ad-hoc", &e);
   EXPECT_EQ(Error::kNotSupported, e.type());
   EXPECT_EQ("service mode is unsupported", e.message());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceOpenNoMode) {
   Error e;
-  GetOpenService("wifi", "an_ssid", NULL, &e);
+  GetOpenService(flimflam::kTypeWifi, "an_ssid", NULL, &e);
   EXPECT_TRUE(e.IsSuccess());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceRSN) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityRsn,
-             "secure password", &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityRsn, "secure password", &e);
   EXPECT_TRUE(e.IsSuccess());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceRSNNoPassword) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityRsn,
-             NULL, &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityRsn, NULL, &e);
   EXPECT_EQ(Error::kInvalidArguments, e.type());
   EXPECT_EQ("must specify passphrase", e.message());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceBadSecurity) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, "rot-13", NULL, &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged, "rot-13",
+             NULL, &e);
   EXPECT_EQ(Error::kNotSupported, e.type());
   EXPECT_EQ("security mode is unsupported", e.message());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceWEPNoPassword) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWep,
-             NULL, &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWep, NULL, &e);
   EXPECT_EQ(Error::kInvalidArguments, e.type());
   EXPECT_EQ("must specify passphrase", e.message());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceWEPEmptyPassword) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWep,
-             "", &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWep, "", &e);
   EXPECT_EQ(Error::kInvalidPassphrase, e.type());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceWEP40ASCII) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWep,
-             "abcde", &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWep, "abcde", &e);
   EXPECT_TRUE(e.IsSuccess());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceWEP104ASCII) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWep,
-             "abcdefghijklm", &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWep, "abcdefghijklm", &e);
   EXPECT_TRUE(e.IsSuccess());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceWEP40ASCIIWithKeyIndex) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWep,
-             "0:abcdefghijklm", &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWep, "0:abcdefghijklm", &e);
   EXPECT_TRUE(e.IsSuccess());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceWEP40Hex) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWep,
-             "0102030405", &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWep, "0102030405", &e);
   EXPECT_TRUE(e.IsSuccess());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceWEP40HexBadPassphrase) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWep,
-             "O102030405", &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWep, "O102030405", &e);
   EXPECT_EQ(Error::kInvalidPassphrase, e.type());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceWEP40HexWithKeyIndexBadPassphrase) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWep,
-             "1:O102030405", &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWep, "1:O102030405", &e);
   EXPECT_EQ(Error::kInvalidPassphrase, e.type());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceWEP40HexWithKeyIndexAndBaseBadPassphrase) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWep,
-             "1:0xO102030405", &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWep, "1:0xO102030405", &e);
   EXPECT_EQ(Error::kInvalidPassphrase, e.type());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceWEP40HexWithBaseBadPassphrase) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWep,
-             "0xO102030405", &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWep, "0xO102030405", &e);
   EXPECT_EQ(Error::kInvalidPassphrase, e.type());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceWEP104Hex) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWep,
-             "0102030405060708090a0b0c0d", &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWep, "0102030405060708090a0b0c0d", &e);
   EXPECT_TRUE(e.IsSuccess());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceWEP104HexUppercase) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWep,
-             "0102030405060708090A0B0C0D", &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWep, "0102030405060708090A0B0C0D", &e);
   EXPECT_TRUE(e.IsSuccess());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceWEP104HexWithKeyIndex) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWep,
-             "0:0102030405060708090a0b0c0d", &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWep, "0:0102030405060708090a0b0c0d", &e);
   EXPECT_TRUE(e.IsSuccess());
 }
 
 TEST_F(WiFiMainTest, GetWifiServiceWEP104HexWithKeyIndexAndBase) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWep,
-             "0:0x0102030405060708090a0b0c0d", &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWep, "0:0x0102030405060708090a0b0c0d", &e);
   EXPECT_TRUE(e.IsSuccess());
 }
 
@@ -647,15 +673,15 @@
 
 TEST_P(WiFiGetServiceSuccessTest, Passphrase) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWpa,
-             GetParam().c_str(), &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWpa, GetParam().c_str(), &e);
   EXPECT_TRUE(e.IsSuccess());
 }
 
 TEST_P(WiFiGetServiceFailureTest, Passphrase) {
   Error e;
-  GetService("wifi", "an_ssid", flimflam::kModeManaged, flimflam::kSecurityWpa,
-             GetParam().c_str(), &e);
+  GetService(flimflam::kTypeWifi, "an_ssid", flimflam::kModeManaged,
+             flimflam::kSecurityWpa, GetParam().c_str(), &e);
   EXPECT_EQ(Error::kInvalidPassphrase, e.type());
 }
 
@@ -681,54 +707,52 @@
   const string ssid("an_ssid");
   {
     Error e;
-    GetService("wifi", ssid.c_str(), flimflam::kModeManaged,
+    GetService(flimflam::kTypeWifi, ssid.c_str(), flimflam::kModeManaged,
                flimflam::kSecurityWep, "abcde", &e);
     EXPECT_TRUE(e.IsSuccess());
   }
   vector<uint8_t> ssid_bytes(ssid.begin(), ssid.end());
 
-  EXPECT_TRUE(wifi()->FindService(ssid_bytes, flimflam::kModeManaged,
-                                  flimflam::kSecurityWep).get());
-  EXPECT_FALSE(wifi()->FindService(ssid_bytes, flimflam::kModeManaged,
-                                   flimflam::kSecurityWpa).get());
+  EXPECT_TRUE(FindService(ssid_bytes, flimflam::kModeManaged,
+                          flimflam::kSecurityWep).get());
+  EXPECT_FALSE(FindService(ssid_bytes, flimflam::kModeManaged,
+                           flimflam::kSecurityWpa).get());
 }
 
 TEST_F(WiFiMainTest, FindServiceWPA) {
   const string ssid("an_ssid");
   {
     Error e;
-    GetService("wifi", ssid.c_str(), flimflam::kModeManaged,
+    GetService(flimflam::kTypeWifi, ssid.c_str(), flimflam::kModeManaged,
                flimflam::kSecurityRsn, "abcdefgh", &e);
     EXPECT_TRUE(e.IsSuccess());
   }
   vector<uint8_t> ssid_bytes(ssid.begin(), ssid.end());
-  WiFiServiceRefPtr wpa_service(
-      wifi()->FindService(ssid_bytes, flimflam::kModeManaged,
-                          flimflam::kSecurityWpa));
+  WiFiServiceRefPtr wpa_service(FindService(ssid_bytes, flimflam::kModeManaged,
+                                            flimflam::kSecurityWpa));
   EXPECT_TRUE(wpa_service.get());
-  WiFiServiceRefPtr rsn_service(
-      wifi()->FindService(ssid_bytes, flimflam::kModeManaged,
-                          flimflam::kSecurityRsn));
+  WiFiServiceRefPtr rsn_service(FindService(ssid_bytes, flimflam::kModeManaged,
+                                            flimflam::kSecurityRsn));
   EXPECT_TRUE(rsn_service.get());
   EXPECT_EQ(wpa_service.get(), rsn_service.get());
-  WiFiServiceRefPtr psk_service(
-      wifi()->FindService(ssid_bytes, flimflam::kModeManaged,
-                          flimflam::kSecurityPsk));
+  WiFiServiceRefPtr psk_service(FindService(ssid_bytes, flimflam::kModeManaged,
+                                            flimflam::kSecurityPsk));
   EXPECT_EQ(wpa_service.get(), psk_service.get());
   // Indirectly test FindService by doing a GetService on something that
   // already exists.
   {
     Error e;
     WiFiServiceRefPtr wpa_service2(
-        GetServiceInner("wifi", ssid.c_str(), flimflam::kModeManaged,
-                        flimflam::kSecurityWpa, "abcdefgh", false, &e));
+        GetServiceInner(flimflam::kTypeWifi, ssid.c_str(),
+                        flimflam::kModeManaged, flimflam::kSecurityWpa,
+                        "abcdefgh", false, &e));
     EXPECT_TRUE(e.IsSuccess());
     EXPECT_EQ(wpa_service.get(), wpa_service2.get());
   }
 }
 
 MATCHER_P(HasHiddenSSID, ssid, "") {
-  std::map<string, DBus::Variant>::const_iterator it =
+  map<string, DBus::Variant>::const_iterator it =
       arg.find(wpa_supplicant::kPropertyScanSSIDs);
   if (it == arg.end()) {
     return false;
@@ -754,10 +778,36 @@
           DBus::Error(
               "fi.w1.wpa_supplicant1.InterfaceUnknown",
               "test threw fi.w1.wpa_supplicant1.InterfaceUnknown")));
-  Error e;
-  GetHiddenService("wifi", "an_ssid", flimflam::kModeManaged, &e);
-  EXPECT_TRUE(e.IsSuccess());
-  EXPECT_CALL(*supplicant_interface_proxy_, Scan(HasHiddenSSID("an_ssid")));
+  {
+    // Create a hidden, favorite service.
+    Error e;
+    WiFiServiceRefPtr service =
+        GetServiceInner(flimflam::kTypeWifi, "ssid0", flimflam::kModeManaged,
+                        NULL, NULL, true, &e);
+    EXPECT_TRUE(e.IsSuccess());
+    EXPECT_TRUE(service->hidden_ssid());
+    service->set_favorite(true);
+  }
+  {
+    // Create a hidden, non-favorite service.
+    Error e;
+    WiFiServiceRefPtr service =
+        GetServiceInner(flimflam::kTypeWifi, "ssid1", flimflam::kModeManaged,
+                        NULL, NULL, true, &e);
+    EXPECT_TRUE(e.IsSuccess());
+    EXPECT_TRUE(service->hidden_ssid());
+  }
+  {
+    // Create a non-hidden, favorite service.
+    Error e;
+    WiFiServiceRefPtr service =
+        GetServiceInner(flimflam::kTypeWifi, "ssid2", flimflam::kModeManaged,
+                        NULL, NULL, false, &e);
+    EXPECT_TRUE(e.IsSuccess());
+    EXPECT_FALSE(service->hidden_ssid());
+    service->set_favorite(true);
+  }
+  EXPECT_CALL(*supplicant_interface_proxy_, Scan(HasHiddenSSID("ssid0")));
   StartWiFi();
   dispatcher_.DispatchPendingEvents();
 }
@@ -802,4 +852,63 @@
   Mock::VerifyAndClearExpectations(service.get());
 }
 
+TEST_F(WiFiMainTest, LoadHiddenServicesFailWithNoGroups) {
+  StrictMock<MockStore> storage;
+  EXPECT_CALL(storage, GetGroupsWithKey(flimflam::kWifiHiddenSsid))
+      .WillOnce(Return(set<string>()));
+  EXPECT_FALSE(LoadHiddenServices(&storage));
+}
+
+TEST_F(WiFiMainTest, LoadHiddenServicesFailWithMissingHidden) {
+  string id;
+  StrictMock<MockStore> storage;
+  SetupHiddenStorage(&storage, "an_ssid", &id);
+  // Missing "Hidden" property.
+  EXPECT_CALL(storage, GetBool(StrEq(id), flimflam::kWifiHiddenSsid, _))
+      .WillOnce(Return(false));
+  EXPECT_FALSE(LoadHiddenServices(&storage));
+}
+
+TEST_F(WiFiMainTest, LoadHiddenServicesFailWithFalseHidden) {
+  string id;
+  StrictMock<MockStore> storage;
+  SetupHiddenStorage(&storage, "an_ssid", &id);
+  // "Hidden" property set to "false".
+  EXPECT_CALL(storage, GetBool(StrEq(id), flimflam::kWifiHiddenSsid, _))
+      .WillOnce(DoAll(SetArgumentPointee<2>(true), Return(false)));
+  EXPECT_FALSE(LoadHiddenServices(&storage));
+}
+
+TEST_F(WiFiMainTest, LoadHiddenServicesFailWithMissingSSID) {
+  string id;
+  StrictMock<MockStore> storage;
+  SetupHiddenStorage(&storage, "an_ssid", &id);
+  // Missing "SSID" property.
+  EXPECT_CALL(storage, GetString(StrEq(id), flimflam::kSSIDProperty, _))
+      .WillOnce(Return(false));
+  EXPECT_FALSE(LoadHiddenServices(&storage));
+}
+
+
+TEST_F(WiFiMainTest, LoadHiddenServicesFailWithFoundService) {
+  StrictMock<MockStore> storage;
+  string id;
+  SetupHiddenStorage(&storage, "an_ssid", &id);
+  Error e;
+  GetOpenService(flimflam::kTypeWifi, "an_ssid", NULL, &e);
+  ASSERT_TRUE(e.IsSuccess());
+  EXPECT_FALSE(LoadHiddenServices(&storage));
+}
+
+TEST_F(WiFiMainTest, LoadHiddenServicesSuccess) {
+  StrictMock<MockStore> storage;
+  string ssid("an_ssid");
+  string id;
+  SetupHiddenStorage(&storage, ssid, &id);
+  EXPECT_TRUE(LoadHiddenServices(&storage));
+  vector<uint8_t> ssid_bytes(ssid.begin(), ssid.end());
+  EXPECT_TRUE(FindService(ssid_bytes, flimflam::kModeManaged,
+                          flimflam::kSecurityNone).get());
+}
+
 }  // namespace shill