Add background portal
ClosedPublic

Authored by jgrulich on Mar 23 2020, 9:51 AM.

Details

Summary

Implements Background portal, providing APIS related to applications running in the background.

Implemented according to: https://github.com/flatpak/xdg-desktop-portal/blob/master/data/org.freedesktop.impl.portal.Background.xml

Test Plan

Tested with manual invocations over DBus and checking what I return back through dbus-monitor.

Diff Detail

Repository
R838 Flatpak Support: KDE Portal for XDG Desktop
Branch
background-portal
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 24094
Build 24112: arc lint + arc unit
jgrulich created this revision.Mar 23 2020, 9:51 AM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 23 2020, 9:51 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
jgrulich requested review of this revision.Mar 23 2020, 9:51 AM
jgrulich edited the summary of this revision. (Show Details)
jgrulich edited the test plan for this revision. (Show Details)
jgrulich updated this revision to Diff 78271.Mar 23 2020, 10:16 AM

Replace my custom function for quoting of arguments with KShell

jgrulich updated this revision to Diff 78272.Mar 23 2020, 10:24 AM

Avoid potential crash when captured variable goes out of scope

apol added a subscriber: apol.Apr 6 2020, 1:12 PM
apol added inline comments.
src/background.cpp
70

I don't really understand why we're exposing this. Is it for contained apps?

134

const QVariantMap map = { {QStringLiteral("result"), static_cast<uint>(result)} };

195

Will dbus look in autostart though?

208

context should be [this]

211

[this]

src/background.h
80

I'd call it windows, it's better not include the type. It will change to QVector with Qt6 anyway.

jgrulich updated this revision to Diff 79515.Apr 6 2020, 6:24 PM
jgrulich marked 5 inline comments as done.

Update to fix review comments

jgrulich added inline comments.Apr 6 2020, 6:25 PM
src/background.cpp
70

This is actually not API exposed to the clients, this is purely for xdg-desktop-portal needs, while the client interface (xdg-desktop-portal) has limited stuff available, see https://github.com/flatpak/xdg-desktop-portal/blob/master/data/org.freedesktop.portal.Background.xml.

195

Doesn't need to? I think it just specifies that the desktop file should ignore the exec line and start it through DBus (assuming correct dbus service file is installed etc.). Or am I wrong?

apol accepted this revision.Apr 7 2020, 12:40 AM
apol added inline comments.
src/background.cpp
195

Ah, alright.

This revision is now accepted and ready to land.Apr 7 2020, 12:40 AM
This revision was automatically updated to reflect the committed changes.
davidedmundson added inline comments.Apr 7 2020, 12:00 PM
src/background.cpp
166

From the docs

Commandline to use add when autostarting at login.
     If this is not specified, the Exec line from the
     desktop file will be used.

I can't see us doing that last part

195

Doesn't need to? I think it just specifies that the desktop file should ignore the exec line and start it through DBus (assuming correct dbus service file is installed etc.). Or am I wrong?

You are right. We don't support it though.

jgrulich added inline comments.Apr 7 2020, 12:12 PM
src/background.cpp
166

The commandline is actually the Exec line, it's just named that way from the portal specification.

You can see below:

desktopEntryConfigGroup.writeEntry(QStringLiteral("Exec"), KShell::joinArgs(commandline));

Yes, I know why and didn't realize it. See the mail about PipeWire dependency on plasma-devel, I added you to CC list so you don't miss it.