MD Settings: Second iteration of search within settings.

chrome/browser/resources/settings/search_settings.js

Index: chrome/browser/resources/settings/search_settings.js
diff --git a/chrome/browser/resources/settings/search_settings.js b/chrome/browser/resources/settings/search_settings.js
index 53d5d22986b3799ea336f7df4233775d81e69604..33070b8621bf05e07a7463a7dee8152db64183df 100644
--- a/chrome/browser/resources/settings/search_settings.js
+++ b/chrome/browser/resources/settings/search_settings.js
@@ -2,6 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+/**
+ * @typedef {{
+ *   id: number,
+ *   rawQuery: ?string,
+ *   regExp: ?RegExp
+ * }}
+ */
+var SearchContext;
+
 cr.define('settings', function() {
   /** @const {string} */
   var WRAPPER_CSS_CLASS = 'search-highlight-wrapper';
@@ -97,18 +106,46 @@ cr.define('settings', function() {
   }
 
   /**
+   * Checks whether the given |node| requires force rendering and schedules an
+   * async RenderTask if necessary.
+   *
+   * @param {!SearchContext} context
+   * @param {!Node} node
+   * @return {boolean} Whether a forced rendering task was scheduled.
+   * @private
+   */
+  function forceRenderNeeded_(context, node) {
+    if (node.tagName != 'TEMPLATE' ||
+        node.getAttribute('name') === null ||
+        node.if) {
+      return false;
+    }
+
+    // TODO(dpapad): Temporarily ignore site-settings because it throws an
+    // assertion error during force-rendering.
+    if (node.getAttribute('name').indexOf('site-') != -1)
+      return false;
+
+    SearchManager.getInstance().queue_.addRenderTask(
+        new RenderTask(context, node));
+    return true;
+  }
+
+  /**
    * Traverses the entire DOM (including Shadow DOM), finds text nodes that
    * match the given regular expression and applies the highlight UI. It also
    * ensures that <settings-section> instances become visible if any matches
    * occurred under their subtree.
    *
-   * @param {!Element} page The page to be searched, should be either
-   *     <settings-basic-page> or <settings-advanced-page>.
-   * @param {!RegExp} regExp The regular expression to detect matches.
+   * @param {!SearchContext} context
+   * @param {!Node} root The root of the sub-tree to be searched
    * @private
    */
-  function findAndHighlightMatches_(page, regExp) {
+  function findAndHighlightMatches_(context, root) {
     function doSearch(node) {
+      if (forceRenderNeeded_(context, node))
+        return;
+
       if (IGNORED_ELEMENTS.has(node.tagName))
         return;
 
@@ -117,9 +154,9 @@ cr.define('settings', function() {
         if (textContent.length == 0)
           return;
 
-        if (regExp.test(textContent)) {
+        if (context.regExp.test(textContent)) {
           revealParentSection_(node);
-          highlight_(node, textContent.split(regExp));
+          highlight_(node, textContent.split(context.regExp));
         }
         // Returning early since TEXT_NODE nodes never have children.
         return;
@@ -139,7 +176,7 @@ cr.define('settings', function() {
         doSearch(shadowRoot);
     }
 
-    doSearch(page);
+    doSearch(root);
   }
 
   /**
@@ -158,7 +195,7 @@ cr.define('settings', function() {
   }
 
   /**
-   * @param {!Element} page
+   * @param {!Node} page
    * @param {boolean} visible
    * @private
    */
@@ -169,24 +206,235 @@ cr.define('settings', function() {
   }
 
   /**
-   * Performs hierarchical search, starting at the given page element.
+   * @constructor
+   *
+   * @param {!SearchContext} context
+   * @param {!Node} node
+   */
+  function Task(context, node) {
+    /** @protected {!SearchContext} */
+    this.context = context;
+
+    /** @protected {!Node} */
+    this.node = node;
+  }
+
+  Task.prototype = {
+    /**
+     * @abstract
+     * @return {!Promise}
+     */
+    exec: function() {},
+  };
+
+  /**
+   * @constructor
+   * @extends {Task}
+   *
+   * @param {!SearchContext} context
+   * @param {!Node} node
+   */
+  function RenderTask(context, node) {
+    Task.call(this, context, node);
+  }
+
+  RenderTask.prototype = {
+    /** @override */
+    exec: function() {
+      var subpageTemplate =
+          this.node['_content'].querySelector('settings-subpage');
+      subpageTemplate.id = subpageTemplate.id || this.node.getAttribute('name');
+      assert(!this.node.if);
+      this.node.if = true;
+
+      return this.getRenderedNode_(subpageTemplate.id).then(function(node) {
+        // Register a SearchTask for the part of the DOM that was just
+        // rendered.
+        SearchManager.getInstance().queue_.addSearchTask(
+            new SearchTask(this.context, node));
+      }.bind(this));
+    },
+
+    /**
+     * @param {string} id
+     * @return {!Promise<!Node>}
+     */
+    getRenderedNode_: function(id) {
+      var parent = this.node.parentNode;
+      return new Promise(function(resolve, reject) {
+        parent.async(function() {
+          resolve(parent.querySelector('#' + id));
+        });
+      });
+    },
+  };
+
+  /**
+   * @constructor
+   * @extends {Task}
+   *
+   * @param {!SearchContext} context
+   * @param {!Node} node
+   */
+  function SearchTask(context, node) {
+    Task.call(this, context, node);
+  }
+
+  SearchTask.prototype = {
+    /** @override */
+    exec: function() {
+      findAndHighlightMatches_(this.context, this.node);
+      return Promise.resolve();
+    },
+  };
+
+  /**
+   * @constructor
+   * @extends {Task}
+   *
+   * @param {!SearchContext} context
+   * @param {!Node} page
+   */
+  function TopLevelSearchTask(context, page) {
+    Task.call(this, context, page);
+  }
+
+  TopLevelSearchTask.prototype = {
+    /** @override */
+    exec: function() {
+      findAndRemoveHighlights_(this.node);
+
+      if (this.context.regExp === null) {
+        setSectionsVisibility_(this.node, true);
+        return Promise.resolve();
+      }
+
+      setSectionsVisibility_(this.node, false);
+      findAndHighlightMatches_(this.context, this.node);
+      return Promise.resolve();
+    },
+  };
+
+  /**
+   * @constructor
+   */
+  function TaskQueue() {
+    /**
+     * @private {{
+     *   high: !Array<!Task>,
+     *   middle: !Array<!Task>,
+     *   low: !Array<!Task>
+     * }}
+     */
+    this.queues_ = {high: [], middle: [], low: []};
+
+    /** @private {?Task} */
+    this.activeTask_ = null;

[Dan Beam, 2016/07/11 21:48:22]

i understand why it's helpful to use a task here, but i might prefer a simple
bool named |running_| or something, so that we discourage use of a global

I'd far prefer to access tasks in local/private state and not count on globals
still being around when we resume

[Dan Beam, 2016/07/12 00:08:41]

ping

[dpapad, 2016/07/12 02:20:27]

Done.

+  }
+
+  TaskQueue.prototype = {
+    /** @param {!TopLevelSearchTask} task */
+    addTopLevelSearchTask: function(task) {
+      this.queues_.high.push(task);
+      this.consumePending_();
+    },
+
+    /** @param {!SearchTask} task */
+    addSearchTask: function(task) {
+      this.queues_.middle.push(task);
+      this.consumePending_();
+    },
+
+    /** @param {!RenderTask} task */
+    addRenderTask: function(task) {
+      this.queues_.low.push(task);
+      this.consumePending_();
+    },
+
+    /**
+     * @return {?Task}
+     * @private
+     */
+    popNextTask_: function() {
+      return this.queues_.high.shift() ||
+          this.queues_.middle.shift() ||
+          this.queues_.low.shift() || null;
+    },
+
+    /** @private */
+    consumePending_: function() {
+      if (this.activeTask_)

[Dan Beam, 2016/07/11 21:48:22]

if (this.running_)
  return;

while (1) {
  var task = this.popNextTask_();
  if (!task) {
    this.running_ = false;  // Yes, I understand this is an extra step.
    return;
  }

  ...

  this.running_ = true;  // And so is this.
  window.requestIdleCallback(function() {
    // There's no way |task| could change here, unlike this.activeTask_.
    task.exec().then(this.consumePending_.bind(this));
  }.bind(this));
}

[Dan Beam, 2016/07/12 00:08:41]

that's not to say that we don't need to check its validity (as you discovered
;))
ping

[dpapad, 2016/07/12 02:20:27]

Done.

+        return;
+
+      var manager = SearchManager.getInstance();
+      while (1) {
+        this.activeTask_ = this.popNextTask_();
+        if (!this.activeTask_)
+          return;
+
+        if (this.activeTask_.context.id != manager.activeContext_.id) {
+          // Dropping this task without ever executing it, since a new search
+          // has been issued since this task was submitted.
+          continue;
+        }
+
+        window.requestIdleCallback(function() {
+          this.activeTask_.exec().then(function(result) {
+            this.activeTask_ = null;
+            this.consumePending_();
+          }.bind(this));
+        }.bind(this));
+        return;
+      }
+    },
+  };
+
+  /**
+   * @constructor
+   */
+  var SearchManager = function() {
+    /** @private {!TaskQueue} */
+    this.queue_ = new TaskQueue();
+
+    /** @private {!SearchContext} */
+    this.activeContext_ = {
+      id: 0, rawQuery: null, regExp: null,
+    };
+  };
+  cr.addSingletonGetter(SearchManager);
+
+  SearchManager.prototype = {
+    /**
+     * @param {string} text The text to search for.
+     * @param {!Node} page
+     */
+    search: function(text, page) {
+      if (this.activeContext_.rawQuery != text) {
+        var newId = this.activeContext_.id + 1;
+
+        var regExp = null;
+        // Generate search text by escaping any characters that would be
+        // problematic for regular expressions.
+        var searchText = text.trim().replace(SANITIZE_REGEX, '\\$&');
+        if (searchText.length > 0)
+          regExp = new RegExp('(' + searchText + ')', 'i');
+
+        this.activeContext_ = {
+          id: newId, rawQuery: text, regExp: regExp,
+        };
+      }
+
+      this.queue_.addTopLevelSearchTask(
+          new TopLevelSearchTask(this.activeContext_, page));
+    },
+  };
+
+  /**
    * @param {string} text
-   * @param {!Element} page Must be either <settings-basic-page> or
-   *     <settings-advanced-page>.
+   * @param {!Node} page
    */
   function search(text, page) {
-    findAndRemoveHighlights_(page);
-
-    // Generate search text by escaping any characters that would be problematic
-    // for regular expressions.
-    var searchText = text.trim().replace(SANITIZE_REGEX, '\\$&');
-    if (searchText.length == 0) {
-      setSectionsVisibility_(page, true);
-      return;
-    }
-
-    setSectionsVisibility_(page, false);
-    findAndHighlightMatches_(page, new RegExp('(' + searchText + ')', 'i'));
+    SearchManager.getInstance().search(text, page);
   }
 
   return {