Bug 180812 – [WPE][GTK] Sleep inhibitors do not work under Flatpak

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Michael Catanzaro]

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.

[Michael Catanzaro]

Created attachment 331875 [details]
Patch

[Michael Catanzaro]

(I finally managed to test this using the new flapjack tool from Endless. It works.)

[Michael Catanzaro]

Created attachment 331879 [details]
Patch

[Carlos Garcia Campos]

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?

[Michael Catanzaro]

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?

[Carlos Garcia Campos]

Comment on attachment 331879 [details]
Patch

Ok, I guess it's fine as it is then. Thanks

[WebKit Commit Bot]

Comment on attachment 331879 [details]
Patch

Clearing flags on attachment: 331879

Committed r227415: <https://trac.webkit.org/changeset/227415>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.