shill: WiFiService: Fix-up old-style WiFi Service entries

Add a utility to upgrade WiFi service entries that don't contain
the "Type", "Mode" or "Security" properties, and only contain
this information in the fields of the storage identifier.  Use
this utility whenever the WiFi device loads a new profile.
Follow-on CLs will clean up all remaining direct callers of
WiFiServce::ParseStorageIdentifier().

BUG=chromium-os:38048
TEST=Unit Tests, inspect chrome://histograms for new histogram
entries when profiles are updated.

Change-Id: I0f1d4e75abc1edfe6da1bbfe68183cab1b1c0021
Reviewed-on: https://gerrit.chromium.org/gerrit/41802
Reviewed-by: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>
diff --git a/wifi_service_unittest.cc b/wifi_service_unittest.cc
index 03aa47b..dfa0214 100644
--- a/wifi_service_unittest.cc
+++ b/wifi_service_unittest.cc
@@ -5,9 +5,11 @@
 #include "shill/wifi_service.h"
 
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
+#include <base/stringprintf.h>
 #include <base/string_util.h>
 #include <chromeos/dbus/service_constants.h>
 #include <gmock/gmock.h>
@@ -28,6 +30,7 @@
 #include "shill/wpa_supplicant.h"
 
 using std::map;
+using std::set;
 using std::string;
 using std::vector;
 using ::testing::_;
@@ -39,6 +42,7 @@
 using ::testing::SetArgumentPointee;
 using ::testing::StrEq;
 using ::testing::StrNe;
+using ::testing::StrictMock;
 
 namespace shill {
 
@@ -220,6 +224,48 @@
 const char *WiFiServiceUpdateFromEndpointsTest::kBadEndpointBssId =
     "00:00:00:00:00:03";
 
+class WiFiServiceFixupStorageTest : public WiFiServiceTest {
+ protected:
+  void AddGroup(string group_name) {
+    groups_.insert(group_name);
+  }
+
+  void AddServiceEntry(bool has_type, bool has_mode, bool has_security) {
+    int index = groups_.size();
+    string id = base::StringPrintf("%s_%d_%d_%s_%s", flimflam::kTypeWifi,
+                                   index, index, flimflam::kModeManaged,
+                                   flimflam::kSecurityNone);
+    AddGroup(id);
+    EXPECT_CALL(store_, GetString(id, WiFiService::kStorageType, _))
+        .WillOnce(Return(has_type));
+    if (!has_type) {
+      EXPECT_CALL(store_, SetString(id, WiFiService::kStorageType,
+                                    flimflam::kTypeWifi));
+    }
+    EXPECT_CALL(store_, GetString(id, WiFiService::kStorageMode, _))
+        .WillOnce(Return(has_mode));
+    if (!has_mode) {
+      EXPECT_CALL(store_, SetString(id, WiFiService::kStorageMode,
+                                    flimflam::kModeManaged));
+    }
+    EXPECT_CALL(store_, GetString(id, WiFiService::kStorageSecurity, _))
+        .WillOnce(Return(has_security));
+    if (!has_security) {
+      EXPECT_CALL(store_, SetString(id, WiFiService::kStorageSecurity,
+                                    flimflam::kSecurityNone));
+    }
+  }
+
+  bool FixupServiceEntries() {
+    EXPECT_CALL(store_, GetGroups()).WillOnce(Return(groups_));
+    return WiFiService::FixupServiceEntries(&store_);
+  }
+
+ private:
+  StrictMock<MockStore> store_;
+  set<string> groups_;
+};
+
 TEST_F(WiFiServiceTest, StorageId) {
   vector<uint8_t> ssid(5);
   ssid.push_back(0xff);
@@ -926,6 +972,45 @@
   EXPECT_EQ(flimflam::kSecurity8021x, security);
 }
 
+TEST_F(WiFiServiceFixupStorageTest, FixedEntries) {
+  const string kNonWiFiId = "vpn_foo";
+  const string kUnparsableWiFiId = "wifi_foo";
+
+  AddGroup(kNonWiFiId);
+  AddGroup(kUnparsableWiFiId);
+  AddServiceEntry(true, true, true);
+  AddServiceEntry(false, false, false);
+  AddServiceEntry(true, true, true);
+  AddServiceEntry(false, false, false);
+  EXPECT_TRUE(FixupServiceEntries());
+}
+
+TEST_F(WiFiServiceFixupStorageTest, NoFixedEntries) {
+  const string kNonWiFiId = "vpn_foo";
+  const string kUnparsableWiFiId = "wifi_foo";
+
+  AddGroup(kNonWiFiId);
+  AddGroup(kUnparsableWiFiId);
+  AddServiceEntry(true, true, true);
+  EXPECT_FALSE(FixupServiceEntries());
+}
+
+TEST_F(WiFiServiceFixupStorageTest, MissingTypeProperty) {
+  AddServiceEntry(false, true, true);
+  EXPECT_TRUE(FixupServiceEntries());
+}
+
+TEST_F(WiFiServiceFixupStorageTest, MissingModeProperty) {
+  AddServiceEntry(true, false, true);
+  EXPECT_TRUE(FixupServiceEntries());
+}
+
+TEST_F(WiFiServiceFixupStorageTest, MissingSecurityProperty) {
+  AddServiceEntry(true, true, false);
+  EXPECT_TRUE(FixupServiceEntries());
+}
+
+
 TEST_F(WiFiServiceTest, Connectable) {
   // Open network should be connectable.
   EXPECT_TRUE(CheckConnectable(flimflam::kSecurityNone, NULL, NULL));