diff --git a/chrome/browser/password_manager/android/password_sync_controller_delegate_android.cc b/chrome/browser/password_manager/android/password_sync_controller_delegate_android.cc index 124908e31cce9..225b73c2e03a5 100644 --- a/chrome/browser/password_manager/android/password_sync_controller_delegate_android.cc +++ b/chrome/browser/password_manager/android/password_sync_controller_delegate_android.cc @@ -55,8 +55,7 @@ void PasswordSyncControllerDelegateAndroid::OnSyncServiceInitialized( syncer::SyncService* sync_service) { sync_service_ = sync_service; sync_observation_.Observe(sync_service); - is_sync_enabled_ = IsSyncEnabled(IsPasswordSyncEnabled(sync_service)); - UpdateCredentialManagerSyncStatus(is_sync_enabled_.value()); + is_sync_enabled_ = IsSyncEnabled(IsPasswordSyncEnabled(sync_service_)); } void PasswordSyncControllerDelegateAndroid::OnSyncStarting( @@ -131,7 +130,20 @@ void PasswordSyncControllerDelegateAndroid:: void PasswordSyncControllerDelegateAndroid::OnStateChanged( syncer::SyncService* sync) { - UpdateCredentialManagerSyncStatus(IsSyncEnabled(IsPasswordSyncEnabled(sync))); + // Notify credential manager about current account on startup or if + // password sync setting has changed. + if (sync_util::IsPasswordSyncEnabled(sync) && + (!credential_manager_sync_setting_.has_value() || + credential_manager_sync_setting_ == IsSyncEnabled(false))) { + bridge_->NotifyCredentialManagerWhenSyncing(); + credential_manager_sync_setting_ = IsSyncEnabled(true); + } + if (!sync_util::IsPasswordSyncEnabled(sync) && + (!credential_manager_sync_setting_.has_value() || + credential_manager_sync_setting_ == IsSyncEnabled(true))) { + bridge_->NotifyCredentialManagerWhenNotSyncing(); + credential_manager_sync_setting_ = IsSyncEnabled(false); + } } void PasswordSyncControllerDelegateAndroid::OnSyncShutdown( @@ -161,21 +173,6 @@ void PasswordSyncControllerDelegateAndroid::OnCredentialManagerError( } } -void PasswordSyncControllerDelegateAndroid::UpdateCredentialManagerSyncStatus( - IsSyncEnabled is_enabled) { - if (credential_manager_sync_setting_.has_value() && - credential_manager_sync_setting_ == is_enabled) { - return; - } - - credential_manager_sync_setting_ = is_enabled; - if (is_enabled) { - bridge_->NotifyCredentialManagerWhenSyncing(); - } else { - bridge_->NotifyCredentialManagerWhenNotSyncing(); - } -} - base::WeakPtr PasswordSyncControllerDelegateAndroid::GetWeakPtrToBaseClass() { return weak_ptr_factory_.GetWeakPtr(); diff --git a/chrome/browser/password_manager/android/password_sync_controller_delegate_android.h b/chrome/browser/password_manager/android/password_sync_controller_delegate_android.h index 203223270a162..cb345dfc503e3 100644 --- a/chrome/browser/password_manager/android/password_sync_controller_delegate_android.h +++ b/chrome/browser/password_manager/android/password_sync_controller_delegate_android.h @@ -72,10 +72,6 @@ class PasswordSyncControllerDelegateAndroid private: using IsSyncEnabled = base::StrongAlias; - // Notify credential manager about current account on startup or if - // password sync setting has changed. - void UpdateCredentialManagerSyncStatus(IsSyncEnabled is_enabled); - base::WeakPtr GetWeakPtrToBaseClass(); const std::unique_ptr bridge_; diff --git a/chrome/browser/password_manager/android/password_sync_controller_delegate_android_unittest.cc b/chrome/browser/password_manager/android/password_sync_controller_delegate_android_unittest.cc index b864cb79f8233..f63fac52c731a 100644 --- a/chrome/browser/password_manager/android/password_sync_controller_delegate_android_unittest.cc +++ b/chrome/browser/password_manager/android/password_sync_controller_delegate_android_unittest.cc @@ -45,7 +45,7 @@ class PasswordSyncControllerDelegateAndroidTest : public testing::Test { void RunUntilIdle() { task_environment_.RunUntilIdle(); } MockPasswordSyncControllerDelegateBridge* bridge() { return bridge_; } - syncer::TestSyncService* sync_service() { return &sync_service_; } + syncer::SyncService* sync_service() { return &sync_service_; } PasswordSyncControllerDelegateAndroid* sync_controller_delegate() { return sync_controller_delegate_.get(); } @@ -69,55 +69,16 @@ class PasswordSyncControllerDelegateAndroidTest : public testing::Test { raw_ptr> bridge_; }; -TEST_F(PasswordSyncControllerDelegateAndroidTest, - OnSyncStatusEnabledOnStartup) { - EXPECT_CALL(*bridge(), NotifyCredentialManagerWhenSyncing); - sync_controller_delegate()->OnSyncServiceInitialized(sync_service()); - testing::Mock::VerifyAndClearExpectations(bridge()); - - // Check that observing the same event again will not trigger another - // notification. - sync_controller_delegate()->OnStateChanged(sync_service()); -} - -TEST_F(PasswordSyncControllerDelegateAndroidTest, - OnSyncStatusEnabledWithoutPasswordsOnStartup) { - sync_service()->GetUserSettings()->SetSelectedTypes(/*sync_everything=*/false, - /*types=*/{}); - - EXPECT_CALL(*bridge(), NotifyCredentialManagerWhenNotSyncing); - sync_controller_delegate()->OnStateChanged(sync_service()); - testing::Mock::VerifyAndClearExpectations(bridge()); - - // Check that observing the same event again will not trigger another - // notification. - sync_controller_delegate()->OnStateChanged(sync_service()); -} - -TEST_F(PasswordSyncControllerDelegateAndroidTest, - OnSyncStatusDisabledOnStartup) { - sync_service()->SetDisableReasons( - syncer::SyncService::DISABLE_REASON_NOT_SIGNED_IN); - - EXPECT_CALL(*bridge(), NotifyCredentialManagerWhenNotSyncing); - sync_controller_delegate()->OnStateChanged(sync_service()); - testing::Mock::VerifyAndClearExpectations(bridge()); - - // Check that observing the same event again will not trigger another - // notification. - sync_controller_delegate()->OnStateChanged(sync_service()); -} - TEST_F(PasswordSyncControllerDelegateAndroidTest, OnSyncStatusChangedToEnabledAfterStartup) { syncer::TestSyncService sync_service; EXPECT_CALL(*bridge(), NotifyCredentialManagerWhenSyncing); sync_controller_delegate()->OnStateChanged(&sync_service); - testing::Mock::VerifyAndClearExpectations(bridge()); // Check that observing the same event again will not trigger another // notification. + EXPECT_CALL(*bridge(), NotifyCredentialManagerWhenSyncing).Times(0); sync_controller_delegate()->OnStateChanged(&sync_service); } @@ -127,6 +88,7 @@ TEST_F(PasswordSyncControllerDelegateAndroidTest, sync_service.GetUserSettings()->SetSelectedTypes(/*sync_everything=*/false, /*types=*/{}); + EXPECT_CALL(*bridge(), NotifyCredentialManagerWhenSyncing).Times(0); EXPECT_CALL(*bridge(), NotifyCredentialManagerWhenNotSyncing); sync_controller_delegate()->OnStateChanged(&sync_service); } @@ -155,10 +117,10 @@ TEST_F(PasswordSyncControllerDelegateAndroidTest, EXPECT_CALL(*bridge(), NotifyCredentialManagerWhenNotSyncing); sync_controller_delegate()->OnStateChanged(&sync_service); - testing::Mock::VerifyAndClearExpectations(bridge()); // Check that observing the same event again will not trigger another // notification. + EXPECT_CALL(*bridge(), NotifyCredentialManagerWhenNotSyncing).Times(0); sync_controller_delegate()->OnStateChanged(&sync_service); } @@ -242,7 +204,6 @@ TEST_F(PasswordSyncControllerDelegateAndroidTest, TEST_F(PasswordSyncControllerDelegateAndroidTest, AttachesObserverOnSyncServiceInitialized) { - EXPECT_CALL(*bridge(), NotifyCredentialManagerWhenSyncing); sync_controller_delegate()->OnSyncServiceInitialized(sync_service()); EXPECT_TRUE(sync_service()->HasObserver(sync_controller_delegate())); }