M108 merge: [SafetyCheck] Make notification module header responsive

This CL updates the header of notification permissions module and makes
it responsive to length of the list.

Screenshot: https://bugs.chromium.org/p/chromium/issues/detail?id=1345920#c42

Besides that this CL fixes the calculation bug for Sunday in notification_permission_review_service.cc.

(cherry picked from commit 00f834ea91f55625f444254af8cc8e86a290ce1b)

Bug: 1345920
Change-Id: If63d0560117ad746d2378e4b37ed23db8457de69
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3936630
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Side YILMAZ <sideyilmaz@chromium.org>
Auto-Submit: Side YILMAZ <sideyilmaz@chromium.org>
Reviewed-by: Rainhard Findling <rainhard@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1059213}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3955760
Cr-Commit-Position: refs/branch-heads/5359@{#49}
Cr-Branched-From: 27d3765-refs/heads/main@{#1058933}

chrome/browser/resources/settings/privacy_page/privacy_page.html

@@ -502,7 +502,8 @@ <h2>$i18n{siteSettingsDefaultBehavior}</h2>
             search-label="$i18n{siteSettingsAllSitesSearch}"
             search-term="{{searchFilter_}}">
           <div id="notificationRadioGroup" class="radio-group">
-            <template is="dom-if" if="[[!showNotificationPermissionsReview_]]">
+            <template is="dom-if"
+                if="[[!safetyCheckNotificationPermissionsEnabled_]]">
               <div class="secondary">
                 $i18n{siteSettingsNotificationsDescription}
               </div>

chrome/browser/resources/settings/site_settings/review_notification_permissions.html

@@ -65,7 +65,7 @@
     <iron-icon role="img" icon="settings:notifications-none"></iron-icon>
     <cr-expand-button id="expandButton" class="header-group-wrapper" no-hover
         expanded="{{notificationPermissionReviewListExpanded_}}">
-      <h2>$i18n{safetyCheckNotificationPermissionReviewPrimaryLabel}</h2>
+      <h2>[[headerString_]]</h2>
       <div class="secondary">
         $i18n{safetyCheckNotificationPermissionReviewSecondaryLabel}
       </div>
@@ -74,7 +74,7 @@ <h2>$i18n{safetyCheckNotificationPermissionReviewPrimaryLabel}</h2>
   <iron-collapse class="notification-permissions-list"
       opened="[[notificationPermissionReviewListExpanded_]]">
     <template id="notificationPermissionReviewList" is="dom-repeat"
-        items="[[notificationPermissionReviewList_]]">
+        items="[[sites_]]">
       <div class$="cr-row [[getClassForIndex_(index)]]">
         <site-favicon url="[[item.origin]]"></site-favicon>
         <div id="displayName_[[index]]" class="display-name cr-padded-text">

chrome/browser/resources/settings/site_settings/review_notification_permissions.ts

@@ -13,6 +13,7 @@ import {CrToastElement} from 'chrome://resources/cr_elements/cr_toast/cr_toast.j
 import {I18nMixin} from 'chrome://resources/cr_elements/i18n_mixin.js';
 import {WebUIListenerMixin} from 'chrome://resources/cr_elements/web_ui_listener_mixin.js';
 import {assert, assertNotReached} from 'chrome://resources/js/assert_ts.js';
+import {PluralStringProxyImpl} from 'chrome://resources/js/plural_string_proxy.js';
 import {DomRepeatEvent, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
 import {BaseMixin} from '../base_mixin.js';
@@ -54,9 +55,10 @@ export class SettingsReviewNotificationPermissionsElement extends
   static get properties() {
     return {
       /* List of domains that sends a lot of notifications. */
-      notificationPermissionReviewList_: {
+      sites_: {
         type: Array,
-        value: null,
+        value: [],
+        observer: 'onSitesChanged_',
       },
 
       /* If the list of notification permissions is expanded or collapsed. */
@@ -80,29 +82,34 @@ export class SettingsReviewNotificationPermissionsElement extends
        */
       shouldShowCompletionInfo_: {
         type: Boolean,
-        computed: 'computeShouldShowCompletionInfo_' +
-            '(notificationPermissionReviewList_.*)',
+        computed: 'computeShouldShowCompletionInfo_(sites_.*)',
       },
+
+      /* The string for the primary header label. */
+      headerString_: String,
     };
   }
 
-  private notificationPermissionReviewList_: NotificationPermission[]|null;
+  private sites_: NotificationPermission[];
   private notificationPermissionReviewListExpanded_: boolean;
   private shouldShowCompletionInfo_: boolean;
   private browserProxy_: SiteSettingsPrefsBrowserProxy =
       SiteSettingsPrefsBrowserProxyImpl.getInstance();
   private lastOrigin_: string;
   private lastUserAction_: Actions|null;
+  private headerString_: string;
+  private sitesLoaded_: boolean = false;
 
-  override connectedCallback() {
+  override async connectedCallback() {
     super.connectedCallback();
     // Register for review notification permission list updates.
     this.addWebUIListener(
         'notification-permission-review-list-changed',
         (sites: NotificationPermission[]) =>
             this.onReviewNotificationPermissionListChanged_(sites));
 
-    this.populateList_();
+    this.sites_ = await this.browserProxy_.getNotificationPermissionReview();
+    this.sitesLoaded_ = true;
   }
 
   /**
@@ -159,7 +166,7 @@ export class SettingsReviewNotificationPermissionsElement extends
   /* Repopulate the list when notification permission list is updated. */
   private onReviewNotificationPermissionListChanged_(
       sites: NotificationPermission[]) {
-    this.notificationPermissionReviewList_ = sites;
+    this.sites_ = sites;
   }
 
   private onShowTooltip_(e: Event) {
@@ -261,9 +268,11 @@ export class SettingsReviewNotificationPermissionsElement extends
    * Retrieve the list of domains that send lots of notification and implicitly
    * trigger the update of the display list.
    */
-  private async populateList_() {
-    this.notificationPermissionReviewList_ =
-        await this.browserProxy_.getNotificationPermissionReview();
+  private async onSitesChanged_() {
+    this.headerString_ =
+        await PluralStringProxyImpl.getInstance().getPluralString(
+            'safetyCheckNotificationPermissionReviewPrimaryLabel',
+            this.sites_!.length);
   }
 
   private getMoreActionsAriaLabelText_(): string {
@@ -277,8 +286,7 @@ export class SettingsReviewNotificationPermissionsElement extends
 
   /** Show info that review is completed when there are no permissions left. */
   private computeShouldShowCompletionInfo_(): boolean {
-    return !!this.notificationPermissionReviewList_ &&
-        this.notificationPermissionReviewList_.length === 0;
+    return this.sitesLoaded_ && this.sites_.length === 0;
   }
 }
 

chrome/browser/ui/webui/settings/settings_localized_strings_provider.cc

@@ -1977,8 +1977,6 @@ void AddSafetyCheckStrings(content::WebUIDataSource* html_source) {
      IDS_SETTINGS_SAFETY_CHECK_EXTENSIONS_PRIMARY_LABEL},
     {"safetyCheckExtensionsButtonAriaLabel",
      IDS_SETTINGS_SAFETY_CHECK_EXTENSIONS_BUTTON_ARIA_LABEL},
-    {"safetyCheckNotificationPermissionReviewPrimaryLabel",
-     IDS_SETTINGS_SAFETY_CHECK_REVIEW_NOTIFICATION_PERMISSIONS_PRIMARY_LABEL},
     {"safetyCheckNotificationPermissionReviewSecondaryLabel",
      IDS_SETTINGS_SAFETY_CHECK_REVIEW_NOTIFICATION_PERMISSIONS_SECONDARY_LABEL},
     {"safetyCheckNotificationPermissionReviewIgnoredToastLabel",

chrome/browser/ui/webui/settings/settings_ui.cc

@@ -397,6 +397,9 @@ SettingsUI::SettingsUI(content::WebUI* web_ui)
   plural_string_handler->AddLocalizedString(
       "safetyCheckNotificationPermissionReviewBlockAllToastLabel",
       IDS_SETTINGS_SAFETY_CHECK_NOTIFICATION_PERMISSION_REVIEW_BLOCK_ALL_TOAST_LABEL);
+  plural_string_handler->AddLocalizedString(
+      "safetyCheckNotificationPermissionReviewPrimaryLabel",
+      IDS_SETTINGS_SAFETY_CHECK_REVIEW_NOTIFICATION_PERMISSIONS_PRIMARY_LABEL);
   web_ui->AddMessageHandler(std::move(plural_string_handler));
 
   // Add the metrics handler to write uma stats.

chrome/test/data/webui/settings/review_notification_permissions_test.ts

@@ -4,12 +4,12 @@
 
 // clang-format off
 import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
+import {flushTasks} from 'chrome://webui-test/polymer_test_util.js';
 import {assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js';
 import {SettingsReviewNotificationPermissionsElement, SiteSettingsPrefsBrowserProxyImpl} from 'chrome://settings/lazy_load.js';
 import {CrActionMenuElement} from 'chrome://settings/settings.js';
-import {webUIListenerCallback} from 'chrome://resources/js/cr.m.js';
-import {flushTasks} from 'chrome://webui-test/polymer_test_util.js';
 import {isChildVisible, isVisible} from 'chrome://webui-test/test_util.js';
+import {webUIListenerCallback} from 'chrome://resources/js/cr.m.js';
 
 import {TestSiteSettingsPrefsBrowserProxy} from './test_site_settings_prefs_browser_proxy.js';
 
@@ -23,19 +23,19 @@ suite('CrSettingsReviewNotificationPermissionsTest', function() {
 
   let testElement: SettingsReviewNotificationPermissionsElement;
 
-  const origin_1 = 'www.example1.com';
-  const detail_1 = 'About 4 notifications a day';
-  const origin_2 = 'www.example2.com';
-  const detail_2 = 'About 1 notification a day';
+  const origin1 = 'www.example1.com';
+  const detail1 = 'About 4 notifications a day';
+  const origin2 = 'www.example2.com';
+  const detail2 = 'About 1 notification a day';
 
-  const mock_data = [
+  const mockData = [
     {
-      origin: origin_1,
-      notificationInfoString: detail_1,
+      origin: origin1,
+      notificationInfoString: detail1,
     },
     {
-      origin: origin_2,
-      notificationInfoString: detail_2,
+      origin: origin2,
+      notificationInfoString: detail2,
     },
   ];
 
@@ -69,7 +69,7 @@ suite('CrSettingsReviewNotificationPermissionsTest', function() {
 
   setup(function() {
     browserProxy = new TestSiteSettingsPrefsBrowserProxy();
-    browserProxy.setNotificationPermissionReview(mock_data);
+    browserProxy.setNotificationPermissionReview(mockData);
     SiteSettingsPrefsBrowserProxyImpl.setInstance(browserProxy);
 
     document.body.innerHTML =
@@ -110,16 +110,16 @@ suite('CrSettingsReviewNotificationPermissionsTest', function() {
 
     // Check that the text describing the changed permissions is correct.
     assertEquals(
-        origin_1,
+        origin1,
         entries[0]!.querySelector('.site-representation')!.textContent!.trim());
     assertEquals(
-        detail_1,
+        detail1,
         entries[0]!.querySelector('.second-line')!.textContent!.trim());
     assertEquals(
-        origin_2,
+        origin2,
         entries[1]!.querySelector('.site-representation')!.textContent!.trim());
     assertEquals(
-        detail_2,
+        detail2,
         entries[1]!.querySelector('.second-line')!.textContent!.trim());
   });
 
@@ -298,7 +298,7 @@ suite('CrSettingsReviewNotificationPermissionsTest', function() {
     // The element returns to showing the list of permissions when new items are
     // added while the completion state is visible.
     webUIListenerCallback(
-        'notification-permission-review-list-changed', mock_data);
+        'notification-permission-review-list-changed', mockData);
     await flushTasks();
     assertTrue(isChildVisible(testElement, '#review-header'));
     assertTrue(isChildVisible(testElement, '.notification-permissions-list'));
@@ -334,4 +334,39 @@ suite('CrSettingsReviewNotificationPermissionsTest', function() {
     assertTrue(expandButton.expanded);
     assertTrue(notificationPermissionList.opened);
   });
+
+  /**
+   * Tests whether header string updated based on the notification permission
+   * list size for plural and singular case.
+   */
+  test('Header String', async function() {
+    await browserProxy.whenCalled('getNotificationPermissionReview');
+    await flushTasks();
+
+    // Check header string for plural case.
+    let entries = testElement.shadowRoot!.querySelectorAll('.cr-row');
+    assertEquals(2, entries.length);
+
+    const headerElement =
+        testElement.shadowRoot!.querySelector('#expandButton h2');
+    assertTrue(headerElement !== null);
+    assertEquals(
+        'Review 2 sites that recently sent a lot of notifications',
+        headerElement.textContent!.trim());
+
+    // Check header string for singular case.
+    await webUIListenerCallback(
+        'notification-permission-review-list-changed', [{
+          origin: origin1,
+          notificationInfoString: detail1,
+        }]);
+    await flushTasks();
+
+    entries = testElement.shadowRoot!.querySelectorAll('.cr-row');
+    assertEquals(1, entries.length);
+
+    assertEquals(
+        'Review 1 site that recently sent a lot of notifications',
+        headerElement.textContent!.trim());
+  });
 });