Improve background portal support.

[vchernin]

EasyEffects Version

6.1.4

What package are you using?

Flatpak (Flathub)

Distribution

Fedora 35

Describe the bug

EasyEffects should better support the background portal, which is used for both running in the background, and autostart.

There are multiple issue reports that have popped up, which mostly revolve around autostart not working when the switch is enabled. It might be the patch is at fault, but reproducing it is hard.

I think all those bugs can be avoided if EasyEffects does the following:

1. EasyEffects in Flatpak should not assume it can run after window close, or autostart, without permission.
2. EasyEffects should verify permissions before attempting to run in background or autostart (e.g. check portal on startup if autostart and background switch enabled).

Improvements

Shutdown on window close is disabled by default (at least in Flatpak), if user disables that ask for background portal.

Debug Log

No response

Additional Information

I have noticed a strange case where the autostart switch doesn't actually create the autostart file, and needs to be permission-reset. I haven't figured out steps to reproduce that. This is likely #1279. As stated above, more sanity checks should avoid these situations.

It might be worth revisiting how the background portal and autostart portal can be supported without a patch cleanly, without creating a maintenance burden. Unfortunately both those portals still don't work yet on the host, but eventually they should.

[wwmm]

This probably means shutdown on window close should be enabled by default at least in Flatpak.

I have never used it but GSettings supports some kind of "Vendor Settings". If I am not mistaken distributions can use this to customize the default settings they want for applications that use gsettings. Maybe this could be used when building the Flatpak package.

When I started this project the default was not having effects after closing the window. But people asked for it to be changed. And after so long using this as default I became used to it. Most of the time I have the window closed.

Maybe once the work I am doing on #1243 is finished I can take a look at the portal situation again. Something I have already done there is using libadwaita to handle the switch between light and dark themes

easyeffects/src/application_ui.cpp

Line 53 in 97f144e

adw_style_manager_set_color_scheme(adw_style_manager_get_default(), ADW_COLOR_SCHEME_FORCE_LIGHT);

[vchernin]

I have never used it but GSettings supports some kind of "Vendor Settings". If I am not mistaken distributions can use this to customize the default settings they want for applications that use gsettings. Maybe this could be used when building the Flatpak package.

When I started this project the default was not having effects after closing the window. But people asked for it to be changed. And after so long using this as default I became used to it. Most of the time I have the window closed.

A vendor setting to change the default for Flatpak sounds quite reasonable.

Regardless the main blocker here is not the setting itself, but actually having a portal implementation. So defaults are probably best revisted then I suppose.

Maybe once the work I am doing on #1243 is finished I can take a look at the portal situation again.

Hopefully a portal maintainer can review flatpak/xdg-desktop-portal#581 which should be the missing piece so the portals work for non-Flatpaks, maybe I'll ask around.

Something I have already done there is using libadwaita to handle the switch between light and dark themes

I might test that branch out soon, it sounds quite exciting!

[wwmm]

I might test that branch out soon, it sounds quite exciting!

Just have in mind I still have a lot to do there. Although effects are applied the widgets control to them are not in the window yet. But things like preset importing are already working. It would be good to know if now we are accessing gtk directly something has changed in those issues where the file dialog was causing freezes for some people.

[vchernin]

It would be good to know if now we are accessing gtk directly something has changed in those issues where the file dialog was causing freezes for some people.

Yes, perhaps this is useful to mention in those bug reports.

[vchernin]

The patch needs to be adjusted for the new preferences window, so I thought to review what a ideal fix should do:

I think it’s best to go for the simplest approach possible here.

At a high level:

1. Flatpak EasyEffects should not assume background or autostart permission.

2. Flatpak EasyEffects should verify permissions before attempting to run in background or autostart.

Provided those two are always met by the implementation I think everything is sane.

For the implementation:

1. Flatpak should shutdown on window close by default (use gsettings vendor setting or patch).
2. At startup, EasyEffects should read the current state of the preferences window switches, and request permission accordingly.

