Commit 82443ecb authored by Viktor Semeniuk's avatar Viktor Semeniuk Committed by Commit Bot

[iOS][Password Check] Delay password check result

This change adds delay before revealing password check result. Delay is
added only if it took less than 3 seconds to finish the check.

Bug: 1075494
Change-Id: Ibc1031e870e178a98b1f74f4122c9519a8e15d28
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2351975Reviewed-by: default avatarSergio Collazos <sczs@chromium.org>
Reviewed-by: default avatarIoana Pandele <ioanap@chromium.org>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Viktor Semeniuk <vsemeniuk@google.com>
Cr-Commit-Position: refs/heads/master@{#798601}
parent 289daef6
...@@ -142,6 +142,10 @@ class IOSChromePasswordCheckManager ...@@ -142,6 +142,10 @@ class IOSChromePasswordCheckManager
// delegate is initialized. // delegate is initialized.
bool start_check_on_init_ = false; bool start_check_on_init_ = false;
// Time when password check was started. Used to calculate delay in case
// when password check run less than 3 seconds.
base::Time start_time_;
// A scoped observer for |saved_passwords_presenter_|. // A scoped observer for |saved_passwords_presenter_|.
ScopedObserver<password_manager::SavedPasswordsPresenter, ScopedObserver<password_manager::SavedPasswordsPresenter,
password_manager::SavedPasswordsPresenter::Observer> password_manager::SavedPasswordsPresenter::Observer>
...@@ -159,6 +163,8 @@ class IOSChromePasswordCheckManager ...@@ -159,6 +163,8 @@ class IOSChromePasswordCheckManager
// Observers to listen to password check changes. // Observers to listen to password check changes.
base::ObserverList<Observer> observers_; base::ObserverList<Observer> observers_;
base::WeakPtrFactory<IOSChromePasswordCheckManager> weak_ptr_factory_{this};
}; };
#endif // IOS_CHROME_BROWSER_PASSWORDS_IOS_CHROME_PASSWORD_CHECK_MANAGER_H_ #endif // IOS_CHROME_BROWSER_PASSWORDS_IOS_CHROME_PASSWORD_CHECK_MANAGER_H_
...@@ -26,6 +26,8 @@ using State = password_manager::BulkLeakCheckServiceInterface::State; ...@@ -26,6 +26,8 @@ using State = password_manager::BulkLeakCheckServiceInterface::State;
// Key used to attach UserData to a LeakCheckCredential. // Key used to attach UserData to a LeakCheckCredential.
constexpr char kPasswordCheckDataKey[] = "password-check-manager-data-key"; constexpr char kPasswordCheckDataKey[] = "password-check-manager-data-key";
// Minimum time the check should be running.
constexpr base::TimeDelta kDelay = base::TimeDelta::FromSeconds(3);
// Class which ensures that IOSChromePasswordCheckManager will stay alive // Class which ensures that IOSChromePasswordCheckManager will stay alive
// until password check is completed even if class what initially created // until password check is completed even if class what initially created
...@@ -120,6 +122,7 @@ void IOSChromePasswordCheckManager::StartPasswordCheck() { ...@@ -120,6 +122,7 @@ void IOSChromePasswordCheckManager::StartPasswordCheck() {
bulk_leak_check_service_adapter_.StartBulkLeakCheck(kPasswordCheckDataKey, bulk_leak_check_service_adapter_.StartBulkLeakCheck(kPasswordCheckDataKey,
&data); &data);
is_check_running_ = true; is_check_running_ = true;
start_time_ = base::Time::Now();
} else { } else {
start_check_on_init_ = true; start_check_on_init_ = true;
} }
...@@ -207,6 +210,20 @@ void IOSChromePasswordCheckManager::OnStateChanged(State state) { ...@@ -207,6 +210,20 @@ void IOSChromePasswordCheckManager::OnStateChanged(State state) {
base::Time::Now().ToDoubleT()); base::Time::Now().ToDoubleT());
} }
if (state != State::kRunning) { if (state != State::kRunning) {
// If check was running
if (is_check_running_) {
const base::TimeDelta elapsed = base::Time::Now() - start_time_;
if (elapsed < kDelay) {
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&IOSChromePasswordCheckManager::
NotifyPasswordCheckStatusChanged,
weak_ptr_factory_.GetWeakPtr()),
kDelay - elapsed);
is_check_running_ = false;
return;
}
}
is_check_running_ = false; is_check_running_ = false;
} }
NotifyPasswordCheckStatusChanged(); NotifyPasswordCheckStatusChanged();
......
...@@ -157,6 +157,8 @@ class IOSChromePasswordCheckManagerTest : public PlatformTest { ...@@ -157,6 +157,8 @@ class IOSChromePasswordCheckManagerTest : public PlatformTest {
void RunUntilIdle() { task_env_.RunUntilIdle(); } void RunUntilIdle() { task_env_.RunUntilIdle(); }
void FastForwardBy(base::TimeDelta time) { task_env_.FastForwardBy(time); }
ChromeBrowserState* browser_state() { return browser_state_.get(); } ChromeBrowserState* browser_state() { return browser_state_.get(); }
TestPasswordStore& store() { return *store_; } TestPasswordStore& store() { return *store_; }
MockBulkLeakCheckService* service() { return bulk_leak_check_service_; } MockBulkLeakCheckService* service() { return bulk_leak_check_service_; }
...@@ -389,3 +391,28 @@ TEST_F(IOSChromePasswordCheckManagerTest, EditCompromisedPassword) { ...@@ -389,3 +391,28 @@ TEST_F(IOSChromePasswordCheckManagerTest, EditCompromisedPassword) {
EXPECT_EQ(base::UTF8ToUTF16(kPassword2), EXPECT_EQ(base::UTF8ToUTF16(kPassword2),
store().stored_passwords().at(kExampleCom).at(0).password_value); store().stored_passwords().at(kExampleCom).at(0).password_value);
} }
// Tests expected delay is being added.
TEST_F(IOSChromePasswordCheckManagerTest, CheckFinishedWithDelay) {
store().AddLogin(MakeSavedPassword(kExampleCom, kUsername1));
RunUntilIdle();
StrictMock<MockPasswordCheckManagerObserver> observer;
manager().AddObserver(&observer);
manager().StartPasswordCheck();
RunUntilIdle();
EXPECT_CALL(observer, PasswordCheckStatusChanged(PasswordCheckState::kIdle))
.Times(0);
static_cast<BulkLeakCheckServiceInterface::Observer*>(&manager())
->OnStateChanged(BulkLeakCheckServiceInterface::State::kIdle);
FastForwardBy(base::TimeDelta::FromSeconds(1));
EXPECT_CALL(observer, PasswordCheckStatusChanged(PasswordCheckState::kIdle))
.Times(0);
FastForwardBy(base::TimeDelta::FromSeconds(1));
EXPECT_CALL(observer, PasswordCheckStatusChanged(PasswordCheckState::kIdle))
.Times(1);
FastForwardBy(base::TimeDelta::FromSeconds(1));
}
...@@ -20,7 +20,11 @@ PasswordCheckObserverBridge::~PasswordCheckObserverBridge() = default; ...@@ -20,7 +20,11 @@ PasswordCheckObserverBridge::~PasswordCheckObserverBridge() = default;
void PasswordCheckObserverBridge::PasswordCheckStatusChanged( void PasswordCheckObserverBridge::PasswordCheckStatusChanged(
PasswordCheckState status) { PasswordCheckState status) {
[delegate_ passwordCheckStateDidChange:status]; // Since password check state update can be called with delay from the
// background thread, dispatch aync should be used to update main UI thread.
dispatch_async(dispatch_get_main_queue(), ^{
[delegate_ passwordCheckStateDidChange:status];
});
} }
void PasswordCheckObserverBridge::CompromisedCredentialsChanged( void PasswordCheckObserverBridge::CompromisedCredentialsChanged(
......
...@@ -1369,8 +1369,7 @@ std::vector<std::unique_ptr<autofill::PasswordForm>> CopyOf( ...@@ -1369,8 +1369,7 @@ std::vector<std::unique_ptr<autofill::PasswordForm>> CopyOf(
} }
break; break;
case ItemTypeCheckForProblemsButton: case ItemTypeCheckForProblemsButton:
if (_passwordCheck->GetPasswordCheckState() != if (self.passwordCheckState != PasswordCheckStateRunning) {
PasswordCheckState::kNoPasswords) {
_passwordCheck->StartPasswordCheck(); _passwordCheck->StartPasswordCheck();
} }
break; break;
......
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