Commit aa366463 authored by Ryan Hansberry's avatar Ryan Hansberry Committed by Commit Bot

[CrOS MultiDevice] Make exemption for no EASY_UNLOCK_HOST support.

To support legacy Smart Lock hosts which did not mark themselves as
supporting EASY_UNLOCK_HOST (because they couldn't have), simply
ignore lack of a kSupported bit if kEnabled is present, and set the
host device as an enabled Smart Lock host.

Bug: 896082
Change-Id: Id8fb1ade25113fcb108a7658738e7bf9e0c996a0
Reviewed-on: https://chromium-review.googlesource.com/c/1345123Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610222}
parent 8896ad63
...@@ -145,17 +145,25 @@ SupportedAndEnabledSoftwareFeaturesToDictionaryValue( ...@@ -145,17 +145,25 @@ SupportedAndEnabledSoftwareFeaturesToDictionaryValue(
&software_feature_state) || &software_feature_state) ||
static_cast<SoftwareFeatureState>(software_feature_state) != static_cast<SoftwareFeatureState>(software_feature_state) !=
SoftwareFeatureState::kSupported) { SoftwareFeatureState::kSupported) {
PA_LOG(ERROR) << "A feature is marked as enabled but not as supported: " if (software_feature == cryptauth::SoftwareFeature::EASY_UNLOCK_HOST) {
<< software_feature_key; // Allow this known special-case for legacy purposes; fall-through to
RecordDeviceSyncSoftwareFeaturesResult(false /* success */, // logic which marks this device as enabled.
software_feature); PA_LOG(VERBOSE) << "Encountered legacy case: feature EASY_UNLOCK_HOST "
"is marked as supported but not enabled. Setting as "
"enabled.";
} else {
PA_LOG(ERROR) << "A feature is marked as enabled but not as supported: "
<< software_feature_key << ". Not setting as enabled.";
RecordDeviceSyncSoftwareFeaturesResult(false /* success */,
software_feature);
continue; continue;
} else { }
RecordDeviceSyncSoftwareFeaturesResult(true /* success */,
software_feature);
} }
RecordDeviceSyncSoftwareFeaturesResult(true /* success */,
software_feature);
dictionary->SetInteger(software_feature_key, dictionary->SetInteger(software_feature_key,
static_cast<int>(SoftwareFeatureState::kEnabled)); static_cast<int>(SoftwareFeatureState::kEnabled));
} }
......
...@@ -1170,8 +1170,15 @@ TEST_F(CryptAuthDeviceManagerImplTest, MetricsForEnabledAndNotSupported) { ...@@ -1170,8 +1170,15 @@ TEST_F(CryptAuthDeviceManagerImplTest, MetricsForEnabledAndNotSupported) {
SoftwareFeatureEnumToString(SoftwareFeature::BETTER_TOGETHER_HOST)); SoftwareFeatureEnumToString(SoftwareFeature::BETTER_TOGETHER_HOST));
enabled_not_supported_device.add_enabled_software_features( enabled_not_supported_device.add_enabled_software_features(
SoftwareFeatureEnumToString(SoftwareFeature::BETTER_TOGETHER_HOST)); SoftwareFeatureEnumToString(SoftwareFeature::BETTER_TOGETHER_HOST));
// EASY_UNLOCK_HOST is a special case; it is allowed to not be marked as
// supported, but still be set as enabled.
enabled_not_supported_device.add_enabled_software_features( enabled_not_supported_device.add_enabled_software_features(
SoftwareFeatureEnumToString(SoftwareFeature::EASY_UNLOCK_HOST)); SoftwareFeatureEnumToString(SoftwareFeature::EASY_UNLOCK_HOST));
// These will fail because they are not set as supported.
enabled_not_supported_device.add_enabled_software_features(
SoftwareFeatureEnumToString(SoftwareFeature::MAGIC_TETHER_HOST));
enabled_not_supported_device.add_enabled_software_features( enabled_not_supported_device.add_enabled_software_features(
"MyUnknownFeature"); "MyUnknownFeature");
...@@ -1194,9 +1201,9 @@ TEST_F(CryptAuthDeviceManagerImplTest, MetricsForEnabledAndNotSupported) { ...@@ -1194,9 +1201,9 @@ TEST_F(CryptAuthDeviceManagerImplTest, MetricsForEnabledAndNotSupported) {
success_callback_.Run(response); success_callback_.Run(response);
histogram_tester.ExpectTotalCount( histogram_tester.ExpectTotalCount(
"CryptAuth.DeviceSyncSoftwareFeaturesResult", 3); "CryptAuth.DeviceSyncSoftwareFeaturesResult", 4);
histogram_tester.ExpectBucketCount<bool>( histogram_tester.ExpectBucketCount<bool>(
"CryptAuth.DeviceSyncSoftwareFeaturesResult", true, 1); "CryptAuth.DeviceSyncSoftwareFeaturesResult", true, 2);
histogram_tester.ExpectBucketCount<bool>( histogram_tester.ExpectBucketCount<bool>(
"CryptAuth.DeviceSyncSoftwareFeaturesResult", false, 2); "CryptAuth.DeviceSyncSoftwareFeaturesResult", false, 2);
...@@ -1207,7 +1214,10 @@ TEST_F(CryptAuthDeviceManagerImplTest, MetricsForEnabledAndNotSupported) { ...@@ -1207,7 +1214,10 @@ TEST_F(CryptAuthDeviceManagerImplTest, MetricsForEnabledAndNotSupported) {
cryptauth::SoftwareFeature::BETTER_TOGETHER_HOST, 0); cryptauth::SoftwareFeature::BETTER_TOGETHER_HOST, 0);
histogram_tester.ExpectBucketCount<cryptauth::SoftwareFeature>( histogram_tester.ExpectBucketCount<cryptauth::SoftwareFeature>(
"CryptAuth.DeviceSyncSoftwareFeaturesResult.Failures", "CryptAuth.DeviceSyncSoftwareFeaturesResult.Failures",
cryptauth::SoftwareFeature::EASY_UNLOCK_HOST, 1); cryptauth::SoftwareFeature::EASY_UNLOCK_HOST, 0);
histogram_tester.ExpectBucketCount<cryptauth::SoftwareFeature>(
"CryptAuth.DeviceSyncSoftwareFeaturesResult.Failures",
cryptauth::SoftwareFeature::MAGIC_TETHER_HOST, 1);
histogram_tester.ExpectBucketCount<cryptauth::SoftwareFeature>( histogram_tester.ExpectBucketCount<cryptauth::SoftwareFeature>(
"CryptAuth.DeviceSyncSoftwareFeaturesResult.Failures", "CryptAuth.DeviceSyncSoftwareFeaturesResult.Failures",
cryptauth::SoftwareFeature::UNKNOWN_FEATURE, 1); cryptauth::SoftwareFeature::UNKNOWN_FEATURE, 1);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment