[Settings] Refactor: change createExceptionEntry(); set ids in tests

This CL is simply the analogous of crrev.com/c/2218055 for password
exceptions. This means it changes the signature of this helper function
to: a) make it easier to write tests in a multi-store scenario; b) allow
the deduplication functionality to be introduced without breaking the
existing tests.

Besides that, the CL fixes a comment typo in validatePasswordList() and
also exposes chrome.passwordsPrivate.removeExceptions() API in the
PasswordManagerProxy, so it can later be used at PasswordsSection when
exceptions are deduplicated.

TBR=fhorschig@chromium.org

Bug: 1049141
Change-Id: I3a501283c964904b981a6e2463336f4f147816bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2234755
Commit-Queue: Victor Vianna <victorvianna@google.com>
Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776221}

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

@@ -86,6 +86,13 @@ export class PasswordManagerProxy {
    */
   removeException(id) {}
 
+  /**
+   * Should remove the password exceptions and notify that the list has changed.
+   * @param {!Array<number>} ids The ids for the exception url entries being
+   * removed. Any |id| not in the list is ignored.
+   */
+  removeExceptions(ids) {}
+
   /**
    * Should undo the last saved password or exception removal and notify that
    * the list has changed.
@@ -381,6 +388,11 @@ export class PasswordManagerImpl {
     chrome.passwordsPrivate.removePasswordException(id);
   }
 
+  /** @override */
+  removeExceptions(ids) {
+    chrome.passwordsPrivate.removePasswordExceptions(ids);
+  }
+
   /** @override */
   undoRemoveSavedPasswordOrException() {
     chrome.passwordsPrivate.undoRemoveSavedPasswordOrException();

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

@@ -207,7 +207,10 @@ suite('PasswordsAndForms', function() {
     return createPrefs(true, true).then(function(prefs) {
       const element = createAutofillElement(prefs);
 
-      const list = [createExceptionEntry(), createExceptionEntry()];
+      const list = [
+        createExceptionEntry({url: 'one.com', id: 0}),
+        createExceptionEntry({url: 'two.com', id: 1})
+      ];
       passwordManager.lastCallback.addExceptionListChangedListener(list);
       flush();
 

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

@@ -11,9 +11,9 @@ import {createExceptionEntry} from 'chrome://test/settings/passwords_and_autofil
 
 suite('MultiStoreExceptionEntry', function() {
   test('verifyIds', function() {
-    const deviceEntry = createExceptionEntry('g.com', 0);
+    const deviceEntry = createExceptionEntry({url: 'g.com', id: 0});
     deviceEntry.fromAccountStore = false;
-    const accountEntry = createExceptionEntry('g.com', 1);
+    const accountEntry = createExceptionEntry({url: 'g.com', id: 1});
     accountEntry.fromAccountStore = true;
 
     const multiStoreDeviceEntry = new MultiStoreExceptionEntry(deviceEntry);

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

@@ -6,7 +6,7 @@
 import {assertNotReached} from 'chrome://resources/js/assert.m.js';
 import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 import {AutofillManager, PaymentsManager} from 'chrome://settings/lazy_load.js';
-import {MultiStorePasswordUiEntry} from 'chrome://settings/settings.js';
+import {MultiStoreExceptionEntry, MultiStorePasswordUiEntry} from 'chrome://settings/settings.js';
 
 import {assertEquals} from '../chai_assert.js';
 
@@ -18,7 +18,7 @@ import {TestPasswordManagerProxy} from './test_password_manager_proxy.js';
  * password manager native code. If no |id| is passed, it is set to a default,
  * value so this should probably not be done in tests with multiple entries
  * (|id| is unique). If no |frontendId| is passed, it is set to the same value
- * as |id|.
+ * set for |id|.
  * @param {{ url: (string|undefined),
  *            username: (string|undefined),
  *            federationText: (string|undefined),
@@ -31,24 +31,23 @@ import {TestPasswordManagerProxy} from './test_password_manager_proxy.js';
 export function createPasswordEntry(params) {
   // Generate fake data if param is undefined.
   params = params || {};
-  params.url = params.url !== undefined ? params.url : 'www.foo.com';
-  params.username = params.username || 'user';
-  params.id = params.id !== undefined ? params.id : 42;
-  params.frontendId =
-      params.frontendId !== undefined ? params.frontendId : params.id;
-  params.fromAccountStore = params.fromAccountStore || false;
+  const url = params.url !== undefined ? params.url : 'www.foo.com';
+  const username = params.username !== undefined ? params.username : 'user';
+  const id = params.id !== undefined ? params.id : 42;
+  const frontendId = params.frontendId !== undefined ? params.frontendId : id;
+  const fromAccountStore = params.fromAccountStore || false;
 
   return {
     urls: {
-      origin: 'http://' + params.url + '/login',
-      shown: params.url,
-      link: 'http://' + params.url + '/login',
+      origin: 'http://' + url + '/login',
+      shown: url,
+      link: 'http://' + url + '/login',
     },
-    username: params.username,
+    username: username,
     federationText: params.federationText,
-    id: params.id,
-    frontendId: params.frontendId,
-    fromAccountStore: params.fromAccountStore,
+    id: id,
+    frontendId: frontendId,
+    fromAccountStore: fromAccountStore,
   };
 }
 
@@ -100,14 +99,23 @@ export function createMultiStorePasswordEntry(params) {
 }
 
 /**
- * Creates a single item for the list of password exceptions.
- * @param {string=} url
- * @param {number=} id
+ * Creates a single item for the list of password exceptions. If no |id| is
+ * passed, it is set to a default, value so this should probably not be done in
+ * tests with multiple entries (|id| is unique). If no |frontendId| is passed,
+ * it is set to the same value set for |id|.
+ * @param {{ url: (string|undefined),
+ *           id: (number|undefined),
+ *           frontendId: (number|undefined),
+ *           fromAccountStore: (boolean|undefined)
+ *         }=} params
  * @return {chrome.passwordsPrivate.ExceptionEntry}
  */
-export function createExceptionEntry(url, id) {
-  url = url || patternMaker_('www.xxxxxx.com', 16);
-  id = id || 0;
+export function createExceptionEntry(params) {
+  params = params || {};
+  const url = params.url !== undefined ? params.url : 'www.foo.com';
+  const id = params.id !== undefined ? params.id : 42;
+  const frontendId = params.frontendId !== undefined ? params.frontendId : id;
+  const fromAccountStore = params.fromAccountStore || false;
   return {
     urls: {
       origin: 'http://' + url + '/login',
@@ -115,8 +123,8 @@ export function createExceptionEntry(url, id) {
       link: 'http://' + url + '/login',
     },
     id: id,
-    frontendId: id,
-    fromAccountStore: false,
+    frontendId: frontendId,
+    fromAccountStore: fromAccountStore,
   };
 }
 

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

@@ -46,7 +46,7 @@ function validateMultiStorePasswordList(passwordsSection, expectedPasswords) {
 }
 
 /**
- * Convenience version of validateMultiStorePasswordList for when store
+ * Convenience version of validateMultiStorePasswordList() for when store
  * duplicates don't exist.
  * @param {!Element} passwordsSection The passwords section element that will
  *     be checked.
@@ -573,12 +573,12 @@ suite('PasswordsSection', function() {
 
   test('verifyFilterPasswordExceptions', function() {
     const exceptionList = [
-      createExceptionEntry('docsshoW.google.com'),
-      createExceptionEntry('showmail.com'),
-      createExceptionEntry('google.com'),
-      createExceptionEntry('inbox.google.com'),
-      createExceptionEntry('mapsshow.google.com'),
-      createExceptionEntry('plus.google.comshow'),
+      createExceptionEntry({url: 'docsshoW.google.com', id: 0}),
+      createExceptionEntry({url: 'showmail.com', id: 1}),
+      createExceptionEntry({url: 'google.com', id: 2}),
+      createExceptionEntry({url: 'inbox.google.com', id: 3}),
+      createExceptionEntry({url: 'mapsshow.google.com', id: 4}),
+      createExceptionEntry({url: 'plus.google.comshow', id: 5}),
     ];
 
     const passwordsSection = elementFactory.createPasswordsSection(
@@ -587,10 +587,10 @@ suite('PasswordsSection', function() {
     flush();
 
     const expectedExceptionList = [
-      createExceptionEntry('docsshoW.google.com'),
-      createExceptionEntry('showmail.com'),
-      createExceptionEntry('mapsshow.google.com'),
-      createExceptionEntry('plus.google.comshow'),
+      createExceptionEntry({url: 'docsshoW.google.com', id: 0}),
+      createExceptionEntry({url: 'showmail.com', id: 1}),
+      createExceptionEntry({url: 'mapsshow.google.com', id: 4}),
+      createExceptionEntry({url: 'plus.google.comshow', id: 5}),
     ];
 
     validateExceptionList(
@@ -610,12 +610,12 @@ suite('PasswordsSection', function() {
 
   test('verifyPasswordExceptions', function() {
     const exceptionList = [
-      createExceptionEntry('docs.google.com'),
-      createExceptionEntry('mail.com'),
-      createExceptionEntry('google.com'),
-      createExceptionEntry('inbox.google.com'),
-      createExceptionEntry('maps.google.com'),
-      createExceptionEntry('plus.google.com'),
+      createExceptionEntry({url: 'docs.google.com', id: 0}),
+      createExceptionEntry({url: 'mail.com', id: 1}),
+      createExceptionEntry({url: 'google.com', id: 2}),
+      createExceptionEntry({url: 'inbox.google.com', id: 3}),
+      createExceptionEntry({url: 'maps.google.com', id: 4}),
+      createExceptionEntry({url: 'plus.google.com', id: 5}),
     ];
 
     const passwordsSection = elementFactory.createPasswordsSection(
@@ -631,12 +631,12 @@ suite('PasswordsSection', function() {
   // Test verifies that removing an exception will update the elements.
   test('verifyPasswordExceptionRemove', function() {
     const exceptionList = [
-      createExceptionEntry('docs.google.com'),
-      createExceptionEntry('mail.com'),
-      createExceptionEntry('google.com'),
-      createExceptionEntry('inbox.google.com'),
-      createExceptionEntry('maps.google.com'),
-      createExceptionEntry('plus.google.com'),
+      createExceptionEntry({url: 'docs.google.com', id: 0}),
+      createExceptionEntry({url: 'mail.com', id: 1}),
+      createExceptionEntry({url: 'google.com', id: 2}),
+      createExceptionEntry({url: 'inbox.google.com', id: 3}),
+      createExceptionEntry({url: 'maps.google.com', id: 4}),
+      createExceptionEntry({url: 'plus.google.com', id: 5}),
     ];
 
     const passwordsSection = elementFactory.createPasswordsSection(
@@ -653,21 +653,28 @@ suite('PasswordsSection', function() {
     assertFalse(exceptionsListContainsUrl(exceptionList, 'mail.com'));
     flush();
 
+    const expectedExceptionList = [
+      createExceptionEntry({url: 'docs.google.com', id: 0}),
+      createExceptionEntry({url: 'google.com', id: 2}),
+      createExceptionEntry({url: 'inbox.google.com', id: 3}),
+      createExceptionEntry({url: 'maps.google.com', id: 4}),
+      createExceptionEntry({url: 'plus.google.com', id: 5})
+    ];
     validateExceptionList(
         getDomRepeatChildren(passwordsSection.$.passwordExceptionsList),
-        exceptionList);
+        expectedExceptionList);
   });
 
   // Test verifies that pressing the 'remove' button will trigger a remove
   // event. Does not actually remove any exceptions.
   test('verifyPasswordExceptionRemoveButton', function() {
     const exceptionList = [
-      createExceptionEntry('docs.google.com'),
-      createExceptionEntry('mail.com'),
-      createExceptionEntry('google.com'),
-      createExceptionEntry('inbox.google.com'),
-      createExceptionEntry('maps.google.com'),
-      createExceptionEntry('plus.google.com'),
+      createExceptionEntry({url: 'docs.google.com', id: 0}),
+      createExceptionEntry({url: 'mail.com', id: 1}),
+      createExceptionEntry({url: 'google.com', id: 2}),
+      createExceptionEntry({url: 'inbox.google.com', id: 3}),
+      createExceptionEntry({url: 'maps.google.com', id: 4}),
+      createExceptionEntry({url: 'plus.google.com', id: 5}),
     ];
 
     const passwordsSection = elementFactory.createPasswordsSection(

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

@@ -56,6 +56,7 @@ export class TestPasswordManagerProxy extends TestBrowserProxy {
       'removeSavedPassword',
       'removeSavedPasswords',
       'removeException',
+      'removeExceptions',
     ]);
 
     /** @private {!PasswordManagerExpectations} */
@@ -141,6 +142,12 @@ export class TestPasswordManagerProxy extends TestBrowserProxy {
     this.methodCalled('removeException', id);
   }
 
+  /** @override */
+  removeExceptions(ids) {
+    this.actual_.removed.exceptions += ids.length;
+    this.methodCalled('removeExceptions', ids);
+  }
+
   /** @override */
   requestPlaintextPassword(id, reason) {
     this.methodCalled('requestPlaintextPassword', {id, reason});