[Passwords] CompromisedCredentialsReader::GetAllCompromisedCredentials()

This CL introduce a new API to fetch all compromised from both
the profile and the account stores.

Bug: 1108422

Change-Id: I41998de212ad5d83f2c489aad98b43e343296828
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2378030
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803077}

components/password_manager/core/browser/ui/compromised_credentials_reader.cc

@@ -17,8 +17,14 @@ CompromisedCredentialsReader::CompromisedCredentialsReader(
     : profile_store_(profile_store), account_store_(account_store) {
   DCHECK(profile_store_);
   observed_password_store_.Add(profile_store_);
-  if (account_store_)
+  if (account_store_) {
     observed_password_store_.Add(account_store_);
+  } else {
+    // Since we aren't expecting any response from the account store, mark it as
+    // responded not to block responses from the the profile waiting for the
+    // account store to respond.
+    account_store_responded_ = true;
+  }
 }
 
 CompromisedCredentialsReader::~CompromisedCredentialsReader() = default;
@@ -52,6 +58,8 @@ void CompromisedCredentialsReader::OnGetCompromisedCredentials(
 void CompromisedCredentialsReader::OnGetCompromisedCredentialsFrom(
     PasswordStore* store,
     std::vector<CompromisedCredentials> compromised_credentials) {
+  profile_store_responded_ |= store == profile_store_;
+  account_store_responded_ |= store == account_store_;
   // Remove all previously cached credentials from `store` and then insert
   // the just received `compromised_credentials`.
   autofill::PasswordForm::Store to_remove =
@@ -65,9 +73,41 @@ void CompromisedCredentialsReader::OnGetCompromisedCredentialsFrom(
   util::ranges::move(compromised_credentials,
                      std::back_inserter(compromised_credentials_));
 
-  // Inform the observers
+  // Observers are reptitively notified of compromised credentials, and hence
+  // vbservers can expect partial view of the compromised credentials, so inform
+  // the observers directly.
   for (auto& observer : observers_)
     observer.OnCompromisedCredentialsChanged(compromised_credentials_);
+
+  // For the callbacks waiting for the results of
+  // `GetAllCompromisedCredentials()`, they should be notified only when both
+  // stores responded.
+  if (!profile_store_responded_ || !account_store_responded_)
+    return;
+
+  for (auto& callback :
+       std::exchange(get_all_compromised_credentials_callbacks_, {})) {
+    std::move(callback).Run(compromised_credentials_);
+  }
+}
+
+void CompromisedCredentialsReader::GetAllCompromisedCredentials(
+    GetCompromisedCredentialsCallback cb) {
+  if (profile_store_responded_ && account_store_responded_) {
+    std::move(cb).Run(compromised_credentials_);
+    return;
+  }
+  // Add the callback *before* triggering any of the fetches. This ensures
+  // that we don't miss a notitication if the fetches return synchronously
+  // (which is the case in tests).
+  get_all_compromised_credentials_callbacks_.push_back(std::move(cb));
+
+  if (!profile_store_responded_)
+    profile_store_->GetAllCompromisedCredentials(this);
+  if (!account_store_responded_) {
+    DCHECK(account_store_);
+    account_store_->GetAllCompromisedCredentials(this);
+  }
 }
 
 void CompromisedCredentialsReader::AddObserver(Observer* observer) {

components/password_manager/core/browser/ui/compromised_credentials_reader.h

@@ -17,6 +17,8 @@ class CompromisedCredentialsReader
     : public PasswordStore::DatabaseCompromisedCredentialsObserver,
       public CompromisedCredentialsConsumer {
  public:
+  using GetCompromisedCredentialsCallback =
+      base::OnceCallback<void(std::vector<CompromisedCredentials>)>;
   // Observer interface. Clients can implement this to get notified about
   // changes to the list of compromised credentials. Clients can register and
   // de-register themselves, and are expected to do so before the provider gets
@@ -37,6 +39,9 @@ class CompromisedCredentialsReader
 
   void Init();
 
+  // `callback` must outlive this object.
+  void GetAllCompromisedCredentials(GetCompromisedCredentialsCallback callback);
+
   // Allows clients and register and de-register themselves.
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
@@ -70,6 +75,13 @@ class CompromisedCredentialsReader
       observed_password_store_{this};
 
   base::ObserverList<Observer, /*check_empty=*/true> observers_;
+  std::vector<GetCompromisedCredentialsCallback>
+      get_all_compromised_credentials_callbacks_;
+
+  // Whether we are still waiting for a first response from the profile and
+  // account stores.
+  bool profile_store_responded_ = false;
+  bool account_store_responded_ = false;
 };
 
 }  // namespace password_manager

components/password_manager/core/browser/ui/compromised_credentials_reader_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/memory/scoped_refptr.h"
 #include "base/scoped_observer.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/mock_callback.h"
 #include "base/test/task_environment.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/password_manager/core/browser/test_password_store.h"
@@ -115,4 +116,31 @@ TEST_F(CompromisedCredentialsReaderTest, AddCredentialsToBothStores) {
   reader().RemoveObserver(&mock_observer);
 }
 
+TEST_F(CompromisedCredentialsReaderTest, GetAllCompromisedCredentials) {
+  CompromisedCredentials profile_cred;
+  profile_cred.signon_realm = kTestWebRealm;
+  profile_cred.username = base::ASCIIToUTF16("profile@gmail.com");
+  profile_cred.in_store = PasswordForm::Store::kProfileStore;
+
+  CompromisedCredentials account_cred;
+  account_cred.signon_realm = kTestWebRealm;
+  account_cred.username = base::ASCIIToUTF16("account1@gmail.com");
+  account_cred.in_store = PasswordForm::Store::kAccountStore;
+
+  profile_store().AddCompromisedCredentials(profile_cred);
+  account_store().AddCompromisedCredentials(account_cred);
+
+  base::MockCallback<
+      CompromisedCredentialsReader::GetCompromisedCredentialsCallback>
+      get_all_compromised_credentials_cb;
+
+  reader().GetAllCompromisedCredentials(
+      get_all_compromised_credentials_cb.Get());
+
+  // The callback is run only after the stores respond in RunUntilIdle().
+  EXPECT_CALL(get_all_compromised_credentials_cb,
+              Run(UnorderedElementsAre(profile_cred, account_cred)));
+  RunUntilIdle();
+}
+
 }  // namespace password_manager