Merge ~3v1n0/ubuntu/+source/gnome-shell:ubuntu/bionic-sru3 into ~ubuntu-desktop/ubuntu/+source/gnome-shell:ubuntu/bionic

Proposed by Marco Trevisan (Treviño)
Status: Merged
Merged at revision: 638809248ddac6ee03bfeed977739425f97465b6
Proposed branch: ~3v1n0/ubuntu/+source/gnome-shell:ubuntu/bionic-sru3
Merge into: ~ubuntu-desktop/ubuntu/+source/gnome-shell:ubuntu/bionic
Prerequisite: ~3v1n0/ubuntu/+source/gnome-shell:ubuntu/bionic-touch-two-clicks-fix
Diff against target: 408 lines (+76/-46)
5 files modified
debian/changelog (+3/-6)
debian/patches/series (+2/-4)
debian/patches/st-scroll-view-Handle-the-case-where-scrollbars-are-NULL.patch (+38/-0)
debian/patches/st-scroll-view-Remove-scrollbars-references-on-dispose.patch (+33/-0)
dev/null (+0/-36)
Reviewer Review Type Date Requested Status
Iain Lane Approve
Daniel van Vugt Approve
Review via email: mp+358010@code.launchpad.net

Description of the change

Reverting XUbuntuCancel patches, we will do them soon after this

To post a comment you must log in.
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

The green stuff looks good to me.

I have no opinion on the red parts.

review: Approve
Revision history for this message
Iain Lane (laney) wrote :

thanks!

please keep working to get those fixes upstream

