[Passwords] Run PreventSilentAccessTask against account store

This CL make sure that for account storage users,
CredentialManagerPendingPreventSilentAccessTask runs against both
the profile and account stores.

It also introduced unit testing.

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

components/password_manager/core/browser/BUILD.gn

@@ -543,6 +543,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_prevent_silent_access_task_unittest.cc",
     "credential_manager_pending_request_task_unittest.cc",
     "credentials_cleaner_runner_unittest.cc",
     "credentials_cleaner_unittest.cc",

components/password_manager/core/browser/credential_manager_pending_prevent_silent_access_task.cc

@@ -20,11 +20,24 @@ void CredentialManagerPendingPreventSilentAccessTask::AddOrigin(
     const PasswordStore::FormDigest& form_digest) {
   delegate_->GetProfilePasswordStore()->GetLogins(form_digest, this);
   pending_requests_++;
+  if (PasswordStore* account_store = delegate_->GetAccountPasswordStore()) {
+    account_store->GetLogins(form_digest, this);
+    pending_requests_++;
+  }
 }
 
 void CredentialManagerPendingPreventSilentAccessTask::OnGetPasswordStoreResults(
     std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
-  PasswordStore* store = delegate_->GetProfilePasswordStore();
+  // 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 CredentialManagerPendingPreventSilentAccessTask::
+    OnGetPasswordStoreResultsFrom(
+        scoped_refptr<PasswordStore> store,
+        std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
   for (const auto& form : results) {
     if (!form->skip_zero_click) {
       form->skip_zero_click = true;

components/password_manager/core/browser/credential_manager_pending_prevent_silent_access_task.h

@@ -36,11 +36,14 @@ class CredentialManagerPendingPreventSilentAccessTask
   // Adds an origin to require user mediation.
   void AddOrigin(const PasswordStore::FormDigest& form_digest);
 
- private:
   // PasswordStoreConsumer implementation.
   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:
   CredentialManagerPendingPreventSilentAccessTaskDelegate* const
       delegate_;  // Weak.
 

components/password_manager/core/browser/credential_manager_pending_prevent_silent_access_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_prevent_silent_access_task.h"
+
+#include "base/test/task_environment.h"
+#include "components/password_manager/core/browser/test_password_store.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace password_manager {
+
+namespace {
+
+class CredentialManagerPendingPreventSilentAccessTaskDelegateMock
+    : public CredentialManagerPendingPreventSilentAccessTaskDelegate {
+ public:
+  CredentialManagerPendingPreventSilentAccessTaskDelegateMock() = default;
+  ~CredentialManagerPendingPreventSilentAccessTaskDelegateMock() override =
+      default;
+
+  MOCK_METHOD(PasswordStore*, GetProfilePasswordStore, (), (override));
+  MOCK_METHOD(PasswordStore*, GetAccountPasswordStore, (), (override));
+  MOCK_METHOD(void, DoneRequiringUserMediation, (), (override));
+};
+
+}  // namespace
+
+class CredentialManagerPendingPreventSilentAccessTaskTest
+    : public ::testing::Test {
+ public:
+  CredentialManagerPendingPreventSilentAccessTaskTest() {
+    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);
+  }
+  ~CredentialManagerPendingPreventSilentAccessTaskTest() 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<CredentialManagerPendingPreventSilentAccessTaskDelegateMock>
+      delegate_mock_;
+  scoped_refptr<TestPasswordStore> profile_store_;
+  scoped_refptr<TestPasswordStore> account_store_;
+
+ private:
+  base::test::TaskEnvironment task_environment_;
+};
+
+TEST_F(CredentialManagerPendingPreventSilentAccessTaskTest, ProfileStoreOnly) {
+  ON_CALL(delegate_mock_, GetProfilePasswordStore)
+      .WillByDefault(testing::Return(profile_store_.get()));
+  ON_CALL(delegate_mock_, GetAccountPasswordStore)
+      .WillByDefault(testing::Return(nullptr));
+
+  CredentialManagerPendingPreventSilentAccessTask task(&delegate_mock_);
+
+  task.AddOrigin(PasswordStore::FormDigest(
+      autofill::PasswordForm::Scheme::kHtml,
+      /*signon_realm=*/"www.example.com", GURL("www.example.com")));
+
+  // We are expecting results from only one store, delegate should be called
+  // upon getting a response from the store.
+  EXPECT_CALL(delegate_mock_, DoneRequiringUserMediation);
+  task.OnGetPasswordStoreResultsFrom(profile_store_, {});
+}
+
+TEST_F(CredentialManagerPendingPreventSilentAccessTaskTest,
+       ProfileAndAccountStores) {
+  ON_CALL(delegate_mock_, GetProfilePasswordStore)
+      .WillByDefault(testing::Return(profile_store_.get()));
+  ON_CALL(delegate_mock_, GetAccountPasswordStore)
+      .WillByDefault(testing::Return(account_store_.get()));
+
+  CredentialManagerPendingPreventSilentAccessTask task(&delegate_mock_);
+
+  task.AddOrigin(PasswordStore::FormDigest(
+      autofill::PasswordForm::Scheme::kHtml,
+      /*signon_realm=*/"www.example.com", GURL("www.example.com")));
+
+  // We are expecting results from 2 stores, the delegate shouldn't be called
+  // until both stores respond.
+  EXPECT_CALL(delegate_mock_, DoneRequiringUserMediation).Times(0);
+  task.OnGetPasswordStoreResultsFrom(profile_store_, {});
+
+  testing::Mock::VerifyAndClearExpectations(&delegate_mock_);
+
+  EXPECT_CALL(delegate_mock_, DoneRequiringUserMediation);
+  task.OnGetPasswordStoreResultsFrom(account_store_, {});
+}
+
+}  // namespace password_manager