[Passwords] Query AccountStore for Credentials Management API

Before this CL: only the profile store is accessed when reading
credentials for credential management API.

After this CL: if the account store is available, passwords are read
from it as well.

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

components/password_manager/core/browser/BUILD.gn

@@ -539,6 +539,7 @@ source_set("unit_tests") {
     "credential_manager_impl_unittest.cc",
     "credential_manager_logger_unittest.cc",
     "credential_manager_password_form_manager_unittest.cc",
+    "credential_manager_pending_request_task_unittest.cc",
     "credentials_cleaner_runner_unittest.cc",
     "credentials_cleaner_unittest.cc",
     "export/csv_writer_unittest.cc",

components/password_manager/core/browser/credential_manager_impl.cc

@@ -147,14 +147,20 @@ void CredentialManagerImpl::Get(CredentialMediationRequirement mediation,
         metrics_util::CredentialManagerGetResult::kNoneZeroClickOff, mediation);
     return;
   }
-
+  StoresToQuery stores_to_query = GetAccountPasswordStore()
+                                      ? StoresToQuery::kProfileAndAccountStores
+                                      : StoresToQuery::kProfileStore;
   pending_request_ = std::make_unique<CredentialManagerPendingRequestTask>(
       this, base::BindOnce(&RunGetCallback, std::move(callback)), mediation,
-      include_passwords, federations);
+      include_passwords, federations, stores_to_query);
   // This will result in a callback to
   // PendingRequestTask::OnGetPasswordStoreResults().
   GetProfilePasswordStore()->GetLogins(GetSynthesizedFormForOrigin(),
                                        pending_request_.get());
+  if (GetAccountPasswordStore()) {
+    GetAccountPasswordStore()->GetLogins(GetSynthesizedFormForOrigin(),
+                                         pending_request_.get());
+  }
 }
 
 bool CredentialManagerImpl::IsZeroClickAllowed() const {

components/password_manager/core/browser/credential_manager_pending_request_task.cc

@@ -87,13 +87,23 @@ CredentialManagerPendingRequestTask::CredentialManagerPendingRequestTask(
     SendCredentialCallback callback,
     CredentialMediationRequirement mediation,
     bool include_passwords,
-    const std::vector<GURL>& request_federations)
+    const std::vector<GURL>& request_federations,
+    StoresToQuery stores_to_query)
     : delegate_(delegate),
       send_callback_(std::move(callback)),
       mediation_(mediation),
       origin_(delegate_->GetOrigin()),
       include_passwords_(include_passwords) {
   CHECK(!net::IsCertStatusError(delegate_->client()->GetMainFrameCertStatus()));
+  switch (stores_to_query) {
+    case StoresToQuery::kProfileStore:
+      expected_stores_to_respond_ = 1;
+      break;
+    case StoresToQuery::kProfileAndAccountStores:
+      expected_stores_to_respond_ = 2;
+      break;
+  }
+
   for (const GURL& federation : request_federations)
     federations_.insert(
         url::Origin::Create(federation.GetOrigin()).Serialize());
@@ -111,12 +121,24 @@ void CredentialManagerPendingRequestTask::OnGetPasswordStoreResults(
         origin_, delegate_->client(), this);
     return;
   }
-  ProcessForms(std::move(results));
+  AggregatePasswordStoreResults(std::move(results));
 }
 
 void CredentialManagerPendingRequestTask::ProcessMigratedForms(
     std::vector<std::unique_ptr<autofill::PasswordForm>> forms) {
-  ProcessForms(std::move(forms));
+  AggregatePasswordStoreResults(std::move(forms));
+}
+
+void CredentialManagerPendingRequestTask::AggregatePasswordStoreResults(
+    std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
+  // Store the results.
+  for (auto& form : results)
+    partial_results_.push_back(std::move(form));
+
+  // If we're still awaiting more results, nothing else to do.
+  if (--expected_stores_to_respond_ > 0)
+    return;
+  ProcessForms(std::move(partial_results_));
 }
 
 void CredentialManagerPendingRequestTask::ProcessForms(

components/password_manager/core/browser/credential_manager_pending_request_task.h

@@ -31,6 +31,7 @@ class PasswordManagerClient;
 using SendCredentialCallback =
     base::OnceCallback<void(const CredentialInfo& credential)>;
 
+enum class StoresToQuery { kProfileStore, kProfileAndAccountStores };
 // Sends credentials retrieved from the PasswordStore to CredentialManager API
 // clients and retrieves embedder-dependent information.
 class CredentialManagerPendingRequestTaskDelegate {
@@ -65,7 +66,8 @@ class CredentialManagerPendingRequestTask
       SendCredentialCallback callback,
       CredentialMediationRequirement mediation,
       bool include_passwords,
-      const std::vector<GURL>& request_federations);
+      const std::vector<GURL>& request_federations,
+      StoresToQuery stores_to_query);
   ~CredentialManagerPendingRequestTask() override;
 
   const url::Origin& origin() const { return origin_; }