I guess the revert should be reverted - can you propose an MP for that please?

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index c6ad925..c43929d 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,16 +1,13 @@
6 gnome-shell (3.28.3-0ubuntu0.18.04.3) UNRELEASED; urgency=medium
7
8- * d/p/search-Cancel-search-provider-operations-on-clear.patch,
9- d/p/search-Ignore-search-provider-results-metas-if-search-is-.patch,
10- d/p/viewSelector-Cancel-search-on-overview-hidden.patch,
11- d/p/ubuntu/search-call-XUbuntuCancel-method-on-providers-when-no-dat.patch:
12- - Add support for cancelling remote search providers when the overlay
13- is closed (and actually stop searches when requested from UI, LP: #1756826)
14 * debian/ubuntu.css:
15 - use defined color for menu separators (LP: #1739931)
16 - set StEntry minimun height to work properly with Ubuntu font (LP: #1743058)
17 * debian/patches/st-button-Ignore-pointer-emulated-touch-events.patch:
18 - Don't emit two click events on touch under X11 (LP: #1745888)
19+ * d/p/st-scroll-view-Handle-the-case-where-scrollbars-are-NULL.patch,
20+ d/p/st-scroll-view-Remove-scrollbars-references-on-dispose.patch:
21+ - Handle NULL scroll bars in st-scroll-view (LP: #1725312)
22
23 -- Marco Trevisan (Treviño) <marco@ubuntu.com> Wed, 03 Oct 2018 00:50:42 +0200
24
25diff --git a/debian/patches/search-Cancel-search-provider-operations-on-clear.patch b/debian/patches/search-Cancel-search-provider-operations-on-clear.patch
26deleted file mode 100644
27index 052ee8a..0000000
28--- a/debian/patches/search-Cancel-search-provider-operations-on-clear.patch
29+++ /dev/null
30@@ -1,29 +0,0 @@
31-From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
32-Date: Thu, 23 Aug 2018 18:14:38 +0200
33-Subject: search: Cancel search provider operations on clear
34-
35-Ensure that the search provider operations (just getResultMetas requests in the
36-current implementation) in progress are properly cancelled when we clear the UI,
37-otherwise returned results might still be added when not needed.
38-
39-This is triggered for each provider by the SearchResults reset.
40-
41-Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
42-Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/gnome-shell/+bug/1756826
43-Forwarded: https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/205
44----
45- js/ui/search.js | 1 +
46- 1 file changed, 1 insertion(+)
47-
48-diff --git a/js/ui/search.js b/js/ui/search.js
49-index 1fb54b4..804be95 100644
50---- a/js/ui/search.js
51-+++ b/js/ui/search.js
52-@@ -192,6 +192,7 @@ var SearchResultsBase = new Lang.Class({
53- },
54-
55- clear() {
56-+ this._cancellable.cancel();
57- for (let resultId in this._resultDisplays)
58- this._resultDisplays[resultId].actor.destroy();
59- this._resultDisplays = {};
60diff --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
61deleted file mode 100644
62index 27645a5..0000000
63--- a/debian/patches/search-Ignore-search-provider-results-metas-if-search-is-.patch
64+++ /dev/null
65@@ -1,30 +0,0 @@
66-From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
67-Date: Thu, 30 Aug 2018 07:11:24 +0200
68-Subject: search: Ignore search provider results metas if search is cancelled
69-
70-Call updateSearch callback with no results when the search provider has been
71-cancelled, without doing any logging.
72-
73-Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
74-Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/gnome-shell/+bug/1756826
75-Forwarded: https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/205
76----
77- js/ui/search.js | 5 +++--
78- 1 file changed, 3 insertions(+), 2 deletions(-)
79-
80-diff --git a/js/ui/search.js b/js/ui/search.js
81-index 804be95..dd4bfad 100644
82---- a/js/ui/search.js
83-+++ b/js/ui/search.js
84-@@ -227,8 +227,9 @@ var SearchResultsBase = new Lang.Class({
85-
86- this.provider.getResultMetas(metasNeeded, metas => {
87- if (metas.length != metasNeeded.length) {
88-- log('Wrong number of result metas returned by search provider ' + this.provider.id +
89-- ': expected ' + metasNeeded.length + ' but got ' + metas.length);
90-+ if (!this._cancellable.is_cancelled())
91-+ log(`Wrong number of result metas returned by search provider ${this.provider.id}` +
92-+ `: expected ${metasNeeded.length} but got ${metas.length}`);
93- callback(false);
94- return;
95- }
96diff --git a/debian/patches/series b/debian/patches/series
97index 217740f..6a12345 100644
98--- a/debian/patches/series
99+++ b/debian/patches/series
100@@ -22,8 +22,6 @@ authPrompt-Unset-preemptiveAnswer-on-reset.patch
101 authPrompt-Do-not-enable-sensitivity-if-retries-are-disal.patch
102 gdm-util-Always-allow-to-retry-login-in-unlock-mode.patch
103 popupMenu-Don-t-handle-key-presses-directly-if-there-are-.patch
104-viewSelector-Cancel-search-on-overview-hidden.patch
105-search-Cancel-search-provider-operations-on-clear.patch
106-search-Ignore-search-provider-results-metas-if-search-is-.patch
107 st-button-Ignore-pointer-emulated-touch-events.patch
108-ubuntu/search-call-XUbuntuCancel-method-on-providers-when-no-dat.patch
109+st-scroll-view-Handle-the-case-where-scrollbars-are-NULL.patch
110+st-scroll-view-Remove-scrollbars-references-on-dispose.patch
111diff --git a/debian/patches/st-scroll-view-Handle-the-case-where-scrollbars-are-NULL.patch b/debian/patches/st-scroll-view-Handle-the-case-where-scrollbars-are-NULL.patch
112new file mode 100644
113index 0000000..0c5e93d
114--- /dev/null
115+++ b/debian/patches/st-scroll-view-Handle-the-case-where-scrollbars-are-NULL.patch
116@@ -0,0 +1,38 @@
117+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
118+Date: Fri, 3 Aug 2018 18:51:24 +0200
119+Subject: st/scroll-view: Handle the case where scrollbars are NULL
120+
121+The scrollbars actors in a scroll view can be NULL, in case they get removed
122+with a call to `clutter_actor_remove_child` on a scroll-view (and this is
123+implemented in st_scroll_view_remove).
124+
125+So, we should support the case where `priv->{h,v}scroll` are NULL, not to crash
126+in `st_widget_style_changed`.
127+
128+Fixes #467
129+
130+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/467
131+BUG-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/gnome-shell/+bug/1725312
132+Forwarded: yes, https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/190
133+---
134+ src/st/st-scroll-view.c | 7 +++++--
135+ 1 file changed, 5 insertions(+), 2 deletions(-)
136+
137+diff --git a/src/st/st-scroll-view.c b/src/st/st-scroll-view.c
138+index fc0db1c..a8b6d2e 100644
139+--- a/src/st/st-scroll-view.c
140++++ b/src/st/st-scroll-view.c
141+@@ -741,8 +741,11 @@ st_scroll_view_style_changed (StWidget *widget)
142+ gdouble hfade_offset = st_theme_node_get_length (theme_node, "-st-hfade-offset");
143+ st_scroll_view_update_fade_effect (self, vfade_offset, hfade_offset);
144+
145+- st_widget_style_changed (ST_WIDGET (priv->hscroll));
146+- st_widget_style_changed (ST_WIDGET (priv->vscroll));
147++ if (priv->hscroll)
148++ st_widget_style_changed (ST_WIDGET (priv->hscroll));
149++
150++ if (priv->vscroll)
151++ st_widget_style_changed (ST_WIDGET (priv->vscroll));
152+
153+ ST_WIDGET_CLASS (st_scroll_view_parent_class)->style_changed (widget);
154+ }
155diff --git a/debian/patches/st-scroll-view-Remove-scrollbars-references-on-dispose.patch b/debian/patches/st-scroll-view-Remove-scrollbars-references-on-dispose.patch
156new file mode 100644
157index 0000000..f5f49be
158--- /dev/null
159+++ b/debian/patches/st-scroll-view-Remove-scrollbars-references-on-dispose.patch
160@@ -0,0 +1,33 @@
161+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
162+Date: Fri, 3 Aug 2018 19:06:47 +0200
163+Subject: st/scroll-view: Remove scrollbars references on dispose
164+
165+As we're destroying the scrollbars on destruction, we should remove any
166+reference of it, not to cause multiple-calls to disposal to unreference them
167+again.
168+
169+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/467
170+BUG-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/gnome-shell/+bug/1725312
171+Forwarded: yes, https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/190
172+---
173+ src/st/st-scroll-view.c | 7 ++-----
174+ 1 file changed, 2 insertions(+), 5 deletions(-)
175+
176+diff --git a/src/st/st-scroll-view.c b/src/st/st-scroll-view.c
177+index a8b6d2e..cb38aef 100644
178+--- a/src/st/st-scroll-view.c
179++++ b/src/st/st-scroll-view.c
180+@@ -244,11 +244,8 @@ st_scroll_view_dispose (GObject *object)
181+ priv->fade_effect = NULL;
182+ }
183+
184+- if (priv->vscroll)
185+- clutter_actor_destroy (priv->vscroll);
186+-
187+- if (priv->hscroll)
188+- clutter_actor_destroy (priv->hscroll);
189++ g_clear_pointer (&priv->vscroll, clutter_actor_destroy);
190++ g_clear_pointer (&priv->hscroll, clutter_actor_destroy);
191+
192+ /* For most reliable freeing of memory, an object with signals
193+ * like StAdjustment should be explicitly disposed. Since we own
194diff --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
195deleted file mode 100644
196index 94a1d42..0000000
197--- a/debian/patches/ubuntu/search-call-XUbuntuCancel-method-on-providers-when-no-dat.patch
198+++ /dev/null
199@@ -1,167 +0,0 @@
200-From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
201-Date: Thu, 23 Aug 2018 20:00:57 +0200
202-Subject: search: call XUbuntuCancel method on providers when no data is
203- needed
204-
205-Add XUbuntuCancel method to search providers and call it when a search provider
206-is still doing operations.
207-Ignore the result when the method does not exist or is cancelled.
208-
209-This will allow to stop operations on providers.
210-
211-Fixes LP: #1756826
212-
213-Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
214-Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/gnome-shell/+bug/1756826
215-Forwarded: not-needed
216----
217- data/org.gnome.ShellSearchProvider.xml | 6 ++++++
218- data/org.gnome.ShellSearchProvider2.xml | 6 ++++++
219- js/ui/remoteSearch.js | 15 +++++++++++++++
220- js/ui/search.js | 34 +++++++++++++++++++++++++++++++++
221- 4 files changed, 61 insertions(+)
222-
223-diff --git a/data/org.gnome.ShellSearchProvider.xml b/data/org.gnome.ShellSearchProvider.xml
224-index 78ad305..393cb01 100644
225---- a/data/org.gnome.ShellSearchProvider.xml
226-+++ b/data/org.gnome.ShellSearchProvider.xml
227-@@ -69,5 +69,11 @@
228- <method name="ActivateResult">
229- <arg type="s" name="identifier" direction="in" />
230- </method>
231-+
232-+ <!--
233-+ XUbuntuCancel:
234-+ Cancel the current search operation
235-+ -->
236-+ <method name="XUbuntuCancel" />
237- </interface>
238- </node>
239-diff --git a/data/org.gnome.ShellSearchProvider2.xml b/data/org.gnome.ShellSearchProvider2.xml
240-index 9502340..8141bc0 100644
241---- a/data/org.gnome.ShellSearchProvider2.xml
242-+++ b/data/org.gnome.ShellSearchProvider2.xml
243-@@ -83,5 +83,11 @@
244- <arg type="as" name="terms" direction="in" />
245- <arg type="u" name="timestamp" direction="in" />
246- </method>
247-+
248-+ <!--
249-+ XUbuntuCancel:
250-+ Cancel the current search operation
251-+ -->
252-+ <method name="XUbuntuCancel" />
253- </interface>
254- </node>
255-diff --git a/js/ui/remoteSearch.js b/js/ui/remoteSearch.js
256-index c6c5a29..8de6148 100644
257---- a/js/ui/remoteSearch.js
258-+++ b/js/ui/remoteSearch.js
259-@@ -30,6 +30,7 @@ const SearchProviderIface = '<node> \
260- <method name="ActivateResult"> \
261- <arg type="s" direction="in" /> \
262- </method> \
263-+<method name="XUbuntuCancel" /> \
264- </interface> \
265- </node>';
266-
267-@@ -57,6 +58,7 @@ const SearchProvider2Iface = '<node> \
268- <arg type="as" direction="in" /> \
269- <arg type="u" direction="in" /> \
270- </method> \
271-+<method name="XUbuntuCancel" /> \
272- </interface> \
273- </node>';
274-
275-@@ -310,6 +312,19 @@ var RemoteSearchProvider = new Lang.Class({
276- cancellable);
277- },
278-
279-+ XUbuntuCancel(cancellable, callback) {
280-+ this.proxy.XUbuntuCancelRemote((results, error) => {
281-+ if (error &&
282-+ !error.matches(Gio.DBusError, Gio.DBusError.UNKNOWN_METHOD) &&
283-+ !error.matches(Gio.IOErrorEnum, Gio.IOErrorEnum.CANCELLED)) {
284-+ log('Received error from DBus search provider %s during XUbuntuCancel: %s'.format(this.id, String(error)));
285-+ } else if (callback && !error) {
286-+ callback();
287-+ }
288-+ },
289-+ cancellable);
290-+ },
291-+
292- activateResult(id) {
293- this.proxy.ActivateResultRemote(id);
294- },
295-diff --git a/js/ui/search.js b/js/ui/search.js
296-index dd4bfad..629664e 100644
297---- a/js/ui/search.js
298-+++ b/js/ui/search.js
299-@@ -225,7 +225,9 @@ var SearchResultsBase = new Lang.Class({
300- this._cancellable.cancel();
301- this._cancellable.reset();
302-
303-+ this.provider.resultsMetasInProgress = true;
304- this.provider.getResultMetas(metasNeeded, metas => {
305-+ this.provider.resultsMetasInProgress = this._cancellable.is_cancelled();
306- if (metas.length != metasNeeded.length) {
307- if (!this._cancellable.is_cancelled())
308- log(`Wrong number of result metas returned by search provider ${this.provider.id}` +
309-@@ -450,6 +452,10 @@ var SearchResults = new Lang.Class({
310-
311- this._searchTimeoutId = 0;
312- this._cancellable = new Gio.Cancellable();
313-+ this._searchCancelCancellable = new Gio.Cancellable();
314-+ this._cancellable.connect(() => {
315-+ this._cancelSearchProviderRequest();
316-+ });
317-
318- this._registerProvider(new AppDisplay.AppSearchProvider());
319- this._reloadRemoteProviders();
320-@@ -491,11 +497,32 @@ var SearchResults = new Lang.Class({
321- }
322- },
323-
324-+ _cancelSearchProviderRequest() {
325-+ if (this._terms.length != 0 || this._searchCancelTimeoutId > 0)
326-+ return;
327-+
328-+ this._searchCancelTimeoutId = GLib.timeout_add(GLib.PRIORITY_DEFAULT, 100, () => {
329-+ this._providers.forEach(provider => {
330-+ if (provider.isRemoteProvider &&
331-+ (provider.searchInProgress || provider.resultsMetasInProgress)) {
332-+ provider.XUbuntuCancel(this._searchCancelCancellable, () => {
333-+ provider.searchInProgress = false;
334-+ provider.resultsMetasInProgress = false;
335-+ });
336-+ }
337-+ });
338-+
339-+ delete this._searchCancelTimeoutId;
340-+ return GLib.SOURCE_REMOVE;
341-+ });
342-+ },
343-+
344- _reset() {
345- this._terms = [];
346- this._results = {};
347- this._clearDisplay();
348- this._clearSearchTimeout();
349-+ this._cancelSearchProviderRequest();
350- this._defaultResult = null;
351- this._startingSearch = false;
352-
353-@@ -562,6 +589,13 @@ var SearchResults = new Lang.Class({
354- if (this._terms.length > 0)
355- isSubSearch = searchString.indexOf(previousSearchString) == 0;
356-
357-+ this._searchCancelCancellable.cancel();
358-+ this._searchCancelCancellable.reset();
359-+ if (this._searchCancelTimeoutId > 0) {
360-+ GLib.source_remove(this._searchCancelTimeoutId);
361-+ delete this._searchCancelTimeoutId;
362-+ }
363-+
364- this._terms = terms;
365- this._isSubSearch = isSubSearch;
366- this._updateSearchProgress();
367diff --git a/debian/patches/viewSelector-Cancel-search-on-overview-hidden.patch b/debian/patches/viewSelector-Cancel-search-on-overview-hidden.patch
368deleted file mode 100644
369index 88ae73e..0000000
370--- a/debian/patches/viewSelector-Cancel-search-on-overview-hidden.patch
371+++ /dev/null
372@@ -1,36 +0,0 @@
373-From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
374-Date: Thu, 23 Aug 2018 16:11:10 +0200
375-Subject: viewSelector: Cancel search on overview hidden
376-
377-Currently when the overview is hidden, any pending search is kept alive, not only
378-at remote search provider level (as per issue #183), but even the shell providers
379-proxies continue to get and process data. This happens even if this is not needed
380-anymore, while the UI reset is performed only next time that the overview is
381-shown (causing some more computation presentation time).
382-
383-In order to stop this to happen, when the overview is hidden, we have to unset
384-the search entry to an empty value as this would make SearchResults to have empty
385-terms list and that would make the proxies cancellable to be triggered (without
386-causing any further search to start).
387-
388-https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/205
389-
390-Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
391-Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/gnome-shell/+bug/1756826
392-Forwarded: https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/205
393----
394- js/ui/viewSelector.js | 1 +
395- 1 file changed, 1 insertion(+)
396-
397-diff --git a/js/ui/viewSelector.js b/js/ui/viewSelector.js
398-index 91bc222..c46afc5 100644
399---- a/js/ui/viewSelector.js
400-+++ b/js/ui/viewSelector.js
401-@@ -311,6 +311,7 @@ var ViewSelector = new Lang.Class({
402- },
403-
404- hide() {
405-+ this.reset();
406- this._workspacesDisplay.hide();
407- },
408-

Subscribers

People subscribed via source and target branches