Merge lp:~3v1n0/unity-settings-daemon/redirect-fdo-screensaver-calls into lp:unity-settings-daemon

[Marco Trevisan (Treviño)]

ScreensaverProxy: redirect supported calls to gnome screensaver APIs

[Sebastien Bacher (seb128)]

Thanks for the work, some small questions/comments inline.

Seems like you included some small cleanups unrelated to the changeset, it's fine but it might make review easier/more sense to split that in a different commit/(upstreamable)merge request next time?

review: Needs Information

[Marco Trevisan (Treviño) (3v1n0)]

I was actually thinking of sending upstream the whole thing, since I don't see this wrong even in Gnome environment, but I'd like to discuss with upstream.

I don't know if they just disable this plugin in gnome-shell leaving the implementation of this to the shell.

[Marco Trevisan (Treviño) (3v1n0)]

Ah, code updated as well.

[Sebastien Bacher (seb128)]

thanks

review: Approve

=== added file 'gnome-settings-daemon/org.gnome.ScreenSaver.xml'
--- gnome-settings-daemon/org.gnome.ScreenSaver.xml	1970-01-01 00:00:00 +0000
+++ gnome-settings-daemon/org.gnome.ScreenSaver.xml	2016-05-30 18:37:12 +0000
@@ -0,0 +1,43 @@
+<!DOCTYPE node PUBLIC
+        "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN"
+        "http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd">
+
+<!--
+        Copyright (C) 2013 Red Hat, Inc.
+
+        This library is free software; you can redistribute it and/or
+        modify it under the terms of the GNU Lesser General Public
+        License as published by the Free Software Foundation; either
+        version 2 of the License, or (at your option) any later version.
+
+        This library is distributed in the hope that it will be useful,
+        but WITHOUT ANY WARRANTY; without even the implied warranty of
+        MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+        Lesser General Public License for more details.
+
+        You should have received a copy of the GNU Lesser General
+        Public License along with this library; if not, see <http://www.gnu.org/licenses/>.
+-->
+
+<node name="/" xmlns:doc="http://www.freedesktop.org/dbus/1.0/doc.dtd">
+        <!--
+        org.gnome.ScreenSaver:
+
+        An interface used for managing the lock screen.
+        -->
+        <interface name="org.gnome.ScreenSaver">
+                <method name="Lock" />
+                <method name="GetActive">
+                        <arg name="active" direction="out" type="b" />
+                </method>
+                <method name="SetActive">
+                        <arg name="value" direction="in" type="b" />
+                </method>
+                <method name="GetActiveTime">
+                        <arg name="value" direction="out" type="u" />
+                </method>
+                <signal name="ActiveChanged">
+                        <arg name="new_value" type="b" />
+                </signal>
+        </interface>
+</node>
 
=== modified file 'plugins/screensaver-proxy/gsd-screensaver-proxy-manager.c'
--- plugins/screensaver-proxy/gsd-screensaver-proxy-manager.c	2016-05-30 18:37:11 +0000
+++ plugins/screensaver-proxy/gsd-screensaver-proxy-manager.c	2016-05-30 18:37:12 +0000
@@ -28,15 +28,12 @@
 #include <string.h>
 #include <errno.h>
 
-#include <locale.h>
-
-#include <glib.h>
-#include <glib/gi18n.h>
-#include <gdk/gdk.h>
+#include <gio/gio.h>
 
 #include "gnome-settings-bus.h"
 #include "gnome-settings-profile.h"
 #include "gsd-screensaver-proxy-manager.h"
+#include "gsd-idle-monitor.h"
 
 #define GSD_SCREENSAVER_PROXY_MANAGER_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), GSD_TYPE_SCREENSAVER_PROXY_MANAGER, GsdScreensaverProxyManagerPrivate))
 
