From d1d2a43e4d0ec842cb9a1a1c7d383314c2ab6a55 Mon Sep 17 00:00:00 2001 From: Gustavo Marques Date: Mon, 5 Jun 2023 15:46:55 -0300 Subject: [PATCH 1/2] Background: minor changes * move dbus interfaces out of the classes bodies, use constants for path and name. * use enumeration values where applicable and skip the namespace when possible. * simplify the response callback in the notify_background(). * make the notification_closed() callback return ALLOW_ONCE in more types of reasons. * remove temporary variables where applicable. Signed-off-by: Gustavo Marques --- src/Background/NotificationRequest.vala | 93 ++++++++++++++----------- src/Background/Portal.vala | 87 +++++++++++------------ 2 files changed, 92 insertions(+), 88 deletions(-) diff --git a/src/Background/NotificationRequest.vala b/src/Background/NotificationRequest.vala index 997e4bd6..e1f261a9 100644 --- a/src/Background/NotificationRequest.vala +++ b/src/Background/NotificationRequest.vala @@ -3,19 +3,48 @@ * SPDX-License-Identifier: LGPL-2.1-or-later */ +[DBus (name = "org.freedesktop.Notifications")] +private interface Fdo.Notifications : Object { + public signal void action_invoked (uint32 id, string action_key); + public signal void notification_closed (uint32 id, CloseReason reason); + + public const string NAME = "org.freedesktop.Notifications"; + public const string PATH = "/org/freedesktop/Notifications"; + + [CCode (type_signature = "u")] + public enum CloseReason { + EXPIRED = 1, + DIMISSED, + CANCELLED + } + + public abstract void close_notification (uint32 id) throws DBusError, IOError; + public abstract uint32 notify ( + string app_name, + uint32 replaces_id, + string app_icon, + string summary, + string body, + string[] actions, + HashTable hints, + int32 expire_timeout + ) throws DBusError, IOError; +} + [DBus (name = "org.freedesktop.impl.portal.Request")] -public class NotificationRequest : Object { +public class Background.NotificationRequest : Object { [DBus (visible = false)] - public signal void response (uint32 result); + public signal void response (NotifyBackgroundResult result); private const string ACTION_ALLOW_BACKGROUND = "background.allow"; private const string ACTION_FORBID_BACKGROUND = "background.forbid"; - private static HashTable notification_by_id; - private static Notifications notifications; + private static HashTable requests; + private static Fdo.Notifications notifications; private uint32 id = 0; + [CCode (type_signature = "u")] public enum NotifyBackgroundResult { FORBID, ALLOW, @@ -24,60 +53,41 @@ public class NotificationRequest : Object { FAILED } - [DBus (name = "org.freedesktop.Notifications")] - public interface Notifications : Object { - public signal void action_invoked (uint32 id, string action_key); - public signal void notification_closed (uint32 id, uint32 reason); - - public abstract void close_notification (uint32 id) throws DBusError, IOError; - public abstract uint32 notify ( - string app_name, - uint32 replaces_id, - string app_icon, - string summary, - string body, - string[] actions, - HashTable hints, - int32 expire_timeout - ) throws DBusError, IOError; - } - - [DBus (visible = false)] public static void init (DBusConnection connection) { - notification_by_id = new HashTable (null, null); + requests = new HashTable (null, null); try { - notifications = connection.get_proxy_sync ("org.freedesktop.Notifications", "/org/freedesktop/Notifications"); - notifications.action_invoked.connect (on_action_invoked); - notifications.notification_closed.connect (on_notification_closed); + notifications = connection.get_proxy_sync (Fdo.Notifications.NAME, Fdo.Notifications.PATH); + notifications.action_invoked.connect (action_invoked); + notifications.notification_closed.connect (notification_closed); } catch { - warning ("Cannot connect to notifications dbus, background portal working with reduced functionality."); + warning ("Cannot connect to notifications dbus, portal working with reduced functionality."); } } - private static void on_action_invoked (uint32 id, string action_key) { - var notification = notification_by_id.take (id); + private static void action_invoked (uint32 id, string action_key) { + unowned var notification = requests.take (id); if (notification == null) { return; } if (action_key == ACTION_ALLOW_BACKGROUND) { - notification.response (NotifyBackgroundResult.ALLOW); + notification.response (ALLOW); } else { - notification.response (NotifyBackgroundResult.FORBID); + notification.response (FORBID); } } - private static void on_notification_closed (uint32 id, uint32 reason) { - var notification = notification_by_id.take (id); + private static void notification_closed (uint32 id, Fdo.Notifications.CloseReason reason) { + unowned var notification = requests.take (id); if (notification == null) { return; } - if (reason == 2) { //Dismissed by user - notification.response (NotifyBackgroundResult.ALLOW_ONCE); - } else if (reason == 3) { //Closed via DBus call - notification.response (NotifyBackgroundResult.CANCELLED); + if (reason == CANCELLED) { // Closed via DBus call + notification.response (CANCELLED); + } else { // Dismissed, Expired, or something internal to the server + notification.response (ALLOW_ONCE); } } @@ -89,6 +99,7 @@ public class NotificationRequest : Object { ACTION_FORBID_BACKGROUND, _("Forbid") }; + var hints = new HashTable (null, null); hints["desktop-entry"] = app_id; hints["urgency"] = (uint8) 1; @@ -103,10 +114,10 @@ public class NotificationRequest : Object { -1 ); - notification_by_id.set (id, this); + requests[id] = this; } catch (Error e) { - warning ("Failed to send notification for app id '%s': %s", app_id, e.message); - response (NotifyBackgroundResult.FAILED); + warning ("Failed to notify background activity from '%s': %s", app_id, e.message); + response (FAILED); } } diff --git a/src/Background/Portal.vala b/src/Background/Portal.vala index 21e47f45..cefb9090 100644 --- a/src/Background/Portal.vala +++ b/src/Background/Portal.vala @@ -3,35 +3,41 @@ * SPDX-License-Identifier: LGPL-2.1-or-later */ +[DBus (name = "org.pantheon.gala.DesktopIntegration")] +private interface Gala.DesktopIntegration : Object { + public signal void running_applications_changed (); + + public const string NAME = "org.pantheon.gala"; + public const string PATH = "/org/pantheon/gala/DesktopInterface"; + + public struct RunningApplications { + string app_id; + HashTable details; + } + + public abstract RunningApplications[] get_running_applications () throws DBusError, IOError; +} + [DBus (name = "org.freedesktop.impl.portal.Background")] public class Background.Portal : Object { public signal void running_applications_changed (); + private Gala.DesktopIntegration? desktop_integration; private DBusConnection connection; - private DesktopIntegration desktop_integration; public Portal (DBusConnection connection) { this.connection = connection; NotificationRequest.init (connection); try { - desktop_integration = connection.get_proxy_sync ("org.pantheon.gala", "/org/pantheon/gala/DesktopInterface"); + desktop_integration = connection.get_proxy_sync (Gala.DesktopIntegration.NAME, Gala.DesktopIntegration.PATH); desktop_integration.running_applications_changed.connect (() => running_applications_changed ()); } catch { - warning ("Cannot connect to compositor, background portal working with reduced functionality."); - } - } - - [DBus (name = "org.pantheon.gala.DesktopIntegration")] - public interface DesktopIntegration : Object { - public struct RunningApplications { - string app_id; - HashTable details; + warning ("Cannot connect to compositor, portal working with reduced functionality."); } - public signal void running_applications_changed (); - public abstract RunningApplications[] get_running_applications () throws DBusError, IOError; } + [CCode (type_signature = "u")] private enum ApplicationState { BACKGROUND, RUNNING, @@ -43,18 +49,18 @@ public class Background.Portal : Object { throw new DBusError.FAILED ("No connection to compositor."); } - var apps = desktop_integration.get_running_applications (); var results = new HashTable (null, null); - foreach (var app in apps) { + debug ("getting application states"); + + foreach (var app in desktop_integration.get_running_applications ()) { var app_id = app.app_id; if (app_id.has_suffix (".desktop")) { app_id = app_id.slice (0, app_id.last_index_of_char ('.')); } var app_state = ApplicationState.RUNNING; //FIXME: Don't hardcode: needs implementation on the gala side - - results[app_id] = (uint32) app_state; debug ("App state of '%s' set to %u (= %s).", app_id, app_state, app_state.to_string ()); + results[app_id] = app_state; } return results; @@ -69,40 +75,32 @@ public class Background.Portal : Object { ) throws DBusError, IOError { debug ("Notify background for '%s'.", app_id); - uint32 _response = 2; var _results = new HashTable (str_hash, str_equal); + uint32 _response = 2; - var notification_request = new NotificationRequest (); - - notification_request.response.connect ((result) => { - switch (result) { - case NotificationRequest.NotifyBackgroundResult.CANCELLED: - _response = 1; - break; - case NotificationRequest.NotifyBackgroundResult.FAILED: - break; - default: - _response = 0; - _results["result"] = (uint32) result; - break; + var notification = new NotificationRequest (); + notification.response.connect ((result) => { + if (result == CANCELLED) { + _response = 1; + } else if (result != FAILED) { + _response = 0; + _results["result"] = result; } debug ("Response to background activity of '%s': %s.", app_id, result.to_string ()); - notify_background.callback (); }); uint register_id = 0; try { - register_id = connection.register_object (handle, notification_request); + register_id = connection.register_object (handle, notification); } catch (Error e) { warning ("Failed to export request object: %s", e.message); throw new DBusError.OBJECT_PATH_IN_USE (e.message); } debug ("Sending desktop notification for '%s'.", app_id); - notification_request.send_notification (app_id, name); - + notification.send_notification (app_id, name); yield; connection.unregister_object (register_id); @@ -135,11 +133,10 @@ public class Background.Portal : Object { */ var app_info = new DesktopAppInfo (_app_id + ".desktop"); if (app_info == null) { - _app_id = string.joinv ("-", commandline).replace ("--", "-"); + _app_id = string.joinv ("-", commandline).replace ("--", "-").replace ("--", "-"); } var path = Path.build_filename (Environment.get_user_config_dir (), "autostart", _app_id + ".desktop"); - if (!enable) { FileUtils.unlink (path); return false; @@ -158,13 +155,10 @@ public class Background.Portal : Object { key_file.set_string (KeyFileDesktop.GROUP, KeyFileDesktop.KEY_COMMENT, string.joinv (" ", commandline)); } - key_file.set_string (KeyFileDesktop.GROUP, KeyFileDesktop.KEY_EXEC, flatpak_quote_argv (commandline)); - - if (flags == AutostartFlags.DBUS_ACTIVATABLE) { - key_file.set_boolean (KeyFileDesktop.GROUP, KeyFileDesktop.KEY_DBUS_ACTIVATABLE, true); - } + key_file.set_string (KeyFileDesktop.GROUP, KeyFileDesktop.KEY_EXEC, quote_argv (commandline)); + key_file.set_boolean (KeyFileDesktop.GROUP, KeyFileDesktop.KEY_DBUS_ACTIVATABLE, flags == DBUS_ACTIVATABLE); - if (app_id.strip () != "") { + if (app_id != "") { key_file.set_string (KeyFileDesktop.GROUP, "X-Flatpak", app_id); } @@ -172,15 +166,14 @@ public class Background.Portal : Object { key_file.save_to_file (path); } catch (Error e) { warning ("Failed to write autostart file: %s", e.message); - return false; + throw new DBusError.FAILED (e.message); } - debug ("Autostart file installed for '%s'.", app_id); - + debug ("Autostart file installed at '%s'.", path); return true; } - private string flatpak_quote_argv (string[] argv) { + private string quote_argv (string[] argv) { var builder = new StringBuilder (); foreach (var arg in argv) { From 46848f68650eb5d3a7e77ed1d28d6eb29c7b2fb0 Mon Sep 17 00:00:00 2001 From: Gustavo Marques Date: Mon, 5 Jun 2023 16:04:27 -0300 Subject: [PATCH 2/2] Background: make asynchronous, get notifications proxy lazily make all of the dbus calls and methods asynchronous so that we don't end up blocking others interfaces, methods and initialization. the notification proxy is now also started when we first call send_notification() so we don't end starting it early than necessary or blocking the initialization for too much time. Signed-off-by: Gustavo Marques --- src/Background/NotificationRequest.vala | 67 ++++++++++++++----------- src/Background/Portal.vala | 42 ++++++++++------ 2 files changed, 65 insertions(+), 44 deletions(-) diff --git a/src/Background/NotificationRequest.vala b/src/Background/NotificationRequest.vala index e1f261a9..71f0751c 100644 --- a/src/Background/NotificationRequest.vala +++ b/src/Background/NotificationRequest.vala @@ -18,8 +18,8 @@ private interface Fdo.Notifications : Object { CANCELLED } - public abstract void close_notification (uint32 id) throws DBusError, IOError; - public abstract uint32 notify ( + public async abstract void close_notification (uint32 id) throws DBusError, IOError; + public async abstract uint32 notify ( string app_name, uint32 replaces_id, string app_icon, @@ -40,7 +40,7 @@ public class Background.NotificationRequest : Object { private const string ACTION_FORBID_BACKGROUND = "background.forbid"; private static HashTable requests; - private static Fdo.Notifications notifications; + private static Fdo.Notifications? notifications; private uint32 id = 0; @@ -53,16 +53,8 @@ public class Background.NotificationRequest : Object { FAILED } - public static void init (DBusConnection connection) { + static construct { requests = new HashTable (null, null); - - try { - notifications = connection.get_proxy_sync (Fdo.Notifications.NAME, Fdo.Notifications.PATH); - notifications.action_invoked.connect (action_invoked); - notifications.notification_closed.connect (notification_closed); - } catch { - warning ("Cannot connect to notifications dbus, portal working with reduced functionality."); - } } private static void action_invoked (uint32 id, string action_key) { @@ -93,6 +85,22 @@ public class Background.NotificationRequest : Object { [DBus (visible = false)] public void send_notification (string app_id, string app_name) { + if (notifications == null) { + Bus.get_proxy.begin (SESSION, Fdo.Notifications.NAME, Fdo.Notifications.PATH, NONE, null, + (obj, res) => { + try { + notifications = Bus.get_proxy.end (res); + notifications.action_invoked.connect (action_invoked); + notifications.notification_closed.connect (notification_closed); + send_notification (app_id, app_name); + } catch { + warning ("Cannot connect to notifications server, skipping notify request for '%s'", app_id); + response (FAILED); + } + }); + return; + } + string[] actions = { ACTION_ALLOW_BACKGROUND, _("Allow"), @@ -104,26 +112,27 @@ public class Background.NotificationRequest : Object { hints["desktop-entry"] = app_id; hints["urgency"] = (uint8) 1; - try { - id = notifications.notify ( - app_name, 0, "", - _("Background activity"), - _("“%s” is running in the background without appropriate permission").printf (app_name), - actions, - hints, - -1 - ); - - requests[id] = this; - } catch (Error e) { - warning ("Failed to notify background activity from '%s': %s", app_id, e.message); - response (FAILED); - } + notifications.notify.begin ( + app_name, 0, "", + _("Background activity"), + _("“%s” is running in the background without appropriate permission").printf (app_name), + actions, + hints, + -1, (obj, res) => { + try { + id = notifications.notify.end (res); + requests[id] = this; + } catch (Error e) { + warning ("Failed to notify background activity from '%s': %s", app_id, e.message); + response (FAILED); + } + } + ); } - public void close () throws DBusError, IOError { + public async void close () throws DBusError, IOError { try { - notifications.close_notification (id); + yield notifications.close_notification (id); } catch (Error e) { // the notification was already closed, or we lost the connection to the server } diff --git a/src/Background/Portal.vala b/src/Background/Portal.vala index cefb9090..bd8cf483 100644 --- a/src/Background/Portal.vala +++ b/src/Background/Portal.vala @@ -15,7 +15,7 @@ private interface Gala.DesktopIntegration : Object { HashTable details; } - public abstract RunningApplications[] get_running_applications () throws DBusError, IOError; + public abstract async RunningApplications[] get_running_applications () throws DBusError, IOError; } [DBus (name = "org.freedesktop.impl.portal.Background")] @@ -27,14 +27,19 @@ public class Background.Portal : Object { public Portal (DBusConnection connection) { this.connection = connection; - NotificationRequest.init (connection); - try { - desktop_integration = connection.get_proxy_sync (Gala.DesktopIntegration.NAME, Gala.DesktopIntegration.PATH); - desktop_integration.running_applications_changed.connect (() => running_applications_changed ()); - } catch { - warning ("Cannot connect to compositor, portal working with reduced functionality."); - } + connection.get_proxy.begin ( + Gala.DesktopIntegration.NAME, + Gala.DesktopIntegration.PATH, + NONE, null, (obj, res) => { + try { + desktop_integration = connection.get_proxy.end (res); + desktop_integration.running_applications_changed.connect (() => running_applications_changed ()); + } catch { + warning ("Cannot connect to compositor, portal working with reduced functionality."); + } + } + ); } [CCode (type_signature = "u")] @@ -44,7 +49,7 @@ public class Background.Portal : Object { ACTIVE } - public HashTable get_app_state () throws DBusError, IOError { + public async HashTable get_app_state () throws DBusError, IOError { if (desktop_integration == null) { throw new DBusError.FAILED ("No connection to compositor."); } @@ -52,7 +57,7 @@ public class Background.Portal : Object { var results = new HashTable (null, null); debug ("getting application states"); - foreach (var app in desktop_integration.get_running_applications ()) { + foreach (var app in yield desktop_integration.get_running_applications ()) { var app_id = app.app_id; if (app_id.has_suffix (".desktop")) { app_id = app_id.slice (0, app_id.last_index_of_char ('.')); @@ -114,7 +119,7 @@ public class Background.Portal : Object { DBUS_ACTIVATABLE } - public bool enable_autostart ( + public async bool enable_autostart ( string app_id, bool enable, string[] commandline, @@ -136,9 +141,9 @@ public class Background.Portal : Object { _app_id = string.joinv ("-", commandline).replace ("--", "-").replace ("--", "-"); } - var path = Path.build_filename (Environment.get_user_config_dir (), "autostart", _app_id + ".desktop"); + var file = File.new_build_filename (Environment.get_user_config_dir (), "autostart", _app_id + ".desktop"); if (!enable) { - FileUtils.unlink (path); + try { yield file.delete_async (); } catch {} return false; } @@ -162,14 +167,21 @@ public class Background.Portal : Object { key_file.set_string (KeyFileDesktop.GROUP, "X-Flatpak", app_id); } + FileCreateFlags create_flags = PRIVATE | REPLACE_DESTINATION; + var contents = key_file.to_data (); + try { - key_file.save_to_file (path); + try { + yield file.replace_contents_async (contents.data, null, true, create_flags, null, null); + } catch (IOError.CANT_CREATE_BACKUP e) { + yield file.replace_contents_async (contents.data, null, false, create_flags, null, null); + } } catch (Error e) { warning ("Failed to write autostart file: %s", e.message); throw new DBusError.FAILED (e.message); } - debug ("Autostart file installed at '%s'.", path); + debug ("Autostart file installed at '%s'.", file.get_path ()); return true; }