[PwdCheckAndroid] Add progress tracker to PasswordCheckManager

The progress tracker is responsible for counting the credentials
that have been processed by the check, as well as the number of
credentials that are still to be processed.

Since the bulk leak check tries to be as efficient as possible, it
performs a deduplication step before starting to check passwords. In
this step it canonicalizes each credential, and only processes the
combinations that are unique. Since this number likely does not match
the total number of saved passwords, the tracker updates the counts
with the number of saved passwords that share the same canonicalized
credential.

TBR=andzaytsev@google.com

Bug: 1102025, 1092444
Change-Id: I09dd04c0c563a70998e5a415956a619315d27656
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2363759
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800164}

chrome/browser/password_check/android/internal/java/src/org/chromium/chrome/browser/password_check/PasswordCheckBridge.java

@@ -45,6 +45,13 @@ class PasswordCheckBridge {
          * @param status The current status of the password check.
          */
         void onPasswordCheckStatusChanged(@PasswordCheckUIStatus int status);
+
+        /**
+         * Called during a check when a credential has finished being processed.
+         * @param alreadyProcessed Number of credentials that the check already processed.
+         * @param remainingInQueue Number of credentials that still need to be processed.
+         */
+        void onPasswordCheckProgressChanged(int alreadyProcessed, int remainingInQueue);
     }
 
     PasswordCheckBridge(PasswordCheckObserver passwordCheckObserver) {
@@ -82,6 +89,11 @@ class PasswordCheckBridge {
         mPasswordCheckObserver.onPasswordCheckStatusChanged(state);
     }
 
+    @CalledByNative
+    void onPasswordCheckProgressChanged(int alreadyProcessed, int remainingInQueue) {
+        mPasswordCheckObserver.onPasswordCheckProgressChanged(alreadyProcessed, remainingInQueue);
+    }
+
     @CalledByNative
     private static void insertCredential(CompromisedCredential[] credentials, int index,
             String signonRealm, GURL origin, String username, String displayOrigin,

chrome/browser/password_check/android/internal/java/src/org/chromium/chrome/browser/password_check/PasswordCheckImpl.java

@@ -75,6 +75,13 @@ class PasswordCheckImpl implements PasswordCheck, PasswordCheckObserver {
         }
     }
 
+    @Override
+    public void onPasswordCheckProgressChanged(int alreadyProcessed, int remainingInQueue) {
+        for (Observer obs : mObserverList) {
+            obs.onPasswordCheckProgressChanged(alreadyProcessed, remainingInQueue);
+        }
+    }
+
     @Override
     public void updateCredential(CompromisedCredential credential, String newPassword) {
         mPasswordCheckBridge.updateCredential(credential, newPassword);

chrome/browser/password_check/android/internal/java/src/org/chromium/chrome/browser/password_check/PasswordCheckMediator.java

@@ -138,15 +138,17 @@ class PasswordCheckMediator
         }
     }
 
