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

[SmartLock] Do not cache FeatureState.

Caching FeatureState as was done previously may have been introducing
subtle bugs. In any case, this refactor is cleaner and more direct.

Bug: 883100
Change-Id: Ife296e88bf561c228cd34c6cb80714a4c9767ae5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1618561
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661133}
parent 8f8bf0ef
...@@ -125,8 +125,7 @@ void EasyUnlockServiceRegular::LoadRemoteDevices() { ...@@ -125,8 +125,7 @@ void EasyUnlockServiceRegular::LoadRemoteDevices() {
return; return;
} }
if (feature_state_ != if (!IsEnabled()) {
multidevice_setup::mojom::FeatureState::kEnabledByUser) {
// OnFeatureStatesChanged() will call back on this method when feature state // OnFeatureStatesChanged() will call back on this method when feature state
// changes. // changes.
PA_LOG(VERBOSE) << "Smart Lock is not enabled by user; aborting."; PA_LOG(VERBOSE) << "Smart Lock is not enabled by user; aborting.";
...@@ -410,19 +409,18 @@ bool EasyUnlockServiceRegular::IsAllowedInternal() const { ...@@ -410,19 +409,18 @@ bool EasyUnlockServiceRegular::IsAllowedInternal() const {
if (!ProfileHelper::IsPrimaryProfile(profile())) if (!ProfileHelper::IsPrimaryProfile(profile()))
return false; return false;
if (feature_state_ == if (multidevice_setup_client_->GetFeatureState(
multidevice_setup::mojom::Feature::kSmartLock) ==
multidevice_setup::mojom::FeatureState::kProhibitedByPolicy) { multidevice_setup::mojom::FeatureState::kProhibitedByPolicy) {
return false; return false;
} }
if (!profile()->GetPrefs()->GetBoolean(prefs::kEasyUnlockAllowed))
return false;
return true; return true;
} }
bool EasyUnlockServiceRegular::IsEnabled() const { bool EasyUnlockServiceRegular::IsEnabled() const {
return feature_state_ == return multidevice_setup_client_->GetFeatureState(
multidevice_setup::mojom::Feature::kSmartLock) ==
multidevice_setup::mojom::FeatureState::kEnabledByUser; multidevice_setup::mojom::FeatureState::kEnabledByUser;
} }
...@@ -472,15 +470,6 @@ void EasyUnlockServiceRegular::OnNewDevicesSynced() { ...@@ -472,15 +470,6 @@ void EasyUnlockServiceRegular::OnNewDevicesSynced() {
void EasyUnlockServiceRegular::OnFeatureStatesChanged( void EasyUnlockServiceRegular::OnFeatureStatesChanged(
const multidevice_setup::MultiDeviceSetupClient::FeatureStatesMap& const multidevice_setup::MultiDeviceSetupClient::FeatureStatesMap&
feature_states_map) { feature_states_map) {
const auto it =
feature_states_map.find(multidevice_setup::mojom::Feature::kSmartLock);
if (it == feature_states_map.end()) {
feature_state_ =
multidevice_setup::mojom::FeatureState::kUnavailableNoVerifiedHost;
return;
}
feature_state_ = it->second;
LoadRemoteDevices(); LoadRemoteDevices();
UpdateAppState(); UpdateAppState();
} }
...@@ -489,7 +478,7 @@ void EasyUnlockServiceRegular::ShowChromebookAddedNotification() { ...@@ -489,7 +478,7 @@ void EasyUnlockServiceRegular::ShowChromebookAddedNotification() {
// The user may have decided to disable Smart Lock or the whole multidevice // The user may have decided to disable Smart Lock or the whole multidevice
// suite immediately after completing setup, so ensure that Smart Lock is // suite immediately after completing setup, so ensure that Smart Lock is
// enabled. // enabled.
if (feature_state_ == multidevice_setup::mojom::FeatureState::kEnabledByUser) if (IsEnabled())
notification_controller_->ShowChromebookAddedNotification(); notification_controller_->ShowChromebookAddedNotification();
} }
......
...@@ -157,7 +157,7 @@ class EasyUnlockServiceRegular ...@@ -157,7 +157,7 @@ class EasyUnlockServiceRegular
// Used to fetch local device and remote device data. // Used to fetch local device and remote device data.
device_sync::DeviceSyncClient* device_sync_client_; device_sync::DeviceSyncClient* device_sync_client_;
// Used to determine the FeatureState of Smart Lock. See |feature_state_|. // Used to determine the FeatureState of Smart Lock.
multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client_; multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client_;
// Stores the unlock keys for EasyUnlock before the current device sync, so we // Stores the unlock keys for EasyUnlock before the current device sync, so we
...@@ -165,12 +165,6 @@ class EasyUnlockServiceRegular ...@@ -165,12 +165,6 @@ class EasyUnlockServiceRegular
std::vector<cryptauth::ExternalDeviceInfo> unlock_keys_before_sync_; std::vector<cryptauth::ExternalDeviceInfo> unlock_keys_before_sync_;
multidevice::RemoteDeviceRefList remote_device_unlock_keys_before_sync_; multidevice::RemoteDeviceRefList remote_device_unlock_keys_before_sync_;
// Caches feature state of Smart Lock. This service should only actively be
// running if its value is kEnabledByUser. Populated by using
// |multidevice_setup_client_|.
multidevice_setup::mojom::FeatureState feature_state_ =
multidevice_setup::mojom::FeatureState::kUnavailableNoVerifiedHost;
// True if the pairing changed notification was shown, so that the next time // True if the pairing changed notification was shown, so that the next time
// the Chromebook is unlocked, we can show the subsequent 'pairing applied' // the Chromebook is unlocked, we can show the subsequent 'pairing applied'
// notification. // notification.
......
...@@ -115,7 +115,8 @@ void ProximityAuthProfilePrefManager::SetIsEasyUnlockEnabled( ...@@ -115,7 +115,8 @@ void ProximityAuthProfilePrefManager::SetIsEasyUnlockEnabled(
} }
bool ProximityAuthProfilePrefManager::IsEasyUnlockEnabled() const { bool ProximityAuthProfilePrefManager::IsEasyUnlockEnabled() const {
return feature_state_ == return multidevice_setup_client_->GetFeatureState(
chromeos::multidevice_setup::mojom::Feature::kSmartLock) ==
chromeos::multidevice_setup::mojom::FeatureState::kEnabledByUser; chromeos::multidevice_setup::mojom::FeatureState::kEnabledByUser;
} }
...@@ -165,15 +166,6 @@ bool ProximityAuthProfilePrefManager::IsChromeOSLoginEnabled() const { ...@@ -165,15 +166,6 @@ bool ProximityAuthProfilePrefManager::IsChromeOSLoginEnabled() const {
void ProximityAuthProfilePrefManager::OnFeatureStatesChanged( void ProximityAuthProfilePrefManager::OnFeatureStatesChanged(
const chromeos::multidevice_setup::MultiDeviceSetupClient::FeatureStatesMap& const chromeos::multidevice_setup::MultiDeviceSetupClient::FeatureStatesMap&
feature_states_map) { feature_states_map) {
const auto it = feature_states_map.find(
chromeos::multidevice_setup::mojom::Feature::kSmartLock);
if (it == feature_states_map.end()) {
feature_state_ = chromeos::multidevice_setup::mojom::FeatureState::
kUnavailableNoVerifiedHost;
return;
}
feature_state_ = it->second;
if (local_state_ && account_id_.is_valid()) if (local_state_ && account_id_.is_valid())
SyncPrefsToLocalState(); SyncPrefsToLocalState();
} }
......
...@@ -89,15 +89,10 @@ class ProximityAuthProfilePrefManager ...@@ -89,15 +89,10 @@ class ProximityAuthProfilePrefManager
// The account id of the current profile. // The account id of the current profile.
AccountId account_id_; AccountId account_id_;
// Used to determine the FeatureState of Smart Lock. See |feature_state_|. // Used to determine the FeatureState of Smart Lock.
chromeos::multidevice_setup::MultiDeviceSetupClient* chromeos::multidevice_setup::MultiDeviceSetupClient*
multidevice_setup_client_ = nullptr; multidevice_setup_client_ = nullptr;
// Caches feature state of Smart Lock. Populated by using
// |multidevice_setup_client_|.
chromeos::multidevice_setup::mojom::FeatureState feature_state_ = chromeos::
multidevice_setup::mojom::FeatureState::kUnavailableNoVerifiedHost;
base::WeakPtrFactory<ProximityAuthProfilePrefManager> weak_ptr_factory_; base::WeakPtrFactory<ProximityAuthProfilePrefManager> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(ProximityAuthProfilePrefManager); DISALLOW_COPY_AND_ASSIGN(ProximityAuthProfilePrefManager);
......
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