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

Persistent remote desktop sessions #1004

Merged
merged 11 commits into from
Jul 13, 2023

Conversation

jadahl
Copy link
Collaborator

@jadahl jadahl commented Apr 14, 2023

This adds persistent remote desktop sessions similar to persistent
screen cast sessions. What it means is that applications can ask for
sessions to be "saved" either temporarily in the portal, or in the
permission store, and later be restored at will, without any interactive
dialog showing up.

It works the same way, by the Start() method returning a restore token,
and similarly to the screen cast variant, passing the same restore token
to the SelectDevices() method the next time.

Closes: #850


Also contains a bunch of cleanups to the screen cast and remote desktop portal specification.

@jadahl
Copy link
Collaborator Author

jadahl commented Apr 14, 2023

For backend support in GNOME, see https://gitlab.gnome.org/GNOME/xdg-desktop-portal-gnome/-/merge_requests/88.

Copy link
Member

@GeorgesStavracas GeorgesStavracas left a comment

Choose a reason for hiding this comment

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

Left a few comments, but the general approach seems great and the code flow looks good to me. Thanks!

src/xdp-utils.h Outdated Show resolved Hide resolved
src/remote-desktop.c Outdated Show resolved Hide resolved
src/remote-desktop.c Show resolved Hide resolved
src/restore-token.h Outdated Show resolved Hide resolved
@jadahl
Copy link
Collaborator Author

jadahl commented Apr 18, 2023

Left a few comments, but the general approach seems great and the code flow looks good to me. Thanks!

And thanks for giving me an already established concept to just tweak a bit!

Copy link
Contributor

@whot whot left a comment

Choose a reason for hiding this comment

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

#762 has some extensions to the pytest test suite for testing RD, maybe it's worth stealing those so you can add the tests for restore tokens here?

data/org.freedesktop.impl.portal.RemoteDesktop.xml Outdated Show resolved Hide resolved
src/remote-desktop.c Show resolved Hide resolved
@bjaraujo
Copy link

Any updates on this?

@jadahl
Copy link
Collaborator Author

jadahl commented Jul 11, 2023

Any updates on this?

Was somewhat waiting for #852 to land so this could be rebased to include a clipboard part too. My plan is to get it into the coming release of xdg-desktop-portal.

@jadahl jadahl force-pushed the wip/remote-desktop-restore-token branch from b83f28a to 33bac33 Compare July 11, 2023 15:30
@jadahl
Copy link
Collaborator Author

jadahl commented Jul 11, 2023

#762 has some extensions to the pytest test suite for testing RD, maybe it's worth stealing those so you can add the tests for restore tokens here?

I tried, and came so far that I was lacking the permission store mock, so stopped there for now. Will pick it up later, but would be nice if it didn't block this.

src/xdp-utils.c Outdated Show resolved Hide resolved
src/remote-desktop.c Show resolved Hide resolved
Copy link
Member

@GeorgesStavracas GeorgesStavracas left a comment

Choose a reason for hiding this comment

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

I cannot overstate how happy the last pair of commits make me feel. Clearly this is how this code should have been implemented initially. Thank you so much for doing it. Today is a good day.

jadahl added 11 commits July 13, 2023 14:36
This makes it reusable for a future remote-desktop variant.
The available devices lists available devices, not sources.
Nothing related to persistence will be communicated via the
SelectSources response signal, it's all sent via the response signal to
the Start method.
Document "streams" equivalent to the response signal for starting a
screen cast session, and the selected device types.
Copy paste error from the remote desktop portal.
This adds persistent remote desktop sessions similar to persistent
screen cast sessions. What it means is that applications can ask for
sessions to be "saved" either temporarily in the portal, or in the
permission store, and later be restored at will, without any interactive
dialog showing up.

It works the same way, by the Start() method returning a restore token,
and similarly to the screen cast variant, passing the same restore token
to the SelectDevices() method the next time.

Closes: flatpak#850
This will be emitted unconditionally, so that auxiliary session
functionality can clean up things when the session closes.
This means all methods take a session that already carries some relevant
information, e.g. D-Bus peer name, and app ID. It also means transient
permission handling can be hooked up to the 'internal-closed' signal
instead of being handled manually.
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.

Extend persist/restore_token concept to RemoteDesktop
4 participants