[Passwords] Enable CredentialManagerImplTest for account storage feature

This CL makes CredentialManagerImplTest paramtereized to make sure
they pass when the the account storage feature is enabled.

This uncovered some bug where the http-->https was invoked twice.

Bug: 1093286
Change-Id: Ieb8db5de2d5f19c144b14cb72797d70888fa2b9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2307255
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790054}

components/password_manager/core/browser/credential_manager_pending_request_task.cc

@@ -114,8 +114,21 @@ CredentialManagerPendingRequestTask::~CredentialManagerPendingRequestTask() =
 
 void CredentialManagerPendingRequestTask::OnGetPasswordStoreResults(
     std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
+  // This class overrides OnGetPasswordStoreResultsFrom() (the version of this
+  // method that also receives the originating store), so the store-less version
+  // never gets called.
+  NOTREACHED();
+}
+
+void CredentialManagerPendingRequestTask::OnGetPasswordStoreResultsFrom(
+    scoped_refptr<PasswordStore> store,
+    std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
   // localhost is a secure origin but not https.
-  if (results.empty() && origin_.scheme() == url::kHttpsScheme) {
+  if (store.get() == delegate_->client()->GetProfilePasswordStore() &&
+      results.empty() && origin_.scheme() == url::kHttpsScheme) {
+    // TODO(crbug.com/1093286): Consider also supporting HTTP->HTTPS migration
+    // for the account store.
+
     // Try to migrate the HTTP passwords and process them later.
     http_migrator_ = std::make_unique<HttpPasswordStoreMigrator>(
         origin_, delegate_->client(), this);

components/password_manager/core/browser/credential_manager_pending_request_task.h

@@ -75,6 +75,9 @@ class CredentialManagerPendingRequestTask
   // PasswordStoreConsumer:
   void OnGetPasswordStoreResults(
       std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
+  void OnGetPasswordStoreResultsFrom(
+      scoped_refptr<PasswordStore> store,
+      std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
 
  private:
   // HttpPasswordStoreMigrator::Consumer:

components/password_manager/core/browser/credential_manager_pending_request_task_unittest.cc

@@ -4,7 +4,9 @@
 
 #include "components/password_manager/core/browser/credential_manager_pending_request_task.h"
 
+#include "base/test/task_environment.h"
 #include "components/password_manager/core/browser/stub_password_manager_client.h"
+#include "components/password_manager/core/browser/test_password_store.h"
 #include "components/password_manager/core/common/credential_manager_types.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -12,6 +14,24 @@
 namespace password_manager {
 
 namespace {
+
+class TestPasswordManagerClient : public StubPasswordManagerClient {
+ public:
+  TestPasswordManagerClient(PasswordStore* profile_store,
+                            PasswordStore* account_store)
+      : profile_store_(profile_store), account_store_(account_store) {}
+  PasswordStore* GetProfilePasswordStore() const override {
+    return profile_store_;
+  }
+  PasswordStore* GetAccountPasswordStore() const override {
+    return account_store_;
+  }
+
+ private:
+  PasswordStore* profile_store_;
+  PasswordStore* account_store_;
+};
+
 class CredentialManagerPendingRequestTaskDelegateMock
     : public CredentialManagerPendingRequestTaskDelegate {
  public:
@@ -37,16 +57,36 @@ class CredentialManagerPendingRequestTaskDelegateMock
 class CredentialManagerPendingRequestTaskTest : public ::testing::Test {
  public:
   CredentialManagerPendingRequestTaskTest() {
-    ON_CALL(delegate_mock_, client).WillByDefault(testing::Return(&client_));
+    profile_store_ = new TestPasswordStore(/*is_account_store=*/false);
+    profile_store_->Init(/*prefs=*/nullptr);
+
+    account_store_ = new TestPasswordStore(/*is_account_store=*/true);
+    account_store_->Init(/*prefs=*/nullptr);
+
+    client_ = std::make_unique<TestPasswordManagerClient>(profile_store_.get(),
+                                                          account_store_.get());
+
+    ON_CALL(delegate_mock_, client)
+        .WillByDefault(testing::Return(client_.get()));
   }
   ~CredentialManagerPendingRequestTaskTest() override = default;
 
+  void TearDown() override {
+    account_store_->ShutdownOnUIThread();
+    profile_store_->ShutdownOnUIThread();
+    // It's needed to cleanup the password store asynchronously.
+    task_environment_.RunUntilIdle();
+  }
+
  protected:
   testing::NiceMock<CredentialManagerPendingRequestTaskDelegateMock>
       delegate_mock_;
+  scoped_refptr<TestPasswordStore> profile_store_;
+  scoped_refptr<TestPasswordStore> account_store_;
 
  private:
-  StubPasswordManagerClient client_;
+  base::test::TaskEnvironment task_environment_;
+  std::unique_ptr<TestPasswordManagerClient> client_;
 };
 
 TEST_F(CredentialManagerPendingRequestTaskTest, QueryProfileStore) {
@@ -58,7 +98,7 @@ TEST_F(CredentialManagerPendingRequestTaskTest, QueryProfileStore) {
   // We are expecting results from only one store, delegate should be called
   // upon getting a response from the store.
   EXPECT_CALL(delegate_mock_, SendCredential);
-  task.OnGetPasswordStoreResults({});
+  task.OnGetPasswordStoreResultsFrom(profile_store_, {});
 }
 
 TEST_F(CredentialManagerPendingRequestTaskTest, QueryProfileAndAccountStores) {
@@ -70,12 +110,12 @@ TEST_F(CredentialManagerPendingRequestTaskTest, QueryProfileAndAccountStores) {
   // We are expecting results from 2 stores, the delegate shouldn't be called
   // until both stores respond.
   EXPECT_CALL(delegate_mock_, SendCredential).Times(0);
-  task.OnGetPasswordStoreResults({});
+  task.OnGetPasswordStoreResultsFrom(profile_store_, {});
 
   testing::Mock::VerifyAndClearExpectations(&delegate_mock_);
 
   EXPECT_CALL(delegate_mock_, SendCredential);
-  task.OnGetPasswordStoreResults({});
+  task.OnGetPasswordStoreResultsFrom(account_store_, {});
 }
 
 }  // namespace password_manager