Merge ~3v1n0/ubuntu/+source/gnome-shell:ubuntu/bionic-xubuntu-cancel-search into ~ubuntu-desktop/ubuntu/+source/gnome-shell:ubuntu/bionic
- Git
- lp:~3v1n0/ubuntu/+source/gnome-shell
- ubuntu/bionic-xubuntu-cancel-search
- Merge into 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) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Didier Roche-Tolomelli | Approve | ||
Review via email: mp+354321@code.launchpad.net |
Commit message
Description of the change
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/debian/changelog b/debian/changelog | |||
2 | index 3df98cd..610ea63 100644 | |||
3 | --- a/debian/changelog | |||
4 | +++ b/debian/changelog | |||
5 | @@ -1,3 +1,14 @@ | |||
6 | 1 | gnome-shell (3.28.3-0ubuntu0.18.04.3) UNRELEASED; urgency=medium | ||
7 | 2 | |||
8 | 3 | * d/p/search-Cancel-search-provider-operations-on-clear.patch, | ||
9 | 4 | d/p/search-Ignore-search-provider-results-metas-if-search-is-.patch, | ||
10 | 5 | d/p/viewSelector-Cancel-search-on-overview-hidden.patch, | ||
11 | 6 | d/p/ubuntu/search-call-XUbuntuCancel-method-on-providers-when-no-dat.patch: | ||
12 | 7 | - Add support for cancelling remote search providers when the overlay | ||
13 | 8 | is closed (and actually stop searches when requested from UI, LP: #1756826) | ||
14 | 9 | |||
15 | 10 | -- Marco Trevisan (Treviño) <marco@ubuntu.com> Wed, 05 Sep 2018 14:31:58 +0200 | ||
16 | 11 | |||
17 | 1 | gnome-shell (3.28.3-0ubuntu0.18.04.2) bionic; urgency=medium | 12 | gnome-shell (3.28.3-0ubuntu0.18.04.2) bionic; urgency=medium |
18 | 2 | 13 | ||
19 | 3 | * New upstream release (LP: #1718931, LP: #1782614) | 14 | * New upstream release (LP: #1718931, LP: #1782614) |
20 | diff --git a/debian/patches/search-Cancel-search-provider-operations-on-clear.patch b/debian/patches/search-Cancel-search-provider-operations-on-clear.patch | |||
21 | 4 | new file mode 100644 | 15 | new file mode 100644 |
22 | index 0000000..052ee8a | |||
23 | --- /dev/null | |||
24 | +++ b/debian/patches/search-Cancel-search-provider-operations-on-clear.patch | |||
25 | @@ -0,0 +1,29 @@ | |||
26 | 1 | From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net> | ||
27 | 2 | Date: Thu, 23 Aug 2018 18:14:38 +0200 | ||
28 | 3 | Subject: search: Cancel search provider operations on clear | ||
29 | 4 | |||
30 | 5 | Ensure that the search provider operations (just getResultMetas requests in the | ||
31 | 6 | current implementation) in progress are properly cancelled when we clear the UI, | ||
32 | 7 | otherwise returned results might still be added when not needed. | ||
33 | 8 | |||
34 | 9 | This is triggered for each provider by the SearchResults reset. | ||
35 | 10 | |||
36 | 11 | Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183 | ||
37 | 12 | Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/gnome-shell/+bug/1756826 | ||
38 | 13 | Forwarded: https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/205 | ||
39 | 14 | --- | ||
40 | 15 | js/ui/search.js | 1 + | ||
41 | 16 | 1 file changed, 1 insertion(+) | ||
42 | 17 | |||
43 | 18 | diff --git a/js/ui/search.js b/js/ui/search.js | ||
44 | 19 | index 1fb54b4..804be95 100644 | ||
45 | 20 | --- a/js/ui/search.js | ||
46 | 21 | +++ b/js/ui/search.js | ||
47 | 22 | @@ -192,6 +192,7 @@ var SearchResultsBase = new Lang.Class({ | ||
48 | 23 | }, | ||
49 | 24 | |||
50 | 25 | clear() { | ||
51 | 26 | + this._cancellable.cancel(); | ||
52 | 27 | for (let resultId in this._resultDisplays) | ||
53 | 28 | this._resultDisplays[resultId].actor.destroy(); | ||
54 | 29 | this._resultDisplays = {}; | ||
55 | 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 | |||
56 | 0 | new file mode 100644 | 30 | new file mode 100644 |
57 | index 0000000..27645a5 | |||
58 | --- /dev/null | |||
59 | +++ b/debian/patches/search-Ignore-search-provider-results-metas-if-search-is-.patch | |||
60 | @@ -0,0 +1,30 @@ | |||
61 | 1 | From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net> | ||
62 | 2 | Date: Thu, 30 Aug 2018 07:11:24 +0200 | ||
63 | 3 | Subject: search: Ignore search provider results metas if search is cancelled | ||
64 | 4 | |||
65 | 5 | Call updateSearch callback with no results when the search provider has been | ||
66 | 6 | cancelled, without doing any logging. | ||
67 | 7 | |||
68 | 8 | Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183 | ||
69 | 9 | Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/gnome-shell/+bug/1756826 | ||
70 | 10 | Forwarded: https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/205 | ||
71 | 11 | --- | ||
72 | 12 | js/ui/search.js | 5 +++-- | ||
73 | 13 | 1 file changed, 3 insertions(+), 2 deletions(-) | ||
74 | 14 | |||
75 | 15 | diff --git a/js/ui/search.js b/js/ui/search.js | ||
76 | 16 | index 804be95..dd4bfad 100644 | ||
77 | 17 | --- a/js/ui/search.js | ||
78 | 18 | +++ b/js/ui/search.js | ||
79 | 19 | @@ -227,8 +227,9 @@ var SearchResultsBase = new Lang.Class({ | ||
80 | 20 | |||
81 | 21 | this.provider.getResultMetas(metasNeeded, metas => { | ||
82 | 22 | if (metas.length != metasNeeded.length) { | ||
83 | 23 | - log('Wrong number of result metas returned by search provider ' + this.provider.id + | ||
84 | 24 | - ': expected ' + metasNeeded.length + ' but got ' + metas.length); | ||
85 | 25 | + if (!this._cancellable.is_cancelled()) | ||
86 | 26 | + log(`Wrong number of result metas returned by search provider ${this.provider.id}` + | ||
87 | 27 | + `: expected ${metasNeeded.length} but got ${metas.length}`); | ||
88 | 28 | callback(false); | ||
89 | 29 | return; | ||
90 | 30 | } | ||
91 | diff --git a/debian/patches/series b/debian/patches/series | |||
92 | index 3e0fd79..ad2c4be 100644 | |||
93 | --- a/debian/patches/series | |||
94 | +++ b/debian/patches/series | |||
95 | @@ -22,3 +22,7 @@ authPrompt-Unset-preemptiveAnswer-on-reset.patch | |||
96 | 22 | authPrompt-Do-not-enable-sensitivity-if-retries-are-disal.patch | 22 | authPrompt-Do-not-enable-sensitivity-if-retries-are-disal.patch |
97 | 23 | gdm-util-Always-allow-to-retry-login-in-unlock-mode.patch | 23 | gdm-util-Always-allow-to-retry-login-in-unlock-mode.patch |
98 | 24 | popupMenu-Don-t-handle-key-presses-directly-if-there-are-.patch | 24 | popupMenu-Don-t-handle-key-presses-directly-if-there-are-.patch |
99 | 25 | viewSelector-Cancel-search-on-overview-hidden.patch | ||
100 | 26 | search-Cancel-search-provider-operations-on-clear.patch | ||
101 | 27 | search-Ignore-search-provider-results-metas-if-search-is-.patch | ||
102 | 28 | ubuntu/search-call-XUbuntuCancel-method-on-providers-when-no-dat.patch | ||
103 | 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 | |||
104 | 25 | new file mode 100644 | 29 | new file mode 100644 |
105 | index 0000000..94a1d42 | |||
106 | --- /dev/null | |||
107 | +++ b/debian/patches/ubuntu/search-call-XUbuntuCancel-method-on-providers-when-no-dat.patch | |||
108 | @@ -0,0 +1,167 @@ | |||
109 | 1 | From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net> | ||
110 | 2 | Date: Thu, 23 Aug 2018 20:00:57 +0200 | ||
111 | 3 | Subject: search: call XUbuntuCancel method on providers when no data is | ||
112 | 4 | needed | ||
113 | 5 | |||
114 | 6 | Add XUbuntuCancel method to search providers and call it when a search provider | ||
115 | 7 | is still doing operations. | ||
116 | 8 | Ignore the result when the method does not exist or is cancelled. | ||
117 | 9 | |||
118 | 10 | This will allow to stop operations on providers. | ||
119 | 11 | |||
120 | 12 | Fixes LP: #1756826 | ||
121 | 13 | |||
122 | 14 | Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183 | ||
123 | 15 | Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/gnome-shell/+bug/1756826 | ||
124 | 16 | Forwarded: not-needed | ||
125 | 17 | --- | ||
126 | 18 | data/org.gnome.ShellSearchProvider.xml | 6 ++++++ | ||
127 | 19 | data/org.gnome.ShellSearchProvider2.xml | 6 ++++++ | ||
128 | 20 | js/ui/remoteSearch.js | 15 +++++++++++++++ | ||
129 | 21 | js/ui/search.js | 34 +++++++++++++++++++++++++++++++++ | ||
130 | 22 | 4 files changed, 61 insertions(+) | ||
131 | 23 | |||
132 | 24 | diff --git a/data/org.gnome.ShellSearchProvider.xml b/data/org.gnome.ShellSearchProvider.xml | ||
133 | 25 | index 78ad305..393cb01 100644 | ||
134 | 26 | --- a/data/org.gnome.ShellSearchProvider.xml | ||
135 | 27 | +++ b/data/org.gnome.ShellSearchProvider.xml | ||
136 | 28 | @@ -69,5 +69,11 @@ | ||
137 | 29 | <method name="ActivateResult"> | ||
138 | 30 | <arg type="s" name="identifier" direction="in" /> | ||
139 | 31 | </method> | ||
140 | 32 | + | ||
141 | 33 | + <!-- | ||
142 | 34 | + XUbuntuCancel: | ||
143 | 35 | + Cancel the current search operation | ||
144 | 36 | + --> | ||
145 | 37 | + <method name="XUbuntuCancel" /> | ||
146 | 38 | </interface> | ||
147 | 39 | </node> | ||
148 | 40 | diff --git a/data/org.gnome.ShellSearchProvider2.xml b/data/org.gnome.ShellSearchProvider2.xml | ||
149 | 41 | index 9502340..8141bc0 100644 | ||
150 | 42 | --- a/data/org.gnome.ShellSearchProvider2.xml | ||
151 | 43 | +++ b/data/org.gnome.ShellSearchProvider2.xml | ||
152 | 44 | @@ -83,5 +83,11 @@ | ||
153 | 45 | <arg type="as" name="terms" direction="in" /> | ||
154 | 46 | <arg type="u" name="timestamp" direction="in" /> | ||
155 | 47 | </method> | ||
156 | 48 | + | ||
157 | 49 | + <!-- | ||
158 | 50 | + XUbuntuCancel: | ||
159 | 51 | + Cancel the current search operation | ||
160 | 52 | + --> | ||
161 | 53 | + <method name="XUbuntuCancel" /> | ||
162 | 54 | </interface> | ||
163 | 55 | </node> | ||
164 | 56 | diff --git a/js/ui/remoteSearch.js b/js/ui/remoteSearch.js | ||
165 | 57 | index c6c5a29..8de6148 100644 | ||
166 | 58 | --- a/js/ui/remoteSearch.js | ||
167 | 59 | +++ b/js/ui/remoteSearch.js | ||
168 | 60 | @@ -30,6 +30,7 @@ const SearchProviderIface = '<node> \ | ||
169 | 61 | <method name="ActivateResult"> \ | ||
170 | 62 | <arg type="s" direction="in" /> \ | ||
171 | 63 | </method> \ | ||
172 | 64 | +<method name="XUbuntuCancel" /> \ | ||
173 | 65 | </interface> \ | ||
174 | 66 | </node>'; | ||
175 | 67 | |||
176 | 68 | @@ -57,6 +58,7 @@ const SearchProvider2Iface = '<node> \ | ||
177 | 69 | <arg type="as" direction="in" /> \ | ||
178 | 70 | <arg type="u" direction="in" /> \ | ||
179 | 71 | </method> \ | ||
180 | 72 | +<method name="XUbuntuCancel" /> \ | ||
181 | 73 | </interface> \ | ||
182 | 74 | </node>'; | ||
183 | 75 | |||
184 | 76 | @@ -310,6 +312,19 @@ var RemoteSearchProvider = new Lang.Class({ | ||
185 | 77 | cancellable); | ||
186 | 78 | }, | ||
187 | 79 | |||
188 | 80 | + XUbuntuCancel(cancellable, callback) { | ||
189 | 81 | + this.proxy.XUbuntuCancelRemote((results, error) => { | ||
190 | 82 | + if (error && | ||
191 | 83 | + !error.matches(Gio.DBusError, Gio.DBusError.UNKNOWN_METHOD) && | ||
192 | 84 | + !error.matches(Gio.IOErrorEnum, Gio.IOErrorEnum.CANCELLED)) { | ||
193 | 85 | + log('Received error from DBus search provider %s during XUbuntuCancel: %s'.format(this.id, String(error))); | ||
194 | 86 | + } else if (callback && !error) { | ||
195 | 87 | + callback(); | ||
196 | 88 | + } | ||
197 | 89 | + }, | ||
198 | 90 | + cancellable); | ||
199 | 91 | + }, | ||
200 | 92 | + | ||
201 | 93 | activateResult(id) { | ||
202 | 94 | this.proxy.ActivateResultRemote(id); | ||
203 | 95 | }, | ||
204 | 96 | diff --git a/js/ui/search.js b/js/ui/search.js | ||
205 | 97 | index dd4bfad..629664e 100644 | ||
206 | 98 | --- a/js/ui/search.js | ||
207 | 99 | +++ b/js/ui/search.js | ||
208 | 100 | @@ -225,7 +225,9 @@ var SearchResultsBase = new Lang.Class({ | ||
209 | 101 | this._cancellable.cancel(); | ||
210 | 102 | this._cancellable.reset(); | ||
211 | 103 | |||
212 | 104 | + this.provider.resultsMetasInProgress = true; | ||
213 | 105 | this.provider.getResultMetas(metasNeeded, metas => { | ||
214 | 106 | + this.provider.resultsMetasInProgress = this._cancellable.is_cancelled(); | ||
215 | 107 | if (metas.length != metasNeeded.length) { | ||
216 | 108 | if (!this._cancellable.is_cancelled()) | ||
217 | 109 | log(`Wrong number of result metas returned by search provider ${this.provider.id}` + | ||
218 | 110 | @@ -450,6 +452,10 @@ var SearchResults = new Lang.Class({ | ||
219 | 111 | |||
220 | 112 | this._searchTimeoutId = 0; | ||
221 | 113 | this._cancellable = new Gio.Cancellable(); | ||
222 | 114 | + this._searchCancelCancellable = new Gio.Cancellable(); | ||
223 | 115 | + this._cancellable.connect(() => { | ||
224 | 116 | + this._cancelSearchProviderRequest(); | ||
225 | 117 | + }); | ||
226 | 118 | |||
227 | 119 | this._registerProvider(new AppDisplay.AppSearchProvider()); | ||
228 | 120 | this._reloadRemoteProviders(); | ||
229 | 121 | @@ -491,11 +497,32 @@ var SearchResults = new Lang.Class({ | ||
230 | 122 | } | ||
231 | 123 | }, | ||
232 | 124 | |||
233 | 125 | + _cancelSearchProviderRequest() { | ||
234 | 126 | + if (this._terms.length != 0 || this._searchCancelTimeoutId > 0) | ||
235 | 127 | + return; | ||
236 | 128 | + | ||
237 | 129 | + this._searchCancelTimeoutId = GLib.timeout_add(GLib.PRIORITY_DEFAULT, 100, () => { | ||
238 | 130 | + this._providers.forEach(provider => { | ||
239 | 131 | + if (provider.isRemoteProvider && | ||
240 | 132 | + (provider.searchInProgress || provider.resultsMetasInProgress)) { | ||
241 | 133 | + provider.XUbuntuCancel(this._searchCancelCancellable, () => { | ||
242 | 134 | + provider.searchInProgress = false; | ||
243 | 135 | + provider.resultsMetasInProgress = false; | ||
244 | 136 | + }); | ||
245 | 137 | + } | ||
246 | 138 | + }); | ||
247 | 139 | + | ||
248 | 140 | + delete this._searchCancelTimeoutId; | ||
249 | 141 | + return GLib.SOURCE_REMOVE; | ||
250 | 142 | + }); | ||
251 | 143 | + }, | ||
252 | 144 | + | ||
253 | 145 | _reset() { | ||
254 | 146 | this._terms = []; | ||
255 | 147 | this._results = {}; | ||
256 | 148 | this._clearDisplay(); | ||
257 | 149 | this._clearSearchTimeout(); | ||
258 | 150 | + this._cancelSearchProviderRequest(); | ||
259 | 151 | this._defaultResult = null; | ||
260 | 152 | this._startingSearch = false; | ||
261 | 153 | |||
262 | 154 | @@ -562,6 +589,13 @@ var SearchResults = new Lang.Class({ | ||
263 | 155 | if (this._terms.length > 0) | ||
264 | 156 | isSubSearch = searchString.indexOf(previousSearchString) == 0; | ||
265 | 157 | |||
266 | 158 | + this._searchCancelCancellable.cancel(); | ||
267 | 159 | + this._searchCancelCancellable.reset(); | ||
268 | 160 | + if (this._searchCancelTimeoutId > 0) { | ||
269 | 161 | + GLib.source_remove(this._searchCancelTimeoutId); | ||
270 | 162 | + delete this._searchCancelTimeoutId; | ||
271 | 163 | + } | ||
272 | 164 | + | ||
273 | 165 | this._terms = terms; | ||
274 | 166 | this._isSubSearch = isSubSearch; | ||
275 | 167 | this._updateSearchProgress(); | ||
276 | diff --git a/debian/patches/viewSelector-Cancel-search-on-overview-hidden.patch b/debian/patches/viewSelector-Cancel-search-on-overview-hidden.patch | |||
277 | 0 | new file mode 100644 | 168 | new file mode 100644 |
278 | index 0000000..88ae73e | |||
279 | --- /dev/null | |||
280 | +++ b/debian/patches/viewSelector-Cancel-search-on-overview-hidden.patch | |||
281 | @@ -0,0 +1,36 @@ | |||
282 | 1 | From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net> | ||
283 | 2 | Date: Thu, 23 Aug 2018 16:11:10 +0200 | ||
284 | 3 | Subject: viewSelector: Cancel search on overview hidden | ||
285 | 4 | |||
286 | 5 | Currently when the overview is hidden, any pending search is kept alive, not only | ||
287 | 6 | at remote search provider level (as per issue #183), but even the shell providers | ||
288 | 7 | proxies continue to get and process data. This happens even if this is not needed | ||
289 | 8 | anymore, while the UI reset is performed only next time that the overview is | ||
290 | 9 | shown (causing some more computation presentation time). | ||
291 | 10 | |||
292 | 11 | In order to stop this to happen, when the overview is hidden, we have to unset | ||
293 | 12 | the search entry to an empty value as this would make SearchResults to have empty | ||
294 | 13 | terms list and that would make the proxies cancellable to be triggered (without | ||
295 | 14 | causing any further search to start). | ||
296 | 15 | |||
297 | 16 | https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/205 | ||
298 | 17 | |||
299 | 18 | Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183 | ||
300 | 19 | Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/gnome-shell/+bug/1756826 | ||
301 | 20 | Forwarded: https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/205 | ||
302 | 21 | --- | ||
303 | 22 | js/ui/viewSelector.js | 1 + | ||
304 | 23 | 1 file changed, 1 insertion(+) | ||
305 | 24 | |||
306 | 25 | diff --git a/js/ui/viewSelector.js b/js/ui/viewSelector.js | ||
307 | 26 | index 91bc222..c46afc5 100644 | ||
308 | 27 | --- a/js/ui/viewSelector.js | ||
309 | 28 | +++ b/js/ui/viewSelector.js | ||
310 | 29 | @@ -311,6 +311,7 @@ var ViewSelector = new Lang.Class({ | ||
311 | 30 | }, | ||
312 | 31 | |||
313 | 32 | hide() { | ||
314 | 33 | + this.reset(); | ||
315 | 34 | this._workspacesDisplay.hide(); | ||
316 | 35 | }, | ||
317 | 36 |
Same code than in cosmic, +1