[Passwords] Suppress retry leak checks for passphrases/non-sync users

With this CL, the error case "Too many passwords" will no longer offer
to check passwords in the account for users that are:
- not syncing
- syncing but use a passphrase.

If no check can be offered, always show the total count of found leaks.
screenshots can be found at https://crbug.com/1047726#c58

Translation screenshot for
IDS_SETTINGS_CHECK_PASSWORDS_ERROR_INTERRUPTED_TOO_MANY_PASSWORDS:
https://storage.cloud.google.com/chromium-translation-screenshots/cf5f1e0f138864731990c89272cb55030f2e380b

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

chrome/app/settings_strings.grdp

@@ -335,6 +335,9 @@
   <message name="IDS_SETTINGS_CHECK_PASSWORDS_DESCRIPTION" desc="Explanation of the passwords bulk check feature found within the password settings.">
     Keep your passwords safe from data breaches and other security issues
   </message>
+  <message name="IDS_SETTINGS_CHECK_PASSWORDS_ERROR_INTERRUPTED_TOO_MANY_PASSWORDS" desc="Error message when the password check was started and found some results but can't be completed since the user saved too many passwords.">
+    Chrome couldn't check all of your passwords because there are too many.
+  </message>
   <message name="IDS_SETTINGS_CHECK_PASSWORDS_ERROR_OFFLINE" desc="Error message when the password check can't be completed because the user is offline.">
     Chrome can't check your passwords. Try checking your internet connection.
   </message>

chrome/app/settings_strings_grdp/IDS_SETTINGS_CHECK_PASSWORDS_ERROR_INTERRUPTED_TOO_MANY_PASSWORDS.png.sha1

+cf5f1e0f138864731990c89272cb55030f2e380b
\ No newline at end of file

chrome/browser/resources/settings/autofill_page/BUILD.gn

@@ -478,6 +478,7 @@ polymer_modulizer("password_check") {
   html_type = "dom-module"
   auto_imports = [
     "chrome/browser/resources/settings/autofill_page/password_manager_proxy.html|PasswordManagerImpl,PasswordManagerProxy",
+    "chrome/browser/resources/settings/people_page/sync_browser_proxy.html|SyncBrowserProxyImpl,SyncPrefs,SyncStatus",
     "chrome/browser/resources/settings/plural_string_proxy.html|PluralStringProxyImpl",
     "ui/webui/resources/html/assert.html|assert",
   ]

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

@@ -3,14 +3,16 @@
 <link rel="import" href="chrome://resources/cr_elements/cr_action_menu/cr_action_menu.html">
 <link rel="import" href="chrome://resources/cr_elements/cr_button/cr_button.html">
 <link rel="import" href="../i18n_setup.html">
+<link rel="import" href="../people_page/sync_browser_proxy.html">
 <link rel="import" href="../plural_string_proxy.html">
 <link rel="import" href="../settings_shared_css.html">
 <link rel="import" href="chrome://resources/cr_elements/icons.html">
 <link rel="import" href="chrome://resources/html/assert.html">
+<link rel="import" href="chrome://resources/html/i18n_behavior.html">
+<link rel="import" href="chrome://resources/html/web_ui_listener_behavior.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/iron-list/iron-list.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-spinner/paper-spinner-lite.html">
-<link rel="import" href="chrome://resources/html/i18n_behavior.html">
 <link rel="import" href="password_check_list_item.html">
 <link rel="import" href="password_manager_proxy.html">
 
@@ -71,11 +73,11 @@
 
       <div class="start settings-box-text">
         <div id="title">
-          [[getTitle_(status_)]]
+          [[getTitle_(status_, suppressCheckupLink_)]]
           <a class="secondary inline" id="linkToGoogleAccount"
              href="[[passwordCheckUrl_]]"
              target="_blank"
-             hidden$="[[!hasTooManyPasswords_(status_)]]">
+             hidden$="[[suppressCheckupLink_]]">
             $i18n{checkPasswordsAgainInAccount}
           </a>
           <span class="secondary inline" id="lastCompletedCheck"
@@ -84,7 +86,8 @@
           </span>
         </div>
         <div class="secondary" id="subtitle"
-             hidden$="[[!showsPasswordsCount_(status_, leakedPasswords)]]">
+             hidden$="[[!showsPasswordsCount_(status_, leakedPasswords,
+                                              suppressCheckupLink_)]]">
           [[compromisedPasswordsCount_]]
         </div>
       </div>

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

