Introduce save/restore portal #1818

AdrianVovk · 2025-09-25T03:25:17Z

Implements the portal proposed in #1698

Links to other MRs:

Backend impl in gnome-session: https://gitlab.gnome.org/GNOME/gnome-session/-/merge_requests/162
GTK client for this portal: https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/8980

data/org.freedesktop.portal.SaveRestore.xml

Mikenux · 2025-09-26T03:48:35Z

About the naming...

Why SaveHint instead of Save, given that it's Save on the SaveRestore interface? Or SaveState?

AdrianVovk · 2025-09-26T04:33:00Z

Why SaveHint instead of Save

I renamed it to make it a bit more clear

given that it's Save on the SaveRestore interface

Sorry, where? It's SaveHint in all the dbus interfaces here

Mikenux · 2025-09-27T03:27:05Z

Sorry, where? It's SaveHint in all the dbus interfaces here

Sorry, mistake.

I renamed it to make it a bit more clear

Relative to? If you are looking for something meaningful, I think SaveState or SaveAppState are better options. Same for the interface name.

AdrianVovk · 2025-09-27T07:23:01Z

Relative to? If you are looking for something meaningful, I think SaveState or SaveAppState are better options. Same for the interface name

Whoops didn't finish typing my comment. SaveHint makes it a bit more clear that it's just a hint and that the app shouldn't be using it as the primary source for deciding to save state. If it was just Save it might seem like the portal will emit the signal regularly or whatever. Anyway, not the most important thing in the world, but that's why I renamed it

AdrianVovk · 2025-09-27T07:43:36Z

CC @davidedmundson and @technobaboo as the two folks from other desktops who I've interacted with in the discussion

This is the real API I ended up implementing. PTAL and let me know if there's some blocker to getting something like this working in your desktop, or you see something missing, or etc.

For your reference, here's GNOME's implementation of this portal's backend: https://gitlab.gnome.org/GNOME/gnome-session/-/blob/avovk/session-save-portal/gnome-session/gsm-session-save.c

technobaboo · 2025-09-27T08:43:36Z

wait how does the app get the instance ID and know to restore its data using that ID? afaik there's no standard key name for it or such in the registration map it gets as a return arg for registering

AdrianVovk · 2025-09-27T18:27:00Z

how does the app get the instance ID and know to restore its data using that ID

It's there - it's the first arg called instance-id. And it knows to restore data using that ID via the second arg, restore-reason

I'm guessing you might have been looking at org.freedesktop.impl.portal.SaveRestore, which I don't document as thoroughly because I can link to the portal frontend dbus API instead. Check the docs for org.freedesktop.portal.SaveRestore, which should be more complete w.r.t. known keys in the various vardicts

technobaboo · 2025-09-27T21:41:48Z

okay that's nice to know, so if it's required why isn't it an argument instead of inside the map?

data/org.freedesktop.portal.SaveRestore.xml

davidedmundson · 2025-10-07T09:51:17Z

What's the goal of:

[session: Communicate close reason to impl]

to allow identify crash exits vs clean application shutdowns? Do any other portal impls gain anything from this? If not, we would be better off with a bespoke signal just on the session restore before Close.

swick

Just looked through the PR to understand what is being proposed here and found a bunch of things.

data/org.freedesktop.impl.portal.SaveRestore.xml

data/org.freedesktop.impl.portal.Session.xml

swick · 2025-10-16T16:57:52Z

src/xdp-session.c

+      else
+        xdp_dbus_impl_session_call_close_sync (session->impl_session,
+                                               NULL, &error);


Suggested change

else

xdp_dbus_impl_session_call_close_sync (session->impl_session,

NULL, &error);

else

{

xdp_dbus_impl_session_call_close_sync (session->impl_session,

NULL, &error);

}

Same issue in multiple places

src/xdp-session.c

src/save-restore.h

src/save-restore.c

swick · 2025-10-16T17:24:19Z

src/save-restore.c

+{
+  XdpDbusImplSaveRestore *impl_proxy = XDP_DBUS_IMPL_SAVE_RESTORE (source_object);
+  g_autofree RegisterData *data = user_data;
+  g_autoptr (GError) error = NULL;


In xdp and flatpak for some reason the autoptr style is g_autoptr(GError).

This lets the impl differentiate between a session that's closing because the app voluntarily called `Close()`, or because the app disconnected from the bus / some other reason.

AdrianVovk · 2025-10-23T21:56:03Z

data/org.freedesktop.portal.SaveRestore.xml

+        3 seconds of receiving this signal. The app may be transparently restarted
+        by the system at a later date.
+    -->
+    <signal name="Quit">


Hmm so I'm not really sure that this is actually necessary...

Instead of asking the app to quit, we can just put it in a cgroup freezer. This removes it from the run queue. We could then adjust the memory limits on the cgroup, which as far as I understand will force the app's memory out to swap and out of RAM. At that point, is there really any point to actually shutting down the app? We can leave the app "running", just frozen in a cgroup freezer and swapped out to disk. If the goal is to free up resources like memory and CPU, we've achieved that.

We can use something like SaveHint to ask the app to save its state just in case it somehow gets killed while in the freezer.

Alternatively, apps should just save their state immediately before quitting in response to something like SIGTERM. Then we could handle this simply by sending SIGTERM to the apps, rather than having them subscribe to this signal.

AdrianVovk · 2025-10-23T21:58:46Z

src/save-restore.c

+#define XDP_TYPE_SAVE_RESTORE (xdp_save_restore_get_type ())
+G_DECLARE_FINAL_TYPE (XdpSaveRestore,
+                      xdp_save_restore,
+                      XDP, SAVE_RESTORE,


Hmm so the USB portal names itself XdpUsb, but apparently most every other portal doesn't have an Xdp prefix. And apparently that was intentional

So, IDK am I supposed to rename this portal to just SaveRestore?

Perhaps if we want to maintain a separation, we could have a Portal prefix, so that all the portals are PortalAccounts, PortalSaveRestore, PortalUsb, etc and the utility classes/functions can keep XdpSession, XdpContext, etc

AdrianVovk · 2025-10-23T21:59:20Z

src/save-restore.c

+
+G_DEFINE_TYPE (XdpSaveRestoreSession,
+               xdp_save_restore_session,
+               xdp_session_get_type ())


XDP_TYPE_SESSION?

AdrianVovk · 2025-10-23T22:01:02Z

src/save-restore.c

+{
+  XdpSaveRestoreSession *save_restore_session = XDP_SAVE_RESTORE_SESSION (session);
+
+  save_restore_session->closed = TRUE;


I think a couple of the session implementations do nothing but keep track of this closed variable.

Maybe that should move up into the XdpSession object?

On the other hand, this class separation actually lets us check that the caller is giving us a session path to a session of a type we expect. So we'd need to have a subclass anyway. Or at the very least some sort of accounting of what portal created any given session

AdrianVovk · 2025-10-24T16:54:31Z

src/save-restore.c

+  XdpDbusImplSaveRestore *impl = XDP_DBUS_IMPL_SAVE_RESTORE (source_object);
+  g_autoptr(RegisterData) data = user_data;
+  g_autoptr(GError) error = NULL;
+  g_autoptr(GVariant) out_restore = NULL;


I renamed this to registration

davidedmundson · 2025-10-31T11:27:46Z

src/save-restore.c

+  g_autoptr(GError) error = NULL;
+
+  impl_config = xdp_portal_config_find (config, SAVE_RESTORE_DBUS_IMPL_IFACE);
+  if (impl_config != NULL)


This is backwards.

AdrianVovk commented Sep 25, 2025

View reviewed changes

data/org.freedesktop.portal.SaveRestore.xml Outdated Show resolved Hide resolved

AdrianVovk force-pushed the save-restore-portal branch from 29e6746 to 189855a Compare September 27, 2025 06:27

Mikenux reviewed Sep 27, 2025

View reviewed changes

data/org.freedesktop.portal.SaveRestore.xml Outdated Show resolved Hide resolved

Mikenux reviewed Sep 27, 2025

View reviewed changes

data/org.freedesktop.portal.SaveRestore.xml Outdated Show resolved Hide resolved

davidedmundson reviewed Oct 7, 2025

View reviewed changes

data/org.freedesktop.portal.SaveRestore.xml Show resolved Hide resolved

swick reviewed Oct 16, 2025

View reviewed changes

AdrianVovk added 2 commits October 23, 2025 14:35

session: Communicate close reason to impl

3374bc2

This lets the impl differentiate between a session that's closing because the app voluntarily called `Close()`, or because the app disconnected from the bus / some other reason.

Introduce save/restore portal

0677ad5

AdrianVovk force-pushed the save-restore-portal branch from 189855a to 0677ad5 Compare October 23, 2025 18:35

AdrianVovk commented Oct 24, 2025

View reviewed changes

davidedmundson reviewed Oct 31, 2025

View reviewed changes

Introduce save/restore portal #1818

Are you sure you want to change the base?

Introduce save/restore portal #1818

Uh oh!

Conversation

AdrianVovk commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Mikenux commented Sep 26, 2025

Uh oh!

AdrianVovk commented Sep 26, 2025

Uh oh!

Mikenux commented Sep 27, 2025

Uh oh!

AdrianVovk commented Sep 27, 2025

Uh oh!

AdrianVovk commented Sep 27, 2025

Uh oh!

technobaboo commented Sep 27, 2025

Uh oh!

AdrianVovk commented Sep 27, 2025

Uh oh!

technobaboo commented Sep 27, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davidedmundson commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

swick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AdrianVovk commented Sep 25, 2025 •

edited

Loading

davidedmundson commented Oct 7, 2025 •

edited

Loading