• If autostart key is enabled it will ask for background access + autostart. If only background key is enabled just ask for background access (no autostart). This effectively is a way to check portal status, since the user won't be notified if EasyEffects asks for a permission it already has (I believe, haven't tested).

• This is since somehow users ended up losing permission to autostart/background despite having previously enabled the autostart switch. So they could flip the switch in settings but then no permission prompt would appear, meaning EasyEffects thought they already had permission despite EasyEffects not actually autostarting. I believe I experienced this, but I have no idea how to reproduce it, it might well be a portal bug. I think printing portal info at startup would help understand this behaviour if it still occurs.

• Either a new gsettings key or creating a useless autostart file is probably still needed to remember the autostart state when using libportal.

I don’t know what the best approach is for non sandboxed builds. People have different preferences whether they want background or autostart by default. So I think non-Flatpak builds can be left alone for now.

In master without libportal, enabling the autostart switch directly creates the autostart desktop file correct? Is that how EasyEffects remembers the state of the autostart switch? The rest of the switches all use gsettings keys right?

[wwmm]

Flatpak should shutdown on window close by default (use gsettings vendor setting or patch).

More information about the vendor overrides feature in GSettings https://docs.gtk.org/gio/class.Settings.html#vendor-overrides. I have never tried to use it as it is not something done on the application side.

In master without libportal, enabling the autostart switch directly creates the autostart desktop file correct?

Yes. A file is created inside ~/.config/autostart/

Is that how EasyEffects remembers the state of the autostart switch? The rest of the switches all use gsettings keys right?

It checks if there is an autostart file for it in the autostart folder. This setting is not saved in any gsettings key.

[vchernin]

It checks if there is an autostart file for it in the autostart folder. This setting is not saved in any gsettings key.

Yeah, that's how I understood it.

[vchernin]

So the portal is better supported in the flathub-beta branch now (with the latest upstream master branch). Both the autostart and shutdown on window close switches correctly use the portal. I enabled shutdown on window close by default with a patch (since the gschemas were being patched anyways).

There are still some things I want to improve. Most importantly, I still don't fully understand what caused xdg-desktop-portal killing EasyEffects in those several duplicate "EasyEffects is crashing after 2 minutes" bug reports. Hopefully shutdown on window close being enabled by default for Flatpak will avoid that. But if it still occurs on this rebased patch hopefully an explanation can be found...

[vchernin]

I think I've found a portal bug. EE and a portal test app ashpd (both flatpaks) seem to run into the same issue.

Basically, if I launch either app fresh (config cleared) the background portal in both apps works. But after the Flatpak app has been updated, the previously working portal request will be "cancelled" when using the updated version. At least I think that's the issue, I will try to reproduce it properly. It might be a very weird config issue with my system or something.

Actually ashpd demo never got an update itself, I wonder how to reproduce the bug...

On second thought I'm not condident this is actually a portal bug, I will blame it on user error. The older patch was not as good so hopefully that explains it.

[vchernin]

So interestingly with the gtkmm patch version, I find
if I flatpak permission-reset com.github.wwmm.easyeffects and then run the app, the portal call will always return cancelled. I am honestly not sure why the previous version does this, but I can't reproduce it with the new one which is good. The patch is fairly elaborate now but I think it's necessary...

The last thing I want is to have some kind of dialog when the portal request fails. Currently it prints warnings, but obviously most users will not see those.

[wwmm]

I am honestly not sure why the previous version does this

I also have no idea. In theory there should be no difference in behavior between gtkmm and pure gtk. But as gtkmm adds the usual c++ memory handling procedures on top of gtk it is possible some things behave differently.

[vchernin]

In theory there should be no difference in behavior between gtkmm and pure gtk.

Yeah. One thing I realized we now use a newer libportal version, maybe that's why. But otherwise I'm not sure. There's a lot more sanity checks now (and the switches correctly follow what permissions are actually given), but none of those actually change how the portal is called, they change when the portal is called.

In any case hopefully fewer people run into trouble..

[vchernin]

This is a bit improved now, see #1500 (comment)