@@ -79,6 +81,9 @@ class CredentialManagerPendingRequestTask
   void ProcessMigratedForms(
       std::vector<std::unique_ptr<autofill::PasswordForm>> forms) override;
 
+  void AggregatePasswordStoreResults(
+      std::vector<std::unique_ptr<autofill::PasswordForm>> results);
+
   void ProcessForms(
       std::vector<std::unique_ptr<autofill::PasswordForm>> results);
 
@@ -88,6 +93,11 @@ class CredentialManagerPendingRequestTask
   const url::Origin origin_;
   const bool include_passwords_;
   std::set<std::string> federations_;
+  int expected_stores_to_respond_;
+  // In case of querying both the profile and account stores, it contains the
+  // partial results received from one store until the second store responds and
+  // then all results are processed.
+  std::vector<std::unique_ptr<autofill::PasswordForm>> partial_results_;
 
   std::unique_ptr<HttpPasswordStoreMigrator> http_migrator_;
 

components/password_manager/core/browser/credential_manager_pending_request_task_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/password_manager/core/browser/credential_manager_pending_request_task.h"
+
+#include "components/password_manager/core/browser/stub_password_manager_client.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"
+
+namespace password_manager {
+
+namespace {
+class CredentialManagerPendingRequestTaskDelegateMock
+    : public CredentialManagerPendingRequestTaskDelegate {
+ public:
+  CredentialManagerPendingRequestTaskDelegateMock() = default;
+  ~CredentialManagerPendingRequestTaskDelegateMock() = default;
+
+  MOCK_METHOD(bool, IsZeroClickAllowed, (), (const, override));
+  MOCK_METHOD(url::Origin, GetOrigin, (), (const, override));
+  MOCK_METHOD(PasswordManagerClient*, client, (), (const, override));
+  MOCK_METHOD(void,
+              SendCredential,
+              (SendCredentialCallback send_callback,
+               const CredentialInfo& credential),
+              (override));
+  MOCK_METHOD(void,
+              SendPasswordForm,
+              (SendCredentialCallback send_callback,
+               CredentialMediationRequirement mediation,
+               const autofill::PasswordForm* form),
+              (override));
+};
+}  // namespace
+class CredentialManagerPendingRequestTaskTest : public ::testing::Test {
+ public:
+  CredentialManagerPendingRequestTaskTest() {
+    ON_CALL(delegate_mock_, client).WillByDefault(testing::Return(&client_));
+  }
+  ~CredentialManagerPendingRequestTaskTest() override = default;
+
+ protected:
+  testing::NiceMock<CredentialManagerPendingRequestTaskDelegateMock>
+      delegate_mock_;
+
+ private:
+  StubPasswordManagerClient client_;
+};
+
+TEST_F(CredentialManagerPendingRequestTaskTest, QueryProfileStore) {
+  CredentialManagerPendingRequestTask task(
+      &delegate_mock_, /*callback=*/base::DoNothing(),
+      CredentialMediationRequirement::kSilent, /*include_passwords=*/false,
+      /*request_federations=*/{}, StoresToQuery::kProfileStore);
+
+  // 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({});
+}
+
+TEST_F(CredentialManagerPendingRequestTaskTest, QueryProfileAndAccountStores) {
+  CredentialManagerPendingRequestTask task(
+      &delegate_mock_, /*callback=*/base::DoNothing(),
+      CredentialMediationRequirement::kSilent, /*include_passwords=*/false,
+      /*request_federations=*/{}, StoresToQuery::kProfileAndAccountStores);
+
+  // 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({});
+
+  testing::Mock::VerifyAndClearExpectations(&delegate_mock_);
+
+  EXPECT_CALL(delegate_mock_, SendCredential);
+  task.OnGetPasswordStoreResults({});
+}
+
+}  // namespace password_manager