Marco Trevisan (Treviño) has proposed merging
~3v1n0/ubuntu/+source/gnome-shell:ubuntu/master-xubuntu-cancel-search into
~ubuntu-desktop/ubuntu/+source/gnome-shell:ubuntu/master with
~3v1n0/ubuntu/+source/gnome-shell:ubuntu/master-3.29.91 as a prerequisite.
Requested reviews:
Ubuntu Desktop (ubuntu-desktop)
Related bugs:
Bug #1756826 in nautilus (Ubuntu): "hangs when locate search provider matches
a lot of files"
https://bugs.launchpad.net/ubuntu/+source/nautilus/+bug/1756826
For more details, see:
https://code.launchpad.net/~3v1n0/ubuntu/+source/gnome-shell/+git/gnome-shell/+merge/353825
Added XUbuntuCancel method call for search providers
--
Your team Ubuntu Desktop is requested to review the proposed merge of
~3v1n0/ubuntu/+source/gnome-shell:ubuntu/master-xubuntu-cancel-search into
~ubuntu-desktop/ubuntu/+source/gnome-shell:ubuntu/master.
diff --git a/debian/changelog b/debian/changelog
index 3b2e50f..699905e 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -60,6 +60,11 @@ gnome-shell (3.29.91-1ubuntu1) UNRELEASED; urgency=medium
- Run dh_install with --fail-missing
* d/p/st-scroll-view-Handle-the-case-where-scrollbars-are-NULL.patch:
- Updated as per upstream review
+ * d/p/js-viewSelector-Cancel-search-on-overview-hiding.patch,
+ d/p/search-Cancel-search-provider-operations-on-clear.patch,
+ d/p/ubuntu/search-call-Cancel-method-on-providers-when-no-data-is-ne.patch:
+ - Add support for cancelling remote search providers when the overlay
+ is closed (and actually stop searches when requested from UI, LP: #1756826)
-- Marco Trevisan (Treviño) <ma...@ubuntu.com> Tue, 28 Aug 2018 00:35:19 +0200
diff --git a/debian/patches/js-viewSelector-Cancel-search-on-overview-hiding.patch b/debian/patches/js-viewSelector-Cancel-search-on-overview-hiding.patch
new file mode 100644
index 0000000..438c221
--- /dev/null
+++ b/debian/patches/js-viewSelector-Cancel-search-on-overview-hiding.patch
@@ -0,0 +1,87 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <m...@3v1n0.net>
+Date: Thu, 23 Aug 2018 16:11:10 +0200
+Subject: js/viewSelector: Cancel search on overview hiding
+
+Currently when the overview is hidden, any pending search is kept alive, not only
+at remote search provider level (as per issue #183), but even the shell providers
+proxies continue to get and process data, even if this is not something needed
+anymore, while the UI reset is performed only next time that the overview is
+shown (also causing some more computation at presenting it).
+
+In order to stop this to happen, when the overview is hiding, we have to unset
+the search entry to an empty value as this would make SearchResults to have empty
+terms list and that would make the proxies cancellable to be triggered.
+
+To be more accurate in not doing unneeded operations at this stage, is better to
+split the old `reset()` function into `stopSearch()` and `_resetSearchField()`.
+While the first cancel the search in act, the second only operates on the search
+field cursor and selection (removing some duplication).
+
+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: yes, https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/205
+---
+ js/ui/viewSelector.js | 21 +++++++++++++--------
+ 1 file changed, 13 insertions(+), 8 deletions(-)
+
+diff --git a/js/ui/viewSelector.js b/js/ui/viewSelector.js
+index 2d5c33f..e0c724d 100644
+--- a/js/ui/viewSelector.js
++++ b/js/ui/viewSelector.js
+@@ -311,6 +311,7 @@ var ViewSelector = new Lang.Class({
+ },
+
+ hide() {
++ this.stopSearch();
+ this._workspacesDisplay.hide();
+ },
+
+@@ -458,20 +459,20 @@ var ViewSelector = new Lang.Class({
+ this.reset();
+ },
+
+- reset() {
+- global.stage.set_key_focus(null);
+-
+- this._entry.text = '';
+-
++ _resetSearchField() {
+ this._text.set_cursor_visible(true);
+ this._text.set_selection(0, 0);
+ },
+
++ reset() {
++ this.stopSearch();
++ this._resetSearchField();
++ },
++
+ _onStageKeyFocusChanged() {
+ let focus = global.stage.get_key_focus();
+ let appearFocused = (this._entry.contains(focus) ||
+ this._searchResults.actor.contains(focus));
+-
+ this._text.set_cursor_visible(appearFocused);
+
+ if (appearFocused)
+@@ -485,8 +486,7 @@ var ViewSelector = new Lang.Class({
+ // Enable 'find-as-you-type'
+ this._capturedEventId = global.stage.connect('captured-event',
+ this._onCapturedEvent.bind(this));
+- this._text.set_cursor_visible(true);
+- this._text.set_selection(0, 0);
++ this._resetSearchField();
+ } else {
+ // Disable 'find-as-you-type'
+ if (this._capturedEventId > 0)
+@@ -520,6 +520,11 @@ var ViewSelector = new Lang.Class({
+ this._text.event(synthEvent, false);
+ },
+
++ stopSearch() {
++ this._entry.text = '';
++ global.stage.set_key_focus(null);
++ },
++
+ // the entry does not show the hint
+ _isActivated() {
+ return this._text.text == this._entry.get_text();
diff --git a/debian/patches/search-Cancel-search-provider-operations-on-clear.patch b/debian/patches/search-Cancel-search-provider-operations-on-clear.patch
new file mode 100644
index 0000000..90c06dc
--- /dev/null
+++ b/debian/patches/search-Cancel-search-provider-operations-on-clear.patch
@@ -0,0 +1,42 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <m...@3v1n0.net>
+Date: Thu, 23 Aug 2018 18:14:38 +0200
+Subject: search: Cancel search provider operations on clear
+
+Ensure that the search provider operations (just getResultMetas requests in the
+current implementation) in progress are properly cancelled when we clear the
+UI, otherwise returned results might still be added when not needed.
+In such case, also handle the callback of GetResultMetas calling updateSearch's
+callback with no results and without printing any error in the log.
+
+This is triggered for each provider by the SearchResults reset.
+
+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: yes, https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/205
+---
+ js/ui/search.js | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+diff --git a/js/ui/search.js b/js/ui/search.js
+index 1fb54b4..f6fa719 100644
+--- a/js/ui/search.js
++++ b/js/ui/search.js
+@@ -192,6 +192,7 @@ var SearchResultsBase = new Lang.Class({
+ },
+
+ clear() {
++ this._cancellable.cancel();
+ for (let resultId in this._resultDisplays)
+ this._resultDisplays[resultId].actor.destroy();
+ this._resultDisplays = {};
+@@ -225,6 +226,10 @@ var SearchResultsBase = new Lang.Class({
+ this._cancellable.reset();
+
+ this.provider.getResultMetas(metasNeeded, metas => {
++ if (this._cancellable.is_cancelled()) {
++ callback(false);
++ return;
++ }
+ if (metas.length != metasNeeded.length) {
+ log('Wrong number of result metas returned by search provider ' + this.provider.id +
+ ': expected ' + metasNeeded.length + ' but got ' + metas.length);
diff --git a/debian/patches/series b/debian/patches/series
index d38aac0..288f628 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -18,6 +18,9 @@ sessionMode-add-support-for-debugFlags-parameter.patch
st-scroll-view-Handle-the-case-where-scrollbars-are-NULL.patch
st-scroll-view-Remove-scrollbars-references-on-dispose.patch
js-main-Throw-error-if-no-valid-default-stylesheet-is-fou.patch
+js-viewSelector-Cancel-search-on-overview-hiding.patch
+search-Cancel-search-provider-operations-on-clear.patch
+ubuntu/search-call-Cancel-method-on-providers-when-no-data-is-ne.patch
tweener-Save-handlers-on-target-and-remove-them-on-destro.patch
dnd-Nullify-_dragActor-after-we-ve-destroyed-it-and-avoid.patch
messageList-stop-syncing-if-closeButton-has-been-destroye.patch
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?= <m...@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
+@@ -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;
+ 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();
--
ubuntu-desktop mailing list
ubuntu-desktop@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-desktop
