Marco Trevisan (Treviño) has proposed merging
~3v1n0/ubuntu/+source/gnome-calculator:ubuntu/bionic-xubuntu-cancel-search into
~ubuntu-desktop/ubuntu/+source/gnome-calculator:ubuntu/bionic with
~3v1n0/ubuntu/+source/gnome-calculator:ubuntu/bionic as a prerequisite.
Requested reviews:
Ubuntu Desktop (ubuntu-desktop)
Related bugs:
Bug #1756826 in nautilus (Ubuntu): "hangs when remote search provider
performs expensive operation"
https://bugs.launchpad.net/ubuntu/+source/nautilus/+bug/1756826
For more details, see:
https://code.launchpad.net/~3v1n0/ubuntu/+source/gnome-calculator/+git/gnome-calculator/+merge/354335
--
Your team Ubuntu Desktop is requested to review the proposed merge of
~3v1n0/ubuntu/+source/gnome-calculator:ubuntu/bionic-xubuntu-cancel-search into
~ubuntu-desktop/ubuntu/+source/gnome-calculator:ubuntu/bionic.
diff --git a/debian/changelog b/debian/changelog
index a7346e1..ae09540 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -7,6 +7,17 @@ gnome-calculator (1:3.28.2-1ubuntu1) UNRELEASED; urgency=medium
- Use Ubuntu Vcs-* URIs, and move debian's to XS-Debian-Vcs-*
* debian/gbp.conf:
- update branches to point to ubuntu/bionic
+ d/p/search-provider-Use-async-calls-cancel-search-on-inactivi.patch,
+ d/p/search-provider-renew-inactivity-timeout-at-each-calculat.patch,
+ d/p/search-provider-Use-lower-inactivity-timeout.patch,
+ d/p/search-provider-simplify-solve_subprocess.patch,
+ d/p/search-provider-cache-equations-avoiding-spawning-calcula.patch,
+ d/p/search-provider-cancel-the-current-process-on-new-calcula.patch,
+ d/p/search-provider-cache-only-a-limited-number-of-equations.patch,
+ d/p/ubuntu/search-provider-Cancel-operations-on-XUbuntuCancel.patch:
+ - Make search provider async and support XUbuntuCancel method to
+ stop expensive operations that might lead to an unresponsive
+ process (LP: #1756826)
-- Marco Trevisan (Treviño) <ma...@ubuntu.com> Wed, 05 Sep 2018 15:26:32 +0200
diff --git a/debian/control b/debian/control
index cd4706c..0782192 100644
--- a/debian/control
+++ b/debian/control
@@ -5,7 +5,8 @@
Source: gnome-calculator
Section: math
Priority: optional
-Maintainer: Debian GNOME Maintainers <pkg-gnome-maintain...@lists.alioth.debian.org>
+Maintainer: Ubuntu Developers <ubuntu-devel-disc...@lists.ubuntu.com>
+XSBC-Original-Maintainer: Debian GNOME Maintainers <pkg-gnome-maintain...@lists.alioth.debian.org>
Uploaders: Jeremy Bicha <jbi...@debian.org>, Laurent Bigonville <bi...@debian.org>, Michael Biebl <bi...@debian.org>
Build-Depends: appstream-util,
debhelper (>= 11.1.3),
diff --git a/debian/control.in b/debian/control.in
index b5194ee..d9e0327 100644
--- a/debian/control.in
+++ b/debian/control.in
@@ -1,7 +1,8 @@
Source: gnome-calculator
Section: math
Priority: optional
-Maintainer: Debian GNOME Maintainers <pkg-gnome-maintain...@lists.alioth.debian.org>
+Maintainer: Ubuntu Developers <ubuntu-devel-disc...@lists.ubuntu.com>
+XSBC-Original-Maintainer: Debian GNOME Maintainers <pkg-gnome-maintain...@lists.alioth.debian.org>
Uploaders: @GNOME_TEAM@
Build-Depends: appstream-util,
debhelper (>= 11.1.3),
diff --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
new file mode 100644
index 0000000..324be46
--- /dev/null
+++ b/debian/patches/search-provider-Use-async-calls-cancel-search-on-inactivi.patch
@@ -0,0 +1,232 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <m...@3v1n0.net>
+Date: Fri, 24 Aug 2018 06:57:03 +0200
+Subject: search-provider: Use async calls, cancel search on inactivity
+
+Move to async methods everywhere and factorize the `solve` calls to a single
+method that only uses Subprocess and that can be cancelled.
+
+As per this, if the the search-provider is trying to compute some complex
+operation, the daemon won't hang and once the applications' inactivity
+timeout is hit, any running instance of gnome-calculator will be killed and
+the daemon will return accordingly.
+
+This reduces the impact of an issue that can cause gnome-calculator to keep
+running forever if a complex computation is required (10!!!) from the shell,
+and won't ever be killed (see GNOME/gnome-shell#183).
+
+This is also a prerequisite for supporting the search Cancellation that is
+going to be available on next version of the Search provider protocol
+(as per GNOME/gnome-shell#436)
+
+Bug-Ubuntu: https://launchpad.net/bugs/1756826
+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
+Forwarded: https://gitlab.gnome.org/GNOME/gnome-calculator/merge_requests/10
+---
+ search-provider/search-provider.vala | 126 +++++++++++++++++++++++++----------
+ 1 file changed, 89 insertions(+), 37 deletions(-)
+
+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
+index 26c72af..659f73d 100644
+--- a/search-provider/search-provider.vala
++++ b/search-provider/search-provider.vala
+@@ -1,6 +1,7 @@
+ /* -*- Mode: vala; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+ *
+ * Copyright (C) 2014 Michael Catanzaro
++ * Copyright (C) 2018 Marco Trevisan
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License as published by the Free Software
+@@ -12,17 +13,85 @@
+ [DBus (name = "org.gnome.Shell.SearchProvider2")]
+ public class SearchProvider : Object
+ {
++ private Cancellable cancellable = new Cancellable ();
++
++ ~SearchProvider ()
++ {
++ cancel ();
++ }
++
++ [DBus (visible = false)]
++ public void cancel ()
++ {
++ if (cancellable != null)
++ {
++ cancellable.cancel ();
++ }
++ }
++
+ private static string terms_to_equation (string[] terms)
+ {
+ return string.joinv (" ", terms);
+ }
+
+- private static bool can_parse (string[] terms)
++ private async Subprocess solve_subprocess (string equation, bool return_solution = false, out string? solution_buf = null) throws Error
+ {
++ Subprocess subprocess;
++ string[] argv = {"gnome-calculator", "--solve"};
++ argv += equation;
++ argv += null;
++
++ debug (@"Trying to solve $(equation)");
++
+ try
+ {
+- int exit_status;
++ // Eat output so that it doesn't wind up in the journal. It's
++ // expected that most searches are not valid calculator input.
++ var flags = SubprocessFlags.STDERR_PIPE;
++
++ if (return_solution)
++ {
++ flags |= SubprocessFlags.STDOUT_PIPE;
++ }
++
++ subprocess = new Subprocess.newv (argv, flags);
++ }
++ catch (Error e)
++ {
++ error ("Failed to spawn Calculator: %s", e.message);
++ }
++
++ try
++ {
++ string stderr_buf;
++
++ cancellable = new Cancellable ();
++ cancellable.cancelled.connect (() => {
++ subprocess.force_exit ();
++ cancellable = null;
++ });
++
++ yield subprocess.communicate_utf8_async (null, cancellable, out solution_buf, out stderr_buf);
++ }
++ catch (Error e)
++ {
++ if (e is IOError.CANCELLED)
++ {
++ throw e;
++ }
++ else
++ {
++ error ("Failed reading result: %s", e.message);
++ }
++ }
+
++ return subprocess;
++ }
++
++ private async bool can_parse (string[] terms)
++ {
++ try
++ {
+ var tsep_string = Posix.nl_langinfo (Posix.NLItem.THOUSEP);
+ if (tsep_string == null || tsep_string == "")
+ tsep_string = " ";
+@@ -39,14 +108,8 @@ public class SearchProvider : Object
+ return false;
+ }
+
+- // Eat output so that it doesn't wind up in the journal. It's
+- // expected that most searches are not valid calculator input.
+- string stdout_buf;
+- string stderr_buf;
+- Process.spawn_command_line_sync (
+- "gnome-calculator --solve " + Shell.quote (equation),
+- out stdout_buf, out stderr_buf, out exit_status);
+- Process.check_exit_status (exit_status);
++ var subprocess = yield solve_subprocess (equation);
++ yield subprocess.wait_check_async ();
+ }
+ catch (SpawnError e)
+ {
+@@ -60,54 +123,37 @@ public class SearchProvider : Object
+ return true;
+ }
+
+- private static string[] get_result_identifier (string[] terms)
+- ensures (result.length == 0 || result.length == 1)
++ private async string[] get_result_identifier (string[] terms)
+ {
+ /* We have at most one result: the search terms as one string */
+- if (can_parse (terms))
++ if (yield can_parse (terms))
+ return { terms_to_equation (terms) };
+ else
+ return new string[0];
+ }
+
+- public string[] get_initial_result_set (string[] terms)
++ public async string[] get_initial_result_set (string[] terms)
+ {
+- return get_result_identifier (terms);
++ return yield get_result_identifier (terms);
+ }
+
+- public string[] get_subsearch_result_set (string[] previous_results, string[] terms)
++ public async string[] get_subsearch_result_set (string[] previous_results, string[] terms)
+ {
+- return get_result_identifier (terms);
++ return yield get_result_identifier (terms);
+ }
+
+- public HashTable<string, Variant>[] get_result_metas (string[] results)
++ public async HashTable<string, Variant>[] get_result_metas (string[] results, GLib.BusName sender)
+ requires (results.length == 1)
+- ensures (result.length == 1)
+ {
+- Subprocess subprocess;
+ string stdout_buf;
+- string stderr_buf;
+-
+- string[] argv = {"gnome-calculator", "--solve"};
+- argv += results[0];
+- argv += null;
+-
+- try
+- {
+- subprocess = new Subprocess.newv (argv, SubprocessFlags.STDOUT_PIPE | SubprocessFlags.STDERR_PIPE);
+- }
+- catch (Error e)
+- {
+- error ("Failed to spawn Calculator: %s", e.message);
+- }
+
+ try
+ {
+- subprocess.communicate_utf8 (null, null, out stdout_buf, out stderr_buf);
++ yield solve_subprocess (results[0], true, out stdout_buf);
+ }
+ catch (Error e)
+ {
+- error ("Failed reading result: %s", e.message);
++ return new HashTable<string, Variant>[0];
+ }
+
+ var metadata = new HashTable<string, Variant>[1];
+@@ -155,9 +201,11 @@ public class SearchProviderApp : Application
+
+ public override bool dbus_register (DBusConnection connection, string object_path)
+ {
++ SearchProvider search_provider = new SearchProvider ();
++
+ try
+ {
+- connection.register_object (object_path, new SearchProvider ());
++ connection.register_object (object_path, search_provider);
+ }
+ catch (IOError error)
+ {
+@@ -165,6 +213,10 @@ public class SearchProviderApp : Application
+ quit ();
+ }
+
++ shutdown.connect (() => {
++ search_provider.cancel ();
++ });
++
+ return true;
+ }
+ }
diff --git a/debian/patches/search-provider-Use-lower-inactivity-timeout.patch b/debian/patches/search-provider-Use-lower-inactivity-timeout.patch
new file mode 100644
index 0000000..b5b0dd6
--- /dev/null
+++ b/debian/patches/search-provider-Use-lower-inactivity-timeout.patch
@@ -0,0 +1,29 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <m...@3v1n0.net>
+Date: Fri, 24 Aug 2018 07:12:12 +0200
+Subject: search-provider: Use lower inactivity timeout
+
+Since we're renewing it at every call involving a process call, we can just
+set it to a lower value than the default dbus proxy timeout, so that the provider
+will return invalid values before that the timeout has been hit.
+
+Bug-Ubuntu: https://launchpad.net/bugs/1756826
+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
+Forwarded: https://gitlab.gnome.org/GNOME/gnome-calculator/merge_requests/10
+---
+ search-provider/search-provider.vala | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
+index 7e54fa6..9035c58 100644
+--- a/search-provider/search-provider.vala
++++ b/search-provider/search-provider.vala
+@@ -203,7 +203,8 @@ public class SearchProviderApp : Application
+ {
+ Object (application_id: "org.gnome.Calculator.SearchProvider",
+ flags: ApplicationFlags.IS_SERVICE,
+- inactivity_timeout: 60000);
++ inactivity_timeout: 20000);
++ }
+
+ public void renew_inactivity_timeout ()
+ {
diff --git a/debian/patches/search-provider-cache-equations-avoiding-spawning-calcula.patch b/debian/patches/search-provider-cache-equations-avoiding-spawning-calcula.patch
new file mode 100644
index 0000000..b3903df
--- /dev/null
+++ b/debian/patches/search-provider-cache-equations-avoiding-spawning-calcula.patch
@@ -0,0 +1,170 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <m...@3v1n0.net>
+Date: Mon, 27 Aug 2018 18:45:34 +0200
+Subject: search-provider: cache equations avoiding spawning calculator twice
+
+Currently we spawn the calculator two times for each operation, one to check
+if the search provider is valid for such syntax, the other time to actually
+present the results. But since these results are just available at first
+call, we can just keep them around and return them if the shell requires them.
+
+Since the search provider deamon is kept alive for just few moments, there's
+no real need to cleanup the cache using a queue.
+
+In case of multiple async calls, reuse cancellable instead so that we can
+just kill all the related processes one time at once.
+
+Bug-Ubuntu: https://launchpad.net/bugs/1756826
+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
+Forwarded: https://gitlab.gnome.org/GNOME/gnome-calculator/merge_requests/10
+---
+ search-provider/search-provider.vala | 79 ++++++++++++++++++++----------------
+ 1 file changed, 45 insertions(+), 34 deletions(-)
+
+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
+index a3c57f7..af2779e 100644
+--- a/search-provider/search-provider.vala
++++ b/search-provider/search-provider.vala
+@@ -14,10 +14,13 @@
+ public class SearchProvider : Object
+ {
+ private unowned SearchProviderApp application;
+- private Cancellable cancellable = new Cancellable ();
++ private Cancellable cancellable;
++
++ private HashTable<string, string> cached_equations;
+ public SearchProvider (SearchProviderApp app)
+ {
+ application = app;
++ cached_equations = new HashTable<string, string> (str_hash, str_equal);
+ }
+
+ ~SearchProvider ()
+@@ -29,9 +32,7 @@ public class SearchProvider : Object
+ public void cancel ()
+ {
+ if (cancellable != null)
+- {
+ cancellable.cancel ();
+- }
+ }
+
+ private static string terms_to_equation (string[] terms)
+@@ -60,7 +61,9 @@ public class SearchProvider : Object
+ error ("Failed to spawn Calculator: %s", e.message);
+ }
+
+- cancellable = new Cancellable ();
++ if (cancellable == null)
++ cancellable = new Cancellable ();
++
+ cancellable.cancelled.connect (() => {
+ subprocess.force_exit ();
+ cancellable = null;
+@@ -71,29 +74,36 @@ public class SearchProvider : Object
+ return subprocess;
+ }
+
+- private async bool can_parse (string[] terms)
++ private async bool solve_equation (string equation)
+ {
+- try
+- {
+- var tsep_string = Posix.nl_langinfo (Posix.NLItem.THOUSEP);
+- if (tsep_string == null || tsep_string == "")
+- tsep_string = " ";
++ string? result;
+
+- var decimal = Posix.nl_langinfo (Posix.NLItem.RADIXCHAR);
+- if (decimal == null)
+- decimal = "";
++ cancel();
+
+- // "normalize" input to a format known to double.try_parse
+- var equation = terms_to_equation (terms).replace (tsep_string, "").replace (decimal, ".");
++ var tsep_string = Posix.nl_langinfo (Posix.NLItem.THOUSEP);
++ if (tsep_string == null || tsep_string == "")
++ tsep_string = " ";
+
+- cancel();
++ var decimal = Posix.nl_langinfo (Posix.NLItem.RADIXCHAR);
++ if (decimal == null)
++ decimal = "";
+
+- // if the search is a plain number, don't process it
+- if (double.try_parse (equation)) {
+- return false;
+- }
++ // "normalize" input to a format known to double.try_parse
++ var normalized_equation = equation.replace (tsep_string, "").replace (decimal, ".");
+
+- (yield solve_subprocess (equation)).wait_check (cancellable);
++ // if the search is a plain number, don't process it
++ if (double.try_parse (normalized_equation)) {
++ return false;
++ }
++
++ if (cached_equations.lookup (equation) != null)
++ return true;
++
++ try
++ {
++ var subprocess = yield solve_subprocess (normalized_equation);
++ yield subprocess.communicate_utf8_async (null, cancellable, out result, null);
++ subprocess.wait_check (cancellable);
+ }
+ catch (SpawnError e)
+ {
+@@ -104,14 +114,17 @@ public class SearchProvider : Object
+ return false;
+ }
+
++ cached_equations.insert (equation, result.strip ());
++
+ return true;
+ }
+
+ private async string[] get_result_identifier (string[] terms)
+ {
+ /* We have at most one result: the search terms as one string */
+- if (yield can_parse (terms))
+- return { terms_to_equation (terms) };
++ var equation = terms_to_equation (terms);
++ if (yield solve_equation (equation))
++ return { equation };
+ else
+ return new string[0];
+ }
+@@ -129,24 +142,22 @@ public class SearchProvider : Object
+ public async HashTable<string, Variant>[] get_result_metas (string[] results, GLib.BusName sender)
+ requires (results.length == 1)
+ {
+- string stdout_buf;
+- string stderr_buf;
++ string equation;
++ string result;
+
+- try
+- {
+- var subprocess = yield solve_subprocess (results[0]);
+- yield subprocess.communicate_utf8_async (null, cancellable, out stdout_buf, out stderr_buf);
+- }
+- catch (Error e)
+- {
++ equation = terms_to_equation (results);
++
++ if (!yield solve_equation (equation))
+ return new HashTable<string, Variant>[0];
+- }
++
++ result = cached_equations.lookup (equation);
++ assert (result != null);
+
+ var metadata = new HashTable<string, Variant>[1];
+ metadata[0] = new HashTable<string, Variant>(str_hash, str_equal);
+ metadata[0].insert ("id", results[0]);
+ metadata[0].insert ("name", results[0] );
+- metadata[0].insert ("description", " = " + stdout_buf.strip ());
++ metadata[0].insert ("description", @" = $result");
+
+ return metadata;
+ }
diff --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
new file mode 100644
index 0000000..ca1c342
--- /dev/null
+++ b/debian/patches/search-provider-cache-only-a-limited-number-of-equations.patch
@@ -0,0 +1,44 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <m...@3v1n0.net>
+Date: Mon, 27 Aug 2018 18:56:09 +0200
+Subject: search-provider: cache only a limited number of equations
+
+Bug-Ubuntu: https://launchpad.net/bugs/1756826
+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
+Forwarded: https://gitlab.gnome.org/GNOME/gnome-calculator/merge_requests/10
+---
+ search-provider/search-provider.vala | 9 +++++++++
+ 1 file changed, 9 insertions(+)
+
+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
+index af2779e..fc72128 100644
+--- a/search-provider/search-provider.vala
++++ b/search-provider/search-provider.vala
+@@ -16,10 +16,15 @@ public class SearchProvider : Object
+ private unowned SearchProviderApp application;
+ private Cancellable cancellable;
+
++ private const int MAX_CACHED_OPERATIONS = 10;
++ private Queue<string> queued_equations;
+ private HashTable<string, string> cached_equations;
++
+ public SearchProvider (SearchProviderApp app)
+ {
+ application = app;
++
++ queued_equations = new Queue<string> ();
+ cached_equations = new HashTable<string, string> (str_hash, str_equal);
+ }
+
+@@ -114,8 +119,12 @@ public class SearchProvider : Object
+ return false;
+ }
+
++ queued_equations.push_tail (equation);
+ cached_equations.insert (equation, result.strip ());
+
++ if (queued_equations.length > MAX_CACHED_OPERATIONS)
++ cached_equations.remove (queued_equations.pop_head ());
++
+ return true;
+ }
+
diff --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
new file mode 100644
index 0000000..801d844
--- /dev/null
+++ b/debian/patches/search-provider-cancel-the-current-process-on-new-calcula.patch
@@ -0,0 +1,29 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <m...@3v1n0.net>
+Date: Thu, 30 Aug 2018 18:47:16 +0200
+Subject: search-provider: cancel the current process on new calculation
+ request
+
+As there's just one shell running at time, there's no point of supporting
+parallel calls, we can just safely refer to the last equation as the only one
+we need to compute.
+
+Bug-Ubuntu: https://launchpad.net/bugs/1756826
+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
+Forwarded: https://gitlab.gnome.org/GNOME/gnome-calculator/merge_requests/10
+---
+ search-provider/search-provider.vala | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
+index df37b86..a3c57f7 100644
+--- a/search-provider/search-provider.vala
++++ b/search-provider/search-provider.vala
+@@ -86,6 +86,8 @@ public class SearchProvider : Object
+ // "normalize" input to a format known to double.try_parse
+ var equation = terms_to_equation (terms).replace (tsep_string, "").replace (decimal, ".");
+
++ cancel();
++
+ // if the search is a plain number, don't process it
+ if (double.try_parse (equation)) {
+ return false;
diff --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
new file mode 100644
index 0000000..41458ad
--- /dev/null
+++ b/debian/patches/search-provider-renew-inactivity-timeout-at-each-calculat.patch
@@ -0,0 +1,60 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <m...@3v1n0.net>
+Date: Fri, 24 Aug 2018 07:10:17 +0200
+Subject: search-provider: renew inactivity timeout at each calculator run
+
+As per the async rewrite, now the daemon inactivity timeout might happen
+when another call has just been done, while we don't want this to be the case.
+
+So, everytime we do a subprocess call, let's renew the application timeout.
+
+Bug-Ubuntu: https://launchpad.net/bugs/1756826
+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
+Forwarded: https://gitlab.gnome.org/GNOME/gnome-calculator/merge_requests/100
+---
+ search-provider/search-provider.vala | 14 +++++++++++++-
+ 1 file changed, 13 insertions(+), 1 deletion(-)
+
+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
+index 659f73d..7e54fa6 100644
+--- a/search-provider/search-provider.vala
++++ b/search-provider/search-provider.vala
+@@ -13,7 +13,12 @@
+ [DBus (name = "org.gnome.Shell.SearchProvider2")]
+ public class SearchProvider : Object
+ {
++ private unowned SearchProviderApp application;
+ private Cancellable cancellable = new Cancellable ();
++ public SearchProvider (SearchProviderApp app)
++ {
++ application = app;
++ }
+
+ ~SearchProvider ()
+ {
+@@ -71,6 +76,8 @@ public class SearchProvider : Object
+ cancellable = null;
+ });
+
++ application.renew_inactivity_timeout ();
++
+ yield subprocess.communicate_utf8_async (null, cancellable, out solution_buf, out stderr_buf);
+ }
+ catch (Error e)
+@@ -197,11 +204,16 @@ public class SearchProviderApp : Application
+ Object (application_id: "org.gnome.Calculator.SearchProvider",
+ flags: ApplicationFlags.IS_SERVICE,
+ inactivity_timeout: 60000);
++
++ public void renew_inactivity_timeout ()
++ {
++ this.hold ();
++ this.release ();
+ }
+
+ public override bool dbus_register (DBusConnection connection, string object_path)
+ {
+- SearchProvider search_provider = new SearchProvider ();
++ SearchProvider search_provider = new SearchProvider (this);
+
+ try
+ {
diff --git a/debian/patches/search-provider-simplify-solve_subprocess.patch b/debian/patches/search-provider-simplify-solve_subprocess.patch
new file mode 100644
index 0000000..7dc1d78
--- /dev/null
+++ b/debian/patches/search-provider-simplify-solve_subprocess.patch
@@ -0,0 +1,113 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <m...@3v1n0.net>
+Date: Fri, 24 Aug 2018 07:31:37 +0200
+Subject: search-provider: simplify solve_subprocess
+
+Only return a subprocess in solve_subprocess and make the callers deal with
+the actual operation to perform.
+
+Bug-Ubuntu: https://launchpad.net/bugs/1756826
+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
+Forwarded: https://gitlab.gnome.org/GNOME/gnome-calculator/merge_requests/10
+---
+ search-provider/search-provider.vala | 49 ++++++++++--------------------------
+ 1 file changed, 13 insertions(+), 36 deletions(-)
+
+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
+index 9035c58..df37b86 100644
+--- a/search-provider/search-provider.vala
++++ b/search-provider/search-provider.vala
+@@ -39,7 +39,7 @@ public class SearchProvider : Object
+ return string.joinv (" ", terms);
+ }
+
+- private async Subprocess solve_subprocess (string equation, bool return_solution = false, out string? solution_buf = null) throws Error
++ private async Subprocess solve_subprocess (string equation) throws Error
+ {
+ Subprocess subprocess;
+ string[] argv = {"gnome-calculator", "--solve"};
+@@ -52,13 +52,7 @@ public class SearchProvider : Object
+ {
+ // Eat output so that it doesn't wind up in the journal. It's
+ // expected that most searches are not valid calculator input.
+- var flags = SubprocessFlags.STDERR_PIPE;
+-
+- if (return_solution)
+- {
+- flags |= SubprocessFlags.STDOUT_PIPE;
+- }
+-
++ var flags = SubprocessFlags.STDOUT_PIPE | SubprocessFlags.STDERR_PIPE;
+ subprocess = new Subprocess.newv (argv, flags);
+ }
+ catch (Error e)
+@@ -66,31 +60,13 @@ public class SearchProvider : Object
+ error ("Failed to spawn Calculator: %s", e.message);
+ }
+
+- try
+- {
+- string stderr_buf;
+-
+- cancellable = new Cancellable ();
+- cancellable.cancelled.connect (() => {
+- subprocess.force_exit ();
+- cancellable = null;
+- });
+-
+- application.renew_inactivity_timeout ();
++ cancellable = new Cancellable ();
++ cancellable.cancelled.connect (() => {
++ subprocess.force_exit ();
++ cancellable = null;
++ });
+
+- yield subprocess.communicate_utf8_async (null, cancellable, out solution_buf, out stderr_buf);
+- }
+- catch (Error e)
+- {
+- if (e is IOError.CANCELLED)
+- {
+- throw e;
+- }
+- else
+- {
+- error ("Failed reading result: %s", e.message);
+- }
+- }
++ application.renew_inactivity_timeout ();
+
+ return subprocess;
+ }
+@@ -115,8 +91,7 @@ public class SearchProvider : Object
+ return false;
+ }
+
+- var subprocess = yield solve_subprocess (equation);
+- yield subprocess.wait_check_async ();
++ (yield solve_subprocess (equation)).wait_check (cancellable);
+ }
+ catch (SpawnError e)
+ {
+@@ -153,10 +128,12 @@ public class SearchProvider : Object
+ requires (results.length == 1)
+ {
+ string stdout_buf;
++ string stderr_buf;
+
+ try
+ {
+- yield solve_subprocess (results[0], true, out stdout_buf);
++ var subprocess = yield solve_subprocess (results[0]);
++ yield subprocess.communicate_utf8_async (null, cancellable, out stdout_buf, out stderr_buf);
+ }
+ catch (Error e)
+ {
+@@ -167,7 +144,7 @@ public class SearchProvider : Object
+ metadata[0] = new HashTable<string, Variant>(str_hash, str_equal);
+ metadata[0].insert ("id", results[0]);
+ metadata[0].insert ("name", results[0] );
+- metadata[0].insert ("description", " = " + stdout_buf.strip() );
++ metadata[0].insert ("description", " = " + stdout_buf.strip ());
+
+ return metadata;
+ }
diff --git a/debian/patches/series b/debian/patches/series
index e69de29..4fd8ad8 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -0,0 +1,8 @@
+search-provider-Use-async-calls-cancel-search-on-inactivi.patch
+search-provider-renew-inactivity-timeout-at-each-calculat.patch
+search-provider-Use-lower-inactivity-timeout.patch
+search-provider-simplify-solve_subprocess.patch
+search-provider-cancel-the-current-process-on-new-calcula.patch
+search-provider-cache-equations-avoiding-spawning-calcula.patch
+search-provider-cache-only-a-limited-number-of-equations.patch
+ubuntu/search-provider-Cancel-operations-on-XUbuntuCancel.patch
diff --git a/debian/patches/ubuntu/search-provider-Cancel-operations-on-XUbuntuCancel.patch b/debian/patches/ubuntu/search-provider-Cancel-operations-on-XUbuntuCancel.patch
new file mode 100644
index 0000000..77939ec
--- /dev/null
+++ b/debian/patches/ubuntu/search-provider-Cancel-operations-on-XUbuntuCancel.patch
@@ -0,0 +1,118 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <m...@3v1n0.net>
+Date: Tue, 28 Aug 2018 04:12:20 +0200
+Subject: search-provider: Cancel operations on XUbuntuCancel
+
+Stop process and any computation on XUbuntuCancel method, if the caller
+was the same triggering the last operation.
+
+Fixes LP: #1756826
+
+Bug-Ubuntu: https://launchpad.net/bugs/1756826
+Bug-GNOME: https://gitlab.gnome.org/GNOME/gnome-shell/issues/183
+Forwarded: not-needed
+---
+ search-provider/search-provider.vala | 46 ++++++++++++++++++++++++++++++++----
+ 1 file changed, 41 insertions(+), 5 deletions(-)
+
+diff --git a/search-provider/search-provider.vala b/search-provider/search-provider.vala
+index fc72128..b7c869d 100644
+--- a/search-provider/search-provider.vala
++++ b/search-provider/search-provider.vala
+@@ -10,11 +10,33 @@
+ * license.
+ */
+
++public class CallerTracker : Object
++{
++ public BusName? bus { get; set; }
++}
++
++public class Caller
++{
++ private CallerTracker caller_tracker;
++
++ public Caller (CallerTracker c, BusName sender)
++ {
++ caller_tracker = c;
++ caller_tracker.bus = sender;
++ }
++
++ ~Caller ()
++ {
++ caller_tracker.bus = null;
++ }
++}
++
+ [DBus (name = "org.gnome.Shell.SearchProvider2")]
+ public class SearchProvider : Object
+ {
+ private unowned SearchProviderApp application;
+ private Cancellable cancellable;
++ private CallerTracker caller_tracker;
+
+ private const int MAX_CACHED_OPERATIONS = 10;
+ private Queue<string> queued_equations;
+@@ -24,6 +46,7 @@ public class SearchProvider : Object
+ {
+ application = app;
+
++ caller_tracker = new CallerTracker ();
+ queued_equations = new Queue<string> ();
+ cached_equations = new HashTable<string, string> (str_hash, str_equal);
+ }
+@@ -128,8 +151,10 @@ public class SearchProvider : Object
+ return true;
+ }
+
+- private async string[] get_result_identifier (string[] terms)
++ private async string[] get_result_identifier (string[] terms, GLib.BusName sender)
+ {
++ var owner = new Caller (caller_tracker, sender); (void) owner;
++
+ /* We have at most one result: the search terms as one string */
+ var equation = terms_to_equation (terms);
+ if (yield solve_equation (equation))
+@@ -138,14 +163,14 @@ public class SearchProvider : Object
+ return new string[0];
+ }
+
+- public async string[] get_initial_result_set (string[] terms)
++ public async string[] get_initial_result_set (string[] terms, GLib.BusName sender)
+ {
+- return yield get_result_identifier (terms);
++ return yield get_result_identifier (terms, sender);
+ }
+
+- public async string[] get_subsearch_result_set (string[] previous_results, string[] terms)
++ public async string[] get_subsearch_result_set (string[] previous_results, string[] terms, GLib.BusName sender)
+ {
+- return yield get_result_identifier (terms);
++ return yield get_result_identifier (terms, sender);
+ }
+
+ public async HashTable<string, Variant>[] get_result_metas (string[] results, GLib.BusName sender)
+@@ -154,6 +179,8 @@ public class SearchProvider : Object
+ string equation;
+ string result;
+
++ var owner = new Caller (caller_tracker, sender); (void) owner;
++
+ equation = terms_to_equation (results);
+
+ if (!yield solve_equation (equation))
+@@ -171,6 +198,15 @@ public class SearchProvider : Object
+ return metadata;
+ }
+
++ public async void x_ubuntu_cancel (BusName sender)
++ {
++ if (caller_tracker.bus == sender)
++ {
++ debug ("Cancelling Request");
++ cancel ();
++ }
++ }
++
+ private static void spawn_and_display_equation (string[] terms)
+ {
+ try
--
ubuntu-desktop mailing list
ubuntu-desktop@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-desktop
