Update the UI of the Password Check for signed out users

This CL updates the UI in of the Password Check page and adds the
"Check again" button for signed out users. It also adds a new message
for users in the compromised section to indicate that passwords weren't
checked for compromise because the user is signed out.
Screenshots of the Password Check page after adding support for signed
out users:
https://bugs.chromium.org/p/chromium/issues/detail?id=1119752#c19

Bug: 1119752
Change-Id: Ie06f34050dbbe33ceca030936cfccf1674c15938
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2426502Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Irina Fedorova <irfedorova@google.com>
Cr-Commit-Position: refs/heads/master@{#812175}

chrome/app/settings_chromium_strings.grdp

@@ -67,6 +67,9 @@
   <message name="IDS_SETTINGS_NO_COMPROMISED_CREDENTIALS_LABEL" desc="Label which is shown when there were no compromised passwords detected.">
     Chromium will notify you when you sign in with a compromised password
   </message>
+  <message name="IDS_SETTINGS_SIGNED_OUT_USER_LABEL" desc="Label which is shown at the top of the compromised password section if user is signed out.">
+    To check if your passwords are safe from data breaches and other security issues, <ph name="BEGIN_LINK">&lt;a target='_blank' href='$1'&gt;</ph>sign in to Chromium<ph name="END_LINK">&lt;/a&gt;</ph>.
+  </message>
   <message name="IDS_SETTINGS_COMPROMISED_EDIT_DISCLAIMER_DESCRIPTION" desc="A description for the dialog which tells the user to edit password in Chromium if it was changed already.">
     If so, please edit your saved password in Chromium so it matches your new password.
   </message>

chrome/app/settings_chromium_strings_grdp/IDS_SETTINGS_SIGNED_OUT_USER_LABEL.png.sha1

+d103e540373b3615266f2af3584b7fb5c4b2eb53
\ No newline at end of file

chrome/app/settings_google_chrome_strings.grdp

@@ -67,6 +67,9 @@
   <message name="IDS_SETTINGS_NO_COMPROMISED_CREDENTIALS_LABEL" desc="Label which is shown when there were no compromised passwords detected.">
     Chrome will notify you when you sign in with a compromised password
   </message>
+  <message name="IDS_SETTINGS_SIGNED_OUT_USER_LABEL" desc="Label which is shown at the top of the compromised password section if user is signed out.">
+    To check if your passwords are safe from data breaches and other security issues, <ph name="BEGIN_LINK">&lt;a target='_blank' href='$1'&gt;</ph>sign in to Chrome<ph name="END_LINK">&lt;/a&gt;</ph>.
+  </message>
   <message name="IDS_SETTINGS_COMPROMISED_EDIT_DISCLAIMER_DESCRIPTION" desc="A description for the dialog which tells the user to edit password in Chrome if it was changed already.">
     If so, please edit your saved password in Chrome so it matches your new password.
   </message>

chrome/app/settings_google_chrome_strings_grdp/IDS_SETTINGS_SIGNED_OUT_USER_LABEL.png.sha1

+f40d68d53663e68163c7ccdf8e7ef19279c596bc
\ No newline at end of file

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

@@ -52,7 +52,8 @@
       }
 
     </style>
-    <!-- The banner is visible if no insecure password was found (yet). -->
+    <!-- The banner is visible if no insecure password was found (yet) and user
+    is signed in. -->
     <template is="dom-if"
         if="[[shouldShowBanner_(status, leakedPasswords, weakPasswords)]]">
       <picture>
@@ -105,12 +106,21 @@
       </div>
     </div>
     <div id="compromisedCredentialsBody"
-        hidden$="[[!hasLeakedCredentials_(leakedPasswords)]]">
+        hidden$="[[!showCompromisedCredentialsBody_]]">
       <div class="cr-row first">
         <h2>$i18n{compromisedPasswords}</h2>
       </div>
       <div class="list-frame vertical-list">
