[Passwords] Improve UX for Password Check Progress

This change improves the UX of the password check progress by adding an
artificial delay of 1s before transitioning in the idle state, as well
as making sure the WebUI shows a progress starting at 1 / n, as opposed
to 0 / n.

Bug: 1062687
Change-Id: Icc3bbe873196e90d86ba2397fb564384f47aaac6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2107863Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Friedrich [CET] <fhorschig@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751647}

chrome/browser/extensions/api/passwords_private/password_check_delegate.cc

@@ -7,15 +7,16 @@
 #include <stddef.h>
 
 #include <algorithm>
+#include <map>
 #include <memory>
 
-#include "base/containers/flat_map.h"
 #include "base/containers/flat_set.h"
 #include "base/memory/ref_counted.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/optional.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "base/time/time.h"
 #include "chrome/browser/extensions/api/passwords_private/passwords_private_event_router.h"
 #include "chrome/browser/extensions/api/passwords_private/passwords_private_event_router_factory.h"
@@ -105,7 +106,7 @@ class PasswordCheckProgress : public base::RefCounted<PasswordCheckProgress> {
   // canonicalized credential corresponds to.
   size_t already_processed_ = 0;
   size_t remaining_in_queue_ = 0;
-  base::flat_map<CanonicalizedCredential, size_t> counts_;
+  std::map<CanonicalizedCredential, size_t> counts_;
 
   base::WeakPtrFactory<PasswordCheckProgress> weak_ptr_factory_{this};
 };
@@ -466,6 +467,16 @@ void PasswordCheckDelegate::OnStateChanged(BulkLeakCheckService::State state) {
     profile_->GetPrefs()->SetDouble(
         password_manager::prefs::kLastTimePasswordCheckCompleted,
         base::Time::Now().ToDoubleT());
+
+    // In case the check run to completion delay the last Check Status update by
+    // a second. This avoids flickering of the UI if the full check ran from
+    // start to finish almost immediately.
+    base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+        FROM_HERE,
+        base::BindOnce(&PasswordCheckDelegate::NotifyPasswordCheckStatusChanged,
+                       weak_ptr_factory_.GetWeakPtr()),
+        base::TimeDelta::FromSeconds(1));
+    return;
   }
 
   // NotifyPasswordCheckStatusChanged() invokes GetPasswordCheckStatus()
@@ -502,9 +513,12 @@ void PasswordCheckDelegate::OnCredentialDone(
   if (password_check_progress_)
     password_check_progress_->OnProcessed(credential);
 
-  // Trigger an update of the check status, considering that the progress has
-  // changed.
-  NotifyPasswordCheckStatusChanged();
+  // While the check is still running trigger an update of the check status,
+  // considering that the progress has changed.
+  if (bulk_leak_check_service_adapter_.GetBulkLeakCheckState() ==
+      BulkLeakCheckService::State::kRunning) {
+    NotifyPasswordCheckStatusChanged();
+  }
 }
 
 const CredentialWithPassword*

chrome/browser/extensions/api/passwords_private/password_check_delegate.h

@@ -169,6 +169,8 @@ class PasswordCheckDelegate
               int,
               password_manager::PasswordCredentialLess>
       compromised_credential_id_generator_;
+
+  base::WeakPtrFactory<PasswordCheckDelegate> weak_ptr_factory_{this};
 };
 
 }  // namespace extensions

chrome/browser/resources/settings/autofill_page/password_check.js

@@ -281,9 +281,13 @@ Polymer({
       case CheckState.CANCELED:
         return this.i18n('checkPasswordsCanceled');
       case CheckState.RUNNING:
+        // Returns the progress of a running check. Ensures that both numbers
+        // are at least 1.
         return this.i18n(
-            'checkPasswordsProgress', this.status_.alreadyProcessed || 0,
-            this.status_.remainingInQueue + this.status_.alreadyProcessed);
+            'checkPasswordsProgress', (this.status_.alreadyProcessed || 0) + 1,
+            Math.max(
+                this.status_.remainingInQueue + this.status_.alreadyProcessed,
+                1));
       case CheckState.OFFLINE:
         return this.i18n('checkPasswordsErrorOffline');
       case CheckState.SIGNED_OUT:

chrome/test/data/webui/settings/password_check_test.js

@@ -392,8 +392,8 @@ cr.define('settings_passwords_check', function() {
       passwordManager.data.checkStatus =
           autofill_test_util.makePasswordCheckStatus(
               /*state=*/ PasswordCheckState.RUNNING,
-              /*checked=*/ 1,
-              /*remaining=*/ 1);
+              /*checked=*/ 0,
+              /*remaining=*/ 2);
 
       const section = createCheckPasswordSection();
       return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
@@ -409,8 +409,8 @@ cr.define('settings_passwords_check', function() {
         passwordManager.lastCallback.addPasswordCheckStatusListener(
             autofill_test_util.makePasswordCheckStatus(
                 /*state=*/ PasswordCheckState.RUNNING,
-                /*checked=*/ 2,
-                /*remaining=*/ 0));
+                /*checked=*/ 1,
+                /*remaining=*/ 1));
 
         Polymer.dom.flush();
         assertTrue(isElementVisible(section.$.title));
@@ -537,8 +537,8 @@ cr.define('settings_passwords_check', function() {
       passwordManager.data.checkStatus =
           autofill_test_util.makePasswordCheckStatus(
               /*state=*/ PasswordCheckState.RUNNING,
-              /*checked=*/ 1,
-              /*remaining=*/ 3);
+              /*checked=*/ 0,
+              /*remaining=*/ 4);
 
       const section = createCheckPasswordSection();
       return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
@@ -551,13 +551,33 @@ cr.define('settings_passwords_check', function() {
       });
     });
 
+    // Verifies that in case the backend could not obtain the number of checked
+    // and remaining credentials the UI does not surface 0s to the user.
+    test('runningProgressHandlesZeroCaseFromBackend', function() {
+      passwordManager.data.checkStatus =
+          autofill_test_util.makePasswordCheckStatus(
+              /*state=*/ PasswordCheckState.RUNNING,
+              /*checked=*/ 0,
+              /*remaining=*/ 0);
+
+      const section = createCheckPasswordSection();
+      return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
+        Polymer.dom.flush();
+        const title = section.$.title;
+        assertTrue(isElementVisible(title));
+        expectEquals(
+            section.i18n('checkPasswordsProgress', 1, 1), title.innerText);
+        expectFalse(isElementVisible(section.$.subtitle));
+      });
+    });
+
     // While running, show progress and already found leak count.
     test('testShowProgressAndLeaksWhileRunning', function() {
       const data = passwordManager.data;
       data.checkStatus = autofill_test_util.makePasswordCheckStatus(
           /*state=*/ PasswordCheckState.RUNNING,
-          /*checked=*/ 2,
-          /*remaining=*/ 3);
+          /*checked=*/ 1,
+          /*remaining=*/ 4);
       data.leakedCredentials = [
         autofill_test_util.makeCompromisedCredential(
             'one.com', 'test4', 'LEAKED'),