[UPM] Move password settings between Chrome & GMS on sync changes

This CL ensures that password settings are persisted when the user
enabled/disables password sync.

In particular:
When sync is disabled: non-default GMS password settings should be set
in Chrome prefs.

When sync is enabled:
-If there is no non-default setting in GMS, but the setting exists in
Chrome, Chrome setting should be set in GMS.
-If there is a non-default setting in GMS, it should remain there.
-If Chrome & GMS have conflicting settings, the GMS setting should have
a priority.

Bug: 1321094
Change-Id: Id6d0642e6eccda32848f5bdd1ed0d23210a24d30
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3596758
Reviewed-by: Ioana Pandele <ioanap@chromium.org>
Commit-Queue: Maria Kazinova <kazinova@google.com>
Cr-Commit-Position: refs/heads/main@{#1002352}

chrome/browser/password_manager/android/password_manager_settings_service_android_impl.cc

@@ -15,6 +15,7 @@
 #include "components/password_manager/core/common/password_manager_features.h"
 #include "components/password_manager/core/common/password_manager_pref_names.h"
 #include "components/prefs/pref_service.h"
+#include "components/signin/public/base/signin_pref_names.h"
 #include "components/signin/public/identity_manager/account_info.h"
 #include "components/sync/base/user_selectable_type.h"
 #include "components/sync/driver/sync_service.h"
@@ -85,6 +86,8 @@ PasswordManagerSettingsServiceAndroidImpl::
   lifecycle_helper_->RegisterObserver(base::BindRepeating(
       &PasswordManagerSettingsServiceAndroidImpl::OnChromeForegrounded,
       weak_ptr_factory_.GetWeakPtr()));
+  is_password_sync_enabled_ = IsPasswordSyncEnabled(sync_service);
+  sync_service->AddObserver(this);
 }
 
 // Constructor for tests
@@ -108,6 +111,8 @@ PasswordManagerSettingsServiceAndroidImpl::
   lifecycle_helper_->RegisterObserver(base::BindRepeating(
       &PasswordManagerSettingsServiceAndroidImpl::OnChromeForegrounded,
       weak_ptr_factory_.GetWeakPtr()));
+  is_password_sync_enabled_ = IsPasswordSyncEnabled(sync_service);
+  sync_service->AddObserver(this);
 }
 
 PasswordManagerSettingsServiceAndroidImpl::
