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

Proposed by Marco Trevisan (Treviño) on 2016-05-20
Status: Merged
Approved by: Sebastien Bacher on 2016-06-07
Approved revision: 4146
Merged at revision: 4139
Proposed branch: lp:~3v1n0/unity-settings-daemon/redirect-fdo-screensaver-calls
Merge into: lp:unity-settings-daemon
Prerequisite: lp:~3v1n0/unity-settings-daemon/remove-duplicated-dbus-proxies
Diff against target: 199 lines (+89/-28)
2 files modified
gnome-settings-daemon/org.gnome.ScreenSaver.xml (+43/-0)
plugins/screensaver-proxy/gsd-screensaver-proxy-manager.c (+46/-28)
To merge this branch: bzr merge lp:~3v1n0/unity-settings-daemon/redirect-fdo-screensaver-calls
Reviewer Review Type Date Requested Status
Sebastien Bacher 2016-05-20 Approve on 2016-05-24
Review via email: mp+295370@code.launchpad.net

Commit message

ScreensaverProxy: redirect supported calls to gnome screensaver APIs

To post a comment you must log in.
Sebastien Bacher (seb128) wrote :

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) wrote :

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) wrote :

Ah, code updated as well.

Sebastien Bacher (seb128) wrote :

thanks

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'gnome-settings-daemon/org.gnome.ScreenSaver.xml'
2--- gnome-settings-daemon/org.gnome.ScreenSaver.xml 1970-01-01 00:00:00 +0000
3+++ gnome-settings-daemon/org.gnome.ScreenSaver.xml 2016-05-30 18:37:12 +0000
4@@ -0,0 +1,43 @@
5+<!DOCTYPE node PUBLIC
6+ "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN"
7+ "http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd">
8+
9+<!--
10+ Copyright (C) 2013 Red Hat, Inc.
11+
12+ This library is free software; you can redistribute it and/or
13+ modify it under the terms of the GNU Lesser General Public
14+ License as published by the Free Software Foundation; either
15+ version 2 of the License, or (at your option) any later version.
16+
17+ This library is distributed in the hope that it will be useful,
18+ but WITHOUT ANY WARRANTY; without even the implied warranty of
19+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
20+ Lesser General Public License for more details.
21+
22+ You should have received a copy of the GNU Lesser General
23+ Public License along with this library; if not, see <http://www.gnu.org/licenses/>.
24+-->
25+
26+<node name="/" xmlns:doc="http://www.freedesktop.org/dbus/1.0/doc.dtd">
27+ <!--
28+ org.gnome.ScreenSaver:
29+
30+ An interface used for managing the lock screen.
31+ -->
32+ <interface name="org.gnome.ScreenSaver">
33+ <method name="Lock" />
34+ <method name="GetActive">
35+ <arg name="active" direction="out" type="b" />
36+ </method>
37+ <method name="SetActive">
38+ <arg name="value" direction="in" type="b" />
39+ </method>
40+ <method name="GetActiveTime">
41+ <arg name="value" direction="out" type="u" />
42+ </method>
43+ <signal name="ActiveChanged">
44+ <arg name="new_value" type="b" />
45+ </signal>
46+ </interface>
47+</node>
48
49=== modified file 'plugins/screensaver-proxy/gsd-screensaver-proxy-manager.c'
50--- plugins/screensaver-proxy/gsd-screensaver-proxy-manager.c 2016-05-30 18:37:11 +0000
51+++ plugins/screensaver-proxy/gsd-screensaver-proxy-manager.c 2016-05-30 18:37:12 +0000
52@@ -28,15 +28,12 @@
53 #include <string.h>
54 #include <errno.h>
55
56-#include <locale.h>
57-
58-#include <glib.h>
59-#include <glib/gi18n.h>
60-#include <gdk/gdk.h>
61+#include <gio/gio.h>
62
63 #include "gnome-settings-bus.h"
64 #include "gnome-settings-profile.h"
65 #include "gsd-screensaver-proxy-manager.h"
66+#include "gsd-idle-monitor.h"
67
68 #define GSD_SCREENSAVER_PROXY_MANAGER_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), GSD_TYPE_SCREENSAVER_PROXY_MANAGER, GsdScreensaverProxyManagerPrivate))
69
70@@ -135,6 +132,7 @@
71 struct GsdScreensaverProxyManagerPrivate
72 {
73 GsdSessionManager *session;
74+ GsdScreenSaver *screensaver;
75 GDBusConnection *connection;
76 GCancellable *bus_cancellable;
77 GDBusNodeInfo *introspection_data;
78@@ -194,18 +192,19 @@
79 gpointer user_data)
80 {
81 GsdScreensaverProxyManager *manager = GSD_SCREENSAVER_PROXY_MANAGER (user_data);
82+ GError *error = NULL;
83+ GVariant *ret = NULL;
84
85 /* Check session pointer as a proxy for whether the manager is in the
86 start or stop state */
87- if (manager->priv->session == NULL) {
88- return;
89+ if (manager->priv->session == NULL || manager->priv->screensaver == NULL) {
90+ goto unimplemented;
91 }
92
93 g_debug ("Calling method '%s.%s' for ScreenSaver Proxy",
94 interface_name, method_name);
95
96 if (g_strcmp0 (method_name, "Inhibit") == 0) {
97- GVariant *ret;
98 const char *app_id;
99 const char *reason;
100 guint cookie;
101@@ -213,12 +212,12 @@
102 g_variant_get (parameters,
103 "(ss)", &app_id, &reason);
104
105- ret = g_dbus_proxy_call_sync (G_DBUS_PROXY (G_DBUS_PROXY (manager->priv->session)),
106+ ret = g_dbus_proxy_call_sync (G_DBUS_PROXY (manager->priv->session),
107 "Inhibit",
108 g_variant_new ("(susu)",
109 app_id, 0, reason, GSM_INHIBITOR_FLAG_IDLE),
110 G_DBUS_CALL_FLAGS_NONE,
111- -1, NULL, NULL);
112+ -1, NULL, &error);
113 g_variant_get (ret, "(u)", &cookie);
114 g_hash_table_insert (manager->priv->cookie_ht,
115 GUINT_TO_POINTER (cookie),
116@@ -237,7 +236,6 @@
117 g_strdup (sender),
118 GUINT_TO_POINTER (watch_id));
119 }
120- g_dbus_method_invocation_return_value (invocation, ret);
121 } else if (g_strcmp0 (method_name, "UnInhibit") == 0) {
122 guint cookie;
123
124@@ -249,24 +247,41 @@
125 -1, NULL, NULL);
126 g_debug ("Removing cookie %u from the list for %s", cookie, sender);
127 g_hash_table_remove (manager->priv->cookie_ht, GUINT_TO_POINTER (cookie));
128- g_dbus_method_invocation_return_value (invocation, NULL);
129- } else if (g_strcmp0 (method_name, "Throttle") == 0) {
130- g_dbus_method_invocation_return_value (invocation, NULL);
131- } else if (g_strcmp0 (method_name, "UnThrottle") == 0) {
132- g_dbus_method_invocation_return_value (invocation, NULL);
133- } else if (g_strcmp0 (method_name, "Lock") == 0) {
134- goto unimplemented;
135- } else if (g_strcmp0 (method_name, "SimulateUserActivity") == 0) {
136- goto unimplemented;
137- } else if (g_strcmp0 (method_name, "GetActive") == 0) {
138- goto unimplemented;
139- } else if (g_strcmp0 (method_name, "GetActiveTime") == 0) {
140- goto unimplemented;
141+ } else if (g_strcmp0 (method_name, "Lock") == 0 ||
142+ g_strcmp0 (method_name, "SimulateUserActivity") == 0 ||
143+ g_strcmp0 (method_name, "GetActive") == 0 ||
144+ g_strcmp0 (method_name, "GetActiveTime") == 0 ||
145+ g_strcmp0 (method_name, "SetActive") == 0) {
146+
147+ ret = g_dbus_proxy_call_sync (G_DBUS_PROXY (manager->priv->screensaver),
148+ method_name,
149+ parameters,
150+ G_DBUS_CALL_FLAGS_NONE,
151+ -1, NULL, &error);
152+
153+ if (error == NULL && g_strcmp0 (method_name, "SetActive") == 0) {
154+ g_variant_unref (ret);
155+
156+ /* Returning the actual Activate state here is not possible,
157+ * as calling GetActive at this point might return an invalid
158+ * value if the activation process is still ongoing. */
159+ ret = g_variant_ref (parameters);
160+ }
161 } else if (g_strcmp0 (method_name, "GetSessionIdleTime") == 0) {
162- goto unimplemented;
163- } else if (g_strcmp0 (method_name, "SetActive") == 0) {
164- goto unimplemented;
165- }
166+ GsdIdleMonitor *idle_monitor = gsd_idle_monitor_get_core ();
167+ gint64 idle_time_ms = gsd_idle_monitor_get_idletime (idle_monitor);
168+ ret = g_variant_new ("(u)", idle_time_ms / 1000);
169+ } else {
170+ goto unimplemented;
171+ }
172+
173+ if (error != NULL) {
174+ g_dbus_method_invocation_return_gerror (invocation, error);
175+ g_error_free (error);
176+ return;
177+ }
178+
179+ g_dbus_method_invocation_return_value (invocation, ret);
180
181 return;
182
183@@ -354,6 +369,8 @@
184 gnome_settings_profile_start (NULL);
185 manager->priv->session =
186 gnome_settings_bus_get_session_proxy ();
187+ manager->priv->screensaver =
188+ gnome_settings_bus_get_screen_saver_proxy ();
189 manager->priv->watch_ht = g_hash_table_new_full (g_str_hash,
190 g_str_equal,
191 (GDestroyNotify) g_free,
192@@ -371,6 +388,7 @@
193 {
194 g_debug ("Stopping screensaver_proxy manager");
195 g_clear_object (&manager->priv->session);
196+ g_clear_object (&manager->priv->screensaver);
197 g_clear_pointer (&manager->priv->watch_ht, g_hash_table_destroy);
198 g_clear_pointer (&manager->priv->cookie_ht, g_hash_table_destroy);
199 }

Subscribers

People subscribed via source and target branches