CryptAuthDeviceManager: correctly migrate old SoftwareFeature prefs.

When SoftwareFeatures were changed to be stored in prefs as strings
instead of as ints [1], CryptAuthDeviceManagerImpl failed to also
migrate old int prefs to the new string representation when the prefs
were desrialized. This caused all devices migrating from M69 to a newer
release to believe none of their synced devices had any
SoftwareFeatures until another sync occurred.

1) https://chromium-review.googlesource.com/c/chromium/src/+/1145588

Bug: 888031
Change-Id: Icd666106d6ebd4db5620cfab0dac459ef67df23e
Reviewed-on: https://chromium-review.googlesource.com/1258515Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596263}

components/cryptauth/cryptauth_device_manager_impl.cc

@@ -300,6 +300,20 @@ void AddSoftwareFeaturesToExternalDevice(
     bool old_unlock_key_value_from_prefs,
     bool old_mobile_hotspot_supported_from_prefs) {
   for (const auto& it : software_features_dictionary.DictItems()) {
+    std::string software_feature = it.first;
+    if (SoftwareFeatureStringToEnum(software_feature) ==
+        SoftwareFeature::UNKNOWN_FEATURE) {
+      // SoftwareFeatures were previously stored in prefs as ints. Now,
+      // SoftwareFeatures are stored as full string values, e.g.,
+      // "betterTogetherHost". If |it.first| is not recognized by
+      // SoftwareFeatureStringToEnum(), that means it is in the old int
+      // representation of the SoftwareFeature. Convert it to its full string
+      // representation using SoftwareFeatureEnumToString();
+      int software_feature_int = std::atoi(software_feature.c_str());
+      software_feature = SoftwareFeatureEnumToString(
+          static_cast<SoftwareFeature>(software_feature_int));
+    }
+
     int software_feature_state;
     if (!it.second.GetAsInteger(&software_feature_state)) {
       PA_LOG(WARNING) << "Unable to retrieve SoftwareFeature; skipping.";
@@ -308,10 +322,10 @@ void AddSoftwareFeaturesToExternalDevice(
 
     switch (static_cast<SoftwareFeatureState>(software_feature_state)) {
       case SoftwareFeatureState::kEnabled:
-        external_device->add_enabled_software_features(it.first);
+        external_device->add_enabled_software_features(software_feature);
         FALLTHROUGH;
       case SoftwareFeatureState::kSupported:
-        external_device->add_supported_software_features(it.first);
+        external_device->add_supported_software_features(software_feature);
         break;
       default:
         break;

components/cryptauth/cryptauth_device_manager_impl_unittest.cc

@@ -1117,4 +1117,49 @@ TEST_F(CryptAuthDeviceManagerImplTest,
       SoftwareFeatureEnumToString(SoftwareFeature::MAGIC_TETHER_HOST)));
 }
 
+// Regression test for crbug.com/888031.
+TEST_F(CryptAuthDeviceManagerImplTest,
+       TestMigrateFromIntToStringSoftwareFeaturePrefRepresentation) {
+  device_manager_->Start();
+
+  ExternalDeviceInfo device;
+  device.set_public_key("public key");
+  device.set_friendly_device_name("deprecated device");
+
+  // Simulate how older client versions persisted SoftwareFeatures as ints.
+  device.add_supported_software_features(
+      std::to_string(SoftwareFeature::EASY_UNLOCK_HOST));
+  device.add_enabled_software_features(
+      std::to_string(SoftwareFeature::EASY_UNLOCK_HOST));
+  device.add_supported_software_features(
+      std::to_string(SoftwareFeature::MAGIC_TETHER_HOST));
+
+  devices_in_response_.push_back(device);
+  get_my_devices_response_.add_devices()->CopyFrom(device);
+
+  FireSchedulerForSync(INVOCATION_REASON_PERIODIC);
+  ASSERT_FALSE(success_callback_.is_null());
+  EXPECT_CALL(*this, OnSyncFinishedProxy(
+                         CryptAuthDeviceManager::SyncResult::SUCCESS,
+                         CryptAuthDeviceManager::DeviceChangeResult::CHANGED));
+  success_callback_.Run(get_my_devices_response_);
+
+  ExternalDeviceInfo synced_device = device_manager_->GetSyncedDevices()[2];
+
+  // CryptAuthDeviceManager should recognize that the SoftwareFeature prefs had
+  // been stored as refs, and convert them to their full string representations.
+  EXPECT_TRUE(base::ContainsValue(
+      synced_device.supported_software_features(),
+      SoftwareFeatureEnumToString(SoftwareFeature::EASY_UNLOCK_HOST)));
+  EXPECT_TRUE(base::ContainsValue(
+      synced_device.enabled_software_features(),
+      SoftwareFeatureEnumToString(SoftwareFeature::EASY_UNLOCK_HOST)));
+  EXPECT_TRUE(base::ContainsValue(
+      synced_device.supported_software_features(),
+      SoftwareFeatureEnumToString(SoftwareFeature::MAGIC_TETHER_HOST)));
+  EXPECT_FALSE(base::ContainsValue(
+      synced_device.enabled_software_features(),
+      SoftwareFeatureEnumToString(SoftwareFeature::MAGIC_TETHER_HOST)));
+}
+
 }  // namespace cryptauth