Update test setup for features::kEnablePasswordsAccountStorage

With password_manager::features::kEnablePasswordsAccountStorage enabled,
there will be two PasswordStore instances, a profile-scoped one and an
account-scoped one. This CL updates the setup of a few unit tests so
they instantiate the account-scoped store in addition to the
profile-scoped one. Without this change, the tests were hitting some
DCHECKs when run with kEnablePasswordsAccountStorage enabled.

Bug: 1098677
Change-Id: Ic31b5ff593139788b7ffbf8fd67a1d8edb401ff1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2332602Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795458}

chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc

@@ -23,6 +23,7 @@
 #include "chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.h"
 #include "chrome/browser/extensions/api/passwords_private/passwords_private_event_router.h"
 #include "chrome/browser/extensions/api/passwords_private/passwords_private_event_router_factory.h"
+#include "chrome/browser/password_manager/account_password_store_factory.h"
 #include "chrome/browser/password_manager/chrome_password_manager_client.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/common/extensions/api/passwords_private.h"
@@ -36,6 +37,7 @@
 #include "components/password_manager/core/browser/password_manager_test_utils.h"
 #include "components/password_manager/core/browser/reauth_purpose.h"
 #include "components/password_manager/core/browser/test_password_store.h"
+#include "components/password_manager/core/common/password_manager_features.h"
 #include "components/signin/public/base/signin_metrics.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/test/browser_task_environment.h"
@@ -76,6 +78,21 @@ scoped_refptr<TestPasswordStore> CreateAndUseTestPasswordStore(
           .get()));
 }
 
+scoped_refptr<TestPasswordStore> CreateAndUseTestAccountPasswordStore(
+    Profile* profile) {
+  if (!base::FeatureList::IsEnabled(
+          password_manager::features::kEnablePasswordsAccountStorage)) {
+    return nullptr;
+  }
+  return base::WrapRefCounted(static_cast<TestPasswordStore*>(
+      AccountPasswordStoreFactory::GetInstance()
+          ->SetTestingFactoryAndUse(
+              profile,
+              base::BindRepeating(&password_manager::BuildPasswordStore<
+                                  content::BrowserContext, TestPasswordStore>))
+          .get()));
+}
+
 class MockPasswordManagerClient : public ChromePasswordManagerClient {
  public:
   // Creates the mock and attaches it to |web_contents|.
@@ -200,6 +217,8 @@ class PasswordsPrivateDelegateImplTest : public testing::Test {
   extensions::TestEventRouter* event_router_ = nullptr;
   scoped_refptr<TestPasswordStore> store_ =
       CreateAndUseTestPasswordStore(&profile_);
+  scoped_refptr<TestPasswordStore> account_store_ =
+      CreateAndUseTestAccountPasswordStore(&profile_);
   ui::TestClipboard* test_clipboard_ =
       ui::TestClipboard::CreateForCurrentThread();
 

chrome/browser/safe_browsing/chrome_password_protection_service_unittest.cc

@@ -13,6 +13,7 @@
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
+#include "chrome/browser/password_manager/account_password_store_factory.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/safe_browsing/ui_manager.h"
 #include "chrome/browser/signin/chrome_signin_client_factory.h"
@@ -211,16 +212,11 @@ class MockChromePasswordProtectionService
 class ChromePasswordProtectionServiceTest
     : public ChromeRenderViewHostTestHarness {
  public:
-  ChromePasswordProtectionServiceTest() {}
+  ChromePasswordProtectionServiceTest() = default;
 
   void SetUp() override {
     ChromeRenderViewHostTestHarness::SetUp();
 
-    // Use MockPasswordStore to remove a possible race. Normally the
-    // PasswordStore does its database manipulation on the DB thread, which
-    // creates a possible race during navigation. Specifically the
-    // PasswordManager will ignore any forms in a page if the load from the
-    // PasswordStore has not completed.
     password_store_ =
         base::WrapRefCounted(static_cast<password_manager::MockPasswordStore*>(
             PasswordStoreFactory::GetInstance()
@@ -231,6 +227,19 @@ class ChromePasswordProtectionServiceTest
                                         password_manager::MockPasswordStore>))
                 .get()));
 
+    if (base::FeatureList::IsEnabled(
+            password_manager::features::kEnablePasswordsAccountStorage)) {
+      account_password_store_ = base::WrapRefCounted(
+          static_cast<password_manager::MockPasswordStore*>(
+              AccountPasswordStoreFactory::GetInstance()
+                  ->SetTestingFactoryAndUse(
+                      profile(),
+                      base::BindRepeating(&password_manager::BuildPasswordStore<
+                                          content::BrowserContext,
+                                          password_manager::MockPasswordStore>))
+                  .get()));
+    }
+
     profile()->GetPrefs()->SetBoolean(prefs::kSafeBrowsingEnabled, true);
     profile()->GetPrefs()->SetInteger(
         prefs::kPasswordProtectionWarningTrigger,
@@ -273,6 +282,8 @@ class ChromePasswordProtectionServiceTest
     base::RunLoop().RunUntilIdle();
     service_.reset();
     request_ = nullptr;
+    if (account_password_store_)
+      account_password_store_->ShutdownOnUIThread();
     password_store_->ShutdownOnUIThread();
     identity_test_env_profile_adaptor_.reset();
     cache_manager_.reset();
@@ -387,6 +398,7 @@ class ChromePasswordProtectionServiceTest
       identity_test_env_profile_adaptor_;
   MockSecurityEventRecorder* security_event_recorder_;
   scoped_refptr<password_manager::MockPasswordStore> password_store_;
+  scoped_refptr<password_manager::MockPasswordStore> account_password_store_;
   // Owned by KeyedServiceFactory.
   syncer::FakeUserEventService* fake_user_event_service_;
 #if !defined(OS_ANDROID)

chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc

@@ -40,6 +40,7 @@
 #include "chrome/browser/enterprise/connectors/connectors_manager.h"
 #include "chrome/browser/extensions/api/safe_browsing_private/safe_browsing_private_event_router_factory.h"
 #include "chrome/browser/history/history_service_factory.h"
+#include "chrome/browser/password_manager/account_password_store_factory.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service.h"
 #include "chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service_factory.h"
@@ -71,6 +72,7 @@
 #include "components/keyed_service/content/browser_context_keyed_service_factory.h"
 #include "components/password_manager/core/browser/password_manager_test_utils.h"
 #include "components/password_manager/core/browser/test_password_store.h"
+#include "components/password_manager/core/common/password_manager_features.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/safe_browsing/content/web_ui/safe_browsing_ui.h"
@@ -300,6 +302,13 @@ class DownloadProtectionServiceTestBase
         base::BindRepeating(
             &password_manager::BuildPasswordStore<
                 content::BrowserContext, password_manager::TestPasswordStore>));
+    if (base::FeatureList::IsEnabled(
+            password_manager::features::kEnablePasswordsAccountStorage)) {
+      AccountPasswordStoreFactory::GetInstance()->SetTestingFactory(
+          profile(), base::BindRepeating(&password_manager::BuildPasswordStore<
+                                         content::BrowserContext,
+                                         password_manager::TestPasswordStore>));
+    }
 
     // Start real threads for the IO and File threads so that the DCHECKs
     // to test that we're on the correct thread work.

components/autofill_assistant/browser/BUILD.gn

@@ -304,6 +304,7 @@ source_set("unit_tests") {
     "//base/test:test_support",
     "//components/autofill/core/browser:test_support",
     "//components/password_manager/core/browser:test_support",
+    "//components/password_manager/core/common",
     "//components/strings:components_strings_grit",
     "//components/version_info",
     "//content/test:test_support",

components/autofill_assistant/browser/DEPS

@@ -3,6 +3,7 @@ include_rules = [
   "+components/autofill",
   "+components/google/core/common",
   "+components/password_manager/core/browser",
+  "+components/password_manager/core/common",
   "+components/version_info",
   "+content/public/browser",
   "+content/public/test",

components/autofill_assistant/browser/website_login_manager_impl_unittest.cc

@@ -12,6 +12,7 @@
 #include "components/password_manager/core/browser/password_store_consumer.h"
 #include "components/password_manager/core/browser/stub_password_manager_client.h"
 #include "components/password_manager/core/browser/stub_password_manager_driver.h"
+#include "components/password_manager/core/common/password_manager_features.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -45,6 +46,8 @@ class MockPasswordManagerClient
 
   MOCK_CONST_METHOD0(GetProfilePasswordStore,
                      password_manager::PasswordStore*());
+  MOCK_CONST_METHOD0(GetAccountPasswordStore,
+                     password_manager::PasswordStore*());
 };
 
 class MockPasswordManagerDriver
@@ -81,6 +84,7 @@ PasswordForm MakeSimplePasswordForm() {
   form.username_value = ASCIIToUTF16(kFakeUsername);
   form.username_element = ASCIIToUTF16(kUsernameElement);
   form.password_element = ASCIIToUTF16(kPasswordElement);
+  form.in_store = PasswordForm::Store::kProfileStore;
 
   return form;
 }
@@ -90,6 +94,7 @@ PasswordForm MakeSimplePasswordFormWithoutUsername() {
   form.url = GURL(kFakeUrl);
   form.signon_realm = form.url.GetOrigin().spec();
   form.password_value = ASCIIToUTF16(kFakeNewPassword);
+  form.in_store = PasswordForm::Store::kProfileStore;
 
   return form;
 }
@@ -103,11 +108,22 @@ class WebsiteLoginManagerImplTest : public testing::Test {
 
  protected:
   void SetUp() override {
-    store_ = new password_manager::MockPasswordStore;
-    ASSERT_TRUE(store_->Init(/*prefs=*/nullptr));
+    profile_store_ = new password_manager::MockPasswordStore;
+    ON_CALL(*profile_store_, IsAccountStore()).WillByDefault(Return(false));
+    ASSERT_TRUE(profile_store_->Init(/*prefs=*/nullptr));
 
     ON_CALL(client_, GetProfilePasswordStore())
-        .WillByDefault(Return(store_.get()));
+        .WillByDefault(Return(profile_store_.get()));
+
+    if (base::FeatureList::IsEnabled(
+            password_manager::features::kEnablePasswordsAccountStorage)) {
+      account_store_ = new password_manager::MockPasswordStore;
+      ON_CALL(*account_store_, IsAccountStore()).WillByDefault(Return(true));
+      ASSERT_TRUE(account_store_->Init(/*prefs=*/nullptr));
+
+      ON_CALL(client_, GetAccountPasswordStore())
+          .WillByDefault(Return(account_store_.get()));
+    }
 
     ON_CALL(driver_, GetId()).WillByDefault(Return(0));
     ON_CALL(driver_, IsMainFrame()).WillByDefault(Return(true));
@@ -115,10 +131,15 @@ class WebsiteLoginManagerImplTest : public testing::Test {
     manager_ = std::make_unique<WebsiteLoginManagerImpl>(&client_, &driver_);
   }
 
-  void TearDown() override { store_->ShutdownOnUIThread(); }
+  void TearDown() override {
+    if (account_store_) {
+      account_store_->ShutdownOnUIThread();
+    }
+    profile_store_->ShutdownOnUIThread();
+  }
 
   WebsiteLoginManagerImpl* manager() { return manager_.get(); }
-  password_manager::MockPasswordStore* store() { return store_.get(); }
+  password_manager::MockPasswordStore* store() { return profile_store_.get(); }
 
   void WaitForPasswordStore() { task_environment_.RunUntilIdle(); }
 
@@ -126,7 +147,8 @@ class WebsiteLoginManagerImplTest : public testing::Test {
   testing::NiceMock<MockPasswordManagerClient> client_;
   MockPasswordManagerDriver driver_;
   std::unique_ptr<WebsiteLoginManagerImpl> manager_;
-  scoped_refptr<password_manager::MockPasswordStore> store_;
+  scoped_refptr<password_manager::MockPasswordStore> profile_store_;
+  scoped_refptr<password_manager::MockPasswordStore> account_store_;
   content::BrowserTaskEnvironment task_environment_{
       base::test::TaskEnvironment::TimeSource::MOCK_TIME,
       content::BrowserTaskEnvironment::IO_MAINLOOP};