-    void onPasswordCheckProgressChanged(Pair<Integer, Integer> progress) {
+    @Override
+    public void onPasswordCheckProgressChanged(int alreadyProcessed, int remainingInQueue) {
         ListModel<ListItem> items = mModel.get(ITEMS);
         assert items.size() >= 1;
-        assert progress.first >= 0;
-        assert progress.second >= progress.first;
+        assert alreadyProcessed >= 0;
+        assert remainingInQueue >= 0;
 
         PropertyModel header = items.get(0).model;
         header.set(CHECK_STATUS, PasswordCheckUIStatus.RUNNING);
-        header.set(CHECK_PROGRESS, progress);
+        header.set(
+                CHECK_PROGRESS, new Pair<>(alreadyProcessed, alreadyProcessed + remainingInQueue));
         header.set(CHECK_TIMESTAMP, null);
         header.set(COMPROMISED_CREDENTIALS_COUNT, null);
     }

chrome/browser/password_check/android/java/src/org/chromium/chrome/browser/password_check/PasswordCheck.java

@@ -38,6 +38,13 @@ public interface PasswordCheck extends PasswordCheckComponentUi.Delegate {
          * @param leakedCredential The newly found leaked credential.
          */
         void onCompromisedCredentialFound(CompromisedCredential leakedCredential);
+
+        /**
+         * Called during a check when a credential has finished being processed.
+         * @param alreadyProcessed Number of credentials that the check already processed.
+         * @param remainingInQueue Number of credentials that still need to be processed.
+         */
+        void onPasswordCheckProgressChanged(int alreadyProcessed, int remainingInQueue);
     }
 
     /**

chrome/browser/password_check/android/junit/src/org/chromium/chrome/browser/password_check/PasswordCheckControllerTest.java

@@ -165,7 +165,9 @@ public class PasswordCheckControllerTest {
     @Test
     public void testUpdateProgressHeader() {
         assertRunningHeader(mModel.get(ITEMS).get(0), UNKNOWN_PROGRESS);
-        mMediator.onPasswordCheckProgressChanged(PROGRESS_UPDATE);
+        int already_processed = PROGRESS_UPDATE.first;
+        int remaining_in_queue = PROGRESS_UPDATE.second - already_processed;
+        mMediator.onPasswordCheckProgressChanged(already_processed, remaining_in_queue);
         assertRunningHeader(mModel.get(ITEMS).get(0), PROGRESS_UPDATE);
     }
 

chrome/browser/password_check/android/password_check_bridge.cc

@@ -149,3 +149,11 @@ void PasswordCheckBridge::OnPasswordCheckStatusChanged(
       base::android::AttachCurrentThread(), java_bridge_,
       static_cast<int>(status));
 }
+
+void PasswordCheckBridge::OnPasswordCheckProgressChanged(
+    int already_processed,
+    int remaining_in_queue) {
+  Java_PasswordCheckBridge_onPasswordCheckProgressChanged(
+      base::android::AttachCurrentThread(), java_bridge_, already_processed,
+      remaining_in_queue);
+}

chrome/browser/password_check/android/password_check_bridge.h

@@ -81,6 +81,11 @@ class PasswordCheckBridge : public PasswordCheckManager::Observer {
   void OnPasswordCheckStatusChanged(
       password_manager::PasswordCheckUIStatus status) override;
 
+  // Called by the check manager during a running check, every time a credential
+  // has finished being processed.
+  void OnPasswordCheckProgressChanged(int already_processed,
+                                      int remaining_in_queue) override;
+
  private:
   // The corresponding java object.
   base::android::ScopedJavaGlobalRef<jobject> java_bridge_;

chrome/browser/password_check/android/password_check_manager.cc

@@ -95,6 +95,12 @@ void PasswordCheckManager::StartCheck() {
   // The request is being handled, so reset the boolean.
   was_start_requested_ = false;
   is_check_running_ = true;
+
+  progress_ = std::make_unique<PasswordCheckProgress>();
+  for (const auto& password : saved_passwords_presenter_.GetSavedPasswords())
+    progress_->IncrementCounts(password);
+  observer_->OnPasswordCheckProgressChanged(progress_->already_processed(),
+                                            progress_->remaining_in_queue());
   bulk_leak_check_service_adapter_.StartBulkLeakCheck();
 }
 
@@ -139,6 +145,23 @@ void PasswordCheckManager::RemoveCredential(
   compromised_credentials_manager_.RemoveCompromisedCredential(credential);
 }
 
+PasswordCheckManager::PasswordCheckProgress::PasswordCheckProgress() = default;
+PasswordCheckManager::PasswordCheckProgress::~PasswordCheckProgress() = default;
+
+void PasswordCheckManager::PasswordCheckProgress::IncrementCounts(
+    const autofill::PasswordForm& password) {
+  ++remaining_in_queue_;
+  ++counts_[password];
+}
+
+void PasswordCheckManager::PasswordCheckProgress::OnProcessed(
+    const password_manager::LeakCheckCredential& credential) {
+  auto it = counts_.find(credential);
+  const int num_matching = it != counts_.end() ? it->second : 0;
+  already_processed_ += num_matching;
+  remaining_in_queue_ -= num_matching;
+}
+
 void PasswordCheckManager::OnSavedPasswordsChanged(
     password_manager::SavedPasswordsPresenter::SavedPasswordsView passwords) {
   if (!is_initialized_) {
@@ -176,6 +199,7 @@ void PasswordCheckManager::OnStateChanged(State state) {
   }
 
   if (state != State::kRunning) {
+    progress_.reset();
     is_check_running_ = false;
   }
 
@@ -185,7 +209,9 @@ void PasswordCheckManager::OnStateChanged(State state) {
 void PasswordCheckManager::OnCredentialDone(
     const password_manager::LeakCheckCredential& credential,
     password_manager::IsLeaked is_leaked) {
-  // TODO(crbug.com/1092444): Advance progress.
+  progress_->OnProcessed(credential);
+  observer_->OnPasswordCheckProgressChanged(progress_->already_processed(),
+                                            progress_->remaining_in_queue());
   if (is_leaked) {
     // TODO(crbug.com/1092444): Trigger single-credential update.
     compromised_credentials_manager_.SaveCompromisedCredential(credential);

chrome/browser/password_check/android/password_check_manager.h

@@ -32,6 +32,8 @@ class PasswordCheckManager
     virtual void OnCompromisedCredentialsChanged(int count) = 0;
     virtual void OnPasswordCheckStatusChanged(
         password_manager::PasswordCheckUIStatus status) = 0;
+    virtual void OnPasswordCheckProgressChanged(int already_processed,
+                                                int remaining_in_queue) = 0;
   };
 
   struct CompromisedCredentialForUI : password_manager::CredentialWithPassword {
@@ -98,6 +100,40 @@ class PasswordCheckManager
   PasswordCheckManager& operator=(PasswordCheckManager&&) = delete;
 
  private:
+  // Class remembering the state required to update the progress of an ongoing
+  // Password Check.
+  class PasswordCheckProgress {
+   public:
+    PasswordCheckProgress();
+    ~PasswordCheckProgress();
+
+    size_t remaining_in_queue() const { return remaining_in_queue_; }
+    size_t already_processed() const { return already_processed_; }
+
+    // Increments the counts corresponding to `password`. Intended to be called
+    // for each credential that is passed to the bulk check.
+    void IncrementCounts(const autofill::PasswordForm& password);
+
+    // Updates the counts after a `credential` has been processed by the bulk
+    // check.
+    void OnProcessed(const password_manager::LeakCheckCredential& credential);
+
+   private:
+    // Count variables needed to correctly show the progress of the check to the
+    // user. `already_processed_` contains the number of credentials that have
+    // been checked already, while `remaining_in_queue_` remembers how many
+    // passwords still need to be checked.
+    // Since the bulk leak check tries to be as efficient as possible, it
+    // performs a deduplication step before starting to check passwords. In this
+    // step it canonicalizes each credential, and only processes the
+    // combinations that are unique. Since this number likely does not match the
+    // total number of saved passwords, we remember in `counts_` how many saved
+    // passwords a given canonicalized credential corresponds to.
+    size_t already_processed_ = 0;
+    size_t remaining_in_queue_ = 0;
+    std::map<password_manager::CanonicalizedCredential, size_t> counts_;
+  };
+
   // password_manager::SavedPasswordsPresenter::Observer:
   void OnSavedPasswordsChanged(
       password_manager::SavedPasswordsPresenter::SavedPasswordsView passwords)
@@ -146,6 +182,9 @@ class PasswordCheckManager
   // The profile for which the passwords are checked.
   Profile* profile_ = nullptr;
 
+  // Object storing the progress of a running password check.
+  std::unique_ptr<PasswordCheckProgress> progress_;
+
   // Handle to the password store, powering both `saved_passwords_presenter_`
   // and `compromised_credentials_manager_`.
   scoped_refptr<password_manager::PasswordStore> password_store_ =

chrome/browser/password_check/android/password_check_manager_unittest.cc

@@ -59,6 +59,7 @@ using State = password_manager::BulkLeakCheckService::State;
 namespace {
 
 constexpr char kExampleCom[] = "https://example.com";
+constexpr char kExampleOrg[] = "http://www.example.org";
 constexpr char kExampleApp[] = "com.example.app";
 
 constexpr char kUsername1[] = "alice";
@@ -76,6 +77,8 @@ class MockPasswordCheckManagerObserver : public PasswordCheckManager::Observer {
               OnPasswordCheckStatusChanged,
               (password_manager::PasswordCheckUIStatus),
               (override));
+
+  MOCK_METHOD(void, OnPasswordCheckProgressChanged, (int, int), (override));
 };
 
 class MockPasswordScriptsFetcher
@@ -390,3 +393,23 @@ TEST_F(PasswordCheckManagerTest,
           base::nullopt, "https://example.com/",
           CompromiseTypeFlags::kCredentialLeaked, /*has_script=*/true)));
 }
