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

Proposed by Marco Trevisan (Treviño) on 2018-09-05
Status: Needs review
Proposed branch: ~3v1n0/ubuntu/+source/gnome-calculator:ubuntu/bionic-xubuntu-cancel-search
Merge into: ~ubuntu-desktop/ubuntu/+source/gnome-calculator:ubuntu/bionic
Prerequisite: ~3v1n0/ubuntu/+source/gnome-calculator:ubuntu/bionic
Diff against target: 1031 lines (+940/-2)
13 files modified
debian/changelog (+17/-0)
debian/control (+2/-1)
debian/control.in (+2/-1)
debian/patches/search-provider-Handle-errors-gracefully.patch (+108/-0)
debian/patches/search-provider-Use-async-calls-cancel-search-on-inactivi.patch (+233/-0)
debian/patches/search-provider-Use-lower-inactivity-timeout.patch (+30/-0)
debian/patches/search-provider-cache-equations-avoiding-spawning-calcula.patch (+171/-0)
debian/patches/search-provider-cache-only-a-limited-number-of-equations.patch (+45/-0)
debian/patches/search-provider-cancel-the-current-process-on-new-calcula.patch (+30/-0)
debian/patches/search-provider-renew-inactivity-timeout-at-each-calculat.patch (+61/-0)
debian/patches/search-provider-simplify-solve_subprocess.patch (+114/-0)
debian/patches/series (+9/-0)
debian/patches/ubuntu/search-provider-Cancel-operations-on-XUbuntuCancel.patch (+118/-0)
Reviewer Review Type Date Requested Status
Ubuntu Desktop 2018-09-05 Pending
Review via email: mp+354335@code.launchpad.net
To post a comment you must log in.
c11fb50... by Marco Trevisan (Treviño) on 2018-11-08

Finalise changelog

Unmerged commits

c11fb50... by Marco Trevisan (Treviño) on 2018-11-08

Finalise changelog

333ffe8... by Marco Trevisan (Treviño) on 2018-08-28

search-provider: make async and support XUbuntuCancel

