Debouncing of function rescanning directory in File Manager

chrome/browser/resources/file_manager/js/file_manager.js

Index: chrome/browser/resources/file_manager/js/file_manager.js
diff --git a/chrome/browser/resources/file_manager/js/file_manager.js b/chrome/browser/resources/file_manager/js/file_manager.js
index e4e044ed8d5a468c8935a5d7f47988e9ea5969f6..3f514e55f7f3ff8381d7aa6b037154b557f0a3b1 100644
--- a/chrome/browser/resources/file_manager/js/file_manager.js
+++ b/chrome/browser/resources/file_manager/js/file_manager.js
@@ -1794,7 +1794,7 @@ FileManager.prototype = {
       if (event.status == 'success' ||
           event.status == 'error_unknown_filesystem' ||
           event.status == 'error_unsuported_filesystem')
-        self.rescanDirectory_();
+        self.rescanDirectory_(undefined, 300);

[rginda, 2011/08/25 00:29:09]

In js, "undefined" just happens to be a variable that hasn't been defined yet,
like "foo" or "querty".  Use the keyword null any time you mean "no value".

[sidor, 2011/08/25 01:09:58]

Done.

     });
   };
 
@@ -2522,12 +2522,59 @@ FileManager.prototype = {
   };
 
   /**
-   * Rescan the current directory, refreshing the list.
+   * Rescans the current directory, refreshing the list. It decreases the
+   * probability that two such calls are pending simultaneously.
    *
    * @param {function()} opt_callback Optional function to invoke when the
    *     rescan is complete.
+   * @param {string} delayMS Delay during which next rescanDirectory calls
+   *     can happen.
    */
-  FileManager.prototype.rescanDirectory_ = function(opt_callback) {
+
+  FileManager.prototype.rescanDirectory_ = function(opt_callback, delayMS) {
+    function done(count) {
+      // Variable count is introduced because we only want to do callbacks, that
+      // were in the queue at the moment of rescanDirectoryNow invocation.
+      while (count--) {
+        var callback = this.rescanDirectory_.callbacks.shift();

[rginda, 2011/08/25 00:29:09]

s/shift/pop/

push/pop work from the end of the array.  shift/unshift work from the front.  If
you push to the end, and shift from the front, then you'll break in the case
where 'count' matters.

[sidor, 2011/08/25 01:09:58]

Hmm.. Nope. I did it on purpose and I really think it works.  But yeah it might
not be obvious looking at the code. In my case I use this array as a queue.
Please ping me when you are in your office, I will come by and explain you how
it works. 

+        callback();
+      }
+    }
+
+    if (!this.rescanDirectory_)
+      this.rescanDirectory_ = [];

[rginda, 2011/08/25 00:29:09]

this.rescanDirectory_ is the function we're in.  This code will never run.

[sidor, 2011/08/25 01:09:58]

Done.

+
+    if (!this.rescanDirectory_.callbacks)
+      this.rescanDirectory_.callbacks = [];
+
+    if (opt_callback)
+      this.rescanDirectory_.callbacks.push(opt_callback);
+
+    if (!delayMS)
+      var delayMS = 100;

[rginda, 2011/08/25 00:29:09]

No need for var here.  delayMS is already declared as a parameter.  'delayMS =
delayMS || 100' is a common js idiom for what you're doing here.

[sidor, 2011/08/25 01:09:58]

That's kind of cool idiom :)

+
+    // Assumes rescanDirectoryNow_ takes less than 100 ms. If not there is
+    // a possible overlap between two calls.

[rginda, 2011/08/25 00:29:09]

I don't think this comment (or the minimum 100ms policy) applies here.  This is
debouncing the case when callers invoke the function too many times in a row. 
rescanDirectoryNow won't even *start* until the timeout expires.

[sidor, 2011/08/25 01:09:58]

No, no this is not what I mean there. Here's what this is:
1. [0ms] We have one call 1 to rescanDirectory_ .
2. [100ms] We actually call rescanDirectoryNow_ for call 1.
3. [101ms] We have a call 2 to rescanDirectory_.
4. [201ms] We actually call rescanDirectoryNow_ for call 2.
Now if the rescanDirectoryNow_ for call 1 is still running (it took more than
100 ms to resolve), we have two overlapping calls of rescanDirectoryNow_.

+    if (delayMS < 100)
+      delayMS = 100;
+
+    if (this.rescanDirectory_.handle)
+      clearTimeout(this.rescanDirectory_.handle);
+    var self = this;
+    this.rescanDirectory_.handle = setTimeout(function () {
+        self.rescanDirectoryNow_(done.bind(self,

[rginda, 2011/08/25 00:29:09]

done already has access to self, there is no need to bind it here.  You should
move the 'var self = this' line above the declaration of done, however, to make
it clear.

[sidor, 2011/08/25 01:09:58]

OK, but I still need to pass and argument to the function at this point. Is
bind(null, arg) the right solution?

+            self.rescanDirectory_.callbacks.length));
+      }, delayMS);
+  };
+
+  /**
+   * Rescans the current directory immediately, refreshing the list. Should NOT
+   * be used in most cases. Instead use rescanDirectory_.
+   *
+   * @param {function()} opt_callback Optional function to invoke when the
+   *     rescan is complete.
+   */
+  FileManager.prototype.rescanDirectoryNow_ = function(opt_callback) {
     var self = this;
     var reader;