From a8fd158ccd48778ce004c3cb76d6bfefd7ba755d Mon Sep 17 00:00:00 2001 From: Marco Bonardo Date: Thu, 27 Jun 2013 12:43:11 -0400 Subject: [PATCH] Bug 791776 - Convert inline autocomplete host query to async. r=mano --- .../test/browser_urlbarAutoFillTrimURLs.js | 32 ++- .../components/places/nsPlacesAutoComplete.js | 259 +++++++++--------- 2 files changed, 153 insertions(+), 138 deletions(-) diff --git a/browser/base/content/test/browser_urlbarAutoFillTrimURLs.js b/browser/base/content/test/browser_urlbarAutoFillTrimURLs.js index 3219898a980..80f27099e74 100644 --- a/browser/base/content/test/browser_urlbarAutoFillTrimURLs.js +++ b/browser/base/content/test/browser_urlbarAutoFillTrimURLs.js @@ -42,21 +42,21 @@ function continue_test() { gURLBar.selectionEnd = aTyped.length - 1; EventUtils.synthesizeKey(aTyped.substr(-1), {}); - is(gURLBar.value, aExpected, "trim was applied correctly"); - - aCallback(); + waitForSearchComplete(function () { + is(gURLBar.value, aExpected, "trim was applied correctly"); + aCallback(); + }); } test_autoFill("http://", "http://", function () { test_autoFill("http://a", "http://autofilltrimurl.com/", function () { test_autoFill("http://www.autofilltrimurl.com", "http://www.autofilltrimurl.com/", function () { // Now ensure selecting from the popup correctly trims. - waitForSearchComplete(function () { - EventUtils.synthesizeKey("VK_DOWN", {}); - is(gURLBar.value, "www.autofilltrimurl.com", "trim was applied correctly"); - gURLBar.closePopup(); - waitForClearHistory(finish); - }); + is(gURLBar.controller.matchCount, 1, "Found the expected number of matches"); + EventUtils.synthesizeKey("VK_DOWN", {}); + is(gURLBar.value, "www.autofilltrimurl.com", "trim was applied correctly"); + gURLBar.closePopup(); + waitForClearHistory(finish); }); }); }); @@ -70,16 +70,18 @@ function waitForClearHistory(aCallback) { PlacesUtils.bhistory.removeAllPages(); } +let gOnSearchComplete = null; function waitForSearchComplete(aCallback) { info("Waiting for onSearchComplete"); - let onSearchComplete = gURLBar.onSearchComplete; - registerCleanupFunction(function () { - gURLBar.onSearchComplete = onSearchComplete; - }); + if (!gOnSearchComplete) { + gOnSearchComplete = gURLBar.onSearchComplete; + registerCleanupFunction(() => { + gURLBar.onSearchComplete = gOnSearchComplete; + }); + } gURLBar.onSearchComplete = function () { ok(gURLBar.popupOpen, "The autocomplete popup is correctly open"); - is(gURLBar.controller.matchCount, 1, "Found the expected number of matches") - onSearchComplete.apply(gURLBar); + gOnSearchComplete.apply(gURLBar); aCallback(); } } diff --git a/toolkit/components/places/nsPlacesAutoComplete.js b/toolkit/components/places/nsPlacesAutoComplete.js index 57e10ac8d1f..27b0b60679b 100644 --- a/toolkit/components/places/nsPlacesAutoComplete.js +++ b/toolkit/components/places/nsPlacesAutoComplete.js @@ -209,14 +209,17 @@ function safePrefGetter(aPrefBranch, aName, aDefault) { * Wraps a callback and ensures that handleCompletion is not dispatched if the * query is no longer tracked. * - * @param aCallback + * @param aAutocomplete * A reference to a nsPlacesAutoComplete. + * @param aCallback + * A reference to a mozIStorageStatementCallback * @param aDBConnection * The database connection to execute the queries on. */ -function AutoCompleteStatementCallbackWrapper(aCallback, +function AutoCompleteStatementCallbackWrapper(aAutocomplete, aCallback, aDBConnection) { + this._autocomplete = aAutocomplete; this._callback = aCallback; this._db = aDBConnection; } @@ -239,9 +242,9 @@ AutoCompleteStatementCallbackWrapper.prototype = { { // Only dispatch handleCompletion if we are not done searching and are a // pending search. - let callback = this._callback; - if (!callback.isSearchComplete() && callback.isPendingSearch(this._handle)) { - callback.handleCompletion.apply(callback, arguments); + if (!this._autocomplete.isSearchComplete() && + this._autocomplete.isPendingSearch(this._handle)) { + this._callback.handleCompletion.apply(this._callback, arguments); } }, @@ -273,6 +276,7 @@ AutoCompleteStatementCallbackWrapper.prototype = { //////////////////////////////////////////////////////////////////////////////// //// nsPlacesAutoComplete class +//// @mozilla.org/autocomplete/search;1?name=history function nsPlacesAutoComplete() { @@ -761,7 +765,7 @@ nsPlacesAutoComplete.prototype = { // handleCompletion implementation of AutoCompleteStatementCallbackWrapper). // Create our wrapper object and execute the queries. - let wrapper = new AutoCompleteStatementCallbackWrapper(this, this._db); + let wrapper = new AutoCompleteStatementCallbackWrapper(this, this, this._db); this._pendingQuery = wrapper.executeAsync(aQueries); }, @@ -1255,6 +1259,7 @@ nsPlacesAutoComplete.prototype = { //////////////////////////////////////////////////////////////////////////////// //// urlInlineComplete class +//// component @mozilla.org/autocomplete/search;1?name=urlinline function urlInlineComplete() { @@ -1277,14 +1282,14 @@ urlInlineComplete.prototype = { return this.__db; }, - __syncQuery: null, + __hostQuery: null, - get _syncQuery() + get _hostQuery() { - if (!this.__syncQuery) { + if (!this.__hostQuery) { // Add a trailing slash at the end of the hostname, since we always // want to complete up to and including a URL separator. - this.__syncQuery = this._db.createStatement( + this.__hostQuery = this._db.createAsyncStatement( "/* do not warn (bug no): could index on (typed,frecency) but not worth it */ " + "SELECT host || '/', prefix || host || '/' " + "FROM moz_hosts " @@ -1295,15 +1300,15 @@ urlInlineComplete.prototype = { + "LIMIT 1" ); } - return this.__syncQuery; + return this.__hostQuery; }, - __asyncQuery: null, + __urlQuery: null, - get _asyncQuery() + get _urlQuery() { - if (!this.__asyncQuery) { - this.__asyncQuery = this._db.createAsyncStatement( + if (!this.__urlQuery) { + this.__urlQuery = this._db.createAsyncStatement( "/* do not warn (bug no): can't use an index */ " + "SELECT h.url " + "FROM moz_places h " @@ -1317,7 +1322,7 @@ urlInlineComplete.prototype = { + "LIMIT 1" ); } - return this.__asyncQuery; + return this.__urlQuery; }, ////////////////////////////////////////////////////////////////////////////// @@ -1336,18 +1341,17 @@ urlInlineComplete.prototype = { this._originalSearchString = aSearchString; this._currentSearchString = fixupSearchText(this._originalSearchString.toLowerCase()); - // The protocol and the domain are lowercased by nsIURI, so it's fine to + // The protocol and the host are lowercased by nsIURI, so it's fine to // lowercase the typed prefix to add it back to the results later. this._strippedPrefix = this._originalSearchString.slice( 0, this._originalSearchString.length - this._currentSearchString.length ).toLowerCase(); - let result = Cc["@mozilla.org/autocomplete/simple-result;1"]. - createInstance(Ci.nsIAutoCompleteSimpleResult); - result.setSearchString(aSearchString); - result.setTypeAheadResult(true); + this._result = Cc["@mozilla.org/autocomplete/simple-result;1"]. + createInstance(Ci.nsIAutoCompleteSimpleResult); + this._result.setSearchString(aSearchString); + this._result.setTypeAheadResult(true); - this._result = result; this._listener = aListener; // Don't autoFill if the search term is recognized as a keyword, otherwise @@ -1369,61 +1373,75 @@ urlInlineComplete.prototype = { return; } - // Do a synchronous search on the table of domains. - let query = this._syncQuery; - query.params.search_string = this._currentSearchString.toLowerCase(); - - // Domains have no "/" in them. + // Hosts have no "/" in them. let lastSlashIndex = this._currentSearchString.lastIndexOf("/"); - if (lastSlashIndex == -1) { - var hasDomainResult = false; - var domain, untrimmedDomain; - TelemetryStopwatch.start(DOMAIN_QUERY_TELEMETRY); - try { - // Execute the query synchronously. - // This is by design, to avoid race conditions between the - // user typing and the connection searching for the result. - hasDomainResult = query.executeStep(); - if (hasDomainResult) { - domain = query.getString(0); - untrimmedDomain = query.getString(1); - } - } finally { - query.reset(); - } - TelemetryStopwatch.finish(DOMAIN_QUERY_TELEMETRY); - if (hasDomainResult) { - // We got a match for a domain, we can add it immediately. + let maybeSearchURL = () => { + // Search for URLs if there's a slash and it's not at the end of the + // search string. + if (lastSlashIndex < this._currentSearchString.length - 1) + this._queryURL(); + else + this._finishSearch(); + }; + + // Search hosts only if there's no slash. + if (lastSlashIndex != -1) { + maybeSearchURL(); + return; + } + + // Do a synchronous search on the table of hosts. + let query = this._hostQuery; + query.params.search_string = this._currentSearchString.toLowerCase(); + // This is just to measure the delay to reach the UI, not the query time. + TelemetryStopwatch.start(DOMAIN_QUERY_TELEMETRY); + let ac = this; + let wrapper = new AutoCompleteStatementCallbackWrapper(this, { + _hasResult: false, + handleResult: function (aResultSet) { + this._hasResult = true; + let row = aResultSet.getNextRow(); + let trimmedHost = row.getResultByIndex(0); + let untrimmedHost = row.getResultByIndex(1); // If the untrimmed value doesn't preserve the user's input just - // ignore it and complete to the found domain. - if (untrimmedDomain && - !untrimmedDomain.toLowerCase().contains(this._originalSearchString.toLowerCase())) { - untrimmedDomain = null; + // ignore it and complete to the found host. + if (untrimmedHost && + !untrimmedHost.toLowerCase().contains(ac._originalSearchString.toLowerCase())) { + untrimmedHost = null; } // TODO (bug 754265): this is a temporary solution introduced while // waiting for a propert dedicated API. - result.appendMatch(this._strippedPrefix + domain, untrimmedDomain); + ac._result.appendMatch(ac._strippedPrefix + trimmedHost, untrimmedHost); - this._finishSearch(); - return; + // handleCompletion() will cause the result listener to be called, and + // will display the result in the UI. + }, + + handleError: function (aError) { + Components.utils.reportError( + "URL Inline Complete: An async statement encountered an " + + "error: " + aError.result + ", '" + aError.message + "'"); + }, + + handleCompletion: function (aReason) { + TelemetryStopwatch.finish(DOMAIN_QUERY_TELEMETRY); + if (this._hasResult) + ac._finishSearch(); + else + maybeSearchURL(); } - } - - // We did not get a result from the synchronous domain search. - // We now do an asynchronous search through places, and complete - // up to the next URL separator. - - // First, check if this is necessary. - // We don't need to search if we have no "/" separator, or if it's at - // the end of the search text. - if (lastSlashIndex == -1 || - lastSlashIndex == this._currentSearchString.length - 1) { - this._finishSearch(); - return; - } + }, this._db); + this._pendingQuery = wrapper.executeAsync([query]); + }, + /** + * Execute an asynchronous search through places, and complete + * up to the next URL separator. + */ + _queryURL: function UIC__queryURL() + { // The URIs in the database are fixed up, so we can match on a lowercased // host, but the path must be matched in a case sensitive way. let pathIndex = @@ -1435,15 +1453,61 @@ urlInlineComplete.prototype = { // Within the standard autocomplete query, we only search the beginning // of URLs for 1 result. - let query = this._asyncQuery; + let query = this._urlQuery; let (params = query.params) { params.matchBehavior = MATCH_BEGINNING_CASE_SENSITIVE; params.searchBehavior = Ci.mozIPlacesAutoComplete["BEHAVIOR_URL"]; params.searchString = this._currentSearchString; } - // Execute the async query - let wrapper = new AutoCompleteStatementCallbackWrapper(this, this._db); + // Execute the query. + let ac = this; + let wrapper = new AutoCompleteStatementCallbackWrapper(this, { + handleResult: function(aResultSet) { + let row = aResultSet.getNextRow(); + let value = row.getResultByIndex(0); + let url = fixupSearchText(value); + + let prefix = value.slice(0, value.length - stripPrefix(value).length); + + // We must complete the URL up to the next separator (which is /, ? or #). + let separatorIndex = url.slice(ac._currentSearchString.length) + .search(/[\/\?\#]/); + if (separatorIndex != -1) { + separatorIndex += ac._currentSearchString.length; + if (url[separatorIndex] == "/") { + separatorIndex++; // Include the "/" separator + } + url = url.slice(0, separatorIndex); + } + + // Add the result. + // If the untrimmed value doesn't preserve the user's input just + // ignore it and complete to the found url. + let untrimmedURL = prefix + url; + if (untrimmedURL && + !untrimmedURL.toLowerCase().contains(ac._originalSearchString.toLowerCase())) { + untrimmedURL = null; + } + + // TODO (bug 754265): this is a temporary solution introduced while + // waiting for a propert dedicated API. + ac._result.appendMatch(ac._strippedPrefix + url, untrimmedURL); + + // handleCompletion() will cause the result listener to be called, and + // will display the result in the UI. + }, + + handleError: function(aError) { + Components.utils.reportError( + "URL Inline Complete: An async statement encountered an " + + "error: " + aError.result + ", '" + aError.message + "'"); + }, + + handleCompletion: function(aReason) { + ac._finishSearch(); + } + }, this._db); this._pendingQuery = wrapper.executeAsync([query]); }, @@ -1489,56 +1553,6 @@ urlInlineComplete.prototype = { //// nsIAutoCompleteSearchDescriptor get searchType() Ci.nsIAutoCompleteSearchDescriptor.SEARCH_TYPE_IMMEDIATE, - ////////////////////////////////////////////////////////////////////////////// - //// mozIStorageStatementCallback - - handleResult: function UIC_handleResult(aResultSet) - { - let row = aResultSet.getNextRow(); - let value = row.getResultByIndex(0); - let url = fixupSearchText(value); - - let prefix = value.slice(0, value.length - stripPrefix(value).length); - - // We must complete the URL up to the next separator (which is /, ? or #). - let separatorIndex = url.slice(this._currentSearchString.length) - .search(/[\/\?\#]/); - if (separatorIndex != -1) { - separatorIndex += this._currentSearchString.length; - if (url[separatorIndex] == "/") { - separatorIndex++; // Include the "/" separator - } - url = url.slice(0, separatorIndex); - } - - // Add the result. - // If the untrimmed value doesn't preserve the user's input just - // ignore it and complete to the found url. - let untrimmedURL = prefix + url; - if (untrimmedURL && - !untrimmedURL.toLowerCase().contains(this._originalSearchString.toLowerCase())) { - untrimmedURL = null; - } - - // TODO (bug 754265): this is a temporary solution introduced while - // waiting for a propert dedicated API. - this._result.appendMatch(this._strippedPrefix + url, untrimmedURL); - - // handleCompletion() will cause the result listener to be called, and - // will display the result in the UI. - }, - - handleError: function UIC_handleError(aError) - { - Components.utils.reportError("URL Inline Complete: An async statement encountered an " + - "error: " + aError.result + ", '" + aError.message + "'"); - }, - - handleCompletion: function UIC_handleCompletion(aReason) - { - this._finishSearch(); - }, - ////////////////////////////////////////////////////////////////////////////// //// nsIObserver @@ -1571,8 +1585,8 @@ urlInlineComplete.prototype = { { // Finalize the statements that we have used. let stmts = [ - "__syncQuery", - "__asyncQuery", + "__hostQuery", + "__urlQuery", ]; for (let i = 0; i < stmts.length; i++) { // We do not want to create any query we haven't already created, so @@ -1636,7 +1650,6 @@ urlInlineComplete.prototype = { QueryInterface: XPCOMUtils.generateQI([ Ci.nsIAutoCompleteSearch, Ci.nsIAutoCompleteSearchDescriptor, - Ci.mozIStorageStatementCallback, Ci.nsIObserver, Ci.nsISupportsWeakReference, ])