Fixes LP: #1756826

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 44dd584..7fe49c2 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,20 @@
6+gnome-calculator (1:3.28.2-1~ubuntu18.04.2) bionic; urgency=medium
7+
8+ * d/p/search-provider-Use-async-calls-cancel-search-on-inactivi.patch,
9+ d/p/search-provider-renew-inactivity-timeout-at-each-calculat.patch,
10+ d/p/search-provider-Use-lower-inactivity-timeout.patch,
11+ d/p/search-provider-simplify-solve_subprocess.patch,
12+ d/p/search-provider-cache-equations-avoiding-spawning-calcula.patch,
13+ d/p/search-provider-cancel-the-current-process-on-new-calcula.patch,
14+ d/p/search-provider-cache-only-a-limited-number-of-equations.patch,
15+ d/p/search-provider-Handle-errors-gracefully.patch,
16+ d/p/ubuntu/search-provider-Cancel-operations-on-XUbuntuCancel.patch:
17+ - Make search provider async and support XUbuntuCancel method to
18+ stop expensive operations that might lead to an unresponsive
19+ process (LP: #1756826)
20+
21+ -- Marco Trevisan (Treviño) <marco@ubuntu.com> Thu, 08 Nov 2018 01:34:07 -0500
22+
23 gnome-calculator (1:3.28.2-1~ubuntu18.04.1) bionic; urgency=medium
24
25 * New upstream stable release (LP: #1790876)
26diff --git a/debian/control b/debian/control
27index cd4706c..0782192 100644
28--- a/debian/control
29+++ b/debian/control
30@@ -5,7 +5,8 @@
31 Source: gnome-calculator
32 Section: math
33 Priority: optional
34-Maintainer: Debian GNOME Maintainers <pkg-gnome-maintainers@lists.alioth.debian.org>
35+Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
36+XSBC-Original-Maintainer: Debian GNOME Maintainers <pkg-gnome-maintainers@lists.alioth.debian.org>
37 Uploaders: Jeremy Bicha <jbicha@debian.org>, Laurent Bigonville <bigon@debian.org>, Michael Biebl <biebl@debian.org>
38 Build-Depends: appstream-util,
39 debhelper (>= 11.1.3),
40diff --git a/debian/control.in b/debian/control.in
41index b5194ee..d9e0327 100644
42--- a/debian/control.in
43+++ b/debian/control.in
44@@ -1,7 +1,8 @@
45 Source: gnome-calculator
46 Section: math
47 Priority: optional
48-Maintainer: Debian GNOME Maintainers <pkg-gnome-maintainers@lists.alioth.debian.org>
49+Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
50+XSBC-Original-Maintainer: Debian GNOME Maintainers <pkg-gnome-maintainers@lists.alioth.debian.org>
51 Uploaders: @GNOME_TEAM@
52 Build-Depends: appstream-util,
53 debhelper (>= 11.1.3),
54diff --git a/debian/patches/search-provider-Handle-errors-gracefully.patch b/debian/patches/search-provider-Handle-errors-gracefully.patch
55new file mode 100644
56index 0000000..f78c54b
57--- /dev/null
58+++ b/debian/patches/search-provider-Handle-errors-gracefully.patch
59@@ -0,0 +1,108 @@
60+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
61+Date: Wed, 5 Sep 2018 17:11:27 +0200
62+Subject: search-provider: Handle errors gracefully
63+
64+Instead of exiting immediately if a spawn error occurred, inform our dbus
65+client about it with a proper spawn error.
66+
67+As bonus, get rid of the compile warning.
68+
69+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/gnome-calculator/+bug/1795399
70+Forwarded: https://gitlab.gnome.org/GNOME/gnome-calculator/merge_requests/10
71+Applied-Upstream: yes, 3.31.1
72+---
73+ search-provider/search-provider.vala | 24 +++++++++++++-----------
74+ 1 file changed, 13 insertions(+), 11 deletions(-)
75+
76+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
77+index fc72128..a48c440 100644
78+--- a/search-provider/search-provider.vala
79++++ b/search-provider/search-provider.vala
80+@@ -63,7 +63,7 @@ public class SearchProvider : Object
81+ }
82+ catch (Error e)
83+ {
84+- error ("Failed to spawn Calculator: %s", e.message);
85++ throw e;
86+ }
87+
88+ if (cancellable == null)
89+@@ -79,7 +79,7 @@ public class SearchProvider : Object
90+ return subprocess;
91+ }
92+
93+- private async bool solve_equation (string equation)
94++ private async bool solve_equation (string equation) throws DBusError
95+ {
96+ string? result;
97+
98+@@ -112,7 +112,8 @@ public class SearchProvider : Object
99+ }
100+ catch (SpawnError e)
101+ {
102+- error ("Failed to spawn Calculator: %s", e.message);
103++ critical ("Failed to spawn Calculator: %s", e.message);
104++ throw new DBusError.SPAWN_FAILED (e.message);
105+ }
106+ catch (Error e)
107+ {
108+@@ -128,7 +129,7 @@ public class SearchProvider : Object
109+ return true;
110+ }
111+
112+- private async string[] get_result_identifier (string[] terms)
113++ private async string[] get_result_identifier (string[] terms) throws Error
114+ {
115+ /* We have at most one result: the search terms as one string */
116+ var equation = terms_to_equation (terms);
117+@@ -138,17 +139,17 @@ public class SearchProvider : Object
118+ return new string[0];
119+ }
120+
121+- public async string[] get_initial_result_set (string[] terms)
122++ public async string[] get_initial_result_set (string[] terms) throws Error
123+ {
124+ return yield get_result_identifier (terms);
125+ }
126+
127+- public async string[] get_subsearch_result_set (string[] previous_results, string[] terms)
128++ public async string[] get_subsearch_result_set (string[] previous_results, string[] terms) throws Error
129+ {
130+ return yield get_result_identifier (terms);
131+ }
132+
133+- public async HashTable<string, Variant>[] get_result_metas (string[] results, GLib.BusName sender)
134++ public async HashTable<string, Variant>[] get_result_metas (string[] results, GLib.BusName sender) throws Error
135+ requires (results.length == 1)
136+ {
137+ string equation;
138+@@ -171,7 +172,7 @@ public class SearchProvider : Object
139+ return metadata;
140+ }
141+
142+- private static void spawn_and_display_equation (string[] terms)
143++ private static void spawn_and_display_equation (string[] terms) throws Error
144+ {
145+ try
146+ {
147+@@ -180,16 +181,17 @@ public class SearchProvider : Object
148+ }
149+ catch (SpawnError e)
150+ {
151+- error ("Failed to spawn Calculator: %s", e.message);
152++ critical ("Failed to spawn Calculator: %s", e.message);
153++ throw new DBusError.SPAWN_FAILED (e.message);
154+ }
155+ }
156+
157+- public void activate_result (string result, string[] terms, uint32 timestamp)
158++ public void activate_result (string result, string[] terms, uint32 timestamp) throws Error
159+ {
160+ spawn_and_display_equation (terms);
161+ }
162+
163+- public void launch_search (string[] terms, uint32 timestamp)
164++ public void launch_search (string[] terms, uint32 timestamp) throws Error
165+ {
166+ spawn_and_display_equation (terms);
167+ }
168diff --git a/debian/patches/search-provider-Use-async-calls-cancel-search-on-inactivi.patch b/debian/patches/search-provider-Use-async-calls-cancel-search-on-inactivi.patch
169new file mode 100644
170index 0000000..d8e38be
171--- /dev/null
172+++ b/debian/patches/search-provider-Use-async-calls-cancel-search-on-inactivi.patch
173@@ -0,0 +1,233 @@
174+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
175+Date: Fri, 24 Aug 2018 06:57:03 +0200
176+Subject: search-provider: Use async calls, cancel search on inactivity
177+
178+Move to async methods everywhere and factorize the `solve` calls to a single
179+method that only uses Subprocess and that can be cancelled.
180+
181+As per this, if the the search-provider is trying to compute some complex
182+operation, the daemon won't hang and once the applications' inactivity
183+timeout is hit, any running instance of gnome-calculator will be killed and
184+the daemon will return accordingly.
185+
186+This reduces the impact of an issue that can cause gnome-calculator to keep
187+running forever if a complex computation is required (10!!!) from the shell,
188+and won't ever be killed (see GNOME/gnome-shell#183).
189+
190+This is also a prerequisite for supporting the search Cancellation that is
191+going to be available on next version of the Search provider protocol
192+(as per GNOME/gnome-shell#436)
193+
194+Bug-Ubuntu: https://launchpad.net/bugs/1756826
195+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
196+Forwarded: https://gitlab.gnome.org/GNOME/gnome-calculator/merge_requests/10
197+Applied-Upstream: yes, 3.31.1
198+---
199+ search-provider/search-provider.vala | 126 +++++++++++++++++++++++++----------
200+ 1 file changed, 89 insertions(+), 37 deletions(-)
201+
202+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
203+index 26c72af..659f73d 100644
204+--- a/search-provider/search-provider.vala
205++++ b/search-provider/search-provider.vala
206+@@ -1,6 +1,7 @@
207+ /* -*- Mode: vala; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-
208+ *
209+ * Copyright (C) 2014 Michael Catanzaro
210++ * Copyright (C) 2018 Marco Trevisan
211+ *
212+ * This program is free software: you can redistribute it and/or modify it under
213+ * the terms of the GNU General Public License as published by the Free Software
214+@@ -12,17 +13,85 @@
215+ [DBus (name = "org.gnome.Shell.SearchProvider2")]
216+ public class SearchProvider : Object
217+ {
218++ private Cancellable cancellable = new Cancellable ();
219++
220++ ~SearchProvider ()
221++ {
222++ cancel ();
223++ }
224++
225++ [DBus (visible = false)]
226++ public void cancel ()
227++ {
228++ if (cancellable != null)
229++ {
230++ cancellable.cancel ();
231++ }
232++ }
233++
234+ private static string terms_to_equation (string[] terms)
235+ {
236+ return string.joinv (" ", terms);
237+ }
238+
239+- private static bool can_parse (string[] terms)
240++ private async Subprocess solve_subprocess (string equation, bool return_solution = false, out string? solution_buf = null) throws Error
241+ {
242++ Subprocess subprocess;
243++ string[] argv = {"gnome-calculator", "--solve"};
244++ argv += equation;
245++ argv += null;
246++
247++ debug (@"Trying to solve $(equation)");
248++
249+ try
250+ {
251+- int exit_status;
252++ // Eat output so that it doesn't wind up in the journal. It's
253++ // expected that most searches are not valid calculator input.
254++ var flags = SubprocessFlags.STDERR_PIPE;
255++
256++ if (return_solution)
257++ {
258++ flags |= SubprocessFlags.STDOUT_PIPE;
259++ }
260++
261++ subprocess = new Subprocess.newv (argv, flags);
262++ }
263++ catch (Error e)
264++ {
265++ error ("Failed to spawn Calculator: %s", e.message);
266++ }
267++
268++ try
269++ {
270++ string stderr_buf;
271++
272++ cancellable = new Cancellable ();
273++ cancellable.cancelled.connect (() => {
274++ subprocess.force_exit ();
275++ cancellable = null;
276++ });
277++
278++ yield subprocess.communicate_utf8_async (null, cancellable, out solution_buf, out stderr_buf);
279++ }
280++ catch (Error e)
281++ {
282++ if (e is IOError.CANCELLED)
283++ {
284++ throw e;
285++ }
286++ else
287++ {
288++ error ("Failed reading result: %s", e.message);
289++ }
290++ }
291+
292++ return subprocess;
293++ }
294++
295++ private async bool can_parse (string[] terms)
296++ {
297++ try
298++ {
299+ var tsep_string = Posix.nl_langinfo (Posix.NLItem.THOUSEP);
300+ if (tsep_string == null || tsep_string == "")
301+ tsep_string = " ";
302+@@ -39,14 +108,8 @@ public class SearchProvider : Object
303+ return false;
304+ }
305+
306+- // Eat output so that it doesn't wind up in the journal. It's
307+- // expected that most searches are not valid calculator input.
308+- string stdout_buf;
309+- string stderr_buf;
310+- Process.spawn_command_line_sync (
311+- "gnome-calculator --solve " + Shell.quote (equation),
312+- out stdout_buf, out stderr_buf, out exit_status);
313+- Process.check_exit_status (exit_status);
314++ var subprocess = yield solve_subprocess (equation);
315++ yield subprocess.wait_check_async ();
316+ }
317+ catch (SpawnError e)
318+ {
319+@@ -60,54 +123,37 @@ public class SearchProvider : Object
320+ return true;
321+ }
322+
323+- private static string[] get_result_identifier (string[] terms)
324+- ensures (result.length == 0 || result.length == 1)
325++ private async string[] get_result_identifier (string[] terms)
326+ {
327+ /* We have at most one result: the search terms as one string */
328+- if (can_parse (terms))
329++ if (yield can_parse (terms))
330+ return { terms_to_equation (terms) };
331+ else
332+ return new string[0];
333+ }
334+
335+- public string[] get_initial_result_set (string[] terms)
336++ public async string[] get_initial_result_set (string[] terms)
337+ {
338+- return get_result_identifier (terms);
339++ return yield get_result_identifier (terms);
340+ }
341+
342+- public string[] get_subsearch_result_set (string[] previous_results, string[] terms)
343++ public async string[] get_subsearch_result_set (string[] previous_results, string[] terms)
344+ {
345+- return get_result_identifier (terms);
346++ return yield get_result_identifier (terms);
347+ }
348+
349+- public HashTable<string, Variant>[] get_result_metas (string[] results)
350++ public async HashTable<string, Variant>[] get_result_metas (string[] results, GLib.BusName sender)
351+ requires (results.length == 1)
352+- ensures (result.length == 1)
353+ {
354+- Subprocess subprocess;
355+ string stdout_buf;
356+- string stderr_buf;
357+-
358+- string[] argv = {"gnome-calculator", "--solve"};
359+- argv += results[0];
360+- argv += null;
361+-
362+- try
363+- {
364+- subprocess = new Subprocess.newv (argv, SubprocessFlags.STDOUT_PIPE | SubprocessFlags.STDERR_PIPE);
365+- }
366+- catch (Error e)
367+- {
368+- error ("Failed to spawn Calculator: %s", e.message);
369+- }
370+
371+ try
372+ {
373+- subprocess.communicate_utf8 (null, null, out stdout_buf, out stderr_buf);
374++ yield solve_subprocess (results[0], true, out stdout_buf);
375+ }
376+ catch (Error e)
377+ {
378+- error ("Failed reading result: %s", e.message);
379++ return new HashTable<string, Variant>[0];
380+ }
381+
382+ var metadata = new HashTable<string, Variant>[1];
383+@@ -155,9 +201,11 @@ public class SearchProviderApp : Application
384+
385+ public override bool dbus_register (DBusConnection connection, string object_path)
386+ {
387++ SearchProvider search_provider = new SearchProvider ();
388++
389+ try
390+ {
391+- connection.register_object (object_path, new SearchProvider ());
392++ connection.register_object (object_path, search_provider);
393+ }
394+ catch (IOError error)
395+ {
396+@@ -165,6 +213,10 @@ public class SearchProviderApp : Application
397+ quit ();
398+ }
399+
400++ shutdown.connect (() => {
401++ search_provider.cancel ();
402++ });
403++
404+ return true;
405+ }
406+ }
407diff --git a/debian/patches/search-provider-Use-lower-inactivity-timeout.patch b/debian/patches/search-provider-Use-lower-inactivity-timeout.patch
408new file mode 100644
409index 0000000..5008fe5
410--- /dev/null
411+++ b/debian/patches/search-provider-Use-lower-inactivity-timeout.patch
412@@ -0,0 +1,30 @@
413+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
414+Date: Fri, 24 Aug 2018 07:12:12 +0200
415+Subject: search-provider: Use lower inactivity timeout
416+
417+Since we're renewing it at every call involving a process call, we can just
418+set it to a lower value than the default dbus proxy timeout, so that the provider
419+will return invalid values before that the timeout has been hit.
420+
421+Bug-Ubuntu: https://launchpad.net/bugs/1756826
422+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
423+Forwarded: https://gitlab.gnome.org/GNOME/gnome-calculator/merge_requests/10
424+Applied-Upstream: yes, 3.31.1
425+---
426+ search-provider/search-provider.vala | 3 ++-
427+ 1 file changed, 2 insertions(+), 1 deletion(-)
428+
429+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
430+index 7e54fa6..9035c58 100644
431+--- a/search-provider/search-provider.vala
432++++ b/search-provider/search-provider.vala
433+@@ -203,7 +203,8 @@ public class SearchProviderApp : Application
434+ {
435+ Object (application_id: "org.gnome.Calculator.SearchProvider",
436+ flags: ApplicationFlags.IS_SERVICE,
437+- inactivity_timeout: 60000);
438++ inactivity_timeout: 20000);
439++ }
440+
441+ public void renew_inactivity_timeout ()
442+ {
443diff --git a/debian/patches/search-provider-cache-equations-avoiding-spawning-calcula.patch b/debian/patches/search-provider-cache-equations-avoiding-spawning-calcula.patch
444new file mode 100644
445index 0000000..85d83ce
446--- /dev/null
447+++ b/debian/patches/search-provider-cache-equations-avoiding-spawning-calcula.patch
448@@ -0,0 +1,171 @@
449+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
450+Date: Mon, 27 Aug 2018 18:45:34 +0200
451+Subject: search-provider: cache equations avoiding spawning calculator twice
452+
453+Currently we spawn the calculator two times for each operation, one to check
454+if the search provider is valid for such syntax, the other time to actually
455+present the results. But since these results are just available at first
456+call, we can just keep them around and return them if the shell requires them.
457+
458+Since the search provider deamon is kept alive for just few moments, there's
459+no real need to cleanup the cache using a queue.
460+
461+In case of multiple async calls, reuse cancellable instead so that we can
462+just kill all the related processes one time at once.
463+
464+Bug-Ubuntu: https://launchpad.net/bugs/1756826
465+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
466+Forwarded: https://gitlab.gnome.org/GNOME/gnome-calculator/merge_requests/10
467+Applied-Upstream: yes, 3.31.1
468+---
469+ search-provider/search-provider.vala | 79 ++++++++++++++++++++----------------
470+ 1 file changed, 45 insertions(+), 34 deletions(-)
471+
472+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
473+index a3c57f7..af2779e 100644
474+--- a/search-provider/search-provider.vala
475++++ b/search-provider/search-provider.vala
476+@@ -14,10 +14,13 @@
477+ public class SearchProvider : Object
478+ {
479+ private unowned SearchProviderApp application;
480+- private Cancellable cancellable = new Cancellable ();
481++ private Cancellable cancellable;
482++
483++ private HashTable<string, string> cached_equations;
484+ public SearchProvider (SearchProviderApp app)
485+ {
486+ application = app;
487++ cached_equations = new HashTable<string, string> (str_hash, str_equal);
488+ }
489+
490+ ~SearchProvider ()
491+@@ -29,9 +32,7 @@ public class SearchProvider : Object
492+ public void cancel ()
493+ {
494+ if (cancellable != null)
495+- {
496+ cancellable.cancel ();
497+- }
498+ }
499+
500+ private static string terms_to_equation (string[] terms)
501+@@ -60,7 +61,9 @@ public class SearchProvider : Object
502+ error ("Failed to spawn Calculator: %s", e.message);
503+ }
504+
505+- cancellable = new Cancellable ();
506++ if (cancellable == null)
507++ cancellable = new Cancellable ();
508++
509+ cancellable.cancelled.connect (() => {
510+ subprocess.force_exit ();
511+ cancellable = null;
512+@@ -71,29 +74,36 @@ public class SearchProvider : Object
513+ return subprocess;
514+ }
515+
516+- private async bool can_parse (string[] terms)
517++ private async bool solve_equation (string equation)
518+ {
519+- try
520+- {
521+- var tsep_string = Posix.nl_langinfo (Posix.NLItem.THOUSEP);
522+- if (tsep_string == null || tsep_string == "")
523+- tsep_string = " ";
524++ string? result;
525+
526+- var decimal = Posix.nl_langinfo (Posix.NLItem.RADIXCHAR);
527+- if (decimal == null)
528+- decimal = "";
529++ cancel();
530+
531+- // "normalize" input to a format known to double.try_parse
532+- var equation = terms_to_equation (terms).replace (tsep_string, "").replace (decimal, ".");
533++ var tsep_string = Posix.nl_langinfo (Posix.NLItem.THOUSEP);
534++ if (tsep_string == null || tsep_string == "")
535++ tsep_string = " ";
536+
537+- cancel();
538++ var decimal = Posix.nl_langinfo (Posix.NLItem.RADIXCHAR);
539++ if (decimal == null)
540++ decimal = "";
541+
542+- // if the search is a plain number, don't process it
543+- if (double.try_parse (equation)) {
544+- return false;
545+- }
546++ // "normalize" input to a format known to double.try_parse
547++ var normalized_equation = equation.replace (tsep_string, "").replace (decimal, ".");
548+
549+- (yield solve_subprocess (equation)).wait_check (cancellable);
550++ // if the search is a plain number, don't process it
551++ if (double.try_parse (normalized_equation)) {
552++ return false;
553++ }
554++
555++ if (cached_equations.lookup (equation) != null)
556++ return true;
557++
558++ try
559++ {
560++ var subprocess = yield solve_subprocess (normalized_equation);
561++ yield subprocess.communicate_utf8_async (null, cancellable, out result, null);
562++ subprocess.wait_check (cancellable);
563+ }
564+ catch (SpawnError e)
565+ {
566+@@ -104,14 +114,17 @@ public class SearchProvider : Object
567+ return false;
568+ }
569+
570++ cached_equations.insert (equation, result.strip ());
571++
572+ return true;
573+ }
574+
575+ private async string[] get_result_identifier (string[] terms)
576+ {
577+ /* We have at most one result: the search terms as one string */
578+- if (yield can_parse (terms))
579+- return { terms_to_equation (terms) };
580++ var equation = terms_to_equation (terms);
581++ if (yield solve_equation (equation))
582++ return { equation };
583+ else
584+ return new string[0];
585+ }
586+@@ -129,24 +142,22 @@ public class SearchProvider : Object
587+ public async HashTable<string, Variant>[] get_result_metas (string[] results, GLib.BusName sender)
588+ requires (results.length == 1)
589+ {
590+- string stdout_buf;
591+- string stderr_buf;
592++ string equation;
593++ string result;
594+
595+- try
596+- {
597+- var subprocess = yield solve_subprocess (results[0]);
598+- yield subprocess.communicate_utf8_async (null, cancellable, out stdout_buf, out stderr_buf);
599+- }
600+- catch (Error e)
601+- {
602++ equation = terms_to_equation (results);
603++
604++ if (!yield solve_equation (equation))
605+ return new HashTable<string, Variant>[0];
606+- }
607++
608++ result = cached_equations.lookup (equation);
609++ assert (result != null);
610+
611+ var metadata = new HashTable<string, Variant>[1];
612+ metadata[0] = new HashTable<string, Variant>(str_hash, str_equal);
613+ metadata[0].insert ("id", results[0]);
614+ metadata[0].insert ("name", results[0] );
615+- metadata[0].insert ("description", " = " + stdout_buf.strip ());
616++ metadata[0].insert ("description", @" = $result");
617+
618+ return metadata;
619+ }
620diff --git a/debian/patches/search-provider-cache-only-a-limited-number-of-equations.patch b/debian/patches/search-provider-cache-only-a-limited-number-of-equations.patch
621new file mode 100644
622index 0000000..651aa28
623--- /dev/null
624+++ b/debian/patches/search-provider-cache-only-a-limited-number-of-equations.patch
625@@ -0,0 +1,45 @@
626+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
627+Date: Mon, 27 Aug 2018 18:56:09 +0200
628+Subject: search-provider: cache only a limited number of equations
629+
630+Bug-Ubuntu: https://launchpad.net/bugs/1756826
631+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
632+Forwarded: https://gitlab.gnome.org/GNOME/gnome-calculator/merge_requests/10
633+Applied-Upstream: yes, 3.31.1
634+---
635+ search-provider/search-provider.vala | 9 +++++++++
636+ 1 file changed, 9 insertions(+)
637+
638+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
639+index af2779e..fc72128 100644
640+--- a/search-provider/search-provider.vala
641++++ b/search-provider/search-provider.vala
642+@@ -16,10 +16,15 @@ public class SearchProvider : Object
643+ private unowned SearchProviderApp application;
644+ private Cancellable cancellable;
645+
646++ private const int MAX_CACHED_OPERATIONS = 10;
647++ private Queue<string> queued_equations;
648+ private HashTable<string, string> cached_equations;
649++
650+ public SearchProvider (SearchProviderApp app)
651+ {
652+ application = app;
653++
654++ queued_equations = new Queue<string> ();
655+ cached_equations = new HashTable<string, string> (str_hash, str_equal);
656+ }
657+
658+@@ -114,8 +119,12 @@ public class SearchProvider : Object
659+ return false;
660+ }
661+
662++ queued_equations.push_tail (equation);
663+ cached_equations.insert (equation, result.strip ());
664+
665++ if (queued_equations.length > MAX_CACHED_OPERATIONS)
666++ cached_equations.remove (queued_equations.pop_head ());
667++
668+ return true;
669+ }
670+
671diff --git a/debian/patches/search-provider-cancel-the-current-process-on-new-calcula.patch b/debian/patches/search-provider-cancel-the-current-process-on-new-calcula.patch
672new file mode 100644
673index 0000000..26435e2
674--- /dev/null
675+++ b/debian/patches/search-provider-cancel-the-current-process-on-new-calcula.patch
676@@ -0,0 +1,30 @@
677+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
678+Date: Thu, 30 Aug 2018 18:47:16 +0200
679+Subject: search-provider: cancel the current process on new calculation
680+ request
681+
682+As there's just one shell running at time, there's no point of supporting
683+parallel calls, we can just safely refer to the last equation as the only one
684+we need to compute.
685+
686+Bug-Ubuntu: https://launchpad.net/bugs/1756826
687+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
688+Forwarded: https://gitlab.gnome.org/GNOME/gnome-calculator/merge_requests/10
689+Applied-Upstream: yes, 3.31.1
690+---
691+ search-provider/search-provider.vala | 2 ++
692+ 1 file changed, 2 insertions(+)
693+
694+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
695+index df37b86..a3c57f7 100644
696+--- a/search-provider/search-provider.vala
697++++ b/search-provider/search-provider.vala
698+@@ -86,6 +86,8 @@ public class SearchProvider : Object
699+ // "normalize" input to a format known to double.try_parse
700+ var equation = terms_to_equation (terms).replace (tsep_string, "").replace (decimal, ".");
701+
702++ cancel();
703++
704+ // if the search is a plain number, don't process it
705+ if (double.try_parse (equation)) {
706+ return false;
707diff --git a/debian/patches/search-provider-renew-inactivity-timeout-at-each-calculat.patch b/debian/patches/search-provider-renew-inactivity-timeout-at-each-calculat.patch
708new file mode 100644
709index 0000000..274aae1
710--- /dev/null
711+++ b/debian/patches/search-provider-renew-inactivity-timeout-at-each-calculat.patch
712@@ -0,0 +1,61 @@
713+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
714+Date: Fri, 24 Aug 2018 07:10:17 +0200
715+Subject: search-provider: renew inactivity timeout at each calculator run
716+
717+As per the async rewrite, now the daemon inactivity timeout might happen
718+when another call has just been done, while we don't want this to be the case.
719+
720+So, everytime we do a subprocess call, let's renew the application timeout.
721+
722+Bug-Ubuntu: https://launchpad.net/bugs/1756826
723+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
724+Forwarded: https://gitlab.gnome.org/GNOME/gnome-calculator/merge_requests/100
725+Applied-Upstream: yes, 3.31.1
726+---
727+ search-provider/search-provider.vala | 14 +++++++++++++-
728+ 1 file changed, 13 insertions(+), 1 deletion(-)
729+
730+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
731+index 659f73d..7e54fa6 100644
732+--- a/search-provider/search-provider.vala
733++++ b/search-provider/search-provider.vala
734+@@ -13,7 +13,12 @@
735+ [DBus (name = "org.gnome.Shell.SearchProvider2")]
736+ public class SearchProvider : Object
737+ {
738++ private unowned SearchProviderApp application;
739+ private Cancellable cancellable = new Cancellable ();
740++ public SearchProvider (SearchProviderApp app)
741++ {
742++ application = app;
743++ }
744+
745+ ~SearchProvider ()
746+ {
747+@@ -71,6 +76,8 @@ public class SearchProvider : Object
748+ cancellable = null;
749+ });
750+
751++ application.renew_inactivity_timeout ();
752++
753+ yield subprocess.communicate_utf8_async (null, cancellable, out solution_buf, out stderr_buf);
754+ }
755+ catch (Error e)
756+@@ -197,11 +204,16 @@ public class SearchProviderApp : Application
757+ Object (application_id: "org.gnome.Calculator.SearchProvider",
758+ flags: ApplicationFlags.IS_SERVICE,
759+ inactivity_timeout: 60000);
760++
761++ public void renew_inactivity_timeout ()
762++ {
763++ this.hold ();
764++ this.release ();
765+ }
766+
767+ public override bool dbus_register (DBusConnection connection, string object_path)
768+ {
769+- SearchProvider search_provider = new SearchProvider ();
770++ SearchProvider search_provider = new SearchProvider (this);
771+
772+ try
773+ {
774diff --git a/debian/patches/search-provider-simplify-solve_subprocess.patch b/debian/patches/search-provider-simplify-solve_subprocess.patch
775new file mode 100644
776index 0000000..4f9cde8
777--- /dev/null
778+++ b/debian/patches/search-provider-simplify-solve_subprocess.patch
779@@ -0,0 +1,114 @@
780+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
781+Date: Fri, 24 Aug 2018 07:31:37 +0200
782+Subject: search-provider: simplify solve_subprocess
783+
784+Only return a subprocess in solve_subprocess and make the callers deal with
785+the actual operation to perform.
786+
787+Bug-Ubuntu: https://launchpad.net/bugs/1756826
788+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
789+Forwarded: https://gitlab.gnome.org/GNOME/gnome-calculator/merge_requests/10
790+Applied-Upstream: yes, 3.31.1
791+---
792+ search-provider/search-provider.vala | 49 ++++++++++--------------------------
793+ 1 file changed, 13 insertions(+), 36 deletions(-)
794+
795+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
796+index 9035c58..df37b86 100644
797+--- a/search-provider/search-provider.vala
798++++ b/search-provider/search-provider.vala
799+@@ -39,7 +39,7 @@ public class SearchProvider : Object
800+ return string.joinv (" ", terms);
801+ }
802+
803+- private async Subprocess solve_subprocess (string equation, bool return_solution = false, out string? solution_buf = null) throws Error
804++ private async Subprocess solve_subprocess (string equation) throws Error
805+ {
806+ Subprocess subprocess;
807+ string[] argv = {"gnome-calculator", "--solve"};
808+@@ -52,13 +52,7 @@ public class SearchProvider : Object
809+ {
810+ // Eat output so that it doesn't wind up in the journal. It's
811+ // expected that most searches are not valid calculator input.
812+- var flags = SubprocessFlags.STDERR_PIPE;
813+-
814+- if (return_solution)
815+- {
816+- flags |= SubprocessFlags.STDOUT_PIPE;
817+- }
818+-
819++ var flags = SubprocessFlags.STDOUT_PIPE | SubprocessFlags.STDERR_PIPE;
820+ subprocess = new Subprocess.newv (argv, flags);
821+ }
822+ catch (Error e)
823+@@ -66,31 +60,13 @@ public class SearchProvider : Object
824+ error ("Failed to spawn Calculator: %s", e.message);
825+ }
826+
827+- try
828+- {
829+- string stderr_buf;
830+-
831+- cancellable = new Cancellable ();
832+- cancellable.cancelled.connect (() => {
833+- subprocess.force_exit ();
834+- cancellable = null;
835+- });
836+-
837+- application.renew_inactivity_timeout ();
838++ cancellable = new Cancellable ();
839++ cancellable.cancelled.connect (() => {
840++ subprocess.force_exit ();
841++ cancellable = null;
842++ });
843+
844+- yield subprocess.communicate_utf8_async (null, cancellable, out solution_buf, out stderr_buf);
845+- }
846+- catch (Error e)
847+- {
848+- if (e is IOError.CANCELLED)
849+- {
850+- throw e;
851+- }
852+- else
853+- {
854+- error ("Failed reading result: %s", e.message);
855+- }
856+- }
857++ application.renew_inactivity_timeout ();
858+
859+ return subprocess;
860+ }
861+@@ -115,8 +91,7 @@ public class SearchProvider : Object
862+ return false;
863+ }
864+
865+- var subprocess = yield solve_subprocess (equation);
866+- yield subprocess.wait_check_async ();
867++ (yield solve_subprocess (equation)).wait_check (cancellable);
868+ }
869+ catch (SpawnError e)
870+ {
871+@@ -153,10 +128,12 @@ public class SearchProvider : Object
872+ requires (results.length == 1)
873+ {
874+ string stdout_buf;
875++ string stderr_buf;
876+
877+ try
878+ {
879+- yield solve_subprocess (results[0], true, out stdout_buf);
880++ var subprocess = yield solve_subprocess (results[0]);
881++ yield subprocess.communicate_utf8_async (null, cancellable, out stdout_buf, out stderr_buf);
882+ }
883+ catch (Error e)
884+ {
885+@@ -167,7 +144,7 @@ public class SearchProvider : Object
886+ metadata[0] = new HashTable<string, Variant>(str_hash, str_equal);
887+ metadata[0].insert ("id", results[0]);
888+ metadata[0].insert ("name", results[0] );
889+- metadata[0].insert ("description", " = " + stdout_buf.strip() );
890++ metadata[0].insert ("description", " = " + stdout_buf.strip ());
891+
892+ return metadata;
893+ }
894diff --git a/debian/patches/series b/debian/patches/series
895index e69de29..2755c44 100644
896--- a/debian/patches/series
897+++ b/debian/patches/series
898@@ -0,0 +1,9 @@
899+search-provider-Use-async-calls-cancel-search-on-inactivi.patch
900+search-provider-renew-inactivity-timeout-at-each-calculat.patch
901+search-provider-Use-lower-inactivity-timeout.patch
902+search-provider-simplify-solve_subprocess.patch
903+search-provider-cancel-the-current-process-on-new-calcula.patch
904+search-provider-cache-equations-avoiding-spawning-calcula.patch
905+search-provider-cache-only-a-limited-number-of-equations.patch
906+search-provider-Handle-errors-gracefully.patch
907+ubuntu/search-provider-Cancel-operations-on-XUbuntuCancel.patch
908diff --git a/debian/patches/ubuntu/search-provider-Cancel-operations-on-XUbuntuCancel.patch b/debian/patches/ubuntu/search-provider-Cancel-operations-on-XUbuntuCancel.patch
909new file mode 100644
910index 0000000..edc9419
911--- /dev/null
912+++ b/debian/patches/ubuntu/search-provider-Cancel-operations-on-XUbuntuCancel.patch
913@@ -0,0 +1,118 @@
914+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
915+Date: Tue, 28 Aug 2018 04:12:20 +0200
916+Subject: search-provider: Cancel operations on XUbuntuCancel
917+
918+Stop process and any computation on XUbuntuCancel method, if the caller
919+was the same triggering the last operation.
920+
921+Fixes LP: #1756826
922+
923+Bug-Ubuntu: https://launchpad.net/bugs/1756826
924+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
925+Forwarded: not-needed
926+---
927+ search-provider/search-provider.vala | 46 ++++++++++++++++++++++++++++++++----
928+ 1 file changed, 41 insertions(+), 5 deletions(-)
929+
930+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
931+index a48c440..9823ef7 100644
932+--- a/search-provider/search-provider.vala
933++++ b/search-provider/search-provider.vala
934+@@ -10,11 +10,33 @@
935+ * license.
936+ */
937+
938++public class CallerTracker : Object
939++{
940++ public BusName? bus { get; set; }
941++}
942++
943++public class Caller
944++{
945++ private CallerTracker caller_tracker;
946++
947++ public Caller (CallerTracker c, BusName sender)
948++ {
949++ caller_tracker = c;
950++ caller_tracker.bus = sender;
951++ }
952++
953++ ~Caller ()
954++ {
955++ caller_tracker.bus = null;
956++ }
957++}
958++
959+ [DBus (name = "org.gnome.Shell.SearchProvider2")]
960+ public class SearchProvider : Object
961+ {
962+ private unowned SearchProviderApp application;
963+ private Cancellable cancellable;
964++ private CallerTracker caller_tracker;
965+
966+ private const int MAX_CACHED_OPERATIONS = 10;
967+ private Queue<string> queued_equations;
968+@@ -24,6 +46,7 @@ public class SearchProvider : Object
969+ {
970+ application = app;
971+
972++ caller_tracker = new CallerTracker ();
973+ queued_equations = new Queue<string> ();
974+ cached_equations = new HashTable<string, string> (str_hash, str_equal);
975+ }
976+@@ -129,8 +152,10 @@ public class SearchProvider : Object
977+ return true;
978+ }
979+
980+- private async string[] get_result_identifier (string[] terms) throws Error
981++ private async string[] get_result_identifier (string[] terms, GLib.BusName sender) throws Error
982+ {
983++ var owner = new Caller (caller_tracker, sender); (void) owner;
984++
985+ /* We have at most one result: the search terms as one string */
986+ var equation = terms_to_equation (terms);
987+ if (yield solve_equation (equation))
988+@@ -139,14 +164,14 @@ public class SearchProvider : Object
989+ return new string[0];
990+ }
991+
992+- public async string[] get_initial_result_set (string[] terms) throws Error
993++ public async string[] get_initial_result_set (string[] terms, GLib.BusName sender) throws Error
994+ {
995+- return yield get_result_identifier (terms);
996++ return yield get_result_identifier (terms, sender);
997+ }
998+
999+- public async string[] get_subsearch_result_set (string[] previous_results, string[] terms) throws Error
1000++ public async string[] get_subsearch_result_set (string[] previous_results, string[] terms, GLib.BusName sender) throws Error
1001+ {
1002+- return yield get_result_identifier (terms);
1003++ return yield get_result_identifier (terms, sender);
1004+ }
1005+
1006+ public async HashTable<string, Variant>[] get_result_metas (string[] results, GLib.BusName sender) throws Error
1007+@@ -155,6 +180,8 @@ public class SearchProvider : Object
1008+ string equation;
1009+ string result;
1010+
1011++ var owner = new Caller (caller_tracker, sender); (void) owner;
1012++
1013+ equation = terms_to_equation (results);
1014+
1015+ if (!yield solve_equation (equation))
1016+@@ -172,6 +199,15 @@ public class SearchProvider : Object
1017+ return metadata;
1018+ }
1019+
1020++ public async void x_ubuntu_cancel (BusName sender)
1021++ {
1022++ if (caller_tracker.bus == sender)
1023++ {
1024++ debug ("Cancelling Request");
1025++ cancel ();
1026++ }
1027++ }
1028++
1029+ private static void spawn_and_display_equation (string[] terms) throws Error
1030+ {
1031+ try

Subscribers

People subscribed via source and target branches