-        <div class="list-item secondary">
+        <div class="list-item secondary" hidden$="[[!isSignedOut_]]"
+            id="signedOutUserLabel">
+          <div>
+            $i18nRaw{signedOutUserLabel}
+          </div>
+        </div>
+      </div>
+      <div class="list-frame vertical-list">
+        <div class="list-item secondary" id="compromisedPasswordsDescription"
+            hidden$="[[!hasLeakedCredentials_(leakedPasswords)]]">
           $i18n{compromisedPasswordsDescription}
         </div>
       </div>

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

@@ -103,11 +103,18 @@ Polymer({
      */
     activePassword_: Object,
 
+    /** @private */
+    showCompromisedCredentialsBody_: {
+      type: Boolean,
+      computed: 'computeShowCompromisedCredentialsBody_(isSignedOut_, ' +
+          'leakedPasswords, passwordsWeaknessCheckEnabled)',
+    },
+
     /** @private */
     showNoCompromisedPasswordsLabel_: {
       type: Boolean,
-      computed: 'computeShowNoCompromisedPasswordsLabel(' +
-          'syncStatus_, prefs.*, status, leakedPasswords)',
+      computed: 'computeShowNoCompromisedPasswordsLabel_(syncStatus_, ' +
+          'prefs.*, status, leakedPasswords, passwordsWeaknessCheckEnabled)',
     },
 
     /** @private */
@@ -252,12 +259,22 @@ Polymer({
       case CheckState.IDLE:
       case CheckState.CANCELED:
       case CheckState.OFFLINE:
-      case CheckState.SIGNED_OUT:
       case CheckState.OTHER_ERROR:
         this.passwordManager.recordPasswordCheckInteraction(
             PasswordManagerProxy.PasswordCheckInteraction.START_CHECK_MANUALLY);
         this.passwordManager.startBulkPasswordCheck();
         return;
+      case CheckState.SIGNED_OUT:
+        // Runs the startBulkPasswordCheck to check passwords for weakness that
+        // works for both sign in and sign out users.
+        this.passwordManager.recordPasswordCheckInteraction(
+            PasswordManagerProxy.PasswordCheckInteraction.START_CHECK_MANUALLY);
+        this.passwordManager.startBulkPasswordCheck().then(
+            () => {},
+            error => {
+                // Catching error
+            });
+        return;
       case CheckState.NO_PASSWORDS:
       case CheckState.QUOTA_LIMIT:
     }
@@ -440,7 +457,13 @@ Polymer({
       case CheckState.OFFLINE:
         return this.i18n('checkPasswordsErrorOffline');
       case CheckState.SIGNED_OUT:
-        return this.i18n('checkPasswordsErrorSignedOut');
+        // When user is signed out and |passwordsWeaknessCheckEnabled| is
+        // true, we run the password weakness check. Since it works very fast,
+        // we always shows "Checked passwords" in this case.
+        return this.i18n(
+            this.passwordsWeaknessCheckEnabled ?
+                'checkedPasswords' :
+                'checkPasswordsErrorSignedOut');
       case CheckState.NO_PASSWORDS:
         return this.i18n('checkPasswordsErrorNoPasswords');
       case CheckState.QUOTA_LIMIT:
@@ -490,10 +513,17 @@ Polymer({
       case CheckState.RUNNING:
         return this.i18n('checkPasswordsStop');
       case CheckState.OFFLINE:
-      case CheckState.SIGNED_OUT:
       case CheckState.NO_PASSWORDS:
       case CheckState.OTHER_ERROR:
         return this.i18n('checkPasswordsAgainAfterError');
+      case CheckState.SIGNED_OUT:
+        // When |passwordsWeaknessCheckEnabled| is true, we should allow signed
+        // out users to click the "Check again" button to run the passwords
+        // weakness check.
+        return this.i18n(
+            this.passwordsWeaknessCheckEnabled ?
+                'checkPasswordsAgain' :
+                'checkPasswordsAgainAfterError');
       case CheckState.QUOTA_LIMIT:
         return '';  // Undefined behavior. Don't show any misleading text.
     }
@@ -525,7 +555,9 @@ Polymer({
       case CheckState.OTHER_ERROR:
         return false;
       case CheckState.SIGNED_OUT:
-        return this.isSignedOut_;
+        // When |passwordsWeaknessCheckEnabled| is true, we should allow signed
+        // out users to run the passwords weakness check.
+        return !this.passwordsWeaknessCheckEnabled && this.isSignedOut_;
       case CheckState.NO_PASSWORDS:
       case CheckState.QUOTA_LIMIT:
         return true;
@@ -578,11 +610,14 @@ Polymer({
         return false;
       case CheckState.CANCELED:
       case CheckState.OFFLINE:
-      case CheckState.SIGNED_OUT:
       case CheckState.NO_PASSWORDS:
       case CheckState.QUOTA_LIMIT:
       case CheckState.OTHER_ERROR:
         return true;
+      case CheckState.SIGNED_OUT:
+        // If |passwordsWeaknessCheckEnabled| is true and user is signed out,
+        // this is not an error and we can run the password weakness check.
+        return !this.passwordsWeaknessCheckEnabled;
     }
     assertNotReached(
         'Not specified whether to state is an error: ' + this.status.state);
@@ -604,11 +639,14 @@ Polymer({
       case CheckState.CANCELED:
       case CheckState.RUNNING:
       case CheckState.OFFLINE:
-      case CheckState.SIGNED_OUT:
       case CheckState.NO_PASSWORDS:
       case CheckState.QUOTA_LIMIT:
       case CheckState.OTHER_ERROR:
         return false;
+      case CheckState.SIGNED_OUT:
+        // Shows "No security issues found" if user is signed out and doesn't
+        // have insecure credentials.
+        return this.passwordsWeaknessCheckEnabled;
     }
     assertNotReached(
         'Not specified whether to show passwords for state: ' +
@@ -635,11 +673,16 @@ Polymer({
    * @private
    */
   waitsForFirstCheck_() {
+    // We don't run the compromise check if user is signed out and don't need to
+    // wait for the first check.
+    if (this.passwordsWeaknessCheckEnabled && this.isSignedOut_) {
+      return false;
+    }
     return !this.status.elapsedTimeSinceLastCheck;
   },
 
   /**
-   * Returns true iff the user is signed in.
+   * Returns true iff the user is signed out.
    * @return {boolean}
    * @private
    */
@@ -664,7 +707,20 @@ Polymer({
    * @return {boolean}
    * @private
    */
-  computeShowNoCompromisedPasswordsLabel() {
+  computeShowCompromisedCredentialsBody_() {
+    // Always shows compromised credetnials section if
+    // |passwordsWeaknessCheckEnabled| is true and user is signed out.
+    if (this.passwordsWeaknessCheckEnabled && this.isSignedOut_) {
+      return true;
+    }
+    return this.hasLeakedCredentials_();
+  },
+
+  /**
+   * @return {boolean}
+   * @private
+   */
+  computeShowNoCompromisedPasswordsLabel_() {
     // Check if user isn't signed in.
     if (!this.syncStatus_ || !this.syncStatus_.signedIn) {
       return false;

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

@@ -1043,6 +1043,10 @@ void AddAutofillStrings(content::WebUIDataSource* html_source,
       l10n_util::GetStringFUTF16(
           IDS_SETTINGS_WEAK_PASSWORDS_DESCRIPTION,
           base::ASCIIToUTF16(chrome::kSeeMoreSecurityTipsURL)));
+  html_source->AddString("signedOutUserLabel",
+                         l10n_util::GetStringFUTF16(
+                             IDS_SETTINGS_SIGNED_OUT_USER_LABEL,
+                             base::ASCIIToUTF16(chrome::kSyncLearnMoreURL)));
   // The warning message that will be shown if there is a content setting
   // pattern with a wildcard in it. The check for wildcards is done on the js
   // side.

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

@@ -314,6 +314,7 @@ suite('PasswordsCheckSection', function() {
   // Test verifies that 'Try again' is hidden when users encounter a
   // not-signed-in error.
   test('hideRetryAfterSignOutErrorUntilSignedInAgain', async function() {
+    loadTimeData.overrideValues({passwordsWeaknessCheck: false});
     passwordManager.data.checkStatus = makePasswordCheckStatus(
         /*state=*/ PasswordCheckState.SIGNED_OUT);
     const section = createCheckPasswordSection();
@@ -341,6 +342,32 @@ suite('PasswordsCheckSection', function() {
         interaction);
   });
 
+  // Test verifies that 'Check again' is shown when users is signed out and
+  // |passwordsWeaknessCheck| flag is disabled.
+  test('showCheckAgainWhenSignedOut', async function() {
+    loadTimeData.overrideValues({passwordsWeaknessCheck: true});
+    passwordManager.data.checkStatus = makePasswordCheckStatus(
+        /*state=*/ PasswordCheckState.SIGNED_OUT);
+    const section = createCheckPasswordSection();
+    webUIListenerCallback('stored-accounts-updated', []);
+    if (isChromeOS) {
+      simulateSyncStatus({signedIn: false});
+    }
+    await passwordManager.whenCalled('getPasswordCheckStatus');
+    flush();
+    expectTrue(isElementVisible(section.$.controlPasswordCheckButton));
+    expectEquals(
+        section.i18n('checkPasswordsAgain'),
+        section.$.controlPasswordCheckButton.innerText);
+    section.$.controlPasswordCheckButton.click();
+    await passwordManager.whenCalled('startBulkPasswordCheck');
+    const interaction =
+        await passwordManager.whenCalled('recordPasswordCheckInteraction');
+    assertEquals(
+        PasswordManagerProxy.PasswordCheckInteraction.START_CHECK_MANUALLY,
+        interaction);
+  });
+
   // Test verifies that 'Try again' is hidden when users encounter a
   // no-saved-passwords error.
   test('hideRetryAfterNoPasswordsError', async function() {
@@ -371,9 +398,38 @@ suite('PasswordsCheckSection', function() {
         interaction);
   });
 
-  // Test verifies that if no compromised credentials found than list is not
-  // shown
+  // Test verifies that if no compromised credentials found than list of
+  // compromised credentials is not shown, if user is sign in and the
+  // |passwordsWeaknessCheck| is disabled.
   test('noCompromisedCredentials', async function() {
+    loadTimeData.overrideValues({passwordsWeaknessCheck: false});
+    const data = passwordManager.data;
+    data.checkStatus = makePasswordCheckStatus(
+        /*state=*/ PasswordCheckState.IDLE,
+        /*checked=*/ 4,
+        /*remaining=*/ 0,
+        /*lastCheck=*/ 'Just now');
+    data.leakedCredentials = [];
+
+    const section = createCheckPasswordSection();
+    assertFalse(isElementVisible(section.$.noCompromisedCredentials));
+    webUIListenerCallback('sync-prefs-changed', getSyncAllPrefs());
+    simulateSyncStatus({signedIn: true});
+
+    // Initialize with dummy data breach detection settings
+    section.prefs = {profile: {password_manager_leak_detection: {value: true}}};
+
+    await passwordManager.whenCalled('getPasswordCheckStatus');
+    flush();
+    assertFalse(isElementVisible(section.$.compromisedCredentialsBody));
+    assertTrue(isElementVisible(section.$.noCompromisedCredentials));
+  });
+
+  // Test verifies that if no compromised credentials found than list of
+  // compromised credentials is not shown, if user is sign in and the
+  // |passwordsWeaknessCheck| is enabled.
+  test('noCompromisedCredentialsDisableWeakCheck', async function() {
+    loadTimeData.overrideValues({passwordsWeaknessCheck: true});
     const data = passwordManager.data;
     data.checkStatus = makePasswordCheckStatus(
         /*state=*/ PasswordCheckState.IDLE,
@@ -826,6 +882,7 @@ suite('PasswordsCheckSection', function() {
 
   // When signed out, only show an error.
   test('showOnlyErrorWhenSignedOut', async function() {
+    loadTimeData.overrideValues({passwordsWeaknessCheck: false});
     passwordManager.data.checkStatus =
         makePasswordCheckStatus(PasswordCheckState.SIGNED_OUT);
 
@@ -838,6 +895,80 @@ suite('PasswordsCheckSection', function() {
     expectFalse(isElementVisible(section.$.subtitle));
   });
 
+  // If |passwordsWeaknessCheck| is true, user is signed out and has
+  // compromised credentials that were found in the past, shows "Checked
+  // passwords" and correct label in the top of comromised passwords section.
+  test('signedOutHasCompromisedHasWeak', async function() {
+    loadTimeData.overrideValues({passwordsWeaknessCheck: true});
+    passwordManager.data.checkStatus =
+        makePasswordCheckStatus(PasswordCheckState.SIGNED_OUT);
+    passwordManager.data.weakCredentials =
+        [makeInsecureCredential('one.com', 'test1')];
+    passwordManager.data.leakedCredentials =
+        [makeCompromisedCredential('one.com', 'test4', 'LEAKED', 1)];
+    const section = createCheckPasswordSection();
+    simulateSyncStatus({signedIn: false});
+    await passwordManager.whenCalled('getPasswordCheckStatus');
+    flush();
+
+    const title = section.$.title;
+    const subtitle = section.$.subtitle;
+    assertTrue(isElementVisible(title));
+    expectEquals(section.i18n('checkedPasswords'), title.innerText);
+    assertTrue(isElementVisible(subtitle));
+    await PluralStringProxyImpl.getInstance()
+        .getPluralString('insecurePasswords', 2)
+        .then(count => {
+          expectEquals(count, subtitle.textContent.trim());
+        });
+    expectTrue(
+        section.$$('iron-icon').classList.contains('has-security-issues'));
+    expectFalse(
+        section.$$('iron-icon').classList.contains('no-security-issues'));
+
+    assertTrue(isElementVisible(section.$.compromisedCredentialsBody));
+    assertTrue(isElementVisible(section.$.signedOutUserLabel));
+    expectEquals(
+        section.i18n('signedOutUserLabel'),
+        section.$.signedOutUserLabel.textContent.trim());
+    assertTrue(isElementVisible(section.$.compromisedPasswordsDescription));
+    expectEquals(
+        section.i18n('compromisedPasswordsDescription'),
+        section.$.compromisedPasswordsDescription.textContent.trim());
+    expectTrue(isElementVisible(section.$.weakCredentialsBody));
+  });
+
+  // If |passwordsWeaknessCheck| is true, user is signed out and doesn't have
+  // compromised credentials in the past and doesn't have weak credentials,
+  // shows "Checked passwords" and correct label in the top of comromised
+  // passwords section.
+  test('signedOutNoCompromisedNoWeak', async function() {
+    loadTimeData.overrideValues({passwordsWeaknessCheck: true});
+    passwordManager.data.checkStatus =
+        makePasswordCheckStatus(PasswordCheckState.SIGNED_OUT);
+    const section = createCheckPasswordSection();
+    simulateSyncStatus({signedIn: false});
+    await passwordManager.whenCalled('getPasswordCheckStatus');
+    flush();
+
+    const title = section.$.title;
+    const subtitle = section.$.subtitle;
+    assertTrue(isElementVisible(title));
+    expectEquals(section.i18n('checkedPasswords'), title.innerText);
+    assertTrue(isElementVisible(subtitle));
+    expectTrue(
+        section.$$('iron-icon').classList.contains('no-security-issues'));
+    expectFalse(
+        section.$$('iron-icon').classList.contains('has-security-issues'));
+
+    assertTrue(isElementVisible(section.$.compromisedCredentialsBody));
+    assertTrue(isElementVisible(section.$.signedOutUserLabel));
+    expectEquals(
+        section.i18n('signedOutUserLabel'),
+        section.$.signedOutUserLabel.textContent.trim());
+    assertFalse(isElementVisible(section.$.compromisedPasswordsDescription));
+  });
+
   // When no passwords are saved, only show an error.
   test('showOnlyErrorWithoutPasswords', async function() {
     passwordManager.data.checkStatus =