Skip to content

Commit

Permalink
[privacy sandbox] Fix site-list-entry displayed in lazy loaded lists
Browse files Browse the repository at this point in the history
When iron-list has more than n items and some elements are lazy loaded, computeDisplayName_ method is called with only cookiesExceptionType set
and not model. This leads to an error. cookiesExceptionType should only be set statically. Remove cookiesExceptionType from the method's dependency list.

Add a test with a long list of exceptions to verify that it works.

Bug: 1378703
Change-Id: I489db59494cf13ea5780c752e41b6619749b7616
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4111605
Reviewed-by: Theodore Olsauskas-Warren <sauski@google.com>
Reviewed-by: Rainhard Findling <rainhard@chromium.org>
Commit-Queue: Olesia Marukhno <olesiamarukhno@google.com>
Cr-Commit-Position: refs/heads/main@{#1084883}
  • Loading branch information
Olesia Marukhno authored and Chromium LUCI CQ committed Dec 19, 2022
1 parent 3d8766e commit 3343e43
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 12 deletions.
Expand Up @@ -29,8 +29,7 @@

<!-- This div must not contain extra whitespace. -->
<div class="secondary"
id="siteDescription">[[computeSiteDescription_(model,
cookiesExceptionType)]]</div>
id="siteDescription">[[computeSiteDescription_(model)]]</div>
</div>
<template is="dom-if" if="[[allowNavigateToSiteDetail_]]">
<cr-icon-button class="subpage-arrow"
Expand Down
9 changes: 6 additions & 3 deletions chrome/test/data/webui/settings/site_list_entry_tests.ts
Expand Up @@ -164,6 +164,7 @@ suite('SiteListEntry', function() {
// Verify that third-party exceptions in a combined list have an additional
// description.
test('third-party exception in a combined exceptions list', function() {
testElement.cookiesExceptionType = CookiesExceptionType.COMBINED;
testElement.model = {
category: ContentSettingsTypes.COOKIES,
controlledBy: chrome.settingsPrivate.ControlledBy.OWNER,
Expand All @@ -175,7 +176,6 @@ suite('SiteListEntry', function() {
origin: SITE_EXCEPTION_WILDCARD,
setting: ContentSetting.DEFAULT,
};
testElement.cookiesExceptionType = CookiesExceptionType.COMBINED;
flush();
const siteDescription = testElement.$$('#siteDescription')!;
assertEquals(
Expand All @@ -186,6 +186,7 @@ suite('SiteListEntry', function() {
// Verify that third-party exceptions in a third-party exceptions list don't
// have an additional description.
test('third-party exception in a third-party exceptions list', function() {
testElement.cookiesExceptionType = CookiesExceptionType.THIRD_PARTY;
testElement.model = {
category: ContentSettingsTypes.COOKIES,
controlledBy: chrome.settingsPrivate.ControlledBy.OWNER,
Expand All @@ -197,7 +198,6 @@ suite('SiteListEntry', function() {
origin: SITE_EXCEPTION_WILDCARD,
setting: ContentSetting.DEFAULT,
};
testElement.cookiesExceptionType = CookiesExceptionType.THIRD_PARTY;
flush();
const siteDescription = testElement.$$('#siteDescription')!;
assertEquals('', siteDescription.textContent);
Expand All @@ -206,6 +206,7 @@ suite('SiteListEntry', function() {
// Verify that exceptions with both patterns have proper description for both
// lists.
test('cookies exception with both patterns set', function() {
testElement.cookiesExceptionType = CookiesExceptionType.COMBINED;
testElement.model = {
category: ContentSettingsTypes.COOKIES,
controlledBy: chrome.settingsPrivate.ControlledBy.OWNER,
Expand All @@ -217,14 +218,16 @@ suite('SiteListEntry', function() {
origin: 'http://example2.com',
setting: ContentSetting.DEFAULT,
};
testElement.cookiesExceptionType = CookiesExceptionType.COMBINED;
flush();
const siteDescription = testElement.$$('#siteDescription')!;
assertEquals(
loadTimeData.getStringF('embeddedOnHost', 'http://example1.com'),
siteDescription.textContent);

// `cookiesExceptionType` is static, the element is only observing changes
// to the model.
testElement.cookiesExceptionType = CookiesExceptionType.THIRD_PARTY;
testElement.model = {...testElement.model};
flush();
assertEquals(
loadTimeData.getStringF('embeddedOnHost', 'http://example1.com'),
Expand Down
24 changes: 17 additions & 7 deletions chrome/test/data/webui/settings/site_list_tests.ts
Expand Up @@ -306,6 +306,12 @@ function populateTestExceptions() {
createRawSiteException('http://foo-allow.com', {
embeddingOrigin: '',
}),
createRawSiteException('http://bar-allow.com', {
embeddingOrigin: '',
}),
createRawSiteException('http://baz-allow.com', {
embeddingOrigin: '',
}),
createRawSiteException(SITE_EXCEPTION_WILDCARD, {
embeddingOrigin: 'http://3pc-block.com',
setting: ContentSetting.BLOCK,
Expand Down Expand Up @@ -456,12 +462,14 @@ suite('SiteListCookiesExceptionTypes', function() {
ContentSettingsTypes.COOKIES, ContentSetting.ALLOW,
prefsMixedCookiesExceptionTypes);
return browserProxy.whenCalled('getExceptionList').then(() => {
assertEquals(2, testElement.sites.length);
assertEquals(4, testElement.sites.length);
assertEquals(testElement.sites[0]!.origin, 'http://foo-allow.com');
assertEquals(testElement.sites[1]!.origin, 'http://bar-allow.com');
assertEquals(testElement.sites[2]!.origin, 'http://baz-allow.com');
assertEquals(
testElement.sites[1]!.origin, 'http://mixed-primary-allow.com');
testElement.sites[3]!.origin, 'http://mixed-primary-allow.com');
assertEquals(
testElement.sites[1]!.embeddingOrigin,
testElement.sites[3]!.embeddingOrigin,
'http://mixed-secondary-allow.com');
});
});
Expand All @@ -472,14 +480,16 @@ suite('SiteListCookiesExceptionTypes', function() {
ContentSettingsTypes.COOKIES, ContentSetting.ALLOW,
prefsMixedCookiesExceptionTypes);
return browserProxy.whenCalled('getExceptionList').then(() => {
assertEquals(3, testElement.sites.length);
assertEquals(5, testElement.sites.length);
assertEquals(testElement.sites[0]!.origin, 'http://foo-allow.com');
assertEquals(testElement.sites[1]!.origin, 'http://bar-allow.com');
assertEquals(testElement.sites[2]!.origin, 'http://baz-allow.com');
assertEquals(
testElement.sites[1]!.embeddingOrigin, 'http://3pc-allow.com');
testElement.sites[3]!.embeddingOrigin, 'http://3pc-allow.com');
assertEquals(
testElement.sites[2]!.origin, 'http://mixed-primary-allow.com');
testElement.sites[4]!.origin, 'http://mixed-primary-allow.com');
assertEquals(
testElement.sites[2]!.embeddingOrigin,
testElement.sites[4]!.embeddingOrigin,
'http://mixed-secondary-allow.com');
});
});
Expand Down

0 comments on commit 3343e43

Please sign in to comment.