[Passwords] Delete sync-ed entries if recently restored from backup

Analogous to crrev.com/c/4727928 (bookmarks) and crrev.com/c/4748239
(reading list), ported here for passwords.

The new logic is guarded behind feature flags (one of which is a
dedicated kill switch for passwords).

The design follows the very same principles (see description of the
patches linked above), and the implementation is essentially identical
to the one for reading list.

Specifically, ModelTypeSyncBridge::ApplyDisableSyncChanges() is
guaranteed to be invoked in all relevant cases. However, determining
whether the new behavior should be triggered requires careful guarding
during ModelReadyToSync().

Bug: 1468502
Change-Id: I32311efebc5ffa8af5021d3fecf8480e84a597dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4753585
Reviewed-by: Maria Kazinova <kazinova@google.com>
Auto-Submit: Marc Treib <treib@chromium.org>
Commit-Queue: John Wu <jzw@chromium.org>
Reviewed-by: John Wu <jzw@chromium.org>
Reviewed-by: Rushan Suleymanov <rushans@google.com>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1180970}

chrome/browser/password_manager/account_password_store_factory.cc

@@ -171,11 +171,13 @@ AccountPasswordStoreFactory::BuildServiceInstanceFor(
 #if BUILDFLAG(IS_ANDROID)
       new password_manager::PasswordStore(
           std::make_unique<password_manager::PasswordStoreBuiltInBackend>(
-              std::move(login_db)));
+              std::move(login_db),
+              syncer::WipeModelUponSyncDisabledBehavior::kAlways));
 #else
       new password_manager::PasswordStore(
           std::make_unique<password_manager::PasswordStoreBuiltInBackend>(
               std::move(login_db),
+              syncer::WipeModelUponSyncDisabledBehavior::kAlways,
               std::make_unique<UnsyncedCredentialsDeletionNotifierImpl>(
                   profile)));
 #endif

chrome/browser/password_manager/password_store_backend_factory.cc

@@ -30,7 +30,8 @@ std::unique_ptr<PasswordStoreBackend> PasswordStoreBackend::Create(
   TRACE_EVENT0("passwords", "PasswordStoreBackendCreation");
 #if !BUILDFLAG(IS_ANDROID) || BUILDFLAG(USE_LEGACY_PASSWORD_STORE_BACKEND)
   return std::make_unique<PasswordStoreBuiltInBackend>(
-      CreateLoginDatabaseForProfileStorage(login_db_path));
+      CreateLoginDatabaseForProfileStorage(login_db_path),
+      syncer::WipeModelUponSyncDisabledBehavior::kNever);
 #else  // BUILDFLAG(IS_ANDROID) && !USE_LEGACY_PASSWORD_STORE_BACKEND
   if (PasswordStoreAndroidBackendBridgeHelper::CanCreateBackend() &&
       base::FeatureList::IsEnabled(
@@ -49,11 +50,13 @@ std::unique_ptr<PasswordStoreBackend> PasswordStoreBackend::Create(
                               kTimesAttemptedToReenrollToGoogleMobileServices));
     return std::make_unique<PasswordStoreBackendMigrationDecorator>(
         std::make_unique<PasswordStoreBuiltInBackend>(
-            CreateLoginDatabaseForProfileStorage(login_db_path)),
+            CreateLoginDatabaseForProfileStorage(login_db_path),
+            syncer::WipeModelUponSyncDisabledBehavior::kNever),
         std::make_unique<PasswordStoreAndroidBackend>(prefs), prefs);
   }
   return std::make_unique<PasswordStoreBuiltInBackend>(
-      CreateLoginDatabaseForProfileStorage(login_db_path));
+      CreateLoginDatabaseForProfileStorage(login_db_path),
+      syncer::WipeModelUponSyncDisabledBehavior::kNever);
 #endif
 }
 

components/password_manager/core/browser/login_database_async_helper.cc

@@ -27,8 +27,12 @@ constexpr base::TimeDelta kSyncTaskTimeout = base::Seconds(30);
 LoginDatabaseAsyncHelper::LoginDatabaseAsyncHelper(
     std::unique_ptr<LoginDatabase> login_db,
     std::unique_ptr<UnsyncedCredentialsDeletionNotifier> notifier,
-    scoped_refptr<base::SequencedTaskRunner> main_task_runner)
+    scoped_refptr<base::SequencedTaskRunner> main_task_runner,
+    syncer::WipeModelUponSyncDisabledBehavior
+        wipe_model_upon_sync_disabled_behavior)
     : login_db_(std::move(login_db)),
+      wipe_model_upon_sync_disabled_behavior_(
+          wipe_model_upon_sync_disabled_behavior),
       deletion_notifier_(std::move(notifier)),
       main_task_runner_(std::move(main_task_runner)) {
   DETACH_FROM_SEQUENCE(sequence_checker_);
@@ -75,6 +79,7 @@ bool LoginDatabaseAsyncHelper::Initialize(
       std::make_unique<syncer::ClientTagBasedModelTypeProcessor>(
           syncer::PASSWORDS, base::DoNothing()),
       static_cast<PasswordStoreSync*>(this),
+      wipe_model_upon_sync_disabled_behavior_,
       std::move(sync_enabled_or_disabled_cb));
 
 // On Windows encryption capability is expected to be available by default.

components/password_manager/core/browser/login_database_async_helper.h

@@ -11,6 +11,7 @@
 #include "base/task/sequenced_task_runner.h"
 #include "components/password_manager/core/browser/password_store_backend.h"
 #include "components/password_manager/core/browser/password_store_sync.h"
+#include "components/sync/model/wipe_model_upon_sync_disabled_behavior.h"
 
 namespace syncer {
 class ModelTypeControllerDelegate;
@@ -31,7 +32,9 @@ class LoginDatabaseAsyncHelper : private PasswordStoreSync {
   LoginDatabaseAsyncHelper(
       std::unique_ptr<LoginDatabase> login_db,
       std::unique_ptr<UnsyncedCredentialsDeletionNotifier> notifier,
-      scoped_refptr<base::SequencedTaskRunner> main_task_runner);
+      scoped_refptr<base::SequencedTaskRunner> main_task_runner,
+      syncer::WipeModelUponSyncDisabledBehavior
+          wipe_model_upon_sync_disabled_behavior);
 
   ~LoginDatabaseAsyncHelper() override;
 
@@ -119,6 +122,8 @@ class LoginDatabaseAsyncHelper : private PasswordStoreSync {
   std::unique_ptr<LoginDatabase> login_db_
       GUARDED_BY_CONTEXT(sequence_checker_);
 
+  const syncer::WipeModelUponSyncDisabledBehavior
+      wipe_model_upon_sync_disabled_behavior_;
   std::unique_ptr<PasswordSyncBridge> password_sync_bridge_
       GUARDED_BY_CONTEXT(sequence_checker_);
 

components/password_manager/core/browser/password_store_built_in_backend.cc

@@ -50,14 +50,17 @@ base::OnceCallback<Result(Result)> ReportMetricsForResultCallback(
 
 PasswordStoreBuiltInBackend::PasswordStoreBuiltInBackend(
     std::unique_ptr<LoginDatabase> login_db,
+    syncer::WipeModelUponSyncDisabledBehavior
+        wipe_model_upon_sync_disabled_behavior,
     std::unique_ptr<UnsyncedCredentialsDeletionNotifier> notifier) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   background_task_runner_ = base::ThreadPool::CreateSequencedTaskRunner(
       {base::MayBlock(), base::TaskPriority::USER_VISIBLE});
   DCHECK(background_task_runner_);
   helper_ = std::make_unique<LoginDatabaseAsyncHelper>(
       std::move(login_db), std::move(notifier),
-      base::SequencedTaskRunner::GetCurrentDefault());
+      base::SequencedTaskRunner::GetCurrentDefault(),
+      wipe_model_upon_sync_disabled_behavior);
 }
 
 PasswordStoreBuiltInBackend::~PasswordStoreBuiltInBackend() {

components/password_manager/core/browser/password_store_built_in_backend.h

@@ -13,6 +13,7 @@
 #include "base/memory/weak_ptr.h"
 #include "components/password_manager/core/browser/password_store_backend.h"
 #include "components/password_manager/core/browser/smart_bubble_stats_store.h"
+#include "components/sync/model/wipe_model_upon_sync_disabled_behavior.h"
 
 namespace base {
 class SequencedTaskRunner;
@@ -37,6 +38,8 @@ class PasswordStoreBuiltInBackend : public PasswordStoreBackend,
   // a deferred manner on the background sequence.
   PasswordStoreBuiltInBackend(
       std::unique_ptr<LoginDatabase> login_db,
+      syncer::WipeModelUponSyncDisabledBehavior
+          wipe_model_upon_sync_disabled_behavior,
       std::unique_ptr<UnsyncedCredentialsDeletionNotifier> notifier = nullptr);
 
   ~PasswordStoreBuiltInBackend() override;

components/password_manager/core/browser/password_store_built_in_backend_unittest.cc

@@ -127,7 +127,8 @@ class PasswordStoreBuiltInBackendTest : public testing::Test {
                                                  IsAccountStore(false));
     }
 
-    store_ = std::make_unique<PasswordStoreBuiltInBackend>(std::move(database));
+    store_ = std::make_unique<PasswordStoreBuiltInBackend>(
+        std::move(database), syncer::WipeModelUponSyncDisabledBehavior::kNever);
     PasswordStoreBackend* backend = store_.get();
     backend->InitBackend(affiliated_match_helper,
                          /*remote_form_changes_received=*/base::DoNothing(),

components/password_manager/core/browser/password_store_unittest.cc

@@ -204,8 +204,8 @@ class PasswordStoreTest : public testing::Test {
   scoped_refptr<PasswordStore> CreatePasswordStore() {
     return new PasswordStore(std::make_unique<PasswordStoreBuiltInBackend>(
         std::make_unique<LoginDatabase>(
-            test_login_db_file_path(),
-            password_manager::IsAccountStore(false))));
+            test_login_db_file_path(), password_manager::IsAccountStore(false)),
+        syncer::WipeModelUponSyncDisabledBehavior::kNever));
   }
 
   TestingPrefServiceSimple* pref_service() { return &pref_service_; }

components/password_manager/core/browser/sync/password_sync_bridge.cc

@@ -10,9 +10,7 @@
 
 #include "base/auto_reset.h"
 #include "base/check_op.h"
-#include "base/containers/flat_map.h"
 #include "base/feature_list.h"
-#include "base/functional/bind.h"
 #include "base/functional/callback_helpers.h"
 #include "base/memory/raw_ptr.h"
 #include "base/metrics/histogram_functions.h"
@@ -314,9 +312,13 @@ class ScopedStoreTransaction {
 PasswordSyncBridge::PasswordSyncBridge(
     std::unique_ptr<syncer::ModelTypeChangeProcessor> change_processor,
     PasswordStoreSync* password_store_sync,
+    syncer::WipeModelUponSyncDisabledBehavior
+        wipe_model_upon_sync_disabled_behavior,
     const base::RepeatingClosure& sync_enabled_or_disabled_cb)
     : ModelTypeSyncBridge(std::move(change_processor)),
       password_store_sync_(password_store_sync),
+      wipe_model_upon_sync_disabled_behavior_(
+          wipe_model_upon_sync_disabled_behavior),
       sync_enabled_or_disabled_cb_(sync_enabled_or_disabled_cb) {
   DCHECK(password_store_sync_);
   DCHECK(sync_enabled_or_disabled_cb_);
@@ -400,6 +402,22 @@ PasswordSyncBridge::PasswordSyncBridge(
   base::UmaHistogramEnumeration("PasswordManager.SyncMetadataReadError2",
                                 sync_metadata_read_error);
 
+  if (wipe_model_upon_sync_disabled_behavior_ ==
+          syncer::WipeModelUponSyncDisabledBehavior::kOnceIfTrackingMetadata &&
+      (!batch || !syncer::IsInitialSyncDone(
+                     batch->GetModelTypeState().initial_sync_state()))) {
+    // Since the model isn't initially tracking metadata, move away from
+    // kOnceIfTrackingMetadata so the behavior doesn't kick in, in case sync
+    // is turned on later and back to off.
+    //
+    // Note that implementing this using IsInitialSyncDone(), instead of
+    // invoking IsTrackingMetadata() later, is more reliable, because the
+    // function cannot be trusted in ApplyDisableSyncChanges(), as it can
+    // return false negatives.
+    wipe_model_upon_sync_disabled_behavior_ =
+        syncer::WipeModelUponSyncDisabledBehavior::kNever;
+  }
+
   if (batch) {
     this->change_processor()->ModelReadyToSync(std::move(batch));
   }
@@ -981,13 +999,27 @@ bool PasswordSyncBridge::SupportsGetStorageKey() const {
 
 void PasswordSyncBridge::ApplyDisableSyncChanges(
     std::unique_ptr<syncer::MetadataChangeList> delete_metadata_change_list) {
-  if (!password_store_sync_->IsAccountStore()) {
-    password_store_sync_->GetMetadataStore()->DeleteAllSyncMetadata(
-        syncer::PASSWORDS);
-    sync_enabled_or_disabled_cb_.Run();
-    return;
+  switch (wipe_model_upon_sync_disabled_behavior_) {
+    case syncer::WipeModelUponSyncDisabledBehavior::kNever:
+      CHECK(!password_store_sync_->IsAccountStore());
+      // The actual model data should NOT be wiped. Only wipe the metadata.
+      password_store_sync_->GetMetadataStore()->DeleteAllSyncMetadata(
+          syncer::PASSWORDS);
+      sync_enabled_or_disabled_cb_.Run();
+      return;
+    case syncer::WipeModelUponSyncDisabledBehavior::kOnceIfTrackingMetadata:
+      CHECK(!password_store_sync_->IsAccountStore());
+      // Wipe the model data this once, and flip the behavior to kNever so it
+      // doesn't get wiped again.
+      wipe_model_upon_sync_disabled_behavior_ =
+          syncer::WipeModelUponSyncDisabledBehavior::kNever;
+      break;
+    case syncer::WipeModelUponSyncDisabledBehavior::kAlways:
+      CHECK(password_store_sync_->IsAccountStore());
+      break;
   }
-  // For the account store, the data should be deleted too. So do the following:
+
+  // The data should be deleted too. So do the following:
   // 1. Collect the credentials that will be deleted.
   // 2. Collect which credentials out of those to be deleted are unsynced.
   // 3. Delete the metadata and the data.
@@ -1022,13 +1054,15 @@ void PasswordSyncBridge::ApplyDisableSyncChanges(
   password_store_sync_->DeleteAndRecreateDatabaseFile();
   password_store_sync_->NotifyCredentialsChanged(password_store_changes);
 
-  base::UmaHistogramCounts100(
-      "PasswordManager.AccountStorage.UnsyncedPasswordsFoundDuringSignOut",
-      unsynced_credentials_being_deleted.size());
+  if (password_store_sync_->IsAccountStore()) {
+    base::UmaHistogramCounts100(
+        "PasswordManager.AccountStorage.UnsyncedPasswordsFoundDuringSignOut",
+        unsynced_credentials_being_deleted.size());
 
-  if (!unsynced_credentials_being_deleted.empty()) {
-    password_store_sync_->NotifyUnsyncedCredentialsWillBeDeleted(
-        std::move(unsynced_credentials_being_deleted));
+    if (!unsynced_credentials_being_deleted.empty()) {
+      password_store_sync_->NotifyUnsyncedCredentialsWillBeDeleted(
+          std::move(unsynced_credentials_being_deleted));
+    }
   }
 
   sync_enabled_or_disabled_cb_.Run();

components/password_manager/core/browser/sync/password_sync_bridge.h

@@ -12,6 +12,7 @@
 #include "components/password_manager/core/browser/password_store_sync.h"
 #include "components/sync/model/metadata_batch.h"
 #include "components/sync/model/model_type_sync_bridge.h"
+#include "components/sync/model/wipe_model_upon_sync_disabled_behavior.h"
 
 namespace syncer {
 class MetadataChangeList;
@@ -36,6 +37,8 @@ class PasswordSyncBridge : public syncer::ModelTypeSyncBridge {
   PasswordSyncBridge(
       std::unique_ptr<syncer::ModelTypeChangeProcessor> change_processor,
       PasswordStoreSync* password_store_sync,
+      syncer::WipeModelUponSyncDisabledBehavior
+          wipe_model_upon_sync_disabled_behavior,
       const base::RepeatingClosure& sync_enabled_or_disabled_cb);
 
   PasswordSyncBridge(const PasswordSyncBridge&) = delete;
@@ -94,6 +97,10 @@ class PasswordSyncBridge : public syncer::ModelTypeSyncBridge {
   // Password store responsible for persistence.
   const raw_ptr<PasswordStoreSync> password_store_sync_;
 
+  syncer::WipeModelUponSyncDisabledBehavior
+      wipe_model_upon_sync_disabled_behavior_ =
+          syncer::WipeModelUponSyncDisabledBehavior::kNever;
+
   base::RepeatingClosure sync_enabled_or_disabled_cb_;
 
   // True if processing remote sync changes is in progress. Used to ignore