MD Downloads: limit the amount of downloads we send

chrome/browser/resources/md_downloads/action_service.js

Index: chrome/browser/resources/md_downloads/action_service.js
diff --git a/chrome/browser/resources/md_downloads/action_service.js b/chrome/browser/resources/md_downloads/action_service.js
index ee0a1b4a9fdc7384cfca8c7a120ddc7f6f18bf3b..3294632cb469e660a8dd48be742a21c41370f358 100644
--- a/chrome/browser/resources/md_downloads/action_service.js
+++ b/chrome/browser/resources/md_downloads/action_service.js
@@ -36,6 +36,9 @@ cr.define('downloads', function() {
   };
 
   ActionService.prototype = {
+    /** @private {Array<string>} */
+    searchTerms_: [],

[esprehn, 2015/12/10 21:42:59]

this is static, the array is shred between all instances of ActionService, you
don't normally want to do that, instead you want to do this.searchTerms_ = [] in
the constructor

[Dan Beam, 2015/12/10 21:51:15]

yeah, but this class is a singleton.  i understand what you're saying: instances
are shared across the prototype

+
     /** @param {string} id ID of the download to cancel. */
     cancel: chromeSendWithId('cancel'),
 
@@ -61,15 +64,17 @@ cr.define('downloads', function() {
     /** @param {string} id ID of the download that the user started dragging. */
     drag: chromeSendWithId('drag'),
 
-    /** @private {boolean} */
-    isSearching_: false,
+    /** Loads more downloads with the current search terms. */
+    loadMore: function() {
+      chrome.send('getDownloads', this.searchTerms_);
+    },
 
     /**
      * @return {boolean} Whether the user is currently searching for downloads
      *     (i.e. has a non-empty search term).
      */
     isSearching: function() {
-      return this.isSearching_;
+      return this.searchTerms_.length > 0;
     },
 
     /** Opens the current local destination for downloads. */
@@ -97,15 +102,19 @@ cr.define('downloads', function() {
 
     /** @param {string} searchText What to search for. */
     search: function(searchText) {
-      if (this.searchText_ == searchText)
-        return;
+      var searchTerms = ActionService.splitTerms(searchText);
+      var sameTerms = searchTerms.length == this.searchTerms_.length;
 
-      this.searchText_ = searchText;
+      for (var i = 0; sameTerms && i < searchTerms.length; ++i) {

[esprehn, 2015/12/10 21:42:59]

there's no standard library in WebUI like closure? it's sad to write this stuff
manually instead of goog.array.equals or something

[Dan Beam, 2015/12/10 21:51:15]

not currently.  why isn't there an Array equals... in the platform?  :)  because
people can't decide on certain edge cases?

+        if (searchTerms[i] != this.searchTerms_[i])
+          sameTerms = false;
+      }
 
-      var terms = ActionService.splitTerms(searchText);
-      this.isSearching_ = terms.length > 0;
+      if (sameTerms)
+        return;
 
-      chrome.send('getDownloads', terms);
+      this.searchTerms_ = searchTerms;

[esprehn, 2015/12/10 21:42:59]

this is actually shadowing the variable on the prototype

[Dan Beam, 2015/12/10 21:51:15]

that's fine for our use-case, but will send a followup

[Dan Beam, 2015/12/10 22:08:41]

followup here: https://codereview.chromium.org/1513483008/

+      this.loadMore();
     },
 
     /**