Settings: Show the number of cookies set on grouped entries in All Sites.

Show the number of cookies on a domain / eTLD+1 group of sites in All Sites.
Since cookies at their widest scope can be set on eTLD+1s, this display will
be restricted to sites that are grouped.

Bug: 835712
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I66d2c36b2af1ad577d4d71512f3d22541e1c926f
Reviewed-on: https://chromium-review.googlesource.com/1111770Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Patti <patricialor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575098}

chrome/app/settings_strings.grdp

@@ -3114,11 +3114,10 @@
     Manifest
   </message>
 
-  <message name="IDS_SETTINGS_COOKIES_SINGLE_COOKIE" desc="The text displayed when there is one cookie in the Cookies table">
-    1 cookie
-  </message>
-  <message name="IDS_SETTINGS_COOKIES_PLURAL_COOKIES" desc="The text displayed when there are two or more cookies in the Cookies table">
-    <ph name="COOKIES">$1<ex>42</ex></ph> cookies
+  <message name="IDS_SETTINGS_SITE_SETTINGS_NUM_COOKIES" desc="A label showing the number of cookies set on a site.">
+    {NUM_COOKIES, plural,
+        =1 {1 cookie}
+        other {# cookies}}
   </message>
   <message name="IDS_SETTINGS_COOKIES_WEB_DATABASE_DESCRIPTION_LABEL" desc="The Database Description label">
     Description

chrome/browser/resources/settings/site_settings/BUILD.gn

@@ -174,7 +174,9 @@ js_library("site_details_permission") {
 js_library("site_entry") {
   deps = [
     ":constants",
+    ":local_data_browser_proxy",
     ":site_settings_behavior",
+    ":site_settings_prefs_browser_proxy",
     "..:route",
     "//third_party/polymer/v1_0/components-chromium/iron-collapse:iron-collapse-extracted",
     "//ui/webui/resources/cr_elements/cr_action_menu:cr_action_menu",

chrome/browser/resources/settings/site_settings/all_sites.js

@@ -46,6 +46,8 @@ Polymer({
         settings.SiteSettingsPrefsBrowserProxyImpl.getInstance();
     this.addWebUIListener(
         'contentSettingSitePermissionChanged', this.populateList_.bind(this));
+    this.addEventListener(
+        'site-entry-resized', this.resizeListIfScrollTargetActive_.bind(this));
     this.populateList_();
   },
 
@@ -74,4 +76,14 @@ Polymer({
       this.siteGroupList = response;
     });
   },
+
+  /**
+   * Called when a list item changes its size, and thus the positions and sizes
+   * of the items in the entire list also need updating.
+   * @private
+   */
+  resizeListIfScrollTargetActive_: function() {
+    if (settings.getCurrentRoute() == this.subpageRoute)
+      this.$.allSitesList.fire('iron-resize');
+  },
 });

chrome/browser/resources/settings/site_settings/local_data_browser_proxy.js

@@ -68,6 +68,13 @@ cr.define('settings', function() {
      */
     getCookieDetails(site) {}
 
+    /**
+     * Gets the number of cookies formatted in a plural string, given a site.
+     * @param {string} site The site to count cookies for.
+     * @return {!Promise<string>}
+     */
+    getNumCookiesString(site) {}
+
     /**
      * Reloads all local data.
      * TODO(dschuyler): rename function to reload().
@@ -112,6 +119,11 @@ cr.define('settings', function() {
       return cr.sendWithPromise('localData.getCookieDetails', site);
     }
 
+    /** @override */
+    getNumCookiesString(site) {
+      return cr.sendWithPromise('localData.getNumCookiesString', site);
+    }
+
     /** @override */
     reloadCookies() {
       return cr.sendWithPromise('localData.reload');

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

@@ -6,6 +6,7 @@
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-icon-button/paper-icon-button-light.html">
 <link rel="import" href="../route.html">
 <link rel="import" href="../settings_shared_css.html">
+<link rel="import" href="local_data_browser_proxy.html">
 <link rel="import" href="site_settings_behavior.html">
 
 <dom-module id="site-entry">
@@ -24,22 +25,31 @@
       .site-representation {
         display: flex;
       }
+
+      #cookies {
+        margin-top: 0.1em;
+      }
     </style>
     <div id="collapseParent">
       <div class="settings-box list-item">
-        <div id="toggleButton" class="start row-aligned"
+        <div id="toggleButton" class="start row-aligned two-line"
             on-click="onSiteEntryTap_" actionable aria-expanded="false">
           <div class="favicon-image"
               style$="[[getSiteGroupIcon_(siteGroup)]]">
           </div>
-          <div class="site-representation middle text-elide" id="displayName">
-            <span class="url-directionality">[[displayName_]]</span>
-            <span class="secondary" hidden$="[[!scheme_(siteGroup, -1)]]">
-              &nbsp;$i18nPolymer{siteSettingsSiteRepresentationSeparator}&nbsp;
-            </span>
-            <span class="secondary" hidden$="[[!scheme_(siteGroup, -1)]]">
-              [[scheme_(siteGroup, -1)]]
-            </span>
+          <div class="middle text-elide" id="displayName">
+            <div class="site-representation">
+              <span class="url-directionality">[[displayName_]]</span>
+              <span class="secondary" hidden$="[[!scheme_(siteGroup, -1)]]">
+                &nbsp;$i18nPolymer{siteSettingsSiteRepresentationSeparator}&nbsp;
+              </span>
+              <span class="secondary" hidden$="[[!scheme_(siteGroup, -1)]]">
+                [[scheme_(siteGroup, -1)]]
+              </span>
+            </div>
+            <div id="cookies" class="secondary" hidden$="[[!cookieString_]]">
+              [[cookieString_]]
+            </div>
           </div>
           <paper-icon-button-light id="expandIcon" class="icon-expand-more"
               hidden$="[[!grouped_(siteGroup)]]">

chrome/browser/resources/settings/site_settings/site_entry.js

@@ -6,7 +6,6 @@
  * @fileoverview
  * 'site-entry' is an element representing a single eTLD+1 site entity.
  */
-
 Polymer({
   is: 'site-entry',
 
@@ -28,6 +27,24 @@ Polymer({
      * @private
      */
     displayName_: String,
+
+    /**
+     * The string to display when there is a non-zero number of cookies.
+     * @private
+     */
+    cookieString_: {
+      type: String,
+      value: '',
+    },
+  },
+
+  /** @private {?settings.LocalDataBrowserProxy} */
+  localDataBrowserProxy_: null,
+
+  /** @override */
+  created: function() {
+    this.localDataBrowserProxy_ =
+        settings.LocalDataBrowserProxyImpl.getInstance();
   },
 
   /**
@@ -75,9 +92,28 @@ Polymer({
   onSiteGroupChanged_: function(siteGroup) {
     this.displayName_ = this.siteRepresentation_(siteGroup, -1);
 
-    // Ensure ungrouped |siteGroup|s do not get stuck in an opened state.
-    if (!this.grouped_(SiteGroup) && this.$.collapseChild.opened)
-      this.toggleCollapsible_();
+    if (!this.grouped_(SiteGroup)) {
+      // Ensure ungrouped |siteGroup|s do not get stuck in an opened state.
+      if (this.$.collapseChild.opened)
+        this.toggleCollapsible_();
+      // Ungrouped site-entries should not show cookies.
+      if (this.cookieString_) {
+        this.cookieString_ = '';
+        this.fire('site-entry-resized');
+      }
+    }
+    if (!siteGroup || !this.grouped_(siteGroup))
+      return;
+
+    this.localDataBrowserProxy_.getNumCookiesString(this.displayName_)
+        .then(string => {
+          // If there was no cookie string previously and now there is, or vice
+          // versa, the height of this site-entry will have changed.
+          if ((this.cookieString_ == '') != (string == ''))
+            this.fire('site-entry-resized');
+
+          this.cookieString_ = string;
+        });
   },
 
   /**

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

@@ -2127,7 +2127,6 @@ void AddSiteSettingsStrings(content::WebUIDataSource* html_source,
     {"cookieFlashLso", IDS_SETTINGS_COOKIES_FLASH_LSO},
     {"cookieLocalStorage", IDS_SETTINGS_COOKIES_LOCAL_STORAGE},
     {"cookieMediaLicense", IDS_SETTINGS_COOKIES_MEDIA_LICENSE},
-    {"cookiePlural", IDS_SETTINGS_COOKIES_PLURAL_COOKIES},
     {"cookieServiceWorker", IDS_SETTINGS_COOKIES_SERVICE_WORKER},
     {"cookieSharedWorker", IDS_SETTINGS_COOKIES_SHARED_WORKER},
     {"embeddedOnAnyHost", IDS_SETTINGS_EXCEPTIONS_EMBEDDED_ON_ANY_HOST},

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

@@ -52,11 +52,6 @@ int GetCategoryLabelID(CookieTreeNode::DetailedInfo::NodeType node_type) {
   } kCategoryLabels[] = {
       // Multiple keys (node_type) may have the same value (id).
 
-      {CookieTreeNode::DetailedInfo::TYPE_COOKIES,
-       IDS_SETTINGS_COOKIES_SINGLE_COOKIE},
-      {CookieTreeNode::DetailedInfo::TYPE_COOKIE,
-       IDS_SETTINGS_COOKIES_SINGLE_COOKIE},
-
       {CookieTreeNode::DetailedInfo::TYPE_DATABASES,
        IDS_SETTINGS_COOKIES_DATABASE_STORAGE},
       {CookieTreeNode::DetailedInfo::TYPE_DATABASE,
@@ -178,6 +173,10 @@ void CookiesViewHandler::RegisterMessages() {
       "localData.getCookieDetails",
       base::BindRepeating(&CookiesViewHandler::HandleGetCookieDetails,
                           base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      "localData.getNumCookiesString",
+      base::BindRepeating(&CookiesViewHandler::HandleGetNumCookiesString,
+                          base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
       "localData.removeCookie",
       base::BindRepeating(&CookiesViewHandler::HandleRemove,
@@ -310,6 +309,61 @@ void CookiesViewHandler::HandleGetCookieDetails(const base::ListValue* args) {
   SendCookieDetails(node);
 }
 
+void CookiesViewHandler::HandleGetNumCookiesString(
+    const base::ListValue* args) {
+  CHECK_EQ(2U, args->GetSize());
+  std::string callback_id;
+  CHECK(args->GetString(0, &callback_id));
+  base::string16 etld_plus1;
+  CHECK(args->GetString(1, &etld_plus1));
+
+  AllowJavascript();
+  CHECK(cookies_tree_model_.get());
+  // This method is only interested in the number of cookies, so don't save the
+  // filter and keep the existing |sorted_sites_| list.
+  if (etld_plus1 != filter_)
+    cookies_tree_model_->UpdateSearchResults(etld_plus1);
+
+  int num_cookies = 0;
+  const CookieTreeNode* root = cookies_tree_model_->GetRoot();
+  for (int i = 0; i < root->child_count(); ++i) {
+    const CookieTreeNode* site = root->GetChild(i);
+    const base::string16& title = site->GetTitle();
+    if (!base::EndsWith(title, etld_plus1,
+                        base::CompareCase::INSENSITIVE_ASCII)) {
+      continue;
+    }
+
+    for (int j = 0; j < site->child_count(); ++j) {
+      const CookieTreeNode* category = site->GetChild(j);
+      if (category->GetDetailedInfo().node_type !=
+          CookieTreeNode::DetailedInfo::TYPE_COOKIES) {
+        continue;
+      }
+
+      for (int k = 0; k < category->child_count(); ++k) {
+        if (category->GetChild(k)->GetDetailedInfo().node_type !=
+            CookieTreeNode::DetailedInfo::TYPE_COOKIE) {
+          continue;
+        }
+
+        ++num_cookies;
+      }
+    }
+  }
+
+  if (etld_plus1 != filter_) {
+    // Restore the original |filter_|.
+    cookies_tree_model_->UpdateSearchResults(filter_);
+  }
+  const base::string16 string =
+      num_cookies > 0 ? l10n_util::GetPluralStringFUTF16(
+                            IDS_SETTINGS_SITE_SETTINGS_NUM_COOKIES, num_cookies)
+                      : base::string16();
+
+  ResolveJavascriptCallback(base::Value(callback_id), base::Value(string));
+}
+
 void CookiesViewHandler::HandleGetDisplayList(const base::ListValue* args) {
   CHECK(request_.callback_id_.empty());
   CHECK_EQ(2U, args->GetSize());
@@ -417,32 +471,35 @@ void CookiesViewHandler::SendLocalDataList(const CookieTreeNode* parent) {
   std::unique_ptr<base::ListValue> site_list(new base::ListValue);
   for (int i = 0; i < list_item_count; ++i) {
     const CookieTreeNode* site = parent->GetChild(sorted_sites_[i].second);
-    std::string description;
+    base::string16 description;
     for (int k = 0; k < site->child_count(); ++k) {
-      const CookieTreeNode* category = site->GetChild(k);
-      const auto node_type = category->GetDetailedInfo().node_type;
-      if (node_type == CookieTreeNode::DetailedInfo::TYPE_QUOTA) {
-        // TODO(crbug.com/642955): Omit quota values until bug is addressed.
-        continue;
-      }
-      int ids_value = GetCategoryLabelID(node_type);
-      if (!ids_value) {
-        // If we don't have a label to call it by, don't show it. Please add
-        // a label ID if an expected category is not appearing in the UI.
-        continue;
-      }
       if (description.size()) {
-        description += ", ";
+        description += base::ASCIIToUTF16(", ");
       }
+      const CookieTreeNode* category = site->GetChild(k);
+      const auto node_type = category->GetDetailedInfo().node_type;
       int item_count = category->child_count();
-      if (category->GetDetailedInfo().node_type ==
-              CookieTreeNode::DetailedInfo::TYPE_COOKIES &&
-          item_count > 1) {
-        description +=
-            l10n_util::GetStringFUTF8(IDS_SETTINGS_COOKIES_PLURAL_COOKIES,
-                                      base::FormatNumber(item_count));
-      } else {
-        description += l10n_util::GetStringUTF8(ids_value);
+      switch (node_type) {
+        case CookieTreeNode::DetailedInfo::TYPE_QUOTA:
+          // TODO(crbug.com/642955): Omit quota values until bug is addressed.
+          continue;
+        case CookieTreeNode::DetailedInfo::TYPE_COOKIE:
+          DCHECK_EQ(0, item_count);
+          item_count = 1;
+          FALLTHROUGH;
+        case CookieTreeNode::DetailedInfo::TYPE_COOKIES:
+          description += l10n_util::GetPluralStringFUTF16(
+              IDS_SETTINGS_SITE_SETTINGS_NUM_COOKIES, item_count);
+          break;
+        default:
+          int ids_value = GetCategoryLabelID(node_type);
+          if (!ids_value) {
+            // If we don't have a label to call it by, don't show it. Please add
+            // a label ID if an expected category is not appearing in the UI.
+            continue;
+          }
+          description += l10n_util::GetStringUTF16(ids_value);
+          break;
       }
     }
     std::unique_ptr<base::DictionaryValue> list_info(new base::DictionaryValue);

chrome/browser/ui/webui/settings/settings_cookies_view_handler.h

@@ -61,6 +61,9 @@ class CookiesViewHandler : public SettingsPageUIHandler,
   // Retrieve cookie details for a specific site.
   void HandleGetCookieDetails(const base::ListValue* args);
 
+  // Gets the number of cookies formatted in a plural string, given a site.
+  void HandleGetNumCookiesString(const base::ListValue* args);
+
   // Remove all sites data.
   void HandleRemoveAll(const base::ListValue* args);
 

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

@@ -975,6 +975,7 @@ CrSettingsSiteEntryTest.prototype = {
   /** @override */
   extraLibraries: CrSettingsBrowserTest.prototype.extraLibraries.concat([
     '../test_browser_proxy.js',
+    'test_local_data_browser_proxy.js',
     'test_util.js',
     'test_site_settings_prefs_browser_proxy.js',
     'site_entry_tests.js',

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

@@ -5,7 +5,7 @@
 suite('SiteEntry', function() {
   /**
    * An example eTLD+1 Object with multiple origins grouped under it.
-   * @type {SiteGroup}
+   * @type {!SiteGroup}
    */
   const TEST_MULTIPLE_SITE_GROUP = test_util.createSiteGroup('example.com', [
     'http://example.com',
@@ -15,18 +15,34 @@ suite('SiteEntry', function() {
 
   /**
    * An example eTLD+1 Object with a single origin in it.
-   * @type {SiteGroup}
+   * @type {!SiteGroup}
    */
   const TEST_SINGLE_SITE_GROUP = test_util.createSiteGroup('foo.com', [
     'https://login.foo.com',
   ]);
 
+  const TEST_COOKIE_LIST = {
+    id: 'foo',
+    children: [
+      {},
+      {},
+      {},
+      {},
+    ]
+  };
+
   /**
    * The mock proxy object to use during test.
    * @type {TestSiteSettingsPrefsBrowserProxy}
    */
   let browserProxy;
 
+  /**
+   * The mock local data proxy object to use during test.
+   * @type {TestLocalDataBrowserProxy}
+   */
+  let localDataBrowserProxy;
+
   /**
    * A site list element created before each test.
    * @type {SiteList}
@@ -42,7 +58,9 @@ suite('SiteEntry', function() {
   // Initialize a site-list before each test.
   setup(function() {
     browserProxy = new TestSiteSettingsPrefsBrowserProxy();
+    localDataBrowserProxy = new TestLocalDataBrowserProxy();
     settings.SiteSettingsPrefsBrowserProxyImpl.instance_ = browserProxy;
+    settings.LocalDataBrowserProxyImpl.instance_ = localDataBrowserProxy;
 
     PolymerTest.clearBody();
     testElement = document.createElement('site-entry');
@@ -178,4 +196,38 @@ suite('SiteEntry', function() {
         testElement.onSiteGroupChanged_(testElement.siteGroup);
         assertFalse(testElement.$.collapseChild.opened);
       });
+
+  test('cookies only show when non-zero for grouped entries', function() {
+    localDataBrowserProxy.setCookieDetails(TEST_COOKIE_LIST);
+    testElement.siteGroup = TEST_MULTIPLE_SITE_GROUP;
+    Polymer.dom.flush();
+    const cookiesLabel = testElement.$.cookies;
+    assertEquals('', cookiesLabel.textContent.trim());
+    assertTrue(cookiesLabel.hidden);
+
+    // When the number of cookies is more than zero, the label appears.
+    testElement.onSiteGroupChanged_(TEST_MULTIPLE_SITE_GROUP);
+    return localDataBrowserProxy.whenCalled('getNumCookiesString')
+        .then((args) => {
+          assertEquals('example.com', args);
+          assertFalse(cookiesLabel.hidden);
+          assertEquals(
+              `${TEST_COOKIE_LIST.children.length} cookies`,
+              cookiesLabel.textContent.trim());
+        });
+  });
+
+  test('cookies do not show for ungrouped entries', function() {
+    testElement.siteGroup = TEST_SINGLE_SITE_GROUP;
+    Polymer.dom.flush();
+    const cookiesLabel = testElement.$.cookies;
+    assertTrue(cookiesLabel.hidden);
+    assertEquals('', cookiesLabel.textContent.trim());
+
+    testElement.onSiteGroupChanged_(TEST_SINGLE_SITE_GROUP);
+    // Make sure there was never any call to the back end to retrieve cookies.
+    assertEquals(0, localDataBrowserProxy.getCallCount('getNumCookiesString'));
+    assertEquals('', cookiesLabel.textContent.trim());
+    assertTrue(cookiesLabel.hidden);
+  });
 });

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

@@ -17,6 +17,7 @@ class TestLocalDataBrowserProxy extends TestBrowserProxy {
       'removeShownItems',
       'removeItem',
       'getCookieDetails',
+      'getNumCookiesString',
       'reloadCookies',
       'removeCookie',
     ]);
@@ -30,10 +31,10 @@ class TestLocalDataBrowserProxy extends TestBrowserProxy {
 
   /**
    * Test-only helper.
-   * @param {!CookieList} cookieList
+   * @param {!CookieList} cookieDetails
    */
-  setCookieDetails(cookieList) {
-    this.cookieDetails_ = cookieList;
+  setCookieDetails(cookieDetails) {
+    this.cookieDetails_ = cookieDetails;
   }
 
   /**
@@ -80,6 +81,18 @@ class TestLocalDataBrowserProxy extends TestBrowserProxy {
     return Promise.resolve(this.cookieDetails_ || {id: '', children: []});
   }
 
+  /** @override */
+  getNumCookiesString(site) {
+    this.methodCalled('getNumCookiesString', site);
+    if (this.cookieDetails_) {
+      const numCookies = this.cookieDetails_.children.length;
+      return Promise.resolve(
+          `${this.cookieDetails_.children.length} ` +
+          (numCookies == 1 ? 'cookie' : 'cookies'));
+    }
+    return Promise.resolve('');
+  }
+
   /** @override */
   reloadCookies() {
     this.methodCalled('reloadCookies');