Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1964)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/resources/settings/search_settings.js

Issue 2103133007: MD Settings: Second iteration of search within settings. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@cr_search_migration1
Patch Set: Addressing comments. Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..760219222db4575a2a599fad7b45f4dc142086f9 100644
--- a/chrome/browser/resources/settings/search_settings.js
+++ b/chrome/browser/resources/settings/search_settings.js
@@ -2,6 +2,11 @@
// 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}}
+ */
Dan Beam 2016/07/13 01:26:12 /** ... */ is actually 1 line ;)
dpapad 2016/07/13 02:27:01 Done.
+var SearchContext;
+
cr.define('settings', function() {
/** @const {string} */
var WRAPPER_CSS_CLASS = 'search-highlight-wrapper';
@@ -9,9 +14,6 @@ cr.define('settings', function() {
/** @const {string} */
var HIT_CSS_CLASS = 'search-highlight-hit';
- /** @const {!RegExp} */
- var SANITIZE_REGEX = /[-[\]{}()*+?.,\\^$|#\s]/g;
-
/**
* List of elements types that should not be searched at all.
* The only DOM-MODULE node is in <body> which is not searched, therefore
@@ -40,6 +42,7 @@ cr.define('settings', function() {
/**
* Finds all previous highlighted nodes under |node| (both within self and
* children's Shadow DOM) and removes the highlight (yellow rectangle).
+ * TODO(dpapad): Consider making this a private method of TopLevelSearchTask.
* @param {!Node} node
* @private
*/
@@ -97,19 +100,41 @@ cr.define('settings', function() {
}
/**
+ * Checks whether the given |node| requires force rendering.
+ *
+ * @param {!SearchContext} context
+ * @param {!Node} node
+ * @return {boolean} Whether a forced rendering task was scheduled.
+ * @private
+ */
+ function forceRenderNeeded_(context, node) {
+ if (node.nodeName != 'TEMPLATE' || !node.hasAttribute('name') || node.if)
+ return false;
+
+ // TODO(dpapad): Temporarily ignore site-settings because it throws an
+ // assertion error during force-rendering.
+ return node.getAttribute('name').indexOf('site-') != 0;
+ }
+
+ /**
* 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 (IGNORED_ELEMENTS.has(node.tagName))
+ if (forceRenderNeeded_(context, node)) {
+ SearchManager.getInstance().queue_.addRenderTask(
+ new RenderTask(context, node));
+ return;
+ }
+
+ if (IGNORED_ELEMENTS.has(node.nodeName))
return;
if (node.nodeType == Node.TEXT_NODE) {
@@ -117,9 +142,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 +164,7 @@ cr.define('settings', function() {
doSearch(shadowRoot);
}
- doSearch(page);
+ doSearch(root);
}
/**
@@ -149,7 +174,7 @@ cr.define('settings', function() {
function revealParentSection_(node) {
// Find corresponding SETTINGS-SECTION parent and make it visible.
var parent = node;
- while (parent && parent.tagName !== 'SETTINGS-SECTION') {
+ while (parent && parent.nodeName !== 'SETTINGS-SECTION') {
parent = parent.nodeType == Node.DOCUMENT_FRAGMENT_NODE ?
parent.host : parent.parentNode;
}
@@ -158,35 +183,271 @@ cr.define('settings', function() {
}
/**
- * @param {!Element} page
- * @param {boolean} visible
- * @private
+ * @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() {},
+ };
+
+ /**
+ * A task that takes a <template is="dom-if">...</template> node corresponding
+ * to a setting subpage and renders it. A SearchTask is posted for the newly
+ * rendered subtree, once rendering is done.
+ * @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 new Promise(function(resolve, reject) {
+ var parent = this.node.parentNode;
+ parent.async(function() {
+ var renderedNode = parent.querySelector('#' + subpageTemplate.id);
+ // Register a SearchTask for the part of the DOM that was just
+ // rendered.
+ SearchManager.getInstance().queue_.addSearchTask(
+ new SearchTask(this.context, assert(renderedNode)));
+ resolve();
+ }.bind(this));
+ }.bind(this));
+ },
+ };
+
+ /**
+ * @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);
+
+ var shouldSearch = this.context.regExp !== null;
+ this.setSectionsVisibility_(!shouldSearch);
+ if (shouldSearch)
+ findAndHighlightMatches_(this.context, this.node);
+
+ return Promise.resolve();
+ },
+
+ /**
+ * @param {boolean} visible
+ * @private
+ */
+ setSectionsVisibility_: function(visible) {
+ var sections = Polymer.dom(
+ this.node.root).querySelectorAll('settings-section');
+ for (var i = 0; i < sections.length; i++)
+ sections[i].hidden = !visible;
+ },
+ };
+
+ /**
+ * @constructor
*/
- function setSectionsVisibility_(page, visible) {
- var sections = Polymer.dom(page.root).querySelectorAll('settings-section');
- for (var i = 0; i < sections.length; i++)
- sections[i].hidden = !visible;
+ function TaskQueue() {
+ /**
+ * @private {{
+ * high: !Array<!Task>,
+ * middle: !Array<!Task>,
+ * low: !Array<!Task>
+ * }}
+ */
+ this.queues_ = {high: [], middle: [], low: []};
+
+ /**
+ * Whether a task is currently running.
+ * @private {boolean}
+ */
+ this.running_ = false;
}
+ TaskQueue.prototype = {
+ /** Drops all tasks. */
+ reset: function() {
+ this.queues_.high.length = 0;
+ this.queues_.middle.length = 0;
+ this.queues_.low.length = 0;
Dan Beam 2016/07/13 01:26:12 or just make this impl: this.queues_ = {high: [],
dpapad 2016/07/13 02:27:01 Done.
+ },
+
+ /** @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}
Dan Beam 2016/07/13 01:26:12 isn't this technically !Task|undefined now?
dpapad 2016/07/13 02:27:01 Done. As said, either way compiler did not complai
+ * @private
+ */
+ popNextTask_: function() {
+ return this.queues_.high.shift() ||
+ this.queues_.middle.shift() ||
+ this.queues_.low.shift();
+ },
+
+ /**
+ * @param {!Task} task
+ * @return {boolean} Whether the given task is now obsolete (refers to a
+ * previous search query).
+ * @private
+ */
+ isTaskObsolete_: function(task) {
+ return task.context.id != SearchManager.getInstance().activeContext_.id;
+ },
+
+ /** @private */
+ consumePending_: function() {
+ if (this.running_)
+ return;
+
+ while (1) {
+ var task = this.popNextTask_();
+ if (!task) {
+ this.running_ = false;
+ return;
+ }
+
+ if (this.isTaskObsolete_(task)) {
Dan Beam 2016/07/13 01:26:12 when will this happen?
dpapad 2016/07/13 02:27:01 Now that we are clearing the queues immediately, t
+ // Dropping this task without ever executing it, since a new search
+ // has been issued since this task was queued.
+ continue;
+ }
+
+ window.requestIdleCallback(function() {
+ // Need to check if this task is obsolete again, since a new search
+ // might have been issued after requestIdleCallback() was called.
+ if (this.isTaskObsolete_(assert(task))) {
+ this.running_ = false;
+ this.consumePending_();
+ } else {
+ task.exec().then(function(result) {
+ this.running_ = false;
+ this.consumePending_();
+ }.bind(this));
+ }
Dan Beam 2016/07/13 01:26:12 function startNextTask() { this.running_ = false
dpapad 2016/07/13 02:27:01 Done (kept isTaskObsolete() though).
+ }.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);
+
+ /** @private @const {!RegExp} */
+ SearchManager.SANITIZE_REGEX_ = /[-[\]{}()*+?.,\\^$|#\s]/g;
+
+ SearchManager.prototype = {
+ /**
+ * @param {string} text The text to search for.
+ * @param {!Node} page
+ */
+ search: function(text, page) {
+ if (this.activeContext_.rawQuery != text) {
Dan Beam 2016/07/13 01:28:18 i'm still confused: you said that we'll never get
dpapad 2016/07/13 02:27:01 search() is called twice for every user initiated
+ 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(
+ SearchManager.SANITIZE_REGEX_, '\\$&');
+ if (searchText.length > 0)
+ regExp = new RegExp('(' + searchText + ')', 'i');
+
+ this.activeContext_ = {id: newId, rawQuery: text, regExp: regExp};
+
+ // Drop all previously scheduled tasks, since a new search was just
+ // issued.
+ this.queue_.reset();
+ }
+
+ this.queue_.addTopLevelSearchTask(
+ new TopLevelSearchTask(this.activeContext_, page));
+ },
+ };
+
/**
- * Performs hierarchical search, starting at the given page element.
* @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 {
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698