[UPM] Report sync status to the CredentialManager for signed out users

This CL adds calls to CredentialManager setCurrentAutofillAccount()
for non signed-in users with UPM flag set to enabled. For these users
sync service did not reported status changes after PasswordManager
initialisation which resulted in not informing CredentialManager about
the initial sync status. After this CL, CredentialManager will always
be informed on Chrome startup. Sync status should not change after the
sync service is initialised, so for signed in users there is still
only one call but it's performed earlier.

This should not change any user-visible behaviour and only affects
metrics collected on the GMS ChromeSync side.

Bug: 1402964
Change-Id: Ia3618cf007eb9987ea431b4bbd236036cf59800b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4120437
Reviewed-by: Maria Kazinova <kazinova@google.com>
Commit-Queue: Maxim Anufriev <maxan@google.com>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1086367}

chrome/browser/password_manager/android/password_sync_controller_delegate_android.cc

@@ -55,7 +55,8 @@ void PasswordSyncControllerDelegateAndroid::OnSyncServiceInitialized(
     syncer::SyncService* sync_service) {
   sync_service_ = sync_service;
   sync_observation_.Observe(sync_service);
-  is_sync_enabled_ = IsSyncEnabled(IsPasswordSyncEnabled(sync_service_));
+  is_sync_enabled_ = IsSyncEnabled(IsPasswordSyncEnabled(sync_service));
+  UpdateCredentialManagerSyncStatus(is_sync_enabled_.value());
 }
 
 void PasswordSyncControllerDelegateAndroid::OnSyncStarting(
@@ -130,20 +131,7 @@ void PasswordSyncControllerDelegateAndroid::
 
 void PasswordSyncControllerDelegateAndroid::OnStateChanged(
     syncer::SyncService* 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);
-  }
+  UpdateCredentialManagerSyncStatus(IsSyncEnabled(IsPasswordSyncEnabled(sync)));
 }
 
 void PasswordSyncControllerDelegateAndroid::OnSyncShutdown(
@@ -173,6 +161,21 @@ 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<syncer::ModelTypeControllerDelegate>
 PasswordSyncControllerDelegateAndroid::GetWeakPtrToBaseClass() {
   return weak_ptr_factory_.GetWeakPtr();

chrome/browser/password_manager/android/password_sync_controller_delegate_android.h

@@ -72,6 +72,10 @@ class PasswordSyncControllerDelegateAndroid
  private:
   using IsSyncEnabled = base::StrongAlias<struct IsSyncEnabledTag, bool>;
 
+  // Notify credential manager about current account on startup or if
+  // password sync setting has changed.
+  void UpdateCredentialManagerSyncStatus(IsSyncEnabled is_enabled);
+
   base::WeakPtr<syncer::ModelTypeControllerDelegate> GetWeakPtrToBaseClass();
 
   const std::unique_ptr<PasswordSyncControllerDelegateBridge> bridge_;

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::SyncService* sync_service() { return &sync_service_; }
+  syncer::TestSyncService* sync_service() { return &sync_service_; }
   PasswordSyncControllerDelegateAndroid* sync_controller_delegate() {
     return sync_controller_delegate_.get();
   }
@@ -69,16 +69,55 @@ class PasswordSyncControllerDelegateAndroidTest : public testing::Test {
   raw_ptr<StrictMock<MockPasswordSyncControllerDelegateBridge>> 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);
 }
 
@@ -88,7 +127,6 @@ 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);
 }
@@ -117,10 +155,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);
 }
 
@@ -204,6 +242,7 @@ 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()));
 }