Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Draft background: Add ability to specify timers to start app #1098

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Draft background: Add ability to specify timers to start app #1098

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b86edec
xdg-utils: Properly quote flatpak command
jsparber Sep 6, 2023
9fd4bdc
background: Add ability to specify timers to start app
jsparber Sep 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions data/org.freedesktop.impl.portal.Background.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,30 @@
<arg type="b" name="result" direction="out"/>
</method>

<!--
EnableTimer:
@app_id: App id of the application
@timer: List of times the app should be started
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it kind of weird, that this is a list but named with a singular. While flags is named plural. So I guess this should be plural too?

@commandline: The commandline to used to start the app at the specificied time
@flags: Flags influencing the details of the timer
@result: TRUE if timer was enabled, FALSE otherwise

Enables or disables autostart for an application.

The following flags are understood:
<simplelist>
<member>1: Wake system from suspend</member>
<member>2: Use D-Bus activation</member>
</simplelist>
-->
<method name='EnableTimer'>
<arg type="s" name="app_id" direction="in"/>
<arg type="as" name="timer" direction="in"/>
<arg type="as" name="commandline" direction="in"/>
<arg type="u" name="flags" direction="in"/>
<arg type="b" name="result" direction="out"/>
</method>

<!--
RunningApplicationsChanged:

Expand Down
23 changes: 21 additions & 2 deletions data/org.freedesktop.portal.Background.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,28 @@
<varlistentry>
<term>commandline as</term>
<listitem><para>
Commandline to use add when autostarting at login.
Commandline to use add when autostarting at login
or when starting the app with a timer
If this is not specified, the Exec line from the
desktop file will be used.
</para></listitem>
</varlistentry>
<varlistentry>
<term>dbus-activatable b</term>
<listitem><para>
If TRUE, use D-Bus activation for autostart.
If TRUE, use D-Bus activation for autostart and timer start.
</para></listitem>
</varlistentry>
<varlistentry>
<term>timer as</term>
<listitem><para>
List of times the APP should be started
</para></listitem>
</varlistentry>
<varlistentry>
<term>wake-system b</term>
<listitem><para>
If TRUE, the system is woken form susped to start the app
</para></listitem>
</varlistentry>
</variablelist>
Expand All @@ -95,6 +108,12 @@
TRUE if the application is will be autostarted.
</para></listitem>
</varlistentry>
<varlistentry>
<term>timer b</term>
<listitem><para>
TRUE if the application will be started at the specificed times.
</para></listitem>
</varlistentry>
</variablelist>
-->
<method name="RequestBackground">
Expand Down
52 changes: 49 additions & 3 deletions src/background.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ typedef enum {
AUTOSTART_FLAGS_ACTIVATABLE = 1 << 0,
} AutostartFlags;

typedef enum {
TIMER_FLAGS_NONE = 0,
TIMER_FLAGS_ACTIVATABLE = 1 << 0,
TIMER_FLAGS_WAKE = 1 << 1,
} TimerFlags;

