[Settings] Add a component site-favicon to display site favicon

This is a refactor CL and it should not change any existing UI.

The affected pages are as follows:
- On startup site entry in chrome://settings
- chrome://settings/siteData
- chrome://settings/searchEngines
- chrome://settings/content/zoomLevels
- chrome://settings/handlers
- chrome://settings/content/usbDevices
- chrome://settings/content/all
- All the pages in "Content settings" that use site-list-entry, such as
  chrome://settings/content/paymentHandler

Bug: 890118
Change-Id: I35e00406999cf30012db557b10dd418cef67a69d
Reviewed-on: https://chromium-review.googlesource.com/c/1277995
Commit-Queue: Yue Cen <rsgingerrs@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602099}

chrome/browser/resources/settings/on_startup_page/startup_url_entry.html

@@ -3,11 +3,11 @@
 <link rel="import" href="chrome://resources/cr_elements/cr_action_menu/cr_action_menu.html">
 <link rel="import" href="chrome://resources/cr_elements/cr_lazy_render/cr_lazy_render.html">
 <link rel="import" href="chrome://resources/cr_elements/icons.html">
-<link rel="import" href="chrome://resources/html/icon.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-icon-button/paper-icon-button-light.html">
 <link rel="import" href="startup_urls_page_browser_proxy.html">
 <link rel="import" href="../focus_row_behavior.html">
 <link rel="import" href="../settings_shared_css.html">
+<link rel="import" href="../site_favicon.html">
 
 <dom-module id="settings-startup-url-entry">
   <template>
@@ -17,9 +17,7 @@
       }
     </style>
     <div class="list-item" focus-row-container>
-      <div class="favicon-image"
-          style="background-image: [[getIconSet_(model.url)]]">
-      </div>
+      <site-favicon url="[[model.url]]"></site-favicon>
       <div class="middle hide-overflow">
         <div class="text-elide">[[model.title]]</div>
         <div class="text-elide secondary">[[model.url]]</div>

chrome/browser/resources/settings/on_startup_page/startup_url_entry.js

