[Merge] ~3v1n0/ubuntu/+source/gnome-shell:ubuntu/master-xubuntu-cancel-search into ~ubuntu-desktop/ubuntu/+source/gnome-shell:ubuntu/master

[Didier Roche]

Review: Needs Information

Ok, this looks good to me, see some of my questions.

I saw that you made some changes on the MR after Florian's comment on your patches. Is there anything you can borrow and refresh here?

See as well my 2 questions/comments

Diff comments:

> diff --git a/debian/patches/ubuntu/search-call-Cancel-method-on-providers-when-no-data-is-ne.patch b/debian/patches/ubuntu/search-call-Cancel-method-on-providers-when-no-data-is-ne.patch
> new file mode 100644
> index 0000000..de98f8e
> --- /dev/null
> +++ b/debian/patches/ubuntu/search-call-Cancel-method-on-providers-when-no-data-is-ne.patch
> @@ -0,0 +1,160 @@
> +From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail at 3v1n0.net>
> +Date: Thu, 23 Aug 2018 20:00:57 +0200
> +Subject: search: call Cancel method on providers when no data is needed
> +
> +Add XUbuntuCancel method to search providers and call it when a search provider
> +is still doing operations.
> +
> +This will allow to stop operations on providers.
> +
> +Fixes LP: #1756826
> +
> +Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
> +Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/gnome-shell/+bug/1756826
> +Forwarded: not-needed
> +---
> + data/org.gnome.ShellSearchProvider.xml  |  6 ++++++
> + data/org.gnome.ShellSearchProvider2.xml |  6 ++++++
> + js/ui/remoteSearch.js                   | 13 +++++++++++++
> + js/ui/search.js                         | 31 +++++++++++++++++++++++++++++++
> + 4 files changed, 56 insertions(+)
> +
> +diff --git a/data/org.gnome.ShellSearchProvider.xml b/data/org.gnome.ShellSearchProvider.xml
> +index 78ad305..393cb01 100644
> +--- a/data/org.gnome.ShellSearchProvider.xml
> ++++ b/data/org.gnome.ShellSearchProvider.xml
> +@@ -69,5 +69,11 @@
> +     <method name="ActivateResult">
> +       <arg type="s" name="identifier" direction="in" />
> +     </method>
> ++
> ++    <!--
> ++        XUbuntuCancel:
> ++        Cancel the current search operation
> ++    -->
> ++    <method name="XUbuntuCancel" />
> +   </interface>
> + </node>
> +diff --git a/data/org.gnome.ShellSearchProvider2.xml b/data/org.gnome.ShellSearchProvider2.xml
> +index 9502340..8141bc0 100644
> +--- a/data/org.gnome.ShellSearchProvider2.xml
> ++++ b/data/org.gnome.ShellSearchProvider2.xml

Stupid question, but do we use the API XubuntuCancel() on the 2 search provider interface version?

> +@@ -83,5 +83,11 @@
> +       <arg type="as" name="terms" direction="in" />
> +       <arg type="u" name="timestamp" direction="in" />
> +     </method>
> ++
> ++    <!--
> ++        XUbuntuCancel:
> ++        Cancel the current search operation
> ++    -->
> ++    <method name="XUbuntuCancel" />
> +   </interface>
> + </node>
> +diff --git a/js/ui/remoteSearch.js b/js/ui/remoteSearch.js
> +index c6c5a29..79daba1 100644
> +--- a/js/ui/remoteSearch.js
> ++++ b/js/ui/remoteSearch.js
> +@@ -30,6 +30,7 @@ const SearchProviderIface = '<node> \
> + <method name="ActivateResult"> \
> +     <arg type="s" direction="in" /> \
> + </method> \
> ++<method name="XUbuntuCancel" /> \
> + </interface> \
> + </node>';
> + 
> +@@ -57,6 +58,7 @@ const SearchProvider2Iface = '<node> \
> +     <arg type="as" direction="in" /> \
> +     <arg type="u" direction="in" /> \
> + </method> \
> ++<method name="XUbuntuCancel" /> \
> + </interface> \
> + </node>';
> + 
> +@@ -310,6 +312,17 @@ var RemoteSearchProvider = new Lang.Class({
> +                                         cancellable);
> +     },
> + 
> ++    XUbuntuCancel(cancellable) {
> ++        this.proxy.XUbuntuCancelRemote((results, error) => {
> ++                if (error &&
> ++                    !error.matches(Gio.DBusError, Gio.DBusError.UNKNOWN_METHOD) &&
> ++                    !error.matches(Gio.IOErrorEnum, Gio.IOErrorEnum.CANCELLED)) {
> ++                    log('Received error from DBus search provider %s during XUbuntuCancel: %s'.format(this.id, String(error)));
> ++                }
> ++            },
> ++            cancellable);
> ++    },
> ++
> +     activateResult(id) {
> +         this.proxy.ActivateResultRemote(id);
> +     },
> +diff --git a/js/ui/search.js b/js/ui/search.js
> +index f6fa719..fb2a021 100644
> +--- a/js/ui/search.js
> ++++ b/js/ui/search.js
> +@@ -225,7 +225,9 @@ var SearchResultsBase = new Lang.Class({
> +             this._cancellable.cancel();
> +             this._cancellable.reset();
> + 
> ++            this.provider.resultsMetasInProgress = true;

Can't we rather use the cancellable status to know if one is in progress or not rather than another variable?

> +             this.provider.getResultMetas(metasNeeded, metas => {
> ++                this.provider.resultsMetasInProgress = false;
> +                 if (this._cancellable.is_cancelled()) {
> +                     callback(false);
> +                     return;
> +@@ -453,6 +455,10 @@ var SearchResults = new Lang.Class({
> + 
> +         this._searchTimeoutId = 0;
> +         this._cancellable = new Gio.Cancellable();
> ++        this._searchCancelCancellable = new Gio.Cancellable();
> ++        this._cancellable.connect(() => {
> ++            this._cancelSearchProviderRequest();
> ++        });
> + 
> +         this._registerProvider(new AppDisplay.AppSearchProvider());
> +         this._reloadRemoteProviders();
> +@@ -494,11 +500,29 @@ var SearchResults = new Lang.Class({
> +         }
> +     },
> + 
> ++    _cancelSearchProviderRequest() {
> ++        if (this._terms.length != 0 || this._searchCancelTimeoutId > 0)
> ++            return;
> ++
> ++        this._searchCancelTimeoutId = GLib.timeout_add(GLib.PRIORITY_DEFAULT, 100, () => {
> ++            this._providers.forEach(provider => {
> ++                if (provider.isRemoteProvider &&
> ++                    (provider.searchInProgress || provider.resultsMetasInProgress)) {
> ++                    provider.XUbuntuCancel(this._searchCancelCancellable);
> ++                }
> ++            });
> ++
> ++            delete this._searchCancelTimeoutId;
> ++            return GLib.SOURCE_REMOVE;
> ++        });
> ++    },
> ++
> +     _reset() {
> +         this._terms = [];
> +         this._results = {};
> +         this._clearDisplay();
> +         this._clearSearchTimeout();
> ++        this._cancelSearchProviderRequest();
> +         this._defaultResult = null;
> +         this._startingSearch = false;
> + 
> +@@ -565,6 +589,13 @@ var SearchResults = new Lang.Class({
> +         if (this._terms.length > 0)
> +             isSubSearch = searchString.indexOf(previousSearchString) == 0;
> + 
> ++        this._searchCancelCancellable.cancel();
> ++        this._searchCancelCancellable.reset();
> ++        if (this._searchCancelTimeoutId > 0) {
> ++            GLib.source_remove(this._searchCancelTimeoutId);
> ++            delete this._searchCancelTimeoutId;
> ++        }
> ++
> +         this._terms = terms;
> +         this._isSubSearch = isSubSearch;
> +         this._updateSearchProgress();


-- 
https://code.launchpad.net/~3v1n0/ubuntu/+source/gnome-shell/+git/gnome-shell/+merge/353825
Your team Ubuntu Desktop is subscribed to branch ~ubuntu-desktop/ubuntu/+source/gnome-shell:ubuntu/master.