@@ -143,14 +148,19 @@ bool PasswordManagerSettingsServiceAndroidImpl::IsSettingEnabled(
 void PasswordManagerSettingsServiceAndroidImpl::OnChromeForegrounded() {
   if (!IsPasswordSyncEnabled(sync_service_))
     return;
-  // TODO(crbug.com/1289700): Request the settings from the backend.
+
+  RequestSettingsFromBackend();
 }
 
 void PasswordManagerSettingsServiceAndroidImpl::OnSettingValueFetched(
-    password_manager::PasswordManagerSetting setting,
+    PasswordManagerSetting setting,
     bool value) {
-  if (!IsPasswordSyncEnabled(sync_service_))
+  UpdateSettingFetchState(setting);
+  if (!fetch_after_sync_status_change_in_progress_ &&
+      !IsPasswordSyncEnabled(sync_service_)) {
     return;
+  }
+
   const PrefService::Preference* android_pref =
       GetGMSPrefFromSetting(pref_service_, setting);
   pref_service_->SetBoolean(android_pref->name(), value);
@@ -170,8 +180,10 @@ void PasswordManagerSettingsServiceAndroidImpl::OnSettingValueFetched(
 void PasswordManagerSettingsServiceAndroidImpl::OnSettingValueAbsent(
     password_manager::PasswordManagerSetting setting) {
   DCHECK(bridge_);
+  UpdateSettingFetchState(setting);
   if (!IsPasswordSyncEnabled(sync_service_))
     return;
+
   const PrefService::Preference* pref =
       GetGMSPrefFromSetting(pref_service_, setting);
 
@@ -202,13 +214,54 @@ void PasswordManagerSettingsServiceAndroidImpl::MigratePrefsIfNeeded() {
   if (!IsPasswordSyncEnabled(sync_service_))
     return;
 
+  DumpChromePrefsIntoGMSPrefs();
+}
+
+void PasswordManagerSettingsServiceAndroidImpl::OnStateChanged(
+    syncer::SyncService* sync) {
+  // Return early if the setting didn't change.
+  if (IsPasswordSyncEnabled(sync) == is_password_sync_enabled_) {
+    return;
+  }
+
+  if (IsPasswordSyncEnabled(sync))
+    DumpChromePrefsIntoGMSPrefs();
+
+  // Fetch settings from the backend to align values stored in GMS Core and
+  // Chrome.
+  is_password_sync_enabled_ = IsPasswordSyncEnabled(sync);
+  fetch_after_sync_status_change_in_progress_ = true;
+  for (PasswordManagerSetting setting : kAllPasswordSettings)
+    awaited_settings_.insert(setting);
+  RequestSettingsFromBackend();
+}
+
+void PasswordManagerSettingsServiceAndroidImpl::RequestSettingsFromBackend() {
+  for (PasswordManagerSetting setting : kAllPasswordSettings) {
+    bridge_->GetPasswordSettingValue(
+        PasswordSettingsUpdaterAndroidBridge::SyncingAccount(
+            pref_service_->GetString(::prefs::kGoogleServicesLastUsername)),
+        setting);
+  }
+}
+
+void PasswordManagerSettingsServiceAndroidImpl::UpdateSettingFetchState(
+    PasswordManagerSetting received_setting) {
+  if (!fetch_after_sync_status_change_in_progress_)
+    return;
+
+  awaited_settings_.erase(received_setting);
+  if (awaited_settings_.empty())
+    fetch_after_sync_status_change_in_progress_ = false;
+}
+
+void PasswordManagerSettingsServiceAndroidImpl::DumpChromePrefsIntoGMSPrefs() {
   for (PasswordManagerSetting setting : kAllPasswordSettings) {
     const PrefService::Preference* regular_pref =
         GetRegularPrefFromSetting(pref_service_, setting);
 
-    if (!pref_service_->GetUserPrefValue(regular_pref->name())) {
+    if (!pref_service_->GetUserPrefValue(regular_pref->name()))
       continue;
-    }
 
     const PrefService::Preference* gms_pref =
         GetGMSPrefFromSetting(pref_service_, setting);

chrome/browser/password_manager/android/password_manager_settings_service_android_impl.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 
+#include "base/containers/flat_set.h"
 #include "base/memory/raw_ptr.h"
 #include "chrome/browser/password_manager/android/password_manager_lifecycle_helper.h"
 #include "chrome/browser/password_manager/android/password_settings_updater_android_bridge.h"
@@ -22,7 +23,8 @@ class PrefService;
 // the possibility of communicating with GMS.
 class PasswordManagerSettingsServiceAndroidImpl
     : public PasswordManagerSettingsService,
-      public password_manager::PasswordSettingsUpdaterAndroidBridge::Consumer {
+      public password_manager::PasswordSettingsUpdaterAndroidBridge::Consumer,
+      public syncer::SyncServiceObserver {
  public:
   PasswordManagerSettingsServiceAndroidImpl(PrefService* pref_service,
                                             syncer::SyncService* sync_service);
@@ -66,6 +68,21 @@ class PasswordManagerSettingsServiceAndroidImpl
   // to migrate again.
   void MigratePrefsIfNeeded();
 
+  // syncer::SyncServiceObserver implementation
+  void OnStateChanged(syncer::SyncService* sync) override;
+
+  // Asynchronously fetch password settings.
+  void RequestSettingsFromBackend();
+
+  // Updates information about the current setting fetch after receiving
+  // a reply from the backend.
+  void UpdateSettingFetchState(
+      password_manager::PasswordManagerSetting received_setting);
+
+  // Copies the values of chrome prefs that have user-set values into the
+  // GMS prefs.
+  void DumpChromePrefsIntoGMSPrefs();
+
   // Pref service used to read and write password manager user prefs.
   raw_ptr<PrefService> pref_service_ = nullptr;
 
@@ -81,6 +98,17 @@ class PasswordManagerSettingsServiceAndroidImpl
   // can request settings values from Google Mobile Services.
   std::unique_ptr<PasswordManagerLifecycleHelper> lifecycle_helper_;
 
+  // Cached value of the password sync setting.
+  bool is_password_sync_enabled_ = false;
+
+  // True if settings were requested from the backend after password sync
+  // setting was changed, and the fetch is still in progress.
+  bool fetch_after_sync_status_change_in_progress_ = false;
+
+  // Settings requested from the backend after a sunc status change, but not
+  // fetched yet.
+  base::flat_set<password_manager::PasswordManagerSetting> awaited_settings_;
+
   base::WeakPtrFactory<PasswordManagerSettingsServiceAndroidImpl>
       weak_ptr_factory_{this};
 };