From 9cd7ea9371a30e402e93c9258e7492da48ac3b82 Mon Sep 17 00:00:00 2001 From: "Jasper St. Pierre" Date: Sat, 2 Nov 2013 19:45:35 -0400 Subject: [PATCH] search: Make the internal search interface callback-based Long ago, the search system worked in a synchronous manner: providers were given a query, and results were collected in a single array of [provider, results] pairs, and then the search display was updated from that. We introduced an asynchronous search system when we wanted to potentially add a Zeitgeist search provider to the Shell in 3.2. For a while, search providers were either async or sync, which worked by storing a dummy array in the results, and adding a method for search providers to add results later. Later, we removed the search system entirely and ported the remaining search providers to simply use the API to modify the empty array, but the remains of the synchronous search system with its silly array still lingered. Finally, it's time to modernize. Promises^WCallbacks are the future. Port the one remaining in-shell search engine (app search) to the new callback based system, and simplify the remote search system in the process. --- js/ui/appDisplay.js | 8 +++--- js/ui/remoteSearch.js | 32 +++++++++------------ js/ui/search.js | 66 +++++++++++++++++++++---------------------- 3 files changed, 50 insertions(+), 56 deletions(-) diff --git a/js/ui/appDisplay.js b/js/ui/appDisplay.js index 7186d9f3d..0e9243238 100644 --- a/js/ui/appDisplay.js +++ b/js/ui/appDisplay.js @@ -890,12 +890,12 @@ const AppSearchProvider = new Lang.Class({ return results.slice(0, maxNumber); }, - getInitialResultSet: function(terms) { - this.searchSystem.setResults(this, this._appSys.initial_search(terms)); + getInitialResultSet: function(terms, callback, cancellable) { + callback(this._appSys.initial_search(terms)); }, - getSubsearchResultSet: function(previousResults, terms) { - this.searchSystem.setResults(this, this._appSys.subsearch(previousResults, terms)); + getSubsearchResultSet: function(terms, callback, cancellable) { + callbacl(this._appSys.subsearch(previousResults, terms)); }, activateResult: function(result) { diff --git a/js/ui/remoteSearch.js b/js/ui/remoteSearch.js index 520d109ab..e21416e06 100644 --- a/js/ui/remoteSearch.js +++ b/js/ui/remoteSearch.js @@ -203,8 +203,6 @@ const RemoteSearchProvider = new Lang.Class({ this.appInfo = appInfo; this.id = appInfo.get_id(); this.isRemoteProvider = true; - - this._cancellable = new Gio.Cancellable(); }, createIcon: function(size, meta) { @@ -234,29 +232,27 @@ const RemoteSearchProvider = new Lang.Class({ return regularResults.slice(0, maxNumber).concat(specialResults.slice(0, maxNumber)); }, - _getResultsFinished: function(results, error) { + _getResultsFinished: function(results, error, callback) { if (error) { if (!error.matches(Gio.IOErrorEnum, Gio.IOErrorEnum.CANCELLED)) log('Received error from DBus search provider %s: %s'.format(this.id, String(error))); - } else { - this.searchSystem.setResults(this, results[0]); + callback([]); + return; } + + callback(results[0]); }, - getInitialResultSet: function(terms) { - this._cancellable.cancel(); - this._cancellable.reset(); + getInitialResultSet: function(terms, callback, cancellable) { this.proxy.GetInitialResultSetRemote(terms, - Lang.bind(this, this._getResultsFinished), - this._cancellable); + Lang.bind(this, this._getResultsFinished, callback), + cancellable); }, - getSubsearchResultSet: function(previousResults, newTerms) { - this._cancellable.cancel(); - this._cancellable.reset(); + getSubsearchResultSet: function(previousResults, newTerms, cancellable) { this.proxy.GetSubsearchResultSetRemote(previousResults, newTerms, - Lang.bind(this, this._getResultsFinished), - this._cancellable); + Lang.bind(this, this._getResultsFinished, callback), + cancellable); }, _getResultMetasFinished: function(results, error, callback) { @@ -284,12 +280,10 @@ const RemoteSearchProvider = new Lang.Class({ callback(resultMetas); }, - getResultMetas: function(ids, callback) { - this._cancellable.cancel(); - this._cancellable.reset(); + getResultMetas: function(ids, callback, cancellable) { this.proxy.GetResultMetasRemote(ids, Lang.bind(this, this._getResultMetasFinished, callback), - this._cancellable); + cancellable); }, activateResult: function(id) { diff --git a/js/ui/search.js b/js/ui/search.js index b55b76a94..1b2953a98 100644 --- a/js/ui/search.js +++ b/js/ui/search.js @@ -37,6 +37,8 @@ const SearchSystem = new Lang.Class({ this._searchSettings.connect('changed::sort-order', Lang.bind(this, this._reloadRemoteProviders)); this._reloadRemoteProviders(); + + this._cancellable = new Gio.Cancellable(); }, addProvider: function(provider) { @@ -60,14 +62,12 @@ const SearchSystem = new Lang.Class({ }, _registerProvider: function (provider) { - provider.searchSystem = this; this._providers.push(provider); }, _unregisterProvider: function (provider) { let index = this._providers.indexOf(provider); this._providers.splice(index, 1); - provider.searchSystem = null; }, getProviders: function() { @@ -75,54 +75,50 @@ const SearchSystem = new Lang.Class({ }, getTerms: function() { - return this._previousTerms; + return this._terms; }, reset: function() { - this._previousTerms = []; - this._previousResults = []; + this._terms = []; + this._results = {}; }, - setResults: function(provider, results) { - let i = this._providers.indexOf(provider); - if (i == -1) - return; - - this._previousResults[i] = [provider, results]; - this.emit('search-updated', this._previousResults[i]); + _gotResults: function(results, provider) { + this._results[provider.id] = results; + this.emit('search-updated', provider, results); }, setTerms: function(terms) { + this._cancellable.cancel(); + this._cancellable.reset(); + + let previousResults = this._results; + let previousTerms = this._terms; + this.reset(); + if (!terms) return; let searchString = terms.join(' '); - let previousSearchString = this._previousTerms.join(' '); + let previousSearchString = previousTerms.join(' '); if (searchString == previousSearchString) return; let isSubSearch = false; - if (this._previousTerms.length > 0) + if (previousTerms.length > 0) isSubSearch = searchString.indexOf(previousSearchString) == 0; - let previousResultsArr = this._previousResults; - - let results = []; - this._previousTerms = terms; - this._previousResults = results; + this._terms = terms; if (isSubSearch) { - for (let i = 0; i < this._providers.length; i++) { - let [provider, previousResults] = previousResultsArr[i]; - results.push([provider, []]); - provider.getSubsearchResultSet(previousResults, terms); - } + this._providers.forEach(Lang.bind(this, function(provider) { + let previousResults = previousResults[provider.id]; + provider.getSubsearchResultSet(previousResults, terms, Lang.bind(this, this._gotResults, provider), this._cancellable); + })); } else { - for (let i = 0; i < this._providers.length; i++) { - let provider = this._providers[i]; - results.push([provider, []]); - provider.getInitialResultSet(terms); - } + this._providers.forEach(Lang.bind(this, function(provider) { + provider.getInitialResultSet(terms, Lang.bind(this, this._gotResults, provider), this._cancellable); + })); } } }); @@ -309,6 +305,8 @@ const SearchResultsBase = new Lang.Class({ this.actor.add(separator.actor); this._resultDisplays = {}; + + this._cancellable = new Gio.Cancellable(); }, destroy: function() { @@ -345,6 +343,9 @@ const SearchResultsBase = new Lang.Class({ if (metasNeeded.length === 0) { callback(); } else { + this._cancellable.cancel(); + this._cancellable.reset(); + this.provider.getResultMetas(metasNeeded, Lang.bind(this, function(metas) { metasNeeded.forEach(Lang.bind(this, function(resultId, i) { let meta = metas[i]; @@ -354,7 +355,7 @@ const SearchResultsBase = new Lang.Class({ this._resultDisplays[resultId] = display; })); callback(); - })); + }), this._cancellable); } }, @@ -638,12 +639,11 @@ const SearchResults = new Lang.Class({ } }, - _updateResults: function(searchSystem, results) { + _updateResults: function(searchSystem, provider, results) { let terms = searchSystem.getTerms(); - let [provider, providerResults] = results; let display = provider.display; - display.updateSearch(providerResults, terms, Lang.bind(this, function() { + display.updateSearch(results, terms, Lang.bind(this, function() { this._maybeSetInitialSelection(); this._updateStatusText(); }));