@@ -9,7 +9,10 @@ const CheckState = chrome.passwordsPrivate.PasswordCheckState;
 Polymer({
   is: 'settings-password-check',
 
-  behaviors: [I18nBehavior],
+  behaviors: [
+    I18nBehavior,
+    WebUIListenerBehavior,
+  ],
 
   properties: {
     /**
@@ -51,7 +54,19 @@ Polymer({
       value: () => {
         return {state: CheckState.IDLE};
       },
-    }
+    },
+
+    /** @private */
+    suppressCheckupLink_: {
+      type: Boolean,
+      computed: 'suppressesCheckupLink_(status_, syncPrefs_, syncStatus_)',
+    },
+
+    /** @private {settings.SyncPrefs} */
+    syncPrefs_: Object,
+
+    /** @private {settings.SyncStatus} */
+    syncStatus_: Object,
   },
 
   /**
@@ -82,6 +97,7 @@ Polymer({
   attached() {
     // Set the manager. These can be overridden by tests.
     this.passwordManager_ = PasswordManagerImpl.getInstance();
+    const syncBrowserProxy = settings.SyncBrowserProxyImpl.getInstance();
 
     const statusChangeListener = status => this.status_ = status;
     const setLeakedCredentialsListener = compromisedCredentials => {
@@ -93,6 +109,9 @@ Polymer({
             this.compromisedPasswordsCount_ = count;
           });
     };
+    // TODO(crbug.com/1047726): Deduplicate with updates in password_section.js.
+    const syncStatusChanged = syncStatus => this.syncStatus_ = syncStatus;
+    const syncPrefsChanged = syncPrefs => this.syncPrefs_ = syncPrefs;
 
     this.statusChangedListener_ = statusChangeListener;
     this.leakedCredentialsListener_ = setLeakedCredentialsListener;
@@ -102,12 +121,15 @@ Polymer({
         this.statusChangedListener_);
     this.passwordManager_.getCompromisedCredentials().then(
         this.leakedCredentialsListener_);
+    syncBrowserProxy.getSyncStatus().then(syncStatusChanged);
 
     // Listen for changes.
     this.passwordManager_.addPasswordCheckStatusListener(
         this.statusChangedListener_);
     this.passwordManager_.addCompromisedCredentialsListener(
         this.leakedCredentialsListener_);
+    this.addWebUIListener('sync-status-changed', syncStatusChanged);
+    this.addWebUIListener('sync-prefs-changed', syncPrefsChanged);
   },
 
   /** @override */
@@ -222,20 +244,19 @@ Polymer({
 
   /**
    * Returns the title message indicating the state of the last/ongoing check.
-   * @param {!PasswordManagerProxy.PasswordCheckStatus} status
    * @return {string}
    * @private
    */
-  getTitle_(status) {
-    switch (status.state) {
+  getTitle_() {
+    switch (this.status_.state) {
       case CheckState.IDLE:
         return this.i18n('checkPasswords');
       case CheckState.CANCELED:
         return this.i18n('checkPasswordsCanceled');
       case CheckState.RUNNING:
         return this.i18n(
-            'checkPasswordsProgress', status.alreadyProcessed || 0,
-            status.remainingInQueue + status.alreadyProcessed);
+            'checkPasswordsProgress', this.status_.alreadyProcessed || 0,
+            this.status_.remainingInQueue + this.status_.alreadyProcessed);
       case CheckState.OFFLINE:
         return this.i18n('checkPasswordsErrorOffline');
       case CheckState.SIGNED_OUT:
@@ -243,13 +264,15 @@ Polymer({
       case CheckState.NO_PASSWORDS:
         return this.i18n('checkPasswordsErrorNoPasswords');
       case CheckState.TOO_MANY_PASSWORDS:
-        return this.i18n('checkPasswordsErrorTooManyPasswords');
+        return this.suppressesCheckupLink_() ?
+            this.i18n('checkPasswordsErrorInterruptedTooManyPasswords') :
+            this.i18n('checkPasswordsErrorTooManyPasswords');
       case CheckState.QUOTA_LIMIT:
         return this.i18n('checkPasswordsErrorQuota');
       case CheckState.OTHER_ERROR:
         return this.i18n('checkPasswordsErrorGeneric');
     }
-    throw 'Can\'t find a title for state: ' + status.state;
+    assertNotReached('Can\'t find a title for state: ' + this.status_.state);
   },
 
   /**
@@ -321,17 +344,6 @@ Polymer({
     throw 'Can\'t determine button visibility for state: ' + status.state;
   },
 
-  /**
-   * Returns true iff the backend communicated that there are too many saved
-   * passwords to check them locally.
-   * @param {!PasswordManagerProxy.PasswordCheckStatus} status
-   * @return {boolean}
-   * @private
-   */
-  hasTooManyPasswords_(status) {
-    return status.state == CheckState.TOO_MANY_PASSWORDS;
-  },
-
   /**
    * Returns the chrome:// address where the banner image is located.
    * @param {boolean} isDarkMode
@@ -389,28 +401,50 @@ Polymer({
   /**
    * Returns true if there are leaked credentials or the status is unexpected
    * for a regular password check.
-   * @param {!PasswordManagerProxy.PasswordCheckStatus} status
-   * @param {!Array<PasswordManagerProxy.CompromisedCredential>}
-   *     leakedPasswords
    * @return {boolean}
    * @private
    */
-  showsPasswordsCount_(status, leakedPasswords) {
-    switch (status.state) {
+  showsPasswordsCount_() {
+    if (this.hasLeakedCredentials_(this.leakedPasswords)) {
+      return true;
+    }
+    switch (this.status_.state) {
       case CheckState.IDLE:
         return true;
       case CheckState.CANCELED:
       case CheckState.RUNNING:
-        return this.hasLeakedCredentials_(leakedPasswords);
       case CheckState.OFFLINE:
       case CheckState.SIGNED_OUT:
       case CheckState.NO_PASSWORDS:
-      case CheckState.TOO_MANY_PASSWORDS:
       case CheckState.QUOTA_LIMIT:
       case CheckState.OTHER_ERROR:
         return false;
+      case CheckState.TOO_MANY_PASSWORDS:
+        return this.suppressesCheckupLink_();
+    }
+    assertNotReached(
+        'Not specified whether to show passwords for state: ' +
+        this.status_.state);
+  },
+
+  /**
+   * Returns true iff the user cannot retry the password check in their account.
+   * Either because they are syncing or because they use a custom passphrase.
+   * @return {boolean}
+   * @private
+   */
+  suppressesCheckupLink_() {
+    if (!this.isButtonHidden_(this.status_)) {
+      return true;  // Never show the retry link alongside a button.
+    }
+    if (this.status_.state != CheckState.TOO_MANY_PASSWORDS) {
+      return true;  // Never show the retry link for other states.
+    }
+    if (!this.syncStatus_ || !this.syncStatus_.signedIn) {
+      return true;  // Never show the retry link for signed-out users.
     }
-    throw 'Not specified whether to show passwords for state: ' + status.state;
+    // Show the retry link only, if user data is unencrypted.
+    return !!this.syncPrefs_ && !!this.syncPrefs_.encryptAllData;
   },
 
   /**

chrome/browser/ui/webui/settings/settings_localized_strings_provider.cc

@@ -733,6 +733,8 @@ void AddAutofillStrings(content::WebUIDataSource* html_source,
       {"checkPasswordsCanceled", IDS_SETTINGS_CHECK_PASSWORDS_CANCELED},
       {"checkedPasswords", IDS_SETTINGS_CHECKED_PASSWORDS},
       {"checkPasswordsDescription", IDS_SETTINGS_CHECK_PASSWORDS_DESCRIPTION},
+      {"checkPasswordsErrorInterruptedTooManyPasswords",
+       IDS_SETTINGS_CHECK_PASSWORDS_ERROR_INTERRUPTED_TOO_MANY_PASSWORDS},
       {"checkPasswordsErrorOffline",
        IDS_SETTINGS_CHECK_PASSWORDS_ERROR_OFFLINE},
       {"checkPasswordsErrorSignedOut",

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

@@ -360,6 +360,7 @@ CrSettingsPasswordsCheckTest.prototype = {
   extraLibraries: CrSettingsBrowserTest.prototype.extraLibraries.concat([
     '../test_browser_proxy.js',
     'passwords_and_autofill_fake_data.js',
+    'sync_test_util.js',
     'test_password_manager_proxy.js',
     'password_check_test.js',
   ]),

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

@@ -7,6 +7,7 @@
 // clang-format off
 // #import {PasswordManagerImpl} from 'chrome://settings/settings.js';
 // #import {makeCompromisedCredential,  makePasswordCheckStatus} from 'chrome://test/settings/passwords_and_autofill_fake_data.m.js';
+// #import {getSyncAllPrefs,simulateSyncStatus} from 'chrome://test/settings/sync_test_util.m.js';
 // #import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 // #import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 // #import {TestPasswordManagerProxy} from 'chrome://test/settings/test_password_manager_proxy.m.js';
@@ -154,17 +155,65 @@ cr.define('settings_passwords_check', function() {
 
     // Test verifies that 'Try again' is not visible if users have too many
     // passwords. Instead, there should be a link to their Google Account.
-    test('testNoRetryForTooManyPasswords', function() {
+    test('testRetryInAccountForTooManyPasswordsWhenSyncing', function() {
       passwordManager.data.checkStatus =
           autofill_test_util.makePasswordCheckStatus(
               /*state=*/ PasswordCheckState.TOO_MANY_PASSWORDS);
+
       const section = createCheckPasswordSection();
+      cr.webUIListenerCallback(
+          'sync-prefs-changed', sync_test_util.getSyncAllPrefs());
+      sync_test_util.simulateSyncStatus({signedIn: true});
+
       return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
         expectFalse(isElementVisible(section.$.controlPasswordCheckButton));
         expectTrue(isElementVisible(section.$.linkToGoogleAccount));
       });
     });
 
+    test('testNoRetryInAccountForTooManyPasswordsWhenSignedOut', function() {
+      passwordManager.data.checkStatus =
+          autofill_test_util.makePasswordCheckStatus(
+              PasswordCheckState.TOO_MANY_PASSWORDS);
+
+      const section = createCheckPasswordSection();
+      cr.webUIListenerCallback(
+          'sync-prefs-changed', sync_test_util.getSyncAllPrefs());
+      sync_test_util.simulateSyncStatus({signedIn: false});
+
+      return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
+        Polymer.dom.flush();
+        assertTrue(isElementVisible(section.$.title));
+        expectEquals(
+            section.i18n('checkPasswordsErrorInterruptedTooManyPasswords'),
+            section.$.title.innerText);
+        expectFalse(isElementVisible(section.$.linkToGoogleAccount));
+        expectTrue(isElementVisible(section.$.subtitle));
+      });
+    });
+
+    test('testNoRetryInAccountForTooManyPasswordsForEncryption', function() {
+      passwordManager.data.checkStatus =
+          autofill_test_util.makePasswordCheckStatus(
+              PasswordCheckState.TOO_MANY_PASSWORDS);
+
+      const section = createCheckPasswordSection();
+      const syncPrefs = sync_test_util.getSyncAllPrefs();
+      syncPrefs.encryptAllData = true;
+      cr.webUIListenerCallback('sync-prefs-changed', syncPrefs);
+      sync_test_util.simulateSyncStatus({signedIn: true});
+
+      return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
+        Polymer.dom.flush();
+        assertTrue(isElementVisible(section.$.title));
+        expectEquals(
+            section.i18n('checkPasswordsErrorInterruptedTooManyPasswords'),
+            section.$.title.innerText);
+        expectFalse(isElementVisible(section.$.linkToGoogleAccount));
+        expectTrue(isElementVisible(section.$.subtitle));
+      });
+    });
+
     // Test verifies that 'Try again' visible and working when users encounter a
     // generic error.
     test('testShowRetryAfterGenericError', function() {
@@ -587,11 +636,14 @@ cr.define('settings_passwords_check', function() {
               PasswordCheckState.TOO_MANY_PASSWORDS);
 
       const section = createCheckPasswordSection();
+      cr.webUIListenerCallback(
+          'sync-prefs-changed', sync_test_util.getSyncAllPrefs());
+      sync_test_util.simulateSyncStatus({signedIn: true});
+
       return passwordManager.whenCalled('getPasswordCheckStatus').then(() => {
         Polymer.dom.flush();
         const title = section.$.title;
         assertTrue(isElementVisible(title));
-        // TODO(crbug.com/1047726): Check for account redirection.
         expectEquals(
             section.i18n('checkPasswordsErrorTooManyPasswords') + ' ' +
                 section.i18n('checkPasswordsAgainInAccount'),