Commit 97cd34cb authored by Ryan Hansberry's avatar Ryan Hansberry Committed by Commit Bot

[SmartLock] Increase EasyUnlockServiceRegular test coverage.

Bring EasyUnlockServiceRegular above the 85% test coverage expectation.

This CL also removes unnecessary methods which remained in
EasyUnlockServiceRegular, correctly clears stored remote devices in
prefs when Smart Lock is no longer enabled, and includes minor
refactors necessary to make tests pass.

Coverage before this CL:
  easy_unlock_service.cc:         14.72% (91/618)
  easy_unlock_service_regular.cc: 22.58% (98/434)

Coverage as of this CL:
  easy_unlock_service.cc:         35.76% (221/618)
  easy_unlock_service_regular.cc: 91.96% (389/423)

Report created with:
$ python tools/code_coverage/coverage.py unit_tests \
    -b out/coverage \
    -o out/report \
    -c 'out/coverage/unit_tests --gtest_filter=*EasyUnlockService*' \
    -f chrome/browser/chromeos/login/easy_unlock/

Bug: 949209
Change-Id: I037776f197373ff94099d2f5fc38726d60165cc5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1788552
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695843}
parent e473bbcf
...@@ -2882,6 +2882,7 @@ source_set("unit_tests") { ...@@ -2882,6 +2882,7 @@ source_set("unit_tests") {
"//chromeos/audio", "//chromeos/audio",
"//chromeos/components/multidevice:test_support", "//chromeos/components/multidevice:test_support",
"//chromeos/components/proximity_auth", "//chromeos/components/proximity_auth",
"//chromeos/components/proximity_auth:test_support",
"//chromeos/components/tether:test_support", "//chromeos/components/tether:test_support",
"//chromeos/constants", "//chromeos/constants",
"//chromeos/cryptohome:test_support", "//chromeos/cryptohome:test_support",
......
...@@ -127,6 +127,7 @@ void EasyUnlockServiceRegular::LoadRemoteDevices() { ...@@ -127,6 +127,7 @@ void EasyUnlockServiceRegular::LoadRemoteDevices() {
// 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.";
SetStoredRemoteDevices(base::ListValue());
SetProximityAuthDevices(GetAccountId(), multidevice::RemoteDeviceRefList(), SetProximityAuthDevices(GetAccountId(), multidevice::RemoteDeviceRefList(),
base::nullopt /* local_device */); base::nullopt /* local_device */);
return; return;
...@@ -148,6 +149,7 @@ void EasyUnlockServiceRegular::LoadRemoteDevices() { ...@@ -148,6 +149,7 @@ void EasyUnlockServiceRegular::LoadRemoteDevices() {
} else { } else {
PA_LOG(ERROR) << "Smart Lock is enabled by user, but no unlock key is " PA_LOG(ERROR) << "Smart Lock is enabled by user, but no unlock key is "
"present; aborting."; "present; aborting.";
SetStoredRemoteDevices(base::ListValue());
SetProximityAuthDevices(GetAccountId(), multidevice::RemoteDeviceRefList(), SetProximityAuthDevices(GetAccountId(), multidevice::RemoteDeviceRefList(),
base::nullopt /* local_device */); base::nullopt /* local_device */);
...@@ -292,17 +294,6 @@ AccountId EasyUnlockServiceRegular::GetAccountId() const { ...@@ -292,17 +294,6 @@ AccountId EasyUnlockServiceRegular::GetAccountId() const {
return primary_user->GetAccountId(); return primary_user->GetAccountId();
} }
void EasyUnlockServiceRegular::SetHardlockAfterKeyOperation(
EasyUnlockScreenlockStateHandler::HardlockState state_on_success,
bool success) {
if (success)
SetHardlockStateForUser(GetAccountId(), state_on_success);
// Even if the refresh keys operation suceeded, we still fetch and check the
// cryptohome keys against the keys in local preferences as a sanity check.
CheckCryptohomeKeysAndMaybeHardlock();
}
void EasyUnlockServiceRegular::ClearPermitAccess() { void EasyUnlockServiceRegular::ClearPermitAccess() {
DictionaryPrefUpdate pairing_update(profile()->GetPrefs(), DictionaryPrefUpdate pairing_update(profile()->GetPrefs(),
prefs::kEasyUnlockPairing); prefs::kEasyUnlockPairing);
...@@ -319,10 +310,12 @@ const base::ListValue* EasyUnlockServiceRegular::GetRemoteDevices() const { ...@@ -319,10 +310,12 @@ const base::ListValue* EasyUnlockServiceRegular::GetRemoteDevices() const {
} }
std::string EasyUnlockServiceRegular::GetChallenge() const { std::string EasyUnlockServiceRegular::GetChallenge() const {
NOTREACHED();
return std::string(); return std::string();
} }
std::string EasyUnlockServiceRegular::GetWrappedSecret() const { std::string EasyUnlockServiceRegular::GetWrappedSecret() const {
NOTREACHED();
return std::string(); return std::string();
} }
...@@ -338,31 +331,29 @@ void EasyUnlockServiceRegular::RecordPasswordLoginEvent( ...@@ -338,31 +331,29 @@ void EasyUnlockServiceRegular::RecordPasswordLoginEvent(
} }
void EasyUnlockServiceRegular::InitializeInternal() { void EasyUnlockServiceRegular::InitializeInternal() {
pref_manager_.reset(new proximity_auth::ProximityAuthProfilePrefManager(
profile()->GetPrefs(), multidevice_setup_client_));
pref_manager_->StartSyncingToLocalState(g_browser_process->local_state(),
GetAccountId());
registrar_.Init(profile()->GetPrefs());
registrar_.Add(
proximity_auth::prefs::kProximityAuthIsChromeOSLoginEnabled,
base::Bind(&EasyUnlockServiceRegular::RefreshCryptohomeKeysIfPossible,
weak_ptr_factory_.GetWeakPtr()));
// If |device_sync_client_| is not ready yet, wait for it to call back on // If |device_sync_client_| is not ready yet, wait for it to call back on
// OnReady(). // OnReady().
if (device_sync_client_->is_ready()) if (device_sync_client_->is_ready())
OnReady(); OnReady();
device_sync_client_->AddObserver(this); device_sync_client_->AddObserver(this);
OnFeatureStatesChanged(multidevice_setup_client_->GetFeatureStates()); OnFeatureStatesChanged(multidevice_setup_client_->GetFeatureStates());
multidevice_setup_client_->AddObserver(this); multidevice_setup_client_->AddObserver(this);
proximity_auth::ScreenlockBridge::Get()->AddObserver(this); proximity_auth::ScreenlockBridge::Get()->AddObserver(this);
pref_manager_.reset(new proximity_auth::ProximityAuthProfilePrefManager(
profile()->GetPrefs(), multidevice_setup_client_));
pref_manager_->StartSyncingToLocalState(g_browser_process->local_state(),
GetAccountId());
LoadRemoteDevices(); LoadRemoteDevices();
registrar_.Init(profile()->GetPrefs());
registrar_.Add(
proximity_auth::prefs::kProximityAuthIsChromeOSLoginEnabled,
base::Bind(&EasyUnlockServiceRegular::RefreshCryptohomeKeysIfPossible,
weak_ptr_factory_.GetWeakPtr()));
} }
void EasyUnlockServiceRegular::ShutdownInternal() { void EasyUnlockServiceRegular::ShutdownInternal() {
...@@ -501,11 +492,6 @@ void EasyUnlockServiceRegular::ShowNotificationIfNewDevicePresent( ...@@ -501,11 +492,6 @@ void EasyUnlockServiceRegular::ShowNotificationIfNewDevicePresent(
} }
} }
void EasyUnlockServiceRegular::OnForceSyncCompleted(bool success) {
if (!success)
PA_LOG(WARNING) << "Failed to force device sync.";
}
void EasyUnlockServiceRegular::OnScreenDidLock( void EasyUnlockServiceRegular::OnScreenDidLock(
proximity_auth::ScreenlockBridge::LockHandler::ScreenType screen_type) { proximity_auth::ScreenlockBridge::LockHandler::ScreenType screen_type) {
set_will_authenticate_using_easy_unlock(false); set_will_authenticate_using_easy_unlock(false);
......
...@@ -115,8 +115,6 @@ class EasyUnlockServiceRegular ...@@ -115,8 +115,6 @@ class EasyUnlockServiceRegular
const std::set<std::string>& public_keys_before_sync, const std::set<std::string>& public_keys_before_sync,
const std::set<std::string>& public_keys_after_sync); const std::set<std::string>& public_keys_after_sync);
void OnForceSyncCompleted(bool success);
// proximity_auth::ScreenlockBridge::Observer implementation: // proximity_auth::ScreenlockBridge::Observer implementation:
void OnScreenDidLock(proximity_auth::ScreenlockBridge::LockHandler::ScreenType void OnScreenDidLock(proximity_auth::ScreenlockBridge::LockHandler::ScreenType
screen_type) override; screen_type) override;
...@@ -125,12 +123,6 @@ class EasyUnlockServiceRegular ...@@ -125,12 +123,6 @@ class EasyUnlockServiceRegular
override; override;
void OnFocusedUserChanged(const AccountId& account_id) override; void OnFocusedUserChanged(const AccountId& account_id) override;
// Called after a cryptohome RemoveKey or RefreshKey operation to set the
// proper hardlock state if the operation is successful.
void SetHardlockAfterKeyOperation(
EasyUnlockScreenlockStateHandler::HardlockState state_on_success,
bool success);
// Refreshes the ChromeOS cryptohome keys if the user has reauthed recently. // Refreshes the ChromeOS cryptohome keys if the user has reauthed recently.
// Otherwise, hardlock the device. // Otherwise, hardlock the device.
void RefreshCryptohomeKeysIfPossible(); void RefreshCryptohomeKeysIfPossible();
......
...@@ -59,6 +59,12 @@ MultiDeviceSetupDialog* MultiDeviceSetupDialog::Get() { ...@@ -59,6 +59,12 @@ MultiDeviceSetupDialog* MultiDeviceSetupDialog::Get() {
return current_instance_; return current_instance_;
} }
// static
void MultiDeviceSetupDialog::SetInstanceForTesting(
MultiDeviceSetupDialog* instance) {
current_instance_ = instance;
}
void MultiDeviceSetupDialog::AddOnCloseCallback(base::OnceClosure callback) { void MultiDeviceSetupDialog::AddOnCloseCallback(base::OnceClosure callback) {
on_close_callbacks_.push_back(std::move(callback)); on_close_callbacks_.push_back(std::move(callback));
} }
......
...@@ -31,6 +31,8 @@ class MultiDeviceSetupDialog : public SystemWebDialogDelegate { ...@@ -31,6 +31,8 @@ class MultiDeviceSetupDialog : public SystemWebDialogDelegate {
// nullptr. // nullptr.
static MultiDeviceSetupDialog* Get(); static MultiDeviceSetupDialog* Get();
static void SetInstanceForTesting(MultiDeviceSetupDialog* instance);
// Registers a callback which will be called when the dialog is closed. // Registers a callback which will be called when the dialog is closed.
void AddOnCloseCallback(base::OnceClosure callback); void AddOnCloseCallback(base::OnceClosure callback);
......
...@@ -178,7 +178,7 @@ class ScreenlockBridge { ...@@ -178,7 +178,7 @@ class ScreenlockBridge {
ScreenlockBridge(); ScreenlockBridge();
~ScreenlockBridge(); ~ScreenlockBridge();
LockHandler* lock_handler_; // Not owned LockHandler* lock_handler_ = nullptr; // Not owned
// The last focused user's id. // The last focused user's id.
AccountId focused_account_id_; AccountId focused_account_id_;
......
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