+
+TEST_F(PasswordCheckManagerTest, UpdatesProgressCorrectly) {
+  InitializeManager();
+  store().AddLogin(MakeSavedPassword(kExampleCom, kUsername1, kPassword1));
+  store().AddLogin(MakeSavedPassword(kExampleOrg, kUsername1, kPassword1));
+  store().AddLogin(MakeSavedPassword(kExampleCom, kUsername2));
+  RunUntilIdle();
+
+  EXPECT_CALL(mock_observer(), OnPasswordCheckProgressChanged(0, 3));
+  manager().StartCheck();
+
+  // Expect that 2 credentials were processed, even if there is only one
+  // reply, because of the deduplication logic.
+  EXPECT_CALL(mock_observer(), OnPasswordCheckProgressChanged(2, 1));
+  static_cast<password_manager::BulkLeakCheckDelegateInterface*>(service())
+      ->OnFinishedCredential(
+          password_manager::LeakCheckCredential(base::ASCIIToUTF16(kUsername1),
+                                                base::ASCIIToUTF16(kPassword1)),
+          password_manager::IsLeaked(false));
+}

chrome/browser/safety_check/android/java/src/org/chromium/chrome/browser/safety_check/SafetyCheckMediator.java

@@ -360,6 +360,9 @@ class SafetyCheckMediator implements PasswordCheck.Observer, SafetyCheckCommonOb
     @Override
     public void onCompromisedCredentialFound(CompromisedCredential leakedCredential) {}
 
+    @Override
+    public void onPasswordCheckProgressChanged(int alreadyProcessed, int remainingInQueue) {}
+
     /** Cancels any pending callbacks and registered observers.  */
     public void destroy() {
         cancelCallbacks();