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.

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

Background apps monitoring #901

Merged

Conversation

GeorgesStavracas
Copy link
Member

@GeorgesStavracas GeorgesStavracas commented Oct 11, 2022

libportal counterpart: flatpak/libportal#115

See commits.

Most of the heavy lifting is pre-existing - the background portal already does the monitoring. This PR merely exposes this in a separate D-Bus name / path / interface, in a way that's consumable by host system services. This is just the bare minimum to get this whole things up and running.

No backend or client changes are needed, all this work is restricted to xdg-desktop-portal itself.

This should be enough to implement richer interfaces around background running apps, such as this proposal. With this new background monitor interface, implementing such proposal is much easier.

TODO:

  • Add a way for apps to set a status
  • Review Access portal usage
  • Fine tune sleep times and grace period

Related: #899

@GeorgesStavracas GeorgesStavracas force-pushed the gbsneto/background-session branch from c37ba5e to 3b5d51f Compare October 11, 2022 22:27
@GeorgesStavracas GeorgesStavracas force-pushed the gbsneto/background-session branch 6 times, most recently from 17108c4 to a6ab9d3 Compare October 11, 2022 23:43
@GeorgesStavracas GeorgesStavracas marked this pull request as draft October 12, 2022 00:52
Copy link
Collaborator

@jadahl jadahl left a comment

Choose a reason for hiding this comment

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

Looks pretty straight forward to me.

src/background-monitor.c Show resolved Hide resolved
src/background-monitor.c Show resolved Hide resolved
src/background-monitor.c Outdated Show resolved Hide resolved
src/background-monitor.c Outdated Show resolved Hide resolved
src/background.c Outdated Show resolved Hide resolved
@GeorgesStavracas GeorgesStavracas force-pushed the gbsneto/background-session branch 3 times, most recently from 5017a46 to a5b15e3 Compare October 12, 2022 17:35
GeorgesStavracas added a commit to GeorgesStavracas/libportal that referenced this pull request Oct 12, 2022
Add the correspondent background status API.

See flatpak/xdg-desktop-portal#901
@GeorgesStavracas GeorgesStavracas force-pushed the gbsneto/background-session branch 2 times, most recently from d17d33d to 51a10c1 Compare October 12, 2022 20:20
@GeorgesStavracas GeorgesStavracas marked this pull request as ready for review October 12, 2022 20:20
@GeorgesStavracas
Copy link
Member Author

Okay, from an API / services point of view, I think this is ready again. I'm still not entirely happy with the sleep times, but this iteration feels a lot more responsive compared to 30s + 30s.

@GeorgesStavracas GeorgesStavracas force-pushed the gbsneto/background-session branch from 51a10c1 to 03c08ef Compare October 20, 2022 16:24
data/org.freedesktop.background.Monitor.xml Show resolved Hide resolved
src/background.c Outdated Show resolved Hide resolved
@GeorgesStavracas
Copy link
Member Author

I've written a little demo python app to showcase this new background monitoring service: https://gitlab.gnome.org/-/snippets/4305

To run, download the file, then run it in a terminal.

Here's an example of it working:

Gravacao.de.tela.de.2022-10-20.13-20-37.webm

We probably need to be more aggressive with the detection timings...

@GeorgesStavracas GeorgesStavracas force-pushed the gbsneto/background-session branch from 03c08ef to be91e85 Compare October 20, 2022 17:10
@GeorgesStavracas
Copy link
Member Author

Last push should make it a lot more eager in detecting background apps!

@GeorgesStavracas GeorgesStavracas added this to the 1.16 milestone Oct 20, 2022
@GeorgesStavracas GeorgesStavracas mentioned this pull request Oct 21, 2022
6 tasks
src/background.c Outdated Show resolved Hide resolved
@matthiasclasen
Copy link
Contributor

Looks fine, apart from the minor comments I left.

@GeorgesStavracas
Copy link
Member Author

ACKed on the Matrix channel, let's land

This is the minimal representation of what a background monitoring
service would look like.

The bus name must not be under "org.freedesktop.portal." because
otherwise this would accidentally be exposed to sandboxed apps.
In fact, we must create an entirely new connection even, to prevent
GDBus from exposing the org.freedesktop.background.Monitor interface
into org.freedesktop.portal.Desktop accidentally.

Right now, the implementation is stub - we just create the D-Bus
object, a side GDBusConnection, export it, and request the
org.freedesktop.background.Monitor name. If any of these steps fail,
initialization is aborted.
This is just enough to get a background monitoring service. Most
of the heavy-lifting is already done by the background portal
itself, via the monitoring thread. This commit merely serialized
the already up-to-date list of applications into a vardict GVariant,
and updates the 'background_apps' property of the background monitor
interface.

Related: flatpak#899
This will simplify next commits. No functional changes.
This method allows sandboxed applications to set their background
status, which is picked up by the background monitor service, and
can be used to implement richer user experiences on the host side.

This first version has a single property: a status message. This
should be enough to get us going with this idea.
Now that host systems can monitor which applications are running
in background, we have a breathing room for improving the user
experience - or, specifically, an annoying behavior detrimental
to it - in the background portal.

Grant applications that want to run in background the permission
to do so by default, unless the user explicitly sets that as the
ASK permission, which means "always ask".
We want to respect whatever value is returned by the backend, so
stop setting it to YES right before asking the backend for the
permission.
This seems to work a little bit better than 30 + 30. It still gives
apps a grace time, but reports their background state in reasonably
responsive of a time.
@GeorgesStavracas GeorgesStavracas force-pushed the gbsneto/background-session branch from 985145a to 742434a Compare October 25, 2022 14:54
@GeorgesStavracas GeorgesStavracas merged commit f092371 into flatpak:main Oct 25, 2022
@GeorgesStavracas GeorgesStavracas deleted the gbsneto/background-session branch October 25, 2022 15:06
GeorgesStavracas added a commit to GeorgesStavracas/libportal that referenced this pull request Oct 25, 2022
Add the correspondent background status API.

See flatpak/xdg-desktop-portal#901
GeorgesStavracas added a commit to flatpak/libportal that referenced this pull request Oct 25, 2022
Add the correspondent background status API.

See flatpak/xdg-desktop-portal#901
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants