From 3343e43c3e7cabafa2d426b9c03b4e49cbe8eff8 Mon Sep 17 00:00:00 2001
From: Olesia Marukhno <olesiamarukhno@google.com>
Date: Mon, 19 Dec 2022 10:39:44 +0000
Subject: [PATCH] [privacy sandbox] Fix site-list-entry displayed in lazy
 loaded lists

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}
---
 .../site_settings/site_list_entry.html        |  3 +--
 .../webui/settings/site_list_entry_tests.ts   |  9 ++++---
 .../data/webui/settings/site_list_tests.ts    | 24 +++++++++++++------
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/chrome/browser/resources/settings/site_settings/site_list_entry.html b/chrome/browser/resources/settings/site_settings/site_list_entry.html
index 5b1d7eee0360e9..18cb0a40f18684 100644
--- a/chrome/browser/resources/settings/site_settings/site_list_entry.html
+++ b/chrome/browser/resources/settings/site_settings/site_list_entry.html
@@ -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"
diff --git a/chrome/test/data/webui/settings/site_list_entry_tests.ts b/chrome/test/data/webui/settings/site_list_entry_tests.ts
index 363747563f5c3b..e6399ff2a7f1ca 100644
--- a/chrome/test/data/webui/settings/site_list_entry_tests.ts
+++ b/chrome/test/data/webui/settings/site_list_entry_tests.ts
@@ -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,
@@ -175,7 +176,6 @@ suite('SiteListEntry', function() {
       origin: SITE_EXCEPTION_WILDCARD,
       setting: ContentSetting.DEFAULT,
     };
-    testElement.cookiesExceptionType = CookiesExceptionType.COMBINED;
     flush();
     const siteDescription = testElement.$$('#siteDescription')!;
     assertEquals(
@@ -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,
@@ -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);
@@ -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,
@@ -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'),
diff --git a/chrome/test/data/webui/settings/site_list_tests.ts b/chrome/test/data/webui/settings/site_list_tests.ts
index 1964645943e5e5..4917feee17d396 100644
--- a/chrome/test/data/webui/settings/site_list_tests.ts
+++ b/chrome/test/data/webui/settings/site_list_tests.ts
@@ -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,
@@ -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');
     });
   });
@@ -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');
     });
   });