From 03c419890f91d41b3e73042c36277e0ffb6b6a89 Mon Sep 17 00:00:00 2001 From: Harry Twyford Date: Tue, 12 May 2020 16:59:29 +0000 Subject: [PATCH] Bug 1626897 - Part 1 - Move SuggestionsFetch from PlacesSearchAutocompleteProvider to UrlbarProviderSearchSuggestions. r=mak Differential Revision: https://phabricator.services.mozilla.com/D74117 --- .../UrlbarProviderSearchSuggestions.jsm | 107 ++++++++----- .../PlacesSearchAutocompleteProvider.jsm | 144 ------------------ 2 files changed, 65 insertions(+), 186 deletions(-) diff --git a/browser/components/urlbar/UrlbarProviderSearchSuggestions.jsm b/browser/components/urlbar/UrlbarProviderSearchSuggestions.jsm index 3b8926fcfc84..0abdce7e72d0 100644 --- a/browser/components/urlbar/UrlbarProviderSearchSuggestions.jsm +++ b/browser/components/urlbar/UrlbarProviderSearchSuggestions.jsm @@ -17,6 +17,8 @@ XPCOMUtils.defineLazyModuleGetters(this, { Log: "resource://gre/modules/Log.jsm", PlacesSearchAutocompleteProvider: "resource://gre/modules/PlacesSearchAutocompleteProvider.jsm", + SearchSuggestionController: + "resource://gre/modules/SearchSuggestionController.jsm", Services: "resource://gre/modules/Services.jsm", UrlbarPrefs: "resource:///modules/UrlbarPrefs.jsm", UrlbarProvider: "resource:///modules/UrlbarUtils.jsm", @@ -271,7 +273,7 @@ class ProviderSearchSuggestions extends UrlbarProvider { } let alias = (aliasEngine && aliasEngine.alias) || ""; - let results = await this._matchSearchSuggestions( + let results = await this._fetchSearchSuggestions( queryContext, engine, query, @@ -305,74 +307,95 @@ class ProviderSearchSuggestions extends UrlbarProvider { cancelQuery(queryContext) { logger.info(`Canceling query for ${queryContext.searchString}`); - if (this._suggestionsFetch) { - this._suggestionsFetch.stop(); - this._suggestionsFetch = null; + if (this._suggestionsController) { + this._suggestionsController.stop(); + this._suggestionsController = null; } this.queries.delete(queryContext); } - async _matchSearchSuggestions(queryContext, engine, searchString, alias) { - this._suggestionsFetch = PlacesSearchAutocompleteProvider.newSuggestionsFetch( - engine, + async _fetchSearchSuggestions(queryContext, engine, searchString, alias) { + if (!engine || !searchString) { + return null; + } + + this._suggestionsController = new SearchSuggestionController(); + this._suggestionsController.maxLocalResults = UrlbarPrefs.get( + "maxHistoricalSearchSuggestions" + ); + this._suggestionsController.maxRemoteResults = + queryContext.maxResults - + UrlbarPrefs.get("maxHistoricalSearchSuggestions"); + + this._suggestionsFetchCompletePromise = this._suggestionsController.fetch( searchString, queryContext.isPrivate, - UrlbarPrefs.get("maxHistoricalSearchSuggestions"), - queryContext.maxResults - - UrlbarPrefs.get("maxHistoricalSearchSuggestions"), + engine, queryContext.userContextId ); - await this._suggestionsFetch.fetchCompletePromise; - try { - // The fetch has been canceled already. - if (!this._suggestionsFetch) { - return null; - } - // If we don't return many results, then keep track of the query. If the - // user just adds on to the query, we won't fetch more suggestions since - // we are unlikely to get any. - if ( - this._suggestionsFetch.resultsCount >= 0 && - this._suggestionsFetch.resultsCount < 2 - ) { - this._lastLowResultsSearchSuggestion = searchString; - } - let results = []; - let result; - while ((result = this._suggestionsFetch.consume())) { - if ( - !result || - result.suggestion == searchString || - looksLikeUrl(result.suggestion) - ) { - continue; - } + // See `SearchSuggestionsController.fetch` documentation for a description + // of `fetchData`. + let fetchData = await this._suggestionsFetchCompletePromise; + // The fetch was canceled. + if (!fetchData) { + return null; + } + + let suggestions = []; + suggestions.push( + ...fetchData.local.map(r => ({ suggestion: r, historical: true })), + ...fetchData.remote.map(r => ({ suggestion: r, historical: false })) + ); + + // If we don't return many results, then keep track of the query. If the + // user just adds on to the query, we won't fetch more suggestions since + // we are unlikely to get any. + if (suggestions.length >= 0 && suggestions.length < 2) { + this._lastLowResultsSearchSuggestion = searchString; + } + + let results = []; + for (let suggestion of suggestions) { + if ( + !suggestion || + suggestion.suggestion == searchString || + looksLikeUrl(suggestion.suggestion) + ) { + continue; + } + + try { results.push( new UrlbarResult( UrlbarUtils.RESULT_TYPE.SEARCH, UrlbarUtils.RESULT_SOURCE.SEARCH, ...UrlbarResult.payloadAndSimpleHighlights(queryContext.tokens, { engine: [engine.name, UrlbarUtils.HIGHLIGHT.TYPED], - suggestion: [result.suggestion, UrlbarUtils.HIGHLIGHT.SUGGESTED], + suggestion: [ + suggestion.suggestion, + UrlbarUtils.HIGHLIGHT.SUGGESTED, + ], keyword: [alias ? alias : undefined, UrlbarUtils.HIGHLIGHT.TYPED], query: [searchString.trim(), UrlbarUtils.HIGHLIGHT.NONE], isSearchHistory: false, icon: [ - engine.iconURI && !result.suggestion ? engine.iconURI.spec : "", + engine.iconURI && !suggestion.suggestion + ? engine.iconURI.spec + : "", ], keywordOffer: UrlbarUtils.KEYWORD_OFFER.NONE, }) ) ); + } catch (err) { + Cu.reportError(err); + continue; } - - return results; - } catch (err) { - Cu.reportError(err); - return null; } + + return results; } /** diff --git a/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm b/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm index 68403caa7a1c..bea245f550ef 100644 --- a/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm +++ b/toolkit/components/places/PlacesSearchAutocompleteProvider.jsm @@ -12,12 +12,6 @@ var EXPORTED_SYMBOLS = ["PlacesSearchAutocompleteProvider"]; const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm"); -ChromeUtils.defineModuleGetter( - this, - "SearchSuggestionController", - "resource://gre/modules/SearchSuggestionController.jsm" -); - const SEARCH_ENGINE_TOPIC = "browser-search-engine-modified"; const SearchAutocompleteProviderInternal = { @@ -110,101 +104,6 @@ const SearchAutocompleteProviderInternal = { ]), }; -class SuggestionsFetch { - /** - * Create a new instance of this class for each new suggestions fetch. - * - * @param {nsISearchEngine} engine - * The engine from which suggestions will be fetched. - * @param {string} searchString - * The search query string. - * @param {bool} inPrivateContext - * Pass true if the fetch is being done in a private window. - * @param {int} maxLocalResults - * The maximum number of results to fetch from the user's local - * history. - * @param {int} maxRemoteResults - * The maximum number of results to fetch from the search engine. - * @param {int} userContextId - * The user context ID in which the fetch is being performed. - */ - constructor( - engine, - searchString, - inPrivateContext, - maxLocalResults, - maxRemoteResults, - userContextId - ) { - this._controller = new SearchSuggestionController(); - this._controller.maxLocalResults = maxLocalResults; - this._controller.maxRemoteResults = maxRemoteResults; - this._engine = engine; - this._suggestions = []; - this._success = false; - this._promise = this._controller - .fetch(searchString, inPrivateContext, engine, userContextId) - .then(results => { - this._success = true; - if (results) { - this._suggestions.push( - ...results.local.map(r => ({ suggestion: r, historical: true })), - ...results.remote.map(r => ({ suggestion: r, historical: false })) - ); - } - }) - .catch(err => { - // fetch() rejects its promise if there's a pending request. - }); - } - - /** - * {nsISearchEngine} The engine from which suggestions are being fetched. - */ - get engine() { - return this._engine; - } - - /** - * {promise} Resolved when all suggestions have been fetched. - */ - get fetchCompletePromise() { - return this._promise; - } - - /** - * Returns one suggestion, if any are available, otherwise returns null. - * Note that may be multiple reasons why suggestions are not available: - * - all suggestions have already been consumed - * - the fetch failed - * - the fetch didn't complete yet (should have awaited the promise) - * - * @returns {object} An object { suggestion, historical } or null if no - * suggestions are available. - * - suggestion {string} The suggestion. - * - historical {bool} True if the suggestion comes from the user's - * local history (instead of the search engine). - */ - consume() { - return this._suggestions.shift() || null; - } - - /** - * Returns the number of fetched suggestions, or -1 if the fetching was - * incomplete or failed. - */ - get resultsCount() { - return this._success ? this._suggestions.length : -1; - } - - /** - * Stops the fetch. - */ - stop() { - this._controller.stop(); - } -} - var gInitializationPromise = null; var PlacesSearchAutocompleteProvider = Object.freeze({ @@ -324,47 +223,4 @@ var PlacesSearchAutocompleteProvider = Object.freeze({ } ); }, - - /** - * Starts a new suggestions fetch. - * - * @param {nsISearchEngine} engine - * The engine from which suggestions will be fetched. - * @param {string} searchString - * The search query string. - * @param {bool} inPrivateContext - * Pass true if the fetch is being done in a private window. - * @param {int} maxLocalResults - * The maximum number of results to fetch from the user's local - * history. - * @param {int} maxRemoteResults - * The maximum number of results to fetch from the search engine. - * @param {int} userContextId - * The user context ID in which the fetch is being performed. - * @returns {SuggestionsFetch} A new suggestions fetch object you should use - * to track the fetch. - */ - newSuggestionsFetch( - engine, - searchString, - inPrivateContext, - maxLocalResults, - maxRemoteResults, - userContextId - ) { - if (!SearchAutocompleteProviderInternal.initialized) { - throw new Error("The component has not been initialized."); - } - if (!engine) { - throw new Error("`engine` is null"); - } - return new SuggestionsFetch( - engine, - searchString, - inPrivateContext, - maxLocalResults, - maxRemoteResults, - userContextId - ); - }, });