Commit b36df818 authored by Victor Hugo Vianna Silva's avatar Victor Hugo Vianna Silva Committed by Commit Bot

[b4p/Settings] Introduce MultiStoreExceptionEntry

This CL introduces MultiStoreExceptionEntry, which is the analogous of
MultiStorePasswordUiEntry (crrev.com/c/2202237) for exceptions. The
common logic between these two classes is moved to a common base class.
Future CLs will use the new class to deduplicate the passwordExceptions
array in PasswordsSection.

The CL also reorders some includes in settings_resources_v3.grdp that
were not in alphabetical order.

Bug: 1049141
Change-Id: Ibf64de15f49cf20c2f4b310f9a86341a41f6f3b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225408
Commit-Queue: Victor Vianna <victorvianna@google.com>
Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774554}
parent dd9dba50
...@@ -16,6 +16,7 @@ js_type_check("closure_compile_module") { ...@@ -16,6 +16,7 @@ js_type_check("closure_compile_module") {
":blocking_request_manager", ":blocking_request_manager",
":credit_card_edit_dialog", ":credit_card_edit_dialog",
":credit_card_list_entry", ":credit_card_list_entry",
":multi_store_id_handler",
":multi_store_password_ui_entry", ":multi_store_password_ui_entry",
":password_check", ":password_check",
":password_check_behavior", ":password_check_behavior",
...@@ -157,11 +158,26 @@ js_library("password_edit_dialog") { ...@@ -157,11 +158,26 @@ js_library("password_edit_dialog") {
js_library("multi_store_password_ui_entry") { js_library("multi_store_password_ui_entry") {
sources = [ "multi_store_password_ui_entry.js" ] sources = [ "multi_store_password_ui_entry.js" ]
deps = [ deps = [
":multi_store_id_handler",
":password_manager_proxy", ":password_manager_proxy",
"//ui/webui/resources/js:assert.m", "//ui/webui/resources/js:assert.m",
] ]
} }
js_library("multi_store_exception_entry") {
sources = [ "multi_store_password_ui_entry.js" ]
deps = [
":multi_store_id_handler",
":password_manager_proxy",
"//ui/webui/resources/js:assert.m",
]
}
js_library("multi_store_id_handler") {
sources = [ "multi_store_id_handler.js" ]
deps = [ "//ui/webui/resources/js:assert.m" ]
}
js_library("password_remove_confirmation_dialog") { js_library("password_remove_confirmation_dialog") {
deps = [ deps = [
":password_manager_proxy", ":password_manager_proxy",
......
// 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.
/**
* @fileoverview MultiStoreExceptionEntry is used for showing exceptions that
* are duplicated across stores as a single item in the UI.
*/
import {assert} from 'chrome://resources/js/assert.m.js';
import {MultiStoreIdHandler} from './multi_store_id_handler.js';
import {PasswordManagerProxy} from './password_manager_proxy.js';
/**
* A version of PasswordManagerProxy.ExceptionEntry used for deduplicating
* exceptions from the device and the account.
*/
export class MultiStoreExceptionEntry extends MultiStoreIdHandler {
/**
* Creates a multi-store entry from duplicates |entry1| and (optional)
* |entry2|. If both arguments are passed, they should have the same contents
* but should be from different stores.
* @param {!PasswordManagerProxy.ExceptionEntry} entry1
* @param {PasswordManagerProxy.ExceptionEntry=} entry2
*/
constructor(entry1, entry2) {
super();
/** @type {!PasswordManagerProxy.UrlCollection} */
this.urls_ = entry1.urls;
this.setId(entry1.id, entry1.fromAccountStore);
if (entry2) {
this.merge(entry2);
}
}
/**
* Incorporates the id of |otherEntry|, as long as |otherEntry| matches
* |contents_| and the id corresponding to its store is not set.
* @param {!PasswordManagerProxy.ExceptionEntry} otherEntry
*/
// TODO(crbug.com/1049141) Consider asserting frontendId as well.
merge(otherEntry) {
assert(
(this.isPresentInAccount() && !otherEntry.fromAccountStore) ||
(this.isPresentOnDevice() && otherEntry.fromAccountStore));
assert(JSON.stringify(this.urls_) === JSON.stringify(otherEntry.urls));
this.setId(otherEntry.id, otherEntry.fromAccountStore);
}
get urls() {
return this.urls_;
}
}
// 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.
/**
* @fileoverview Common code used by MultiStorePasswordUiEntry and
* MultiStoreIdHandler to deal with ids from different stores.
*/
import {assertNotReached} from 'chrome://resources/js/assert.m.js';
export class MultiStoreIdHandler {
constructor() {
/** @type {number?} */
this.deviceId_ = null;
/** @type {number?} */
this.accountId_ = null;
}
/**
* Get any of the present ids. At least one of the ids must have been set
* before this method is invoked.
* @return {number}
*/
getAnyId() {
if (this.deviceId_ !== null) {
return this.deviceId_;
}
if (this.accountId_ !== null) {
return this.accountId_;
}
assertNotReached();
return 0;
}
/**
* Whether one of the ids is from the account.
* @return {boolean}
*/
isPresentInAccount() {
return this.accountId_ !== null;
}
/**
* Whether one of the ids is from the device.
* @return {boolean}
*/
isPresentOnDevice() {
return this.deviceId_ !== null;
}
get deviceId() {
return this.deviceId_;
}
get accountId() {
return this.accountId_;
}
/**
* @protected
* @param {number} id
* @param {boolean} fromAccountStore
*/
setId(id, fromAccountStore) {
if (fromAccountStore) {
this.accountId_ = id;
} else {
this.deviceId_ = id;
}
}
}
...@@ -7,15 +7,16 @@ ...@@ -7,15 +7,16 @@
* are duplicated across stores as a single item in the UI. * are duplicated across stores as a single item in the UI.
*/ */
import {assert, assertNotReached} from 'chrome://resources/js/assert.m.js'; import {assert} from 'chrome://resources/js/assert.m.js';
import {MultiStoreIdHandler} from './multi_store_id_handler.js';
import {PasswordManagerProxy} from './password_manager_proxy.js'; import {PasswordManagerProxy} from './password_manager_proxy.js';
/** /**
* A version of PasswordManagerProxy.PasswordUiEntry used for deduplicating * A version of PasswordManagerProxy.PasswordUiEntry used for deduplicating
* entries from the device and the account. * entries from the device and the account.
*/ */
export class MultiStorePasswordUiEntry { export class MultiStorePasswordUiEntry extends MultiStoreIdHandler {
/** /**
* Creates a multi-store item from duplicates |entry1| and (optional) * Creates a multi-store item from duplicates |entry1| and (optional)
* |entry2|. If both arguments are passed, they should have the same contents * |entry2|. If both arguments are passed, they should have the same contents
...@@ -24,14 +25,12 @@ export class MultiStorePasswordUiEntry { ...@@ -24,14 +25,12 @@ export class MultiStorePasswordUiEntry {
* @param {PasswordManagerProxy.PasswordUiEntry=} entry2 * @param {PasswordManagerProxy.PasswordUiEntry=} entry2
*/ */
constructor(entry1, entry2) { constructor(entry1, entry2) {
super();
/** @type {!MultiStorePasswordUiEntry.Contents} */ /** @type {!MultiStorePasswordUiEntry.Contents} */
this.contents_ = MultiStorePasswordUiEntry.getContents_(entry1); this.contents_ = MultiStorePasswordUiEntry.getContents_(entry1);
/** @type {number?} */
this.deviceId_ = null;
/** @type {number?} */
this.accountId_ = null;
this.setId_(entry1.id, entry1.fromAccountStore); this.setId(entry1.id, entry1.fromAccountStore);
if (entry2) { if (entry2) {
this.merge(entry2); this.merge(entry2);
...@@ -40,7 +39,7 @@ export class MultiStorePasswordUiEntry { ...@@ -40,7 +39,7 @@ export class MultiStorePasswordUiEntry {
/** /**
* Incorporates the id of |otherEntry|, as long as |otherEntry| matches * Incorporates the id of |otherEntry|, as long as |otherEntry| matches
* |contents_| and the member id corresponding to its store is not set. * |contents_| and the id corresponding to its store is not set.
* @param {!PasswordManagerProxy.PasswordUiEntry} otherEntry * @param {!PasswordManagerProxy.PasswordUiEntry} otherEntry
*/ */
// TODO(crbug.com/1049141) Consider asserting frontendId as well. // TODO(crbug.com/1049141) Consider asserting frontendId as well.
...@@ -51,39 +50,7 @@ export class MultiStorePasswordUiEntry { ...@@ -51,39 +50,7 @@ export class MultiStorePasswordUiEntry {
assert( assert(
JSON.stringify(this.contents_) === JSON.stringify(this.contents_) ===
JSON.stringify(MultiStorePasswordUiEntry.getContents_(otherEntry))); JSON.stringify(MultiStorePasswordUiEntry.getContents_(otherEntry)));
this.setId_(otherEntry.id, otherEntry.fromAccountStore); this.setId(otherEntry.id, otherEntry.fromAccountStore);
}
/**
* Get any of the present ids. The return value is guaranteed to be non-null.
* @return {number}
*/
getAnyId() {
if (this.deviceId_ !== null) {
return this.deviceId_;
}
if (this.accountId_ !== null) {
return this.accountId_;
}
// One of the ids is guaranteed to be non-null by the constructor.
assertNotReached();
return 0;
}
/**
* Whether one of the duplicates is from the account.
* @return {boolean}
*/
isPresentInAccount() {
return this.accountId_ !== null;
}
/**
* Whether one of the duplicates is from the device.
* @return {boolean}
*/
isPresentOnDevice() {
return this.deviceId_ !== null;
} }
get urls() { get urls() {
...@@ -95,12 +62,6 @@ export class MultiStorePasswordUiEntry { ...@@ -95,12 +62,6 @@ export class MultiStorePasswordUiEntry {
get federationText() { get federationText() {
return this.contents_.federationText; return this.contents_.federationText;
} }
get deviceId() {
return this.deviceId_;
}
get accountId() {
return this.accountId_;
}
/** /**
* Extract all the information except for the id and fromPasswordStore. * Extract all the information except for the id and fromPasswordStore.
...@@ -114,25 +75,13 @@ export class MultiStorePasswordUiEntry { ...@@ -114,25 +75,13 @@ export class MultiStorePasswordUiEntry {
federationText: entry.federationText federationText: entry.federationText
}; };
} }
/**
* @param {number} id
* @param {boolean} fromAccountStore
*/
setId_(id, fromAccountStore) {
if (fromAccountStore) {
this.accountId_ = id;
} else {
this.deviceId_ = id;
}
}
} }
/** /**
* @typedef {{ * @typedef {{
* urls: !PasswordManagerProxy.UrlCollection, * urls: !PasswordManagerProxy.UrlCollection,
* username: string, * username: string,
* federationText: (string|undefined) * federationText: (string|undefined)
* }} * }}
*/ */
MultiStorePasswordUiEntry.Contents; MultiStorePasswordUiEntry.Contents;
...@@ -7,6 +7,7 @@ import './settings_ui/settings_ui.js'; ...@@ -7,6 +7,7 @@ import './settings_ui/settings_ui.js';
export {PluralStringProxyImpl as SettingsPluralStringProxyImpl} from 'chrome://resources/js/plural_string_proxy.js'; export {PluralStringProxyImpl as SettingsPluralStringProxyImpl} from 'chrome://resources/js/plural_string_proxy.js';
export {AboutPageBrowserProxy, AboutPageBrowserProxyImpl, PromoteUpdaterStatus, UpdateStatus} from './about_page/about_page_browser_proxy.m.js'; export {AboutPageBrowserProxy, AboutPageBrowserProxyImpl, PromoteUpdaterStatus, UpdateStatus} from './about_page/about_page_browser_proxy.m.js';
export {AppearanceBrowserProxy, AppearanceBrowserProxyImpl} from './appearance_page/appearance_browser_proxy.js'; export {AppearanceBrowserProxy, AppearanceBrowserProxyImpl} from './appearance_page/appearance_browser_proxy.js';
export {MultiStoreExceptionEntry} from './autofill_page/multi_store_exception_entry.js';
export {MultiStorePasswordUiEntry} from './autofill_page/multi_store_password_ui_entry.js'; export {MultiStorePasswordUiEntry} from './autofill_page/multi_store_password_ui_entry.js';
export {PasswordManagerImpl, PasswordManagerProxy} from './autofill_page/password_manager_proxy.js'; export {PasswordManagerImpl, PasswordManagerProxy} from './autofill_page/password_manager_proxy.js';
// <if expr="not chromeos"> // <if expr="not chromeos">
......
...@@ -86,14 +86,15 @@ ...@@ -86,14 +86,15 @@
file="autofill_page/blocking_request_manager.js" file="autofill_page/blocking_request_manager.js"
compress="false" type="BINDATA" /> compress="false" type="BINDATA" />
</if> </if>
<include name="IDR_SETTINGS_AUTOFILL_PAGE_SHOW_PASSWORD_BEHAVIOR_JS" <include name="IDR_SETTINGS_AUTOFILL_PAGE_MULTI_STORE_EXCEPTION_ENTRY_JS"
file="autofill_page/show_password_behavior.js" file="autofill_page/multi_store_exception_entry.js"
compress="false" type="BINDATA" compress="false" type="BINDATA" />
preprocess="true" /> <include name="IDR_SETTINGS_AUTOFILL_PAGE_MULTI_STORE_ID_HANDLER_JS"
<include name="IDR_SETTINGS_AUTOFILL_PAGE_REMOVE_PASSWORD_BEHAVIOR_JS" file="autofill_page/multi_store_id_handler.js"
file="autofill_page/remove_password_behavior.js" compress="false" type="BINDATA" />
compress="false" type="BINDATA" <include name="IDR_SETTINGS_AUTOFILL_PAGE_MULTI_STORE_PASSWORD_UI_ENTRY_JS"
preprocess="true" /> file="autofill_page/multi_store_password_ui_entry.js"
compress="false" type="BINDATA" />
<include name="IDR_SETTINGS_AUTOFILL_PAGE_PASSWORD_CHECK_JS" <include name="IDR_SETTINGS_AUTOFILL_PAGE_PASSWORD_CHECK_JS"
file="${root_gen_dir}/chrome/browser/resources/settings/autofill_page/password_check.js" file="${root_gen_dir}/chrome/browser/resources/settings/autofill_page/password_check.js"
use_base_dir="false" use_base_dir="false"
...@@ -127,9 +128,6 @@ ...@@ -127,9 +128,6 @@
<include name="IDR_SETTINGS_AUTOFILL_PAGE_PASSWORD_MANAGER_PROXY_JS" <include name="IDR_SETTINGS_AUTOFILL_PAGE_PASSWORD_MANAGER_PROXY_JS"
file="autofill_page/password_manager_proxy.js" file="autofill_page/password_manager_proxy.js"
compress="false" type="BINDATA" /> compress="false" type="BINDATA" />
<include name="IDR_SETTINGS_AUTOFILL_PAGE_MULTI_STORE_PASSWORD_UI_ENTRY_JS"
file="autofill_page/multi_store_password_ui_entry.js"
compress="false" type="BINDATA" />
<include name="IDR_SETTINGS_AUTOFILL_PAGE_PASSWORD_REMOVE_CONFIRMATION_DIALOG_JS" <include name="IDR_SETTINGS_AUTOFILL_PAGE_PASSWORD_REMOVE_CONFIRMATION_DIALOG_JS"
file="${root_gen_dir}/chrome/browser/resources/settings/autofill_page/password_remove_confirmation_dialog.js" file="${root_gen_dir}/chrome/browser/resources/settings/autofill_page/password_remove_confirmation_dialog.js"
use_base_dir="false" use_base_dir="false"
...@@ -153,6 +151,14 @@ ...@@ -153,6 +151,14 @@
file="${root_gen_dir}/chrome/browser/resources/settings/autofill_page/payments_section.js" file="${root_gen_dir}/chrome/browser/resources/settings/autofill_page/payments_section.js"
use_base_dir="false" use_base_dir="false"
compress="false" type="BINDATA" /> compress="false" type="BINDATA" />
<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_SHOW_PASSWORD_BEHAVIOR_JS"
file="autofill_page/show_password_behavior.js"
compress="false" type="BINDATA"
preprocess="true" />
<include name="IDR_SETTINGS_BASIC_PAGE_BASIC_PAGE_JS" <include name="IDR_SETTINGS_BASIC_PAGE_BASIC_PAGE_JS"
file="${root_gen_dir}/chrome/browser/resources/settings/basic_page/basic_page.js" file="${root_gen_dir}/chrome/browser/resources/settings/basic_page/basic_page.js"
use_base_dir="false" use_base_dir="false"
......
...@@ -231,6 +231,19 @@ TEST_F('CrSettingsMultiStorePasswordUiEntryV3Test', 'All', function() { ...@@ -231,6 +231,19 @@ TEST_F('CrSettingsMultiStorePasswordUiEntryV3Test', 'All', function() {
mocha.run(); mocha.run();
}); });
// eslint-disable-next-line no-var
var CrSettingsMultiStoreExceptionEntryV3Test =
class extends CrSettingsV3BrowserTest {
/** @override */
get browsePreload() {
return 'chrome://settings/test_loader.html?module=settings/multi_store_exception_entry_test.js';
}
};
TEST_F('CrSettingsMultiStoreExceptionEntryV3Test', 'All', function() {
mocha.run();
});
// eslint-disable-next-line no-var // eslint-disable-next-line no-var
var CrSettingsPasswordsCheckV3Test = class extends CrSettingsV3BrowserTest { var CrSettingsPasswordsCheckV3Test = class extends CrSettingsV3BrowserTest {
/** @override */ /** @override */
......
// 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.
/**
* @fileoverview Tests for MultiStoreExceptionEntry.
*/
import {MultiStoreExceptionEntry} from 'chrome://settings/settings.js';
import {createExceptionEntry} from 'chrome://test/settings/passwords_and_autofill_fake_data.js';
suite('MultiStoreExceptionEntry', function() {
test('verifyIds', function() {
const deviceEntry = createExceptionEntry('g.com', 0);
deviceEntry.fromAccountStore = false;
const accountEntry = createExceptionEntry('g.com', 1);
accountEntry.fromAccountStore = true;
const multiStoreDeviceEntry = new MultiStoreExceptionEntry(deviceEntry);
expectTrue(multiStoreDeviceEntry.isPresentOnDevice());
expectFalse(multiStoreDeviceEntry.isPresentInAccount());
expectEquals(multiStoreDeviceEntry.getAnyId(), 0);
const multiStoreAccountEntry = new MultiStoreExceptionEntry(accountEntry);
expectFalse(multiStoreAccountEntry.isPresentOnDevice());
expectTrue(multiStoreAccountEntry.isPresentInAccount());
expectEquals(multiStoreAccountEntry.getAnyId(), 1);
const multiStoreEntryFromBoth =
new MultiStoreExceptionEntry(deviceEntry, accountEntry);
expectTrue(multiStoreEntryFromBoth.isPresentOnDevice());
expectTrue(multiStoreEntryFromBoth.isPresentInAccount());
expectTrue(
multiStoreEntryFromBoth.getAnyId() === 0 ||
multiStoreEntryFromBoth.getAnyId() === 1);
});
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment