Bug 180812 - [WPE][GTK] Sleep inhibitors do not work under Flatpak
Summary: [WPE][GTK] Sleep inhibitors do not work under Flatpak
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: Other
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-12-14 08:03 PST by Michael Catanzaro
Modified: 2018-01-23 08:51 PST (History)
4 users (show)

See Also:


Attachments
Patch (9.24 KB, patch)
2018-01-21 12:07 PST, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (10.31 KB, patch)
2018-01-21 12:43 PST, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2017-12-14 08:03:31 PST
Sleep inhibitors don't work under flatpak. We need to use the flatpak portal D-Bus API directly. We can't use GtkApplication because this needs to work from the web process.
Comment 1 Michael Catanzaro 2018-01-21 12:07:52 PST
Created attachment 331875 [details]
Patch
Comment 2 Michael Catanzaro 2018-01-21 12:08:57 PST
(I finally managed to test this using the new flapjack tool from Endless. It works.)
Comment 3 Michael Catanzaro 2018-01-21 12:43:50 PST
Created attachment 331879 [details]
Patch
Comment 4 Carlos Garcia Campos 2018-01-21 23:17:13 PST
Comment on attachment 331879 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=331879&action=review

> Source/WebCore/PAL/pal/system/glib/SleepDisablerGLib.cpp:95
> +            // It failed. Try to use the Flatpak portal instead.
> +            g_dbus_proxy_new_for_bus(G_BUS_TYPE_SESSION, static_cast<GDBusProxyFlags>(G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES | G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS),
> +                nullptr, "org.freedesktop.portal.Desktop", "/org/freedesktop/portal/desktop", "org.freedesktop.portal.Inhibit", self->m_cancellable.get(),
> +                [](GObject*, GAsyncResult* result, gpointer userData) {
> +                    GUniqueOutPtr<GError> error;
> +                    GRefPtr<GDBusProxy> proxy = adoptGRef(g_dbus_proxy_new_for_bus_finish(result, &error.outPtr()));
> +                    if (g_error_matches(error.get(), G_IO_ERROR, G_IO_ERROR_CANCELLED))
> +                        return;
> +
> +                    auto* self = static_cast<SleepDisablerGLib*>(userData);
> +                    if (proxy) {
> +                        GUniquePtr<char> nameOwner(g_dbus_proxy_get_name_owner(proxy.get()));
> +                        if (nameOwner) {
> +                            self->m_inhibitPortalProxy = WTFMove(proxy);
> +                            self->acquireInhibitor();
> +                            return;
> +                        }
> +                    }
> +
> +                    // Give up. Don't warn the user: this is expected.
> +                    self->m_cancellable = nullptr;
> +            }, self);

Could we move this to a helper function that receives the DBus names instead of duplicating the code?
Comment 5 Michael Catanzaro 2018-01-22 09:15:56 PST
It would be a bit tricky. I guess the helper function would need to take, as parameters:

 * Three different strings (for the name owner, object path, interface name)
 * Pointer to the GRefPtr<GDBusProxy> to WTFMove(proxy) into, since this is different in the two different paths
 * The action to take on completion is different too, so it'd need a pointer to an optional <void (void)> WTF::Function to called on completion in the event of failure; in the second case, that would just be to destroy the cancellable, but in the first case, it would be a lambda to call the helper function itself again with different parameters

That's my best idea. The helper function, say getDBusProxyIfNameIsOwned, would itself be simple, but the call to it would not be; it would look something like:

getDBusProxyIfNameIsOwned("org.freedesktop.ScreenSaver", "/org/freedesktop/ScreenSaver", "org.freedesktop.ScreenSaver", &m_screenSaverProxy, [this] {
    getDBusProxyIfNameIsOwned("org.freedesktop.portal.Desktop", "/org/freedesktop/portal/desktop", "org.freedesktop.portal.Inhibit", &m_inhibitPortalProxy, [this] {
        m_cancellable = nullptr;
    });
});

So it's certainly possible to reduce the code duplication, but it would be harder to read, so I'm not sure we should. Do you still want me to try it, or do you have a better suggestion?
Comment 6 Carlos Garcia Campos 2018-01-22 22:14:32 PST
Comment on attachment 331879 [details]
Patch

Ok, I guess it's fine as it is then. Thanks
Comment 7 WebKit Commit Bot 2018-01-23 08:51:27 PST
Comment on attachment 331879 [details]
Patch

Clearing flags on attachment: 331879

Committed r227415: <https://trac.webkit.org/changeset/227415>
Comment 8 WebKit Commit Bot 2018-01-23 08:51:28 PST
All reviewed patches have been landed.  Closing bug.