Commit af2c8827 authored by Andrey Zaytsev's avatar Andrey Zaytsev Committed by Commit Bot

Safety check: fixed the corner case of having no passwords

Bug: 1015841
Change-Id: Ibc2a0eaefdfe20ea1484a464024c184b15306548
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2105219
Commit-Queue: Andrey Zaytsev <andzaytsev@google.com>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Auto-Submit: Andrey Zaytsev <andzaytsev@google.com>
Cr-Commit-Position: refs/heads/master@{#750918}
parent a3c52dda
...@@ -174,6 +174,12 @@ void SafetyCheckHandler::CheckPasswords() { ...@@ -174,6 +174,12 @@ void SafetyCheckHandler::CheckPasswords() {
observed_leak_check_.RemoveAll(); observed_leak_check_.RemoveAll();
observed_leak_check_.Add(leak_service_); observed_leak_check_.Add(leak_service_);
passwords_delegate_->StartPasswordCheck(); passwords_delegate_->StartPasswordCheck();
// In the case of no passwords, there is no state transition and no callback.
// Because of that, it is necessary to check the state synchronously.
if (leak_service_->state() !=
password_manager::BulkLeakCheckService::State::kRunning) {
OnStateChanged(leak_service_->state());
}
} }
void SafetyCheckHandler::CheckExtensions() { void SafetyCheckHandler::CheckExtensions() {
......
...@@ -68,6 +68,11 @@ class TestingSafetyCheckHandler : public SafetyCheckHandler { ...@@ -68,6 +68,11 @@ class TestingSafetyCheckHandler : public SafetyCheckHandler {
class TestPasswordsDelegate : public extensions::TestPasswordsPrivateDelegate { class TestPasswordsDelegate : public extensions::TestPasswordsPrivateDelegate {
public: public:
void SetBulkLeakCheckService(
password_manager::BulkLeakCheckService* leak_service) {
leak_service_ = leak_service;
}
void SetNumCompromisedCredentials(int compromised_password_count) { void SetNumCompromisedCredentials(int compromised_password_count) {
compromised_password_count_ = compromised_password_count; compromised_password_count_ = compromised_password_count;
} }
...@@ -77,6 +82,14 @@ class TestPasswordsDelegate : public extensions::TestPasswordsPrivateDelegate { ...@@ -77,6 +82,14 @@ class TestPasswordsDelegate : public extensions::TestPasswordsPrivateDelegate {
state_ = state; state_ = state;
} }
// When |running_state_on_start_| is set to |true|, this class simulates the
// behavior of the real password check by synchronously settings the state to
// |kRunning| once |StartPasswordCheck()| is invoked. Since that is the case
// for the majority of non-error password check flows, the default is |true|.
// In some test cases with no transitions to the |kRunning| state, such as not
// having any passwords, it is necessary to disable this functionality.
void SetRunningStateOnStart(bool value) { running_state_on_start_ = value; }
std::vector<extensions::api::passwords_private::CompromisedCredential> std::vector<extensions::api::passwords_private::CompromisedCredential>
GetCompromisedCredentials() override { GetCompromisedCredentials() override {
std::vector<extensions::api::passwords_private::CompromisedCredential> std::vector<extensions::api::passwords_private::CompromisedCredential>
...@@ -94,10 +107,22 @@ class TestPasswordsDelegate : public extensions::TestPasswordsPrivateDelegate { ...@@ -94,10 +107,22 @@ class TestPasswordsDelegate : public extensions::TestPasswordsPrivateDelegate {
return status; return status;
} }
bool StartPasswordCheck() override {
bool ret_val =
extensions::TestPasswordsPrivateDelegate::StartPasswordCheck();
if (running_state_on_start_) {
leak_service_->set_state_and_notify(
password_manager::BulkLeakCheckService::State::kRunning);
}
return ret_val;
}
private: private:
password_manager::BulkLeakCheckService* leak_service_ = nullptr;
int compromised_password_count_ = 0; int compromised_password_count_ = 0;
extensions::api::passwords_private::PasswordCheckState state_ = extensions::api::passwords_private::PasswordCheckState state_ =
extensions::api::passwords_private::PASSWORD_CHECK_STATE_IDLE; extensions::api::passwords_private::PASSWORD_CHECK_STATE_IDLE;
bool running_state_on_start_ = true;
}; };
class TestSafetyCheckExtensionService : public TestExtensionService { class TestSafetyCheckExtensionService : public TestExtensionService {
...@@ -180,6 +205,7 @@ void SafetyCheckHandlerTest::SetUp() { ...@@ -180,6 +205,7 @@ void SafetyCheckHandlerTest::SetUp() {
auto version_updater = std::make_unique<TestVersionUpdater>(); auto version_updater = std::make_unique<TestVersionUpdater>();
test_leak_service_ = std::make_unique<password_manager::BulkLeakCheckService>( test_leak_service_ = std::make_unique<password_manager::BulkLeakCheckService>(
nullptr, nullptr); nullptr, nullptr);
test_passwords_delegate_.SetBulkLeakCheckService(test_leak_service_.get());
version_updater_ = version_updater.get(); version_updater_ = version_updater.get();
test_web_ui_.set_web_contents(web_contents()); test_web_ui_.set_web_contents(web_contents());
test_extension_prefs_ = extensions::ExtensionPrefs::Get(profile()); test_extension_prefs_ = extensions::ExtensionPrefs::Get(profile());
...@@ -598,6 +624,17 @@ TEST_F(SafetyCheckHandlerTest, CheckPasswords_RunningOneCompromised) { ...@@ -598,6 +624,17 @@ TEST_F(SafetyCheckHandlerTest, CheckPasswords_RunningOneCompromised) {
VerifyDisplayString(event, "1 compromised password"); VerifyDisplayString(event, "1 compromised password");
} }
TEST_F(SafetyCheckHandlerTest, CheckPasswords_NoPasswords) {
test_passwords_delegate_.SetRunningStateOnStart(false);
safety_check_->PerformSafetyCheck();
const base::DictionaryValue* event =
GetSafetyCheckStatusChangedWithDataIfExists(
kPasswords,
static_cast<int>(SafetyCheckHandler::PasswordsStatus::kSafe));
EXPECT_TRUE(event);
VerifyDisplayString(event, "No compromised passwords found");
}
TEST_F(SafetyCheckHandlerTest, CheckExtensions_NoExtensions) { TEST_F(SafetyCheckHandlerTest, CheckExtensions_NoExtensions) {
safety_check_->PerformSafetyCheck(); safety_check_->PerformSafetyCheck();
EXPECT_TRUE(GetSafetyCheckStatusChangedWithDataIfExists( EXPECT_TRUE(GetSafetyCheckStatusChangedWithDataIfExists(
......
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