@@ -32,15 +32,6 @@ Polymer({
     model: Object,
   },
 
-  /**
-   * @param {string} url Location of an image to get a set of icons for.
-   * @return {string} A set of icon URLs.
-   * @private
-   */
-  getIconSet_: function(url) {
-    return cr.icon.getFavicon(url);
-  },
-
   /** @private */
   onRemoveTap_: function() {
     this.$$('cr-action-menu').close();

chrome/browser/resources/settings/search_engines_page/omnibox_extension_entry.html

@@ -2,12 +2,12 @@
 
 <link rel="import" href="chrome://resources/cr_elements/cr_action_menu/cr_action_menu.html">
 <link rel="import" href="chrome://resources/cr_elements/icons.html">
-<link rel="import" href="chrome://resources/html/icon.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-icon-button/paper-icon-button-light.html">
 <link rel="import" href="../extension_control_browser_proxy.html">
 <link rel="import" href="../focus_row_behavior.html">
 <link rel="import" href="search_engine_entry_css.html">
 <link rel="import" href="../settings_shared_css.html">
+<link rel="import" href="../site_favicon.html">
 
 <dom-module id="settings-omnibox-extension-entry">
   <template>
@@ -22,15 +22,10 @@
       .keyword-column {
         flex: 7;
       }
-
-      .favicon-image + span {
-        margin-inline-end: 8px;
-      }
     </style>
     <div class="list-item" focus-row-container>
       <div class="name-column">
-        <span class="favicon-image"
-            style="background-image: [[getIconSet_(engine.iconURL)]]"></span>
+        <site-favicon url="[[engine.iconURL]]"></site-favicon>
         <span>[[engine.displayName]]</span>
       </div>
       <div class="keyword-column">[[engine.keyword]]</div>

chrome/browser/resources/settings/search_engines_page/omnibox_extension_entry.js

@@ -42,15 +42,6 @@ Polymer({
     this.$$('cr-action-menu').close();
   },
 
-  /**
-   * @param {string} url
-   * @return {string} A set of icon URLs.
-   * @private
-   */
-  getIconSet_: function(url) {
-    return cr.icon.getFavicon(url);
-  },
-
   /** @private */
   onDotsTap_: function() {
     /** @type {!CrActionMenuElement} */ (this.$$('cr-action-menu'))

chrome/browser/resources/settings/search_engines_page/search_engine_entry.html

@@ -4,13 +4,13 @@
 <link rel="import" href="chrome://resources/cr_elements/icons.html">
 <link rel="import" href="chrome://resources/html/assert.html">
 <link rel="import" href="chrome://resources/html/cr/ui/focus_without_ink.html">
-<link rel="import" href="chrome://resources/html/icon.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-icon-button/paper-icon-button-light.html">
 <link rel="import" href="../controls/extension_controlled_indicator.html">
 <link rel="import" href="../focus_row_behavior.html">
 <link rel="import" href="search_engine_entry_css.html">
 <link rel="import" href="search_engines_browser_proxy.html">
 <link rel="import" href="../settings_shared_css.html">
+<link rel="import" href="../site_favicon.html">
 
 <dom-module id="settings-search-engine-entry">
   <template>
@@ -30,8 +30,7 @@
         word-break: break-word;
       }
 
-      #keyword-column > div,
-      .favicon-image + div {
+      #keyword-column > div {
         margin-inline-end: 8px;
       }
 
@@ -42,8 +41,7 @@
 
     <div class="list-item" focus-row-container>
       <div id="name-column">
-        <div class="favicon-image"
-            style="background-image: [[getIconSet_(engine.iconURL)]]"></div>
+        <site-favicon url="[[engine.iconURL]]"></site-favicon>
         <div>[[engine.displayName]]</div>
       </div>
       <div id="keyword-column"><div>[[engine.keyword]]</div></div>

chrome/browser/resources/settings/search_engines_page/search_engine_entry.js

@@ -64,16 +64,6 @@ Polymer({
     return canBeDefault || canBeEdited || canBeRemoved;
   },
 
-  /**
-   * @param {?string} url The icon URL if available.
-   * @return {string} A set of icon URLs.
-   * @private
-   */
-  getIconSet_: function(url) {
-    // Force default icon, if no |engine.iconURL| is available.
-    return cr.icon.getFavicon(url || '');
-  },
-
   /** @private */
   onDeleteTap_: function() {
     this.browserProxy_.removeSearchEngine(this.engine.modelIndex);

chrome/browser/resources/settings/search_engines_page/search_engine_entry_css.html

 <dom-module id="search-engine-entry">
   <template>
     <style>
-      .favicon-image {
-        background-repeat: no-repeat;
-        background-size: contain;
-        display: inline-block;
-        height: 20px;
+      site-favicon {
         margin-inline-end: 8px;
-        min-width: 20px;
-        vertical-align: middle;
       }
 
       .dropdown-content {

chrome/browser/resources/settings/settings_resources.grd

@@ -998,6 +998,12 @@
                  file="route.js"
                  type="chrome_html"
                  preprocess="true" />
+      <structure name="IDR_SETTINGS_SITE_FAVICON_HTML"
+                 file="site_favicon.html"
+                 type="chrome_html" />
+      <structure name="IDR_SETTINGS_SITE_FAVICON_JS"
+                 file="site_favicon.js"
+                 type="chrome_html" />
       <structure name="IDR_SETTINGS_SITE_DATA_HTML"
                  file="site_settings/site_data.html"
                  type="chrome_html" />

chrome/browser/resources/settings/settings_shared_css.html

@@ -323,13 +323,6 @@
         margin: 0 16px;
       }
 
-      .favicon-image {
-        background-repeat: no-repeat;
-        background-size: contain;
-        height: 16px;
-        width: 16px;
-      }
-
       .column-header {
         color: var(--google-grey-refresh-700);
         font-size: inherit;

chrome/browser/resources/settings/site_favicon.html

+<link rel="import" href="chrome://resources/html/polymer.html">
+
+<link rel="import" href="chrome://resources/html/icon.html">
+
+<dom-module id="site-favicon">
+  <template>
+    <style>
+      :host {
+        background-repeat: no-repeat;
+        background-size: contain;
+        display: block;
+        height: 16px;
+        width: 16px;
+      }
+    </style>
+  </template>
+  <script src="site_favicon.js"></script>
+</dom-module>

chrome/browser/resources/settings/site_favicon.js

+// Copyright 2018 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 'site-favicon' is the section to display the favicon given the
+ * site URL.
+ */
+
+Polymer({
+  is: 'site-favicon',
+
+  properties: {
+    url: {
+      type: String,
+      value: '',
+      observer: 'urlChanged_',
+    }
+  },
+
+  /** @private */
+  urlChanged_: function() {
+    let url = this.removePatternWildcard_(this.url);
+    url = this.ensureUrlHasScheme_(url);
+    this.style.backgroundImage = cr.icon.getFavicon(url || '');
+  },
+
+  /**
+   * Removes the wildcard prefix from a pattern string.
+   * @param {string} pattern The pattern to remove the wildcard from.
+   * @return {string} The resulting pattern.
+   * @private
+   */
+  removePatternWildcard_: function(pattern) {
+    if (!pattern || pattern.length === 0)
+      return pattern;
+
+    if (pattern.startsWith('http://[*.]'))
+      return pattern.replace('http://[*.]', 'http://');
+    else if (pattern.startsWith('https://[*.]'))
+      return pattern.replace('https://[*.]', 'https://');
+    else if (pattern.startsWith('[*.]'))
+      return pattern.substring(4, pattern.length);
+    return pattern;
+  },
+
+  /**
+   * Ensures the URL has a scheme (assumes http if omitted).
+   * @param {string} url The URL with or without a scheme.
+   * @return {string} The URL with a scheme, or an empty string.
+   * @private
+   */
+  ensureUrlHasScheme_: function(url) {
+    if (!url || url.length === 0)
+      return url;
+    return url.includes('://') ? url : 'http://' + url;
+  },
+});
\ No newline at end of file

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

@@ -7,6 +7,7 @@
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-icon-button/paper-icon-button-light.html">
 <link rel="import" href="../i18n_setup.html">
 <link rel="import" href="../settings_shared_css.html">
+<link rel="import" href="../site_favicon.html">
 <link rel="import" href="site_settings_behavior.html">
 <link rel="import" href="site_settings_prefs_browser_proxy.html">
 
@@ -40,8 +41,7 @@
         <template is="dom-repeat" items="[[protocol.handlers]]">
 
           <div class="list-item">
-            <div class="favicon-image" style$="[[computeSiteIcon(item.host)]]">
-            </div>
+            <site-favicon url="[[item.host]]"></site-favicon>
             <div class="middle" >
               <div class="protocol-host">
                 <span class="url-directionality">[[item.host]]</span>
@@ -76,8 +76,7 @@
       <div class="list-frame menu-content vertical-list">
         <template is="dom-repeat" items="[[ignoredProtocols]]">
           <div class="list-item">
-            <div class="favicon-image" style$="[[computeSiteIcon(item.host)]]">
-            </div>
+            <site-favicon url="[[item.host]]"></site-favicon>
             <div class="middle" >
               <div class="protocol-host">
                 <span class="url-directionality">[[item.host]]</span></div>

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

@@ -113,19 +113,6 @@ Polymer({
     }
   },
 
-  /**
-   * Returns the icon to use for a given site.
-   * @param {string} url The url of the site to fetch the icon for.
-   * @return {string} Value for background-image style.
-   * @private
-   */
-  favicon_: function(url) {
-    // If the url doesn't have a scheme, inject HTTP as the scheme. Otherwise,
-    // the URL isn't valid and no icon will be returned.
-    const urlWithScheme = url.includes('://') ? url : 'http://' + url;
-    return cr.icon.getFavicon(urlWithScheme);
-  },
-
   /**
    * @param {!Map<string, (string|Function)>} newConfig
    * @param {?Map<string, (string|Function)>} oldConfig

chrome/browser/resources/settings/site_settings/site_data_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="../focus_row_behavior.html">
 <link rel="import" href="../settings_shared_css.html">
+<link rel="import" href="../site_favicon.html">
 <link rel="import" href="cookie_info.html">
 
 <dom-module id="site-data-entry">
@@ -16,9 +17,7 @@
       }
     </style>
     <div class="settings-box two-line site-item" focus-row-container actionable>
-      <div class="favicon-image"
-          style$="background-image: [[favicon_]]">
-      </div>
+      <site-favicon url="[[model.site]]"></site-favicon>
       <div class="middle">
         <span class="url-directionality">[[model.site]]</span>
         <div class="secondary">[[model.localData]]</div>

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

@@ -18,15 +18,6 @@ Polymer({
   properties: {
     /** @type {!CookieDataSummaryItem} */
     model: Object,
-
-    /**
-     * Icon to use for a given site.
-     * @private
-     */
-    favicon_: {
-      type: String,
-      computed: 'computeFavicon_(model)',
-    },
   },
 
   /** @private {settings.LocalDataBrowserProxy} */
@@ -37,18 +28,6 @@ Polymer({
     this.browserProxy_ = settings.LocalDataBrowserProxyImpl.getInstance();
   },
 
-  /**
-   * @return {string}
-   * @private
-   */
-  computeFavicon_: function() {
-    const url = this.model.site;
-    // If the url doesn't have a scheme, inject HTTP as the scheme. Otherwise,
-    // the URL isn't valid and no icon will be returned.
-    const urlWithScheme = url.includes('://') ? url : 'http://' + url;
-    return cr.icon.getFavicon(urlWithScheme);
-  },
-
   /**
    * Deletes all site data for this site.
    * @param {!Event} e

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="../site_favicon.html">
 <link rel="import" href="local_data_browser_proxy.html">
 <link rel="import" href="site_settings_behavior.html">
 
@@ -48,9 +49,7 @@
       <div class$="settings-box list-item [[getClassForIndex_(listIndex)]]">
         <div id="toggleButton" class="start row-aligned two-line"
             on-click="onSiteEntryTap_" actionable aria-expanded="false">
-          <div class="favicon-image"
-              style$="[[getSiteGroupIcon_(siteGroup)]]">
-          </div>
+          <site-favicon url="[[getSiteGroupIcon_(siteGroup)]]"></site-favicon>
           <div class="middle text-elide" id="displayName">
             <div class="site-representation">
               <span class="url-directionality">[[displayName_]]</span>
@@ -94,9 +93,7 @@
           <template is="dom-repeat" items="[[siteGroup.origins]]">
             <div class="settings-box list-item" on-click="onOriginTap_"
                 actionable>
-              <div class="favicon-image"
-                  style$="[[computeSiteIcon(item.origin)]]">
-              </div>
+              <site-favicon url="[[item.origin]]"></site-favicon>
               <div class="site-representation middle text-elide">
                 <span id="originSiteRepresentation" class="url-directionality">
                   [[siteRepresentation_(siteGroup, index)]]

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

@@ -182,13 +182,13 @@ Polymer({
    * Get an appropriate favicon that represents this group of eTLD+1 sites as a
    * whole.
    * @param {SiteGroup} siteGroup The eTLD+1 group of origins.
-   * @return {string} CSS to apply to show the appropriate favicon.
+   * @return {string} URL that is used for fetching the favicon
    * @private
    */
   getSiteGroupIcon_: function(siteGroup) {
     // TODO(https://crbug.com/835712): Implement heuristic for finding a good
     // favicon.
-    return this.computeSiteIcon(siteGroup.origins[0].origin);
+    return siteGroup.origins[0].origin;
   },
 
   /**

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

@@ -10,6 +10,7 @@
 <link rel="import" href="../icons.html">
 <link rel="import" href="../route.html">
 <link rel="import" href="../settings_shared_css.html">
+<link rel="import" href="../site_favicon.html">
 <link rel="import" href="constants.html">
 <link rel="import" href="site_settings_behavior.html">
 <link rel="import" href="site_settings_prefs_browser_proxy.html">
@@ -35,9 +36,7 @@
     <div class="list-item" focus-row-container>
       <div class="settings-row"
           actionable$="[[enableSiteSettings_]]" on-click="onOriginTap_">
-        <div class="favicon-image"
-            style$="[[computeSiteIcon(model.origin)]]">
-        </div>
+        <site-favicon url="[[model.origin]]"></site-favicon>
         <div class="middle no-min-width">
           <div class="text-elide">
             <span class="url-directionality">[[model.displayName]]</span>

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

@@ -5,5 +5,4 @@
 <link rel="import" href="site_settings_prefs_browser_proxy.html">
 <link rel="import" href="chrome://resources/cr_elements/icons.html">
 <link rel="import" href="chrome://resources/html/assert.html">
-<link rel="import" href="chrome://resources/html/icon.html">
 <script src="site_settings_behavior.js"></script>

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

@@ -90,33 +90,6 @@ const SiteSettingsBehaviorImpl = {
     return url;
   },
 
-  /**
-   * Removes the wildcard prefix from a pattern string.
-   * @param {string} pattern The pattern to remove the wildcard from.
-   * @return {string} The resulting pattern.
-   * @private
-   */
-  removePatternWildcard: function(pattern) {
-    if (pattern.startsWith('http://[*.]'))
-      return pattern.replace('http://[*.]', 'http://');
-    else if (pattern.startsWith('https://[*.]'))
-      return pattern.replace('https://[*.]', 'https://');
-    else if (pattern.startsWith('[*.]'))
-      return pattern.substring(4, pattern.length);
-    return pattern;
-  },
-
-  /**
-   * Returns the icon to use for a given site.
-   * @param {string} site The url of the site to fetch the icon for.
-   * @return {string} The background-image style with the favicon.
-   */
-  computeSiteIcon: function(site) {
-    site = this.removePatternWildcard(site);
-    const url = this.ensureUrlHasScheme(site);
-    return 'background-image: ' + cr.icon.getFavicon(url);
-  },
-
   /**
    * Returns true if the passed content setting is considered 'enabled'.
    * @param {string} setting

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

@@ -4,6 +4,7 @@
 
 <link rel="import" href="../i18n_setup.html">
 <link rel="import" href="../settings_shared_css.html">
+<link rel="import" href="../site_favicon.html">
 <link rel="import" href="site_settings_behavior.html">
 <link rel="import" href="site_settings_prefs_browser_proxy.html">
 
@@ -29,8 +30,7 @@
 
       <div class="list-frame menu-content vertical-list">
         <div class="list-item">
-          <div class="favicon-image"
-              style$="[[computeSiteIcon(item.origin)]]"></div>
+          <site-favicon url="[[item.origin]]"></site-favicon>
           <div class="middle">
             <span class="url-directionality">[[item.origin]]</span>
           </div>

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

@@ -7,6 +7,7 @@
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-icon-button/paper-icon-button-light.html">
 <link rel="import" href="../i18n_setup.html">
 <link rel="import" href="../settings_shared_css.html">
+<link rel="import" href="../site_favicon.html">
 <link rel="import" href="site_settings_behavior.html">
 <link rel="import" href="site_settings_prefs_browser_proxy.html">
 
@@ -26,7 +27,7 @@
         margin-top: 15px;
       }
 
-      .list-item .favicon-image {
+      .list-item site-favicon {
         flex-shrink: 0;
       }
 
@@ -40,9 +41,7 @@
           class="cr-separators" risk-selection>
         <template>
           <div class="list-item" first$="[[!index]]">
-            <div class="favicon-image"
-                style$="[[computeSiteIcon(item.originForFavicon)]]">
-            </div>
+            <site-favicon url="[[item.originForFavicon]]"></site-favicon>
             <div class="middle">
               <span class="url-directionality">[[item.displayName]]</span>
             </div>

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

@@ -2190,3 +2190,25 @@ CrSettingsFindShortcutBehavior.prototype = {
 TEST_F('CrSettingsFindShortcutBehavior', 'All', function() {
   mocha.run();
 });
+
+/**
+ * @constructor
+ * @extends {CrSettingsBrowserTest}
+ */
+function CrSettingsSiteFaviconTest() {}
+
+CrSettingsSiteFaviconTest.prototype = {
+  __proto__: CrSettingsBrowserTest.prototype,
+
+  /** @override */
+  browsePreload: 'chrome://settings/site_favicon.html',
+
+  /** @override */
+  extraLibraries: CrSettingsBrowserTest.prototype.extraLibraries.concat([
+    'site_favicon_test.js',
+  ]),
+};
+
+TEST_F('CrSettingsSiteFaviconTest', 'All', function() {
+  mocha.run();
+});

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

+// Copyright 2018 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.
+
+suite('SiteFavicon', function() {
+  let siteFavicon;
+
+  setup(function() {
+    PolymerTest.clearBody();
+    siteFavicon = document.createElement('site-favicon');
+    document.body.appendChild(siteFavicon);
+  });
+
+  function assertIconEquals(expected) {
+    const background = siteFavicon.style.backgroundImage;
+    assertEquals(background, expected);
+  }
+
+  function formExpected(url) {
+    return '-webkit-image-set(' +
+        'url("chrome://favicon/size/16@1x/' + url + '") 1x, ' +
+        'url("chrome://favicon/size/16@2x/' + url + '") 2x)';
+  }
+
+  test('normal URL', function() {
+    const url = 'http://www.google.com';
+    siteFavicon.url = url;
+    assertIconEquals(formExpected(url));
+  });
+
+  test('URL without scheme', function() {
+    const url = 'www.google.com';
+    siteFavicon.url = url;
+    assertIconEquals(formExpected(`http://${url}`));
+  });
+
+  test('URLs with wildcard', function() {
+    const url1 = 'http://[*.]google.com';
+    siteFavicon.url = url1;
+    assertIconEquals(formExpected('http://google.com'));
+
+    const url2 = 'https://[*.]google.com';
+    siteFavicon.url = url2;
+    assertIconEquals(formExpected('https://google.com'));
+
+    const url3 = '[*.]google.com';
+    siteFavicon.url = url3;
+    assertIconEquals(formExpected('http://google.com'));
+  });
+});

testing/buildbot/filters/webui_polymer2_browser_tests.filter

@@ -161,6 +161,7 @@ CrSettingsSiteDataTest.*
 CrSettingsSiteDetailsPermissionTest.*
 CrSettingsSiteDetailsTest.*
 CrSettingsSiteEntryTest.*
+CrSettingsSiteFaviconTest.*
 CrSettingsSiteListEntryTest.*
 CrSettingsSiteListTest.*
 CrSettingsSliderTest.*