MD Settings: Fix CrScrollableBehavior

ui/webui/resources/cr_elements/cr_scrollable_behavior.js

Index: ui/webui/resources/cr_elements/cr_scrollable_behavior.js
diff --git a/ui/webui/resources/cr_elements/cr_scrollable_behavior.js b/ui/webui/resources/cr_elements/cr_scrollable_behavior.js
index 59e1e53dab1b3e5885ebf1fc432df3a1ffc67b28..ec362f8fb0c1caa1034c048303ef9de5b1774252 100644
--- a/ui/webui/resources/cr_elements/cr_scrollable_behavior.js
+++ b/ui/webui/resources/cr_elements/cr_scrollable_behavior.js
@@ -56,7 +56,7 @@ var CrScrollableBehavior = {
   },
 
   detached: function() {
-    if (this.intervalId_)
+    if (this.intervalId_ !== undefined)
       clearInterval(this.intervalId_);
   },
 
@@ -68,7 +68,9 @@ var CrScrollableBehavior = {
   updateScrollableContents() {
     let nodeList = this.root.querySelectorAll('[scrollable] iron-list');
     // Use setTimeout to avoid initial render / sizing issues.
-    this.intervalId_ = setInterval(function() {
+    if (this.intervalId_ === undefined)

[dpapad, 2016/11/23 01:12:51]

Shouldn't this be the opposite?

if (this.intervalId_ !== undefined)
  return;

such that we return early if this method has already been called?

[stevenjb, 2016/11/23 18:07:01]

Er, ugh, yes. I must already be in vacation mode. I guess this "fixed" the
problem by never running. I'll have to make sure to more carefully test the
cases where we need this logic.
Done.

+      return;
+    this.intervalId_ = window.setInterval(function() {
       let unreadyNodes = [];
       for (let node of nodeList) {
         if (node.parentNode.scrollHeight == 0) {
@@ -78,10 +80,12 @@ var CrScrollableBehavior = {
         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_ = undefined;

[dpapad, 2016/11/23 01:12:51]

Setting something to |undefined| is counter intuitive. |undefined| in JS is the
default value of something that has not been defined yet like "var a;".

Can we use null instead and initialize intervalId_ to null at line 41? |null|
signifies more explicitly that a value has been assigned, unlike |undefined|
which can be the result of various errors (typos for example).

[stevenjb, 2016/11/23 18:07:01]

It seems odd to me to set a Number to null, but I guess it isn't really
different than setting it to undefined (and I have to pacify Closure either
way). Done.

+      } else {
         nodeList = unreadyNodes;
+      }
     }.bind(this), 10);
   },