@@ -135,6 +132,7 @@
 struct GsdScreensaverProxyManagerPrivate
 {
         GsdSessionManager       *session;
+        GsdScreenSaver          *screensaver;
         GDBusConnection         *connection;
         GCancellable            *bus_cancellable;
         GDBusNodeInfo           *introspection_data;
@@ -194,18 +192,19 @@
                     gpointer               user_data)
 {
         GsdScreensaverProxyManager *manager = GSD_SCREENSAVER_PROXY_MANAGER (user_data);
+        GError *error = NULL;
+        GVariant *ret = NULL;
 
         /* Check session pointer as a proxy for whether the manager is in the
            start or stop state */
-        if (manager->priv->session == NULL) {
-                return;
+        if (manager->priv->session == NULL || manager->priv->screensaver == NULL) {
+                goto unimplemented;
         }
 
         g_debug ("Calling method '%s.%s' for ScreenSaver Proxy",
                  interface_name, method_name);
 
         if (g_strcmp0 (method_name, "Inhibit") == 0) {
-                GVariant *ret;
                 const char *app_id;
                 const char *reason;
                 guint cookie;
@@ -213,12 +212,12 @@
                 g_variant_get (parameters,
                                "(ss)", &app_id, &reason);
 
-                ret = g_dbus_proxy_call_sync (G_DBUS_PROXY (G_DBUS_PROXY (manager->priv->session)),
+                ret = g_dbus_proxy_call_sync (G_DBUS_PROXY (manager->priv->session),
                                               "Inhibit",
                                               g_variant_new ("(susu)",
                                                              app_id, 0, reason, GSM_INHIBITOR_FLAG_IDLE),
                                               G_DBUS_CALL_FLAGS_NONE,
-                                              -1, NULL, NULL);
+                                              -1, NULL, &error);
                 g_variant_get (ret, "(u)", &cookie);
                 g_hash_table_insert (manager->priv->cookie_ht,
                                      GUINT_TO_POINTER (cookie),
@@ -237,7 +236,6 @@
                                              g_strdup (sender),
                                              GUINT_TO_POINTER (watch_id));
                 }
-                g_dbus_method_invocation_return_value (invocation, ret);
         } else if (g_strcmp0 (method_name, "UnInhibit") == 0) {
                 guint cookie;
 
@@ -249,24 +247,41 @@
                                         -1, NULL, NULL);
                 g_debug ("Removing cookie %u from the list for %s", cookie, sender);
                 g_hash_table_remove (manager->priv->cookie_ht, GUINT_TO_POINTER (cookie));
-                g_dbus_method_invocation_return_value (invocation, NULL);
-        } else if (g_strcmp0 (method_name, "Throttle") == 0) {
-                g_dbus_method_invocation_return_value (invocation, NULL);
-        } else if (g_strcmp0 (method_name, "UnThrottle") == 0) {
-                g_dbus_method_invocation_return_value (invocation, NULL);
-        } else if (g_strcmp0 (method_name, "Lock") == 0) {
-                goto unimplemented;
-        } else if (g_strcmp0 (method_name, "SimulateUserActivity") == 0) {
-                goto unimplemented;
-        } else if (g_strcmp0 (method_name, "GetActive") == 0) {
-                goto unimplemented;
-        } else if (g_strcmp0 (method_name, "GetActiveTime") == 0) {
-                goto unimplemented;
+        } else if (g_strcmp0 (method_name, "Lock") == 0 ||
+                   g_strcmp0 (method_name, "SimulateUserActivity") == 0 ||
+                   g_strcmp0 (method_name, "GetActive") == 0 ||
+                   g_strcmp0 (method_name, "GetActiveTime") == 0 ||
+                   g_strcmp0 (method_name, "SetActive") == 0) {
+
+                ret = g_dbus_proxy_call_sync (G_DBUS_PROXY (manager->priv->screensaver),
+                                              method_name,
+                                              parameters,
+                                              G_DBUS_CALL_FLAGS_NONE,
+                                              -1, NULL, &error);
+
+                if (error == NULL && g_strcmp0 (method_name, "SetActive") == 0) {
+                        g_variant_unref (ret);
+
+                        /* Returning the actual Activate state here is not possible,
+                         * as calling GetActive at this point might return an invalid
+                         * value if the activation process is still ongoing. */
+                        ret = g_variant_ref (parameters);
+                }
         } else if (g_strcmp0 (method_name, "GetSessionIdleTime") == 0) {
-                goto unimplemented;
-        } else if (g_strcmp0 (method_name, "SetActive") == 0) {
-                goto unimplemented;
-        }
+                GsdIdleMonitor *idle_monitor = gsd_idle_monitor_get_core ();
+                gint64 idle_time_ms = gsd_idle_monitor_get_idletime (idle_monitor);
+                ret = g_variant_new ("(u)", idle_time_ms / 1000);
+        } else {
+                goto unimplemented;
+        }
+
+        if (error != NULL) {
+                g_dbus_method_invocation_return_gerror (invocation, error);
+                g_error_free (error);
+                return;
+        }
+
+        g_dbus_method_invocation_return_value (invocation, ret);
 
         return;
 
@@ -354,6 +369,8 @@
         gnome_settings_profile_start (NULL);
         manager->priv->session =
                 gnome_settings_bus_get_session_proxy ();
+        manager->priv->screensaver =
+                gnome_settings_bus_get_screen_saver_proxy ();
         manager->priv->watch_ht = g_hash_table_new_full (g_str_hash,
                                                          g_str_equal,
                                                          (GDestroyNotify) g_free,
@@ -371,6 +388,7 @@
 {
         g_debug ("Stopping screensaver_proxy manager");
         g_clear_object (&manager->priv->session);
+        g_clear_object (&manager->priv->screensaver);
         g_clear_pointer (&manager->priv->watch_ht, g_hash_table_destroy);
         g_clear_pointer (&manager->priv->cookie_ht, g_hash_table_destroy);
 }