Settings: Client side infinite scrolling in All Sites.

An extremely large number of sites may appear in All Sites, so make sure the
client doesn't need to render any more site-entries than required at any one
time by converting it to use an iron-list. Also introduce a couple other polish
fixes, such as scrolling to a site-entry when it's expanded, or scrolling to the
same site-entry previously selected (if any) when navigating back to All Sites
from a different route.

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

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

 <link rel="import" href="chrome://resources/html/polymer.html">
 
+<link rel="import" href="chrome://resources/polymer/v1_0/iron-list/iron-list.html">
+<link rel="import" href="../global_scroll_target_behavior.html">
+<link rel="import" href="../route.html">
 <link rel="import" href="../settings_shared_css.html">
 <link rel="import" href="site_entry.html">
 
 <dom-module id="all-sites">
   <template>
     <style include="settings-shared">
-      :host {
-        display: block;
-      }
-
       /* There is only one top-level heading for All Sites, so remove the
        * additional leading padding used for lists. */
       .list-frame.without-heading {
@@ -20,9 +19,12 @@
       <div class="list-item secondary">$i18n{noSitesAdded}</div>
     </div>
     <div class="list-frame without-heading" id="listContainer">
-      <template is="dom-repeat" items="[[siteGroupList]]">
-        <site-entry site-group="[[item]]"></site-entry>
-      </template>
+      <iron-list id="allSitesList" items="[[siteGroupList]]"
+          scroll-target="[[subpageScrollTarget]]">
+        <template>
+          <site-entry site-group="[[item]]"></site-entry>
+        </template>
+      </iron-list>
     </div>
   </template>
   <script src="all_sites.js"></script>

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

@@ -10,7 +10,12 @@
 Polymer({
   is: 'all-sites',
 
-  behaviors: [SiteSettingsBehavior, WebUIListenerBehavior],
+  behaviors: [
+    SiteSettingsBehavior,
+    WebUIListenerBehavior,
+    settings.RouteObserverBehavior,
+    settings.GlobalScrollTargetBehavior,
+  ],
 
   properties: {
     /**
@@ -23,6 +28,16 @@ Polymer({
         return [];
       },
     },
+
+    /**
+     * Needed by GlobalScrollTargetBehavior.
+     * @override
+     */
+    subpageRoute: {
+      type: Object,
+      value: settings.routes.SITE_SETTINGS_ALL,
+      readOnly: true,
+    },
   },
 
   /** @override */
@@ -34,6 +49,15 @@ Polymer({
     this.populateList_();
   },
 
+  /** @override */
+  attached: function() {
+    // Set scrollOffset so the iron-list scrolling accounts for the space the
+    // title takes.
+    Polymer.RenderStatus.afterNextRender(this, () => {
+      this.$.allSitesList.scrollOffset = this.$.allSitesList.offsetTop;
+    });
+  },
+
   /**
    * Retrieves a list of all known sites with site details.
    * @private

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

@@ -16,6 +16,11 @@
         flex-direction: row;
       }
 
+      #toggleButton {
+        /** Use the minimum row height as the minimum click-target height. */
+        min-height: var(--settings-row-min-height);
+      }
+
       /* URLs should always be displayed in a LTR embedding - see
        * https://url.spec.whatwg.org/#url-rendering. */
       .url-directionality {
@@ -36,25 +41,23 @@
           </div>
           <div class="site-representation middle text-elide" id="displayName">
             <span class="url-directionality">[[displayName_]]</span>
-            <span class="secondary" hidden$="[[!scheme_(siteGroup)]]">
+            <span class="secondary" hidden$="[[!scheme_(siteGroup, -1)]]">
               &nbsp;$i18nPolymer{siteSettingsSiteRepresentationSeparator}&nbsp;
             </span>
-            <span class="secondary" hidden$="[[!scheme_(siteGroup)]]">
-              [[scheme_(siteGroup)]]
+            <span class="secondary" hidden$="[[!scheme_(siteGroup, -1)]]">
+              [[scheme_(siteGroup, -1)]]
             </span>
           </div>
-          <div hidden$="[[!grouped_(siteGroup)]]">
-            <paper-icon-button-light id="expandIcon" class="icon-expand-more">
-              <button aria-label$="[[displayName_]]"
-                  aria-describedby="displayName"></button>
-            </paper-icon-button-light>
-          </div>
-          <div hidden$="[[grouped_(siteGroup)]]">
-            <paper-icon-button-light class="subpage-arrow">
-              <button aria-label$="[[displayName_]]"
-                  aria-describedby="displayName"></button>
-            </paper-icon-button-light>
-          </div>
+          <paper-icon-button-light id="expandIcon" class="icon-expand-more"
+              hidden$="[[!grouped_(siteGroup)]]">
+            <button aria-label$="[[displayName_]]"
+                aria-describedby="displayName"></button>
+          </paper-icon-button-light>
+          <paper-icon-button-light class="subpage-arrow"
+              hidden$="[[grouped_(siteGroup)]]">
+            <button aria-label$="[[displayName_]]"
+                aria-describedby="displayName"></button>
+          </paper-icon-button-light>
         </div>
         <div class="row-aligned" hidden$="[[!grouped_(siteGroup)]]">
           <div class="separator"></div>
@@ -71,8 +74,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)]]">
+              <div class="favicon-image" style$="[[computeSiteIcon(item)]]">
               </div>
               <div class="site-representation middle text-elide">
                 <span class="url-directionality">

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

@@ -38,7 +38,9 @@ Polymer({
    * @private
    */
   grouped_: function(siteGroup) {
-    return siteGroup.origins.length != 1;
+    if (siteGroup)
+      return siteGroup.origins.length != 1;
+    return false;
   },
 
   /**
@@ -46,23 +48,22 @@ Polymer({
    * If grouped_() is true and |originIndex| is not provided, returns the eTLD+1
    * for all the origins, otherwise, return the host for that origin.
    * @param {SiteGroup} siteGroup The eTLD+1 group of origins.
-   * @param {!number=} originIndex Optional index of the origin to get a
-   *     user-friendly name for. If not provided, returns the eTLD+1 name, if
-   *     there is one, otherwise defaults to the first origin.
+   * @param {number} originIndex Index of the origin to get a user-friendly name
+   *     for. If -1, returns the eTLD+1 name if any, otherwise defaults to the
+   *     first origin.
    * @return {string} The user-friendly name.
    * @private
    */
   siteRepresentation_: function(siteGroup, originIndex) {
     if (!siteGroup)
       return '';
-    if (this.grouped_(siteGroup) && originIndex === undefined) {
+    if (this.grouped_(siteGroup) && originIndex == -1) {
       if (siteGroup.etldPlus1 != '')
         return siteGroup.etldPlus1;
       // Fall back onto using the host of the first origin, if no eTLD+1 name
       // was computed.
-      return this.toUrl(siteGroup.origins[0]).host;
     }
-    originIndex = originIndex === undefined ? 0 : originIndex;
+    originIndex = this.getIndexBoundToOriginList_(siteGroup, originIndex);
     const url = this.toUrl(siteGroup.origins[originIndex]);
     return url.host;
   },
@@ -72,23 +73,23 @@ Polymer({
    * @private
    */
   onSiteGroupChanged_: function(siteGroup) {
-    this.displayName_ = this.siteRepresentation_(siteGroup);
+    this.displayName_ = this.siteRepresentation_(siteGroup, -1);
   },
 
   /**
    * Returns any non-HTTPS scheme/protocol for the origin corresponding to
    * |originIndex|. Otherwise, returns a empty string.
    * @param {SiteGroup} siteGroup The eTLD+1 group of origins.
-   * @param {!number=} originIndex Optional index of the origin to get the
-   *     non-HTTPS scheme for. If not provided, returns an empty string for
-   *     grouped |siteGroup|s but defaults to 0 for non-grouped.
+   * @param {number} originIndex Index of the origin to get the non-HTTPS scheme
+   *     for. If -1, returns an empty string for the grouped |siteGroup|s but
+   *     defaults to 0 for non-grouped.
    * @return {string} The scheme if non-HTTPS, or empty string if HTTPS.
    * @private
    */
   scheme_: function(siteGroup, originIndex) {
-    if (!siteGroup || (this.grouped_(siteGroup) && originIndex === undefined))
+    if (!siteGroup || (this.grouped_(siteGroup) && originIndex == -1))
       return '';
-    originIndex = originIndex === undefined ? 0 : originIndex;
+    originIndex = this.getIndexBoundToOriginList_(siteGroup, originIndex);
 
     const url = this.toUrl(siteGroup.origins[originIndex]);
     const scheme = url.protocol.replace(new RegExp(':*$'), '');
@@ -151,32 +152,26 @@ Polymer({
     this.$.toggleButton.setAttribute('aria-expanded', collapseChild.opened);
     this.$.expandIcon.toggleClass('icon-expand-more');
     this.$.expandIcon.toggleClass('icon-expand-less');
-  },
+    this.fire('iron-resize');
 
-  /**
-   * Retrieves the overflow menu.
-   * @private
-   */
-  getOverflowMenu_: function() {
-    let menu = /** @type {?CrActionMenuElement} */ (this.$.menu.getIfExists());
-    if (!menu)
-      menu = /** @type {!CrActionMenuElement} */ (this.$.menu.get());
-    return menu;
+    // Make sure the expanded origins can be viewed without further scrolling
+    // (in case |this| is already at the bottom of the viewport).
+    this.scrollIntoViewIfNeeded();
   },
 
   /**
    * Opens the overflow menu at event target.
-   * @param {!{target: Element}} e
+   * @param {!{target: !Element}} e
    * @private
    */
   showOverflowMenu_: function(e) {
-    this.getOverflowMenu_().showAt(e.target);
+    this.$.menu.get().showAt(e.target);
   },
 
   /** @private */
   onCloseDialog_: function(e) {
     e.target.closest('cr-dialog').close();
-    this.getOverflowMenu_().close();
+    this.$.menu.get().close();
   },
 
   /**
@@ -216,4 +211,17 @@ Polymer({
   getFormatString_: function(label, name) {
     return loadTimeData.substituteString(label, name);
   },
+
+  /**
+   * Returns a valid index for an origin contained in |siteGroup.origins| by
+   * clamping the given |index|. This also replaces undefined |index|es with 0.
+   * Use this to prevent being given out-of-bounds indexes by dom-repeat when
+   * scrolling an iron-list storing these site-entries too quickly.
+   * @param {!number=} index
+   * @return {number}
+   * @private
+   */
+  getIndexBoundToOriginList_: function(siteGroup, index) {
+    return Math.max(0, Math.min(index, siteGroup.origins.length - 1));
+  },
 });