MD Downloads: limit the amount of downloads we send

chrome/browser/resources/md_downloads/action_service.js

 // Copyright 2015 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.
 
 cr.define('downloads', function() {
   /**
    * @param {string} chromeSendName
    * @return {function(string):void} A chrome.send() callback with curried name.
    */
   function chromeSendWithId(chromeSendName) {
@@ skipping 18 matching lines @@
   /**
    * @param {string} searchText Input typed by the user into a search box.
    * @return {Array<string>} A list of terms extracted from |searchText|.
    */
   ActionService.splitTerms = function(searchText) {
     // Split quoted terms (e.g., 'The "lazy" dog' => ['The', 'lazy', 'dog']).
     return searchText.split(/"([^"]*)"/).map(trim).filter(truthy);
   };
 
   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'),
 
     /** Instructs the browser to clear all finished downloads. */
     clearAll: function() {
       if (loadTimeData.getBoolean('allowDeletingHistory')) {
         chrome.send('clearAll');
         this.search('');
       }
     },
 
     /** @param {string} id ID of the dangerous download to discard. */
     discardDangerous: chromeSendWithId('discardDangerous'),
 
     /** @param {string} url URL of a file to download. */
     download: function(url) {
       var a = document.createElement('a');
       a.href = url;
       a.setAttribute('download', '');
       a.click();
     },
 
     /** @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. */
     openDownloadsFolder: chrome.send.bind(chrome, 'openDownloadsFolder'),
 
     /**
      * @param {string} id ID of the download to run locally on the user's box.
      */
     openFile: chromeSendWithId('openFile'),
 
     /** @param {string} id ID the of the progressing download to pause. */
     pause: chromeSendWithId('pause'),
 
     /** @param {string} id ID of the finished download to remove. */
     remove: chromeSendWithId('remove'),
 
     /** @param {string} id ID of the paused download to resume. */
     resume: chromeSendWithId('resume'),
 
     /**
      * @param {string} id ID of the dangerous download to save despite
      *     warnings.
      */
     saveDangerous: chromeSendWithId('saveDangerous'),
 
     /** @param {string} searchText What to search for. */
     search: function(searchText) {
-      if (this.searchText_ == searchText)
+      var searchTerms = ActionService.splitTerms(searchText);
+      var sameTerms = searchTerms.length == this.searchTerms_.length;
+
+      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;
+      }
+
+      if (sameTerms)
         return;
 
-      this.searchText_ = searchText;
-
-      var terms = ActionService.splitTerms(searchText);
-      this.isSearching_ = terms.length > 0;
-
-      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();
     },
 
     /**
      * Shows the local folder a finished download resides in.
      * @param {string} id ID of the download to show.
      */
     show: chromeSendWithId('show'),
 
     /** Undo download removal. */
     undo: chrome.send.bind(chrome, 'undo'),
   };
 
   cr.addSingletonGetter(ActionService);
 
   return {ActionService: ActionService};
 });