Update password_edit_dialog and passwords_list_handler to allow changing saved password.

Bug: 377410
Change-Id: Id6bb117f757c37d153637301be89f01560513a32
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2316203
Commit-Queue: Irina Fedorova <irfedorova@google.com>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#793590}

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

@@ -31,7 +31,7 @@
       }
     </style>
     <cr-dialog id="dialog" close-text="$i18n{close}">
-      <div slot="title">$i18n{passwordDetailsTitle}</div>
+      <div slot="title" id="title">[[getTitle_(isEditDialog_)]]</div>
       <div slot="body">
         <div hidden="[[!shouldShowStorageDetails]]" id="storageDetails">
           [[getStorageDetailsMessage_()]]
@@ -45,7 +45,9 @@
         <cr-input id="passwordInput" label="$i18n{editPasswordPasswordLabel}"
             type="[[getPasswordInputType_(password)]]"
             value="[[getPassword_(password)]]" on-blur="onInputBlur_"
-            class="password-input" readonly>
+            class="password-input" readonly="[[!isEditDialog_]]"
+            required="[[isEditDialog_]]" auto-validate="[[isEditDialog_]]"
+            invalid="{{inputInvalid_}}">
           <cr-icon-button id="showPasswordButton"
               class$="[[getIconClass_(password)]]" slot="suffix"
               hidden$="[[entry.federationText]]"
@@ -55,10 +57,18 @@
                   '$i18nPolymer{showPassword}')]]">
           </cr-icon-button>
         </cr-input>
+        <div id="footnote" hidden="[[!isEditDialog_]]">
+          [[getFootnote_()]]
+        </div>
       </div>
       <div slot="button-container">
-        <cr-button class="action-button" on-click="onActionButtonTap_">
-          $i18n{done}
+        <cr-button id="cancel" class="cancel-button" on-click="onCancel_"
+            hidden="[[!isEditDialog_]]">
+          $i18n{cancel}
+        </cr-button>
+        <cr-button id="actionButton" class="action-button"
+            on-click="onActionButtonTap_" disabled="[[inputInvalid_]]">
+          [[getActionButtonName_(isEditDialog_)]]
         </cr-button>
       </div>
     </cr-dialog>

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

@@ -17,12 +17,14 @@ import '../icons.m.js';
 import '../settings_shared_css.m.js';
 import '../settings_vars_css.m.js';
 import './passwords_shared_css.js';
-import {I18nBehavior} from 'chrome://resources/js/i18n_behavior.m.js';
 
+import {I18nBehavior} from 'chrome://resources/js/i18n_behavior.m.js';
 import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
-import {ShowPasswordBehavior} from './show_password_behavior.js';
+import {loadTimeData} from '../i18n_setup.js';
 
+import {PasswordManagerImpl, PasswordManagerProxy} from './password_manager_proxy.js';
+import {ShowPasswordBehavior} from './show_password_behavior.js';
 
 Polymer({
   is: 'password-edit-dialog',
@@ -33,26 +35,91 @@ Polymer({
 
   properties: {
     shouldShowStorageDetails: {type: Boolean, value: false},
+
+    /** @private */
+    editPasswordsInSettings_: {
+      type: Boolean,
+      value() {
+        return loadTimeData.getBoolean('editPasswordsInSettings');
+      }
+    },
+
+    /**
+     * Check if editPasswordsInSettings flag is true and entry isn't federation
+     * credential.
+     * @private
+     * */
+    isEditDialog_: {
+      type: Boolean,
+      computed: 'computeIsEditDialog_(editPasswordsInSettings_, entry)'
+    },
+
+    /**
+     * Whether the input is invalid.
+     * @private
+     */
+    inputInvalid_: Boolean,
   },
 
+  /** @private {?PasswordManagerProxy} */
+  passwordManager_: null,
+
   /** @override */
   attached() {
+    this.passwordManager_ = PasswordManagerImpl.getInstance();
     this.$.dialog.showModal();
   },
 
+  /**
+   * Helper function that checks if editPasswordsInSettings flag is true and
+   * entry isn't federation credential.
+   * @return {boolean}
+   * @private
+   * */
+  computeIsEditDialog_() {
+    return this.editPasswordsInSettings_ && !this.entry.federationText;
+  },
+
   /** Closes the dialog. */
   close() {
     this.$.dialog.close();
   },
 
   /**
-   * Handler for tapping the 'done' button. Should just dismiss the dialog.
+   * Handler for tapping the 'cancel' button. Should just dismiss the dialog.
    * @private
    */
-  onActionButtonTap_() {
+  onCancel_() {
     this.close();
   },
 
+  /**
+   * Handler for tapping the 'done' or 'save' button depending on isEditDialog_.
+   * For 'save' button it should save new password. After pressing action button
+   * button the edit dialog should be closed.
+   * @private
+   */
+  onActionButtonTap_() {
+    if (this.isEditDialog_) {
+      this.passwordManager_
+          .changeSavedPassword(
+              this.entry.getAnyId(), this.$.passwordInput.value)
+          .finally(() => {
+            this.close();
+          });
+    } else {
+      this.close();
+    }
+  },
+
+  /**
+   * @return {string}
+   * @private
+   */
+  getActionButtonName_() {
+    return this.isEditDialog_ ? this.i18n('save') : this.i18n('done');
+  },
+
   /** Manually de-select texts for readonly inputs. */
   onInputBlur_() {
     this.shadowRoot.getSelection().removeAllRanges();
@@ -69,5 +136,24 @@ Polymer({
     return this.entry.isPresentInAccount() ?
         this.i18n('passwordStoredInAccount') :
         this.i18n('passwordStoredOnDevice');
+  },
+
+  /**
+   * @return {string}
+   * @private
+   */
+  getTitle_() {
+    // TODO(crbug.com/377410): Change strings like
+    // 'editCompromisedPasswordTitle' to 'editPasswordTitle'.
+    return this.isEditDialog_ ? this.i18n('editCompromisedPasswordTitle') :
+                                this.i18n('passwordDetailsTitle');
+  },
+
+  /**
+   * @return {string} The text to be displayed as the dialog's footnote.
+   * @private
+   */
+  getFootnote_() {
+    return this.i18n('editCompromisedPasswordFootnote', this.entry.urls.shown);
   }
 });

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

@@ -18,7 +18,9 @@
             on-click="onMenuCopyPasswordButtonTap_">$i18n{copyPassword}</button>
       </template>
       <button id="menuEditPassword" class="dropdown-item"
-          on-click="onMenuEditPasswordTap_">$i18n{passwordViewDetails}</button>
+          on-click="onMenuEditPasswordTap_">
+        [[getMenuEditPasswordName_(isEditDialog_)]]
+      </button>
       <button id="menuRemovePassword" class="dropdown-item"
           on-click="onMenuRemovePasswordTap_">$i18n{removePassword}</button>
       <template is="dom-if" if="[[shouldShowMoveToAccountOption]]">

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

@@ -87,6 +87,16 @@ Polymer({
       }
     },
 
+    /**
+     * Check if editPasswordsInSettings flag is true and entry isn't federation
+     * credential.
+     * @private
+     * */
+    isEditDialog_: {
+      type: Boolean,
+      computed: 'computeIsEditDialog_(editPasswordsInSettings_, activePassword)'
+    },
+
     /** @private */
     showPasswordEditDialog_: {type: Boolean, value: false},
 
@@ -103,7 +113,6 @@ Polymer({
      */
     activeDialogAnchor_: {type: Object, value: null},
 
-
     /**
      * The message displayed in the toast following a password removal.
      */
@@ -130,6 +139,17 @@ Polymer({
     }
   },
 
+  /**
+   * Helper function that checks if editPasswordsInSettings flag is true and
+   * entry isn't federation credential.
+   * @return {boolean}
+   * @private
+   * */
+  computeIsEditDialog_() {
+    return this.editPasswordsInSettings_ &&
+        (!this.activePassword || !this.activePassword.entry.federationText);
+  },
+
   /**
    * Closes the toast manager.
    */
@@ -151,16 +171,25 @@ Polymer({
   },
 
   /**
-   * Shows the edit password dialog.
    * @param {!Event} e
    * @private
    */
   onMenuEditPasswordTap_(e) {
+    // TODO(crbug.com/377410): Add authentication if isEditDialog_ is true.
     e.preventDefault();
     this.$.menu.close();
     this.showPasswordEditDialog_ = true;
   },
 
+  /**
+   * @return {string}
+   * @private
+   */
+  getMenuEditPasswordName_() {
+    return this.isEditDialog_ ? this.i18n('editCompromisedPassword') :
+                                this.i18n('passwordViewDetails');
+  },
+
   /** @private */
   onPasswordEditDialogClosed_() {
     this.showPasswordEditDialog_ = false;

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

@@ -142,6 +142,46 @@ function exceptionsListContainsUrl(exceptionList, url) {
   return false;
 }
 
+/**
+ * Helper function to test for an element is visible.
+ */
+function isElementVisible(element) {
+  return element && !element.hidden;
+}
+
+/**
+ * Helper function to test if all components of edit dialog are shown correctly.
+ */
+function editDialogPartsAreShownCorrectly(passwordDialog) {
+  assertEquals(
+      passwordDialog.i18n('editCompromisedPasswordTitle'),
+      passwordDialog.$.title.textContent.trim());
+  assertFalse(passwordDialog.$.passwordInput.readonly);
+  assertTrue(passwordDialog.$.passwordInput.required);
+  assertTrue(isElementVisible(passwordDialog.$.footnote));
+  assertTrue(isElementVisible(passwordDialog.$.cancel));
+  assertEquals(
+      passwordDialog.i18n('save'),
+      passwordDialog.$.actionButton.textContent.trim());
+}
+
+/**
+ * Helper function to test if all components of details dialog are shown
+ * correctly.
+ */
+function detailsDialogPartsAreShownCorrectly(passwordDialog) {
+  assertEquals(
+      passwordDialog.i18n('passwordDetailsTitle'),
+      passwordDialog.$.title.textContent.trim());
+  assertTrue(passwordDialog.$.passwordInput.readonly);
+  assertFalse(passwordDialog.$.passwordInput.required);
+  assertFalse(isElementVisible(passwordDialog.$.footnote));
+  assertFalse(isElementVisible(passwordDialog.$.cancel));
+  assertEquals(
+      passwordDialog.i18n('done'),
+      passwordDialog.$.actionButton.textContent.trim());
+}
+
 /**
  * Simulates user who is eligible and opted-in for account storage. Should be
  * called after the PasswordsSection element is created. The load time value for
@@ -562,6 +602,82 @@ suite('PasswordsSection', function() {
         passwordsSection.$.passwordsListHandler.$$('#menuCopyPassword').hidden);
   });
 
+  // Test verifies that 'Edit' button is replaced to 'Details' for Federated
+  // (passwordless) credentials. Does not test Details and Edit button.
+  test('verifyEditReplacedToDetailsForFederatedPasswordInMenu', function() {
+    const passwordList = [
+      createPasswordEntry({federationText: 'with chromium.org'}),
+    ];
+    const passwordsSection = elementFactory.createPasswordsSection(
+        passwordManager, passwordList, []);
+
+    getFirstPasswordListItem(passwordsSection).$.moreActionsButton.click();
+    flush();
+    assertEquals(
+        passwordsSection.i18n('passwordViewDetails'),
+        passwordsSection.$.passwordsListHandler.$$('#menuEditPassword')
+            .textContent.trim());
+  });
+
+  // Test verifies that 'Edit' button is replaced to 'Details' for Federated
+  // (passwordless) credentials when EditPasswordsInSettings flag is enabled.
+  // Does not test Details and Edit button.
+  test(
+      'verifyDetailsForFederatedPasswordInMenuEnabledEditPasswordsInSettings',
+      function() {
+        const passwordList = [
+          createPasswordEntry({federationText: 'with chromium.org'}),
+        ];
+        loadTimeData.overrideValues({editPasswordsInSettings: true});
+        const passwordsSection = elementFactory.createPasswordsSection(
+            passwordManager, passwordList, []);
+
+        getFirstPasswordListItem(passwordsSection).$.moreActionsButton.click();
+        flush();
+        assertEquals(
+            passwordsSection.i18n('passwordViewDetails'),
+            passwordsSection.$.passwordsListHandler.$$('#menuEditPassword')
+                .textContent.trim());
+      });
+
+  // Test verifies that 'Edit' button is shown instead of 'Details' for
+  // common credentials when the flag editPasswordsInSettings is enabled.
+  // Does not test Details and Edit button.
+  test('verifyEditButtonInMenuEnabledEditPasswordsInSettings', function() {
+    const passwordList = [
+      createPasswordEntry({url: 'one.com', username: 'hey'}),
+    ];
+    loadTimeData.overrideValues({editPasswordsInSettings: true});
+    const passwordsSection = elementFactory.createPasswordsSection(
+        passwordManager, passwordList, []);
+
+    getFirstPasswordListItem(passwordsSection).$.moreActionsButton.click();
+    flush();
+    assertEquals(
+        passwordsSection.i18n('editCompromisedPassword'),
+        passwordsSection.$.passwordsListHandler.$$('#menuEditPassword')
+            .textContent.trim());
+  });
+
+  // Test verifies that 'Details' button is shown instead of 'Edit' for
+  // non-federated credentials when the flag editPasswordsInSettings is
+  // disabled. Does not test Details and Edit button.
+  test('verifyDetailsButtonInMenuDisabledEditPasswordsInSettings', function() {
+    const passwordList = [
+      createPasswordEntry({url: 'one.com', username: 'hey'}),
+    ];
+    loadTimeData.overrideValues({editPasswordsInSettings: false});
+    const passwordsSection = elementFactory.createPasswordsSection(
+        passwordManager, passwordList, []);
+
+    getFirstPasswordListItem(passwordsSection).$.moreActionsButton.click();
+    flush();
+    assertEquals(
+        passwordsSection.i18n('passwordViewDetails'),
+        passwordsSection.$.passwordsListHandler.$$('#menuEditPassword')
+            .textContent.trim());
+  });
+
   test('verifyFilterPasswords', function() {
     const passwordList = [
       createPasswordEntry({url: 'one.com', username: 'SHOW', id: 0}),
@@ -832,14 +948,81 @@ suite('PasswordsSection', function() {
   });
 
   test('verifyFederatedPassword', function() {
-    const item = createMultiStorePasswordEntry(
+    const federationEntry = createMultiStorePasswordEntry(
         {federationText: 'with chromium.org', username: 'bart', deviceId: 42});
-    const passwordDialog = elementFactory.createPasswordEditDialog(item);
+    const passwordDialog =
+        elementFactory.createPasswordEditDialog(federationEntry);
 
-    assertEquals(item.federationText, passwordDialog.$.passwordInput.value);
+    assertEquals(
+        federationEntry.federationText, passwordDialog.$.passwordInput.value);
     // Text should be readable.
     assertEquals('text', passwordDialog.$.passwordInput.type);
     assertTrue(passwordDialog.$.showPasswordButton.hidden);
+    detailsDialogPartsAreShownCorrectly(passwordDialog);
+  });
+
+  test('verifyDetailsDialogDisabledEditPasswordsInSettings', function() {
+    const federationEntry = createMultiStorePasswordEntry(
+        {federationText: 'with chromium.org', username: 'bart', deviceId: 42});
+    loadTimeData.overrideValues({editPasswordsInSettings: false});
+    const passwordDialogFederation =
+        elementFactory.createPasswordEditDialog(federationEntry);
+    detailsDialogPartsAreShownCorrectly(passwordDialogFederation);
+
+    const commonEntry = createMultiStorePasswordEntry(
+        {url: 'goo.gl', username: 'bart', accountId: 42});
+    const passwordDialogCommon =
+        elementFactory.createPasswordEditDialog(commonEntry);
+    detailsDialogPartsAreShownCorrectly(passwordDialogCommon);
+  });
+
+  test('verifyEditOrDetailsDialogEnabledEditPasswordsInSettings', function() {
+    const federationEntry = createMultiStorePasswordEntry(
+        {federationText: 'with chromium.org', username: 'bart', deviceId: 42});
+    loadTimeData.overrideValues({editPasswordsInSettings: true});
+    const passwordDialogFederation =
+        elementFactory.createPasswordEditDialog(federationEntry);
+    detailsDialogPartsAreShownCorrectly(passwordDialogFederation);
+
+    const commonEntry = createMultiStorePasswordEntry(
+        {url: 'goo.gl', username: 'bart', accountId: 42});
+    const passwordDialogCommon =
+        elementFactory.createPasswordEditDialog(commonEntry);
+    // Should show edit dialog for common credetial when editPasswordsInSettings
+    // flag is enabled.
+    editDialogPartsAreShownCorrectly(passwordDialogCommon);
+  });
+
+  test('editDialogChangePassword', async function() {
+    loadTimeData.overrideValues({editPasswordsInSettings: true});
+
+    const PASSWORD1 = 'hello_world';
+    const commonEntry = createMultiStorePasswordEntry(
+        {url: 'goo.gl', username: 'bart', accountId: 42});
+
+    const editDialog = elementFactory.createPasswordEditDialog(commonEntry);
+
+    editDialog.password = PASSWORD1;
+    flush();
+
+    assertEquals(PASSWORD1, editDialog.$.passwordInput.value);
+
+    // Empty password should be consider invalid and disables the save button.
+    editDialog.$.passwordInput.value = '';
+    assertTrue(editDialog.$.passwordInput.invalid);
+    assertTrue(editDialog.$.actionButton.disabled);
+
+    const PASSWORD2 = 'hello_world_2';
+    editDialog.$.passwordInput.value = PASSWORD2;
+    assertFalse(editDialog.$.passwordInput.invalid);
+    assertFalse(editDialog.$.actionButton.disabled);
+
+    editDialog.$.actionButton.click();
+
+    // Check that the changeSavedPassword is called with the right arguments.
+    const {newPassword} =
+        await passwordManager.whenCalled('changeSavedPassword');
+    assertEquals(PASSWORD2, newPassword);
   });
 
   // Test verifies that the edit dialog informs the password is stored in the

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

@@ -299,8 +299,8 @@ export class TestPasswordManagerProxy extends TestBrowserProxy {
   }
 
   /** override */
-  changeSavedPassword(id, new_password) {
-    this.methodCalled('changeSavedPassword', {id, new_password});
+  changeSavedPassword(id, newPassword) {
+    this.methodCalled('changeSavedPassword', {id, newPassword});
     return Promise.resolve();
   }