MD Settings: Fix CrScrollableBehavior

ui/webui/resources/cr_elements/cr_scrollable_behavior.js

 // Copyright 2016 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 Behavior for scrollable containers with <iron-list>.
  *
  * Any containers with the 'scrollable' attribute set will have the following
  * classes toggled appropriately: can-scroll, is-scrolled, scrolled-to-bottom.
  * These classes are used to style the container div and list elements
@@ skipping 19 matching lines @@
  * </dom-module>
  *
  * NOTE: If 'container' is not fixed size, it is important to call
  * updateScrollableContents() when [[items]] changes, otherwise the container
  * will not be sized correctly.
  */
 
 /** @polymerBehavior */
 var CrScrollableBehavior = {
   properties: {
-    /** @private */
-    intervalId_: Number,
+    /** @private {number|null} */
+    intervalId_: {
+      type: Number,
+      value: null
+    }
   },
 
   ready: function() {
     var scrollableElements = this.root.querySelectorAll('[scrollable]');
 
     // Setup the intial scrolling related classes for each scrollable container.
     requestAnimationFrame(function() {
       for (let scrollable of scrollableElements)
         this.updateScroll_(scrollable);
     }.bind(this));
 
     // Listen to the 'scroll' event for each scrollable container.
     for (let scrollable of scrollableElements)
       scrollable.addEventListener('scroll', this.updateScrollEvent_.bind(this));
   },
 
   detached: function() {
-    if (this.intervalId_)
+    if (this.intervalId_ !== null)
       clearInterval(this.intervalId_);
   },
 
   /**
    * Called any time the contents of a scrollable container may have changed.
    * This ensures that the <iron-list> contents of dynamically sized
    * containers are resized correctly.
    */
   updateScrollableContents() {

[dpapad, 2016/11/23 18:30:21]

updateScrollableContents: function() {

[stevenjb, 2016/11/23 18:37:15]

Sigh. Still a C++ developer at heart. Done.

     let nodeList = this.root.querySelectorAll('[scrollable] iron-list');
-    // Use setTimeout to avoid initial render / sizing issues.
-    this.intervalId_ = setInterval(function() {
+    // Use setInterval to avoid initial render / sizing issues.
+    if (this.intervalId_ !== null)

[dpapad, 2016/11/23 18:30:21]

Can you move this before the querySelectorAll? So that we don't do any
unnecessary work if we are going to return early?

[stevenjb, 2016/11/23 18:37:15]

Good catch. Done.

+      return;
+    this.intervalId_ = window.setInterval(function() {
       let unreadyNodes = [];
       for (let node of nodeList) {
         if (node.parentNode.scrollHeight == 0) {
           unreadyNodes.push(node);
           continue;
         }
         let ironList = /** @type {!IronListElement} */ (node);
         ironList.notifyResize();
       }
-      if (unreadyNodes.length == 0)
-        clearInterval(this.intervalId_);
-      else
+      if (unreadyNodes.length == 0) {
+        window.clearInterval(this.intervalId_);
+        this.intervalId_ = null;
+      } else {
         nodeList = unreadyNodes;
+      }
     }.bind(this), 10);
   },
 
   /**
    * Event wrapper for updateScroll_.
    * @param {!Event} event
    * @private
    */
   updateScrollEvent_(event) {
     let scrollable = /** @type {!HTMLElement} */ (event.target);
     this.updateScroll_(scrollable);
   },
 
   /**
    * This gets called once intially and any time a scrollable container scrolls.
    * @param {!HTMLElement} scrollable
    * @private
    */
   updateScroll_(scrollable) {
     scrollable.classList.toggle(
         'can-scroll', scrollable.clientHeight < scrollable.scrollHeight);
     scrollable.classList.toggle('is-scrolled', scrollable.scrollTop > 0);
     scrollable.classList.toggle(
         'scrolled-to-bottom', scrollable.scrollTop + scrollable.clientHeight >=
             scrollable.scrollHeight);
   },
 };