[b4p/Settings] Refactor: introduce RemovePasswordBehavior

No behavior is changed. This CL introduces a new Polymer behavior
responsible for removing a password entry in Settings. In a multi
store situation, the behavior would delete all existing versions of the
deduplicated entry. The API introduced in crrev.com/c/2199148 for
removing batches of passwords is now called, so that undo-ing restores
every removed version of the password.

<password-list-item> is the only element currently implementing the new
RemovePasswordBehavior. Future CLs will extend this behavior to allow
choosing which versions to remove, and introduce a new dialog that
implements it.

Bug: 1049141
Change-Id: Id62eee3dcce3e3a78e4451371a90e324b8095f46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2209082Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Esmael Elmoslimany <aee@chromium.org>
Reviewed-by: Friedrich [CET] <fhorschig@chromium.org>
Commit-Queue: Victor Vianna <victorvianna@google.com>
Cr-Commit-Position: refs/heads/master@{#771568}

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

@@ -30,6 +30,7 @@ js_type_check("closure_compile_module") {
     ":passwords_section",
     ":payments_list",
     ":payments_section",
+    ":remove_password_behavior",
     ":show_password_behavior",
     ":upi_id_list_entry",
   ]
@@ -173,6 +174,7 @@ js_library("password_remove_confirmation_dialog") {
 js_library("password_list_item") {
   deps = [
     ":blocking_request_manager",
+    ":remove_password_behavior",
     ":show_password_behavior",
     "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
     "//ui/webui/resources/js:load_time_data.m",
@@ -198,6 +200,7 @@ js_library("passwords_section") {
     ":password_edit_dialog",
     ":password_list_item",
     ":password_manager_proxy",
+    ":remove_password_behavior",
     "..:global_scroll_target_behavior.m",
     "..:plural_string_proxy",
     "..:route",
@@ -250,11 +253,19 @@ js_library("payments_section") {
 js_library("show_password_behavior") {
   deps = [
     ":blocking_request_manager",
+    ":multi_store_password_ui_entry",
     ":password_manager_proxy",
   ]
   externs_list = [ "$externs_path/passwords_private.js" ]
 }
 
+js_library("remove_password_behavior") {
+  deps = [
+    ":multi_store_password_ui_entry",
+    ":password_manager_proxy",
+  ]
+}
+
 js_library("upi_id_list_entry") {
   deps = [
     "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",

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

@@ -19,6 +19,7 @@ import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bun
 import {loadTimeData} from '../i18n_setup.js';
 
 import {MultiStorePasswordUiEntryWithPassword} from './multi_store_password_ui_entry.js';
+import {RemovePasswordBehavior} from './remove_password_behavior.js';
 import {ShowPasswordBehavior} from './show_password_behavior.js';
 
 Polymer({
@@ -29,6 +30,7 @@ Polymer({
   behaviors: [
     FocusRowBehavior,
     ShowPasswordBehavior,
+    RemovePasswordBehavior,
   ],
 
   /**

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

@@ -49,6 +49,13 @@ export class PasswordManagerProxy {
    */
   removeSavedPassword(id) {}
 
+  /**
+   * Should remove the saved passwords and notify that the list has changed.
+   * @param {!Array<number>} ids The ids for the password entries being removed.
+   *     Any id not in the list is ignored.
+   */
+  removeSavedPasswords(ids) {}
+
   /**
    * Add an observer to the list of password exceptions.
    * @param {function(!Array<!PasswordManagerProxy.ExceptionEntry>):void}
@@ -352,6 +359,11 @@ export class PasswordManagerImpl {
     chrome.passwordsPrivate.removeSavedPassword(id);
   }
 
+  /** @override */
+  removeSavedPasswords(ids) {
+    chrome.passwordsPrivate.removeSavedPasswords(ids);
+  }
+
   /** @override */
   addExceptionListChangedListener(listener) {
     chrome.passwordsPrivate.onPasswordExceptionsListChanged.addListener(

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

@@ -169,8 +169,8 @@
           class="cr-separators list-with-header"
           scroll-target="[[subpageScrollTarget]]" risk-selection>
         <template>
-          <password-list-item item="[[item]]" tabindex$="[[tabIndex]]"
-              focus-row-index="[[index]]"
+          <password-list-item item="[[item]]" entry-to-remove="[[item.entry]]"
+              tabindex$="[[tabIndex]]" focus-row-index="[[index]]"
 <if expr="chromeos">
               token-request-manager="[[tokenRequestManager_]]"
 </if>

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

@@ -38,6 +38,7 @@ import '../controls/extension_controlled_indicator.m.js';
 import '../controls/settings_toggle_button.m.js';
 import {GlobalScrollTargetBehavior} from '../global_scroll_target_behavior.m.js';
 import {loadTimeData} from '../i18n_setup.js';
+import {RemovalResult} from './remove_password_behavior.js';
 import {SyncBrowserProxyImpl, SyncPrefs, SyncStatus} from '../people_page/sync_browser_proxy.m.js';
 import {PluralStringProxyImpl} from '../plural_string_proxy.js';
 import '../prefs/prefs.m.js';
@@ -551,13 +552,25 @@ Polymer({
    * @private
    */
   onMenuRemovePasswordTap_() {
-    this.passwordManager_.removeSavedPassword(
-        this.activePassword.item.entry.getAnyId());
-    getToastManager().show(
-        this.getRemovePasswordText_(this.activePassword.item));
-    this.fire('iron-announce', {
-      text: this.i18n('undoDescription'),
-    });
+    // TODO(crbug.com/1049141): Open removal dialog if password is present in
+    // both locations. Add tests for when no password is selected in the dialog.
+    /** @type {!RemovalResult} */
+    const result = this.activePassword.requestRemove();
+
+    if (result.removedFromDevice || result.removedFromAccount) {
+      let text = this.i18n('passwordDeleted');
+      if (this.eligibleForAccountStorage_ && this.isOptedInForAccountStorage_) {
+        // TODO(crbug.com/1049141): Style the text according to mocks.
+        // TODO(crbug.com/1049141): Adapt the string when the user can delete
+        // from both account and device.
+        text = result.removedFromAccount ?
+            this.i18n('passwordDeletedFromAccount') :
+            this.i18n('passwordDeletedFromDevice');
+      }
+      getToastManager().show(text);
+      this.fire('iron-announce', {text: this.i18n('undoDescription')});
+    }
+
     /** @type {CrActionMenuElement} */ (this.$.menu).close();
   },
 
@@ -700,23 +713,6 @@ Polymer({
     return showExportPasswords || showImportPasswords;
   },
 
-  /**
-   * @private
-   * @param {!MultiStorePasswordUiEntryWithPassword} item The deleted item.
-   * @return {string}
-   */
-  getRemovePasswordText_(item) {
-    // TODO(crbug.com/1049141): Adapt the string when the user can delete from
-    // both account and device.
-    // TODO(crbug.com/1049141): Style the text according to mocks.
-    if (this.eligibleForAccountStorage_ && this.isOptedInForAccountStorage_) {
-      return item.entry.isPresentInAccount() ?
-          this.i18n('passwordDeletedFromAccount') :
-          this.i18n('passwordDeletedFromDevice');
-    }
-    return this.i18n('passwordDeleted');
-  },
-
   /**
    * @private
    * @return {boolean}

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

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import {MultiStorePasswordUiEntry} from './multi_store_password_ui_entry.js';
+import {PasswordManagerImpl} from './password_manager_proxy.js';
+
+/**
+ * This behavior bundles functionality required to remove a user password.
+ *
+ * @polymerBehavior
+ */
+export const RemovePasswordBehavior = {
+
+  properties: {
+    /**
+     * The password that will be removed.
+     * @type {!MultiStorePasswordUiEntry}
+     */
+    entryToRemove: Object,
+  },
+
+  /**
+   * Removes all existing versions of the password.
+   * @return {!RemovalResult}
+   */
+  // TODO(crbug.com/1049141): Expose APIs in the behavior to allow selecting
+  // which versions to remove. Update the comment on this method then.
+  requestRemove() {
+    /** @type {!RemovalResult} */
+    const result = {
+      removedFromAccount: false,
+      removedFromDevice: false,
+    };
+
+    /** @type {!Array<number>} */
+    const idsToRemove = [];
+    if (this.entryToRemove.isPresentInAccount()) {
+      result.removedFromAccount = true;
+      idsToRemove.push(this.entryToRemove.accountId);
+    }
+    if (this.entryToRemove.isPresentOnDevice()) {
+      result.removedFromDevice = true;
+      idsToRemove.push(this.entryToRemove.deviceId);
+    }
+
+    if (idsToRemove.length) {
+      PasswordManagerImpl.getInstance().removeSavedPasswords(idsToRemove);
+    }
+
+    return result;
+  },
+};
+
+
+/**
+ * Used to inform which password versions were removed.
+ * @typedef {{
+ *   removedFromAccount: boolean,
+ *   removedFromDevice: boolean,
+ * }}
+ */
+export let RemovalResult;

chrome/browser/resources/settings/settings_resources_v3.grdp

@@ -90,6 +90,10 @@
            file="autofill_page/show_password_behavior.js"
            compress="false" type="BINDATA"
            preprocess="true" />
+  <include name="IDR_SETTINGS_AUTOFILL_PAGE_REMOVE_PASSWORD_BEHAVIOR_JS"
+           file="autofill_page/remove_password_behavior.js"
+           compress="false" type="BINDATA"
+           preprocess="true" />
   <include name="IDR_SETTINGS_AUTOFILL_PAGE_PASSWORD_CHECK_JS"
            file="${root_gen_dir}/chrome/browser/resources/settings/autofill_page/password_check.js"
            use_base_dir="false"

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

@@ -274,7 +274,7 @@ suite('PasswordsSection', function() {
 
   // Test verifies that pressing the 'remove' button will trigger a remove
   // event. Does not actually remove any passwords.
-  test('verifyPasswordItemRemoveButton', function() {
+  test('verifyPasswordItemRemoveButton', async function() {
     const passwordList = [
       createPasswordEntry('one', 'six'),
       createPasswordEntry('two', 'five'),
@@ -295,13 +295,12 @@ suite('PasswordsSection', function() {
     firstNode.$$('#passwordMenu').click();
     passwordsSection.$.menuRemovePassword.click();
 
-    return passwordManager.whenCalled('removeSavedPassword').then(id => {
-      // Verify that the expected value was passed to the proxy.
-      assertEquals(firstPassword.id, id);
-      assertEquals(
-          passwordsSection.i18n('passwordDeleted'),
-          getToastManager().$.content.textContent);
-    });
+    const ids = await passwordManager.whenCalled('removeSavedPasswords');
+    // Verify that the expected value was passed to the proxy.
+    assertDeepEquals([firstPassword.id], ids);
+    assertEquals(
+        passwordsSection.i18n('passwordDeleted'),
+        getToastManager().$.content.textContent);
   });
 
   // Test verifies that 'Copy password' button is hidden for Federated

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

@@ -54,6 +54,7 @@ export class TestPasswordManagerProxy extends TestBrowserProxy {
       'recordPasswordCheckInteraction',
       'recordPasswordCheckReferrer',
       'removeSavedPassword',
+      'removeSavedPasswords',
       'removeException',
     ]);
 
@@ -111,6 +112,12 @@ export class TestPasswordManagerProxy extends TestBrowserProxy {
     this.methodCalled('removeSavedPassword', id);
   }
 
+  /** @override */
+  removeSavedPasswords(ids) {
+    this.actual_.removed.passwords += ids.length;
+    this.methodCalled('removeSavedPasswords', ids);
+  }
+
   /** @override */
   addExceptionListChangedListener(listener) {
     this.actual_.listening.exceptions++;