static GVariant *
get_all_permissions (void)
{
Expand Down Expand Up @@ -704,6 +710,10 @@ handle_request_background_in_thread_func (GTask *task,
AutostartFlags autostart_flags = AUTOSTART_FLAGS_NONE;
gboolean activatable = FALSE;
g_auto(GStrv) commandline = NULL;
const char * const *run_times = { NULL };
Copy link
Collaborator

@hfiguiere hfiguiere Nov 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const char * const *run_times = { NULL };
g_autofree char * const *run_times = NULL;

^a&s return like g_variant_get_strv() it will leak otherwise.

gboolean wake_system = FALSE;
gboolean timer_enabled;
TimerFlags timer_flags = TIMER_FLAGS_NONE;

REQUEST_AUTOLOCK (request);

Expand All @@ -712,9 +722,16 @@ handle_request_background_in_thread_func (GTask *task,
g_variant_lookup (options, "autostart", "b", &autostart_requested);
g_variant_lookup (options, "commandline", "^a&s", &autostart_exec);
g_variant_lookup (options, "dbus-activatable", "b", &activatable);
g_variant_lookup (options, "timer", "^a&s", &run_times);
g_variant_lookup (options, "wake-system", "b", &wake_system);

if (activatable)
if (activatable) {
autostart_flags |= AUTOSTART_FLAGS_ACTIVATABLE;
timer_flags |= TIMER_FLAGS_ACTIVATABLE;
}

if (wake_system)
timer_flags |= TIMER_FLAGS_WAKE;

app_id = xdp_app_info_get_id (request->app_info);

Expand Down Expand Up @@ -782,14 +799,16 @@ handle_request_background_in_thread_func (GTask *task,
allowed && autostart_requested ? "enabled" : "disabled");

autostart_enabled = FALSE;
timer_enabled = FALSE;

commandline = xdp_app_info_rewrite_commandline (request->app_info, autostart_exec,
FALSE /* don't quote escape */);
if (commandline == NULL)
{
g_debug ("Autostart not supported for: %s", app_id);
g_debug ("Autostart and schedualed autostart not supported for: %s", app_id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
g_debug ("Autostart and schedualed autostart not supported for: %s", app_id);
g_debug ("Autostart and scheduled autostart not supported for: %s", app_id);

Typo

}
else if (!xdp_dbus_impl_background_call_enable_autostart_sync (background_impl,
else {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the indentaton style doesn't match

if (!xdp_dbus_impl_background_call_enable_autostart_sync (background_impl,
app_id,
allowed && autostart_requested,
(const char * const *)commandline,
Expand All @@ -802,6 +821,20 @@ handle_request_background_in_thread_func (GTask *task,
g_clear_error (&error);
}

if (!xdp_dbus_impl_background_call_enable_timer_sync (background_impl,
app_id,
allowed && run_times != NULL ? run_times : (const char *[]) { NULL },
(const char * const *)commandline,
timer_flags,
&timer_enabled,
NULL,
&error))
{
g_warning ("EnableTimer call failed: %s", error->message);
g_clear_error (&error);
}
}

if (request->exported)
{
XdgDesktopPortalResponseEnum portal_response;
Expand All @@ -810,6 +843,7 @@ handle_request_background_in_thread_func (GTask *task,
g_variant_builder_init (&results, G_VARIANT_TYPE_VARDICT);
g_variant_builder_add (&results, "{sv}", "background", g_variant_new_boolean (allowed));
g_variant_builder_add (&results, "{sv}", "autostart", g_variant_new_boolean (autostart_enabled));
g_variant_builder_add (&results, "{sv}", "timer", g_variant_new_boolean (timer_enabled));

if (allowed)
portal_response = XDG_DESKTOP_PORTAL_RESPONSE_SUCCESS;
Expand Down Expand Up @@ -873,11 +907,23 @@ validate_commandline (const char *key,

return TRUE;
}
static gboolean
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`nit

Suggested change
static gboolean
static gboolean

validate_timer (const char *key,
GVariant *value,
GVariant *options,
GError **error)
{
// TODO: validate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking this todo with a comment

// We want to allow only: OnCalendar=, OnActiveSec=, OnBootSec=, OnStartupSec=, OnUnitActiveSec=, OnUnitInactiveSec=
return TRUE;
}
static XdpOptionKey background_options[] = {
{ "reason", G_VARIANT_TYPE_STRING, validate_reason },
{ "autostart", G_VARIANT_TYPE_BOOLEAN, NULL },
{ "commandline", G_VARIANT_TYPE_STRING_ARRAY, validate_commandline },
{ "dbus-activatable", G_VARIANT_TYPE_BOOLEAN, NULL },
{ "timer", G_VARIANT_TYPE_STRING_ARRAY, validate_timer },
{ "wake-system", G_VARIANT_TYPE_BOOLEAN, NULL },
};

static gboolean
Expand Down
5 changes: 4 additions & 1 deletion src/xdp-utils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,11 @@ xdp_app_info_rewrite_commandline (XdpAppInfo *app_info,
for (i = 1; commandline[i]; i++)
g_ptr_array_add (args, maybe_quote (commandline[i], quote_escape));
}
else
else if (quote_escape) {
g_ptr_array_add (args, g_shell_quote (app_info->id));
} else {
g_ptr_array_add (args, g_strdup (app_info->id));
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the indentation style doesn't match

g_ptr_array_add (args, NULL);

return (char **)g_ptr_array_free (g_steal_pointer (&args), FALSE);
Expand Down
Loading