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

Proposed by Marco Trevisan (Treviño)
Status: Merged
Merged at revision: 5cee2840c88209a5b375d06d5456469e13b4c4ac
Proposed branch: ~3v1n0/ubuntu/+source/gnome-shell:ubuntu/bionic-xubuntu-cancel-search
Merge into: ~ubuntu-desktop/ubuntu/+source/gnome-shell:ubuntu/bionic
Diff against target: 317 lines (+277/-0)
6 files modified
debian/changelog (+11/-0)
debian/patches/search-Cancel-search-provider-operations-on-clear.patch (+29/-0)
debian/patches/search-Ignore-search-provider-results-metas-if-search-is-.patch (+30/-0)
debian/patches/series (+4/-0)
debian/patches/ubuntu/search-call-XUbuntuCancel-method-on-providers-when-no-dat.patch (+167/-0)
debian/patches/viewSelector-Cancel-search-on-overview-hidden.patch (+36/-0)
Reviewer Review Type Date Requested Status
Didier Roche-Tolomelli Approve
Review via email: mp+354321@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Same code than in cosmic, +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index 3df98cd..610ea63 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,14 @@
1gnome-shell (3.28.3-0ubuntu0.18.04.3) UNRELEASED; urgency=medium
2
3 * d/p/search-Cancel-search-provider-operations-on-clear.patch,
4 d/p/search-Ignore-search-provider-results-metas-if-search-is-.patch,
5 d/p/viewSelector-Cancel-search-on-overview-hidden.patch,
6 d/p/ubuntu/search-call-XUbuntuCancel-method-on-providers-when-no-dat.patch:
7 - Add support for cancelling remote search providers when the overlay
8 is closed (and actually stop searches when requested from UI, LP: #1756826)
9
10 -- Marco Trevisan (Treviño) <marco@ubuntu.com> Wed, 05 Sep 2018 14:31:58 +0200
11
1gnome-shell (3.28.3-0ubuntu0.18.04.2) bionic; urgency=medium12gnome-shell (3.28.3-0ubuntu0.18.04.2) bionic; urgency=medium
213
3 * New upstream release (LP: #1718931, LP: #1782614)14 * New upstream release (LP: #1718931, LP: #1782614)
diff --git a/debian/patches/search-Cancel-search-provider-operations-on-clear.patch b/debian/patches/search-Cancel-search-provider-operations-on-clear.patch
4new file mode 10064415new file mode 100644
index 0000000..052ee8a
--- /dev/null
+++ b/debian/patches/search-Cancel-search-provider-operations-on-clear.patch
@@ -0,0 +1,29 @@
1From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
2Date: Thu, 23 Aug 2018 18:14:38 +0200
3Subject: search: Cancel search provider operations on clear
4
5Ensure that the search provider operations (just getResultMetas requests in the
6current implementation) in progress are properly cancelled when we clear the UI,
7otherwise returned results might still be added when not needed.
8
9This is triggered for each provider by the SearchResults reset.
10
11Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
12Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/gnome-shell/+bug/1756826
13Forwarded: https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/205
14---
15 js/ui/search.js | 1 +
16 1 file changed, 1 insertion(+)
17
18diff --git a/js/ui/search.js b/js/ui/search.js
19index 1fb54b4..804be95 100644
20--- a/js/ui/search.js
21+++ b/js/ui/search.js
22@@ -192,6 +192,7 @@ var SearchResultsBase = new Lang.Class({
23 },
24
25 clear() {
26+ this._cancellable.cancel();
27 for (let resultId in this._resultDisplays)
28 this._resultDisplays[resultId].actor.destroy();
29 this._resultDisplays = {};
diff --git a/debian/patches/search-Ignore-search-provider-results-metas-if-search-is-.patch b/debian/patches/search-Ignore-search-provider-results-metas-if-search-is-.patch
0new file mode 10064430new file mode 100644
index 0000000..27645a5
--- /dev/null
+++ b/debian/patches/search-Ignore-search-provider-results-metas-if-search-is-.patch
@@ -0,0 +1,30 @@
1From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
2Date: Thu, 30 Aug 2018 07:11:24 +0200
3Subject: search: Ignore search provider results metas if search is cancelled
4
5Call updateSearch callback with no results when the search provider has been
6cancelled, without doing any logging.
7
8Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
9Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/gnome-shell/+bug/1756826
10Forwarded: https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/205
11---
12 js/ui/search.js | 5 +++--
13 1 file changed, 3 insertions(+), 2 deletions(-)
14
15diff --git a/js/ui/search.js b/js/ui/search.js
16index 804be95..dd4bfad 100644
17--- a/js/ui/search.js
18+++ b/js/ui/search.js
19@@ -227,8 +227,9 @@ var SearchResultsBase = new Lang.Class({
20
21 this.provider.getResultMetas(metasNeeded, metas => {
22 if (metas.length != metasNeeded.length) {
23- log('Wrong number of result metas returned by search provider ' + this.provider.id +
24- ': expected ' + metasNeeded.length + ' but got ' + metas.length);
25+ if (!this._cancellable.is_cancelled())
26+ log(`Wrong number of result metas returned by search provider ${this.provider.id}` +
27+ `: expected ${metasNeeded.length} but got ${metas.length}`);
28 callback(false);
29 return;
30 }
diff --git a/debian/patches/series b/debian/patches/series
index 3e0fd79..ad2c4be 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -22,3 +22,7 @@ authPrompt-Unset-preemptiveAnswer-on-reset.patch
22authPrompt-Do-not-enable-sensitivity-if-retries-are-disal.patch22authPrompt-Do-not-enable-sensitivity-if-retries-are-disal.patch
23gdm-util-Always-allow-to-retry-login-in-unlock-mode.patch23gdm-util-Always-allow-to-retry-login-in-unlock-mode.patch
24popupMenu-Don-t-handle-key-presses-directly-if-there-are-.patch24popupMenu-Don-t-handle-key-presses-directly-if-there-are-.patch
25viewSelector-Cancel-search-on-overview-hidden.patch
26search-Cancel-search-provider-operations-on-clear.patch
27search-Ignore-search-provider-results-metas-if-search-is-.patch
28ubuntu/search-call-XUbuntuCancel-method-on-providers-when-no-dat.patch
diff --git a/debian/patches/ubuntu/search-call-XUbuntuCancel-method-on-providers-when-no-dat.patch b/debian/patches/ubuntu/search-call-XUbuntuCancel-method-on-providers-when-no-dat.patch
25new file mode 10064429new file mode 100644
index 0000000..94a1d42
--- /dev/null
+++ b/debian/patches/ubuntu/search-call-XUbuntuCancel-method-on-providers-when-no-dat.patch
@@ -0,0 +1,167 @@
1From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
2Date: Thu, 23 Aug 2018 20:00:57 +0200
3Subject: search: call XUbuntuCancel method on providers when no data is
4 needed
5
6Add XUbuntuCancel method to search providers and call it when a search provider
7is still doing operations.
8Ignore the result when the method does not exist or is cancelled.
9
10This will allow to stop operations on providers.
11
12Fixes LP: #1756826
13
14Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
15Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/gnome-shell/+bug/1756826
16Forwarded: not-needed
17---
18 data/org.gnome.ShellSearchProvider.xml | 6 ++++++
19 data/org.gnome.ShellSearchProvider2.xml | 6 ++++++
20 js/ui/remoteSearch.js | 15 +++++++++++++++
21 js/ui/search.js | 34 +++++++++++++++++++++++++++++++++
22 4 files changed, 61 insertions(+)
23
24diff --git a/data/org.gnome.ShellSearchProvider.xml b/data/org.gnome.ShellSearchProvider.xml
25index 78ad305..393cb01 100644
26--- a/data/org.gnome.ShellSearchProvider.xml
27+++ b/data/org.gnome.ShellSearchProvider.xml
28@@ -69,5 +69,11 @@
29 <method name="ActivateResult">
30 <arg type="s" name="identifier" direction="in" />
31 </method>
32+
33+ <!--
34+ XUbuntuCancel:
35+ Cancel the current search operation
36+ -->
37+ <method name="XUbuntuCancel" />
38 </interface>
39 </node>
40diff --git a/data/org.gnome.ShellSearchProvider2.xml b/data/org.gnome.ShellSearchProvider2.xml
41index 9502340..8141bc0 100644
42--- a/data/org.gnome.ShellSearchProvider2.xml
43+++ b/data/org.gnome.ShellSearchProvider2.xml
44@@ -83,5 +83,11 @@
45 <arg type="as" name="terms" direction="in" />
46 <arg type="u" name="timestamp" direction="in" />
47 </method>
48+
49+ <!--
50+ XUbuntuCancel:
51+ Cancel the current search operation
52+ -->
53+ <method name="XUbuntuCancel" />
54 </interface>
55 </node>
56diff --git a/js/ui/remoteSearch.js b/js/ui/remoteSearch.js
57index c6c5a29..8de6148 100644
58--- a/js/ui/remoteSearch.js
59+++ b/js/ui/remoteSearch.js
60@@ -30,6 +30,7 @@ const SearchProviderIface = '<node> \
61 <method name="ActivateResult"> \
62 <arg type="s" direction="in" /> \
63 </method> \
64+<method name="XUbuntuCancel" /> \
65 </interface> \
66 </node>';
67
68@@ -57,6 +58,7 @@ const SearchProvider2Iface = '<node> \
69 <arg type="as" direction="in" /> \
70 <arg type="u" direction="in" /> \
71 </method> \
72+<method name="XUbuntuCancel" /> \
73 </interface> \
74 </node>';
75
76@@ -310,6 +312,19 @@ var RemoteSearchProvider = new Lang.Class({
77 cancellable);
78 },
79
80+ XUbuntuCancel(cancellable, callback) {
81+ this.proxy.XUbuntuCancelRemote((results, error) => {
82+ if (error &&
83+ !error.matches(Gio.DBusError, Gio.DBusError.UNKNOWN_METHOD) &&
84+ !error.matches(Gio.IOErrorEnum, Gio.IOErrorEnum.CANCELLED)) {
85+ log('Received error from DBus search provider %s during XUbuntuCancel: %s'.format(this.id, String(error)));
86+ } else if (callback && !error) {
87+ callback();
88+ }
89+ },
90+ cancellable);
91+ },
92+
93 activateResult(id) {
94 this.proxy.ActivateResultRemote(id);
95 },
96diff --git a/js/ui/search.js b/js/ui/search.js
97index dd4bfad..629664e 100644
98--- a/js/ui/search.js
99+++ b/js/ui/search.js
100@@ -225,7 +225,9 @@ var SearchResultsBase = new Lang.Class({
101 this._cancellable.cancel();
102 this._cancellable.reset();
103
104+ this.provider.resultsMetasInProgress = true;
105 this.provider.getResultMetas(metasNeeded, metas => {
106+ this.provider.resultsMetasInProgress = this._cancellable.is_cancelled();
107 if (metas.length != metasNeeded.length) {
108 if (!this._cancellable.is_cancelled())
109 log(`Wrong number of result metas returned by search provider ${this.provider.id}` +
110@@ -450,6 +452,10 @@ var SearchResults = new Lang.Class({
111
112 this._searchTimeoutId = 0;
113 this._cancellable = new Gio.Cancellable();
114+ this._searchCancelCancellable = new Gio.Cancellable();
115+ this._cancellable.connect(() => {
116+ this._cancelSearchProviderRequest();
117+ });
118
119 this._registerProvider(new AppDisplay.AppSearchProvider());
120 this._reloadRemoteProviders();
121@@ -491,11 +497,32 @@ var SearchResults = new Lang.Class({
122 }
123 },
124
125+ _cancelSearchProviderRequest() {
126+ if (this._terms.length != 0 || this._searchCancelTimeoutId > 0)
127+ return;
128+
129+ this._searchCancelTimeoutId = GLib.timeout_add(GLib.PRIORITY_DEFAULT, 100, () => {
130+ this._providers.forEach(provider => {
131+ if (provider.isRemoteProvider &&
132+ (provider.searchInProgress || provider.resultsMetasInProgress)) {
133+ provider.XUbuntuCancel(this._searchCancelCancellable, () => {
134+ provider.searchInProgress = false;
135+ provider.resultsMetasInProgress = false;
136+ });
137+ }
138+ });
139+
140+ delete this._searchCancelTimeoutId;
141+ return GLib.SOURCE_REMOVE;
142+ });
143+ },
144+
145 _reset() {
146 this._terms = [];
147 this._results = {};
148 this._clearDisplay();
149 this._clearSearchTimeout();
150+ this._cancelSearchProviderRequest();
151 this._defaultResult = null;
152 this._startingSearch = false;
153
154@@ -562,6 +589,13 @@ var SearchResults = new Lang.Class({
155 if (this._terms.length > 0)
156 isSubSearch = searchString.indexOf(previousSearchString) == 0;
157
158+ this._searchCancelCancellable.cancel();
159+ this._searchCancelCancellable.reset();
160+ if (this._searchCancelTimeoutId > 0) {
161+ GLib.source_remove(this._searchCancelTimeoutId);
162+ delete this._searchCancelTimeoutId;
163+ }
164+
165 this._terms = terms;
166 this._isSubSearch = isSubSearch;
167 this._updateSearchProgress();
diff --git a/debian/patches/viewSelector-Cancel-search-on-overview-hidden.patch b/debian/patches/viewSelector-Cancel-search-on-overview-hidden.patch
0new file mode 100644168new file mode 100644
index 0000000..88ae73e
--- /dev/null
+++ b/debian/patches/viewSelector-Cancel-search-on-overview-hidden.patch
@@ -0,0 +1,36 @@
1From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
2Date: Thu, 23 Aug 2018 16:11:10 +0200
3Subject: viewSelector: Cancel search on overview hidden
4
5Currently when the overview is hidden, any pending search is kept alive, not only
6at remote search provider level (as per issue #183), but even the shell providers
7proxies continue to get and process data. This happens even if this is not needed
8anymore, while the UI reset is performed only next time that the overview is
9shown (causing some more computation presentation time).
10
11In order to stop this to happen, when the overview is hidden, we have to unset
12the search entry to an empty value as this would make SearchResults to have empty
13terms list and that would make the proxies cancellable to be triggered (without
14causing any further search to start).
15
16https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/205
17
18Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
19Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/gnome-shell/+bug/1756826
20Forwarded: https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/205
21---
22 js/ui/viewSelector.js | 1 +
23 1 file changed, 1 insertion(+)
24
25diff --git a/js/ui/viewSelector.js b/js/ui/viewSelector.js
26index 91bc222..c46afc5 100644
27--- a/js/ui/viewSelector.js
28+++ b/js/ui/viewSelector.js
29@@ -311,6 +311,7 @@ var ViewSelector = new Lang.Class({
30 },
31
32 hide() {
33+ this.reset();
34 this._workspacesDisplay.hide();
35 },
36

Subscribers

People subscribed via source and target branches