Created attachment 182926 [details] [review]
0003-saver-port-listener-to-GDBus.patch

Review of attachment 182926 [details] [review]:

::: src/gs-listener-dbus.c
@@ +99,3 @@
+    </signal>                                                                           \
+  </interface>                                                                          \
+</node>";

You're putting an enormous amount of irrelevant whitespace into the binary. I think it would be better to do it like this:

static const char interface_xml[] = 
"<node>"
  "<interface name='" GS_INTERFACE "'>"
  ...
"</node>"

thus minimising the extra whitespace.

@@ +146,3 @@
+                        gboolean active;
+
+                        g_variant_get (parameters, "(b)", &active);

You should always check the variant signature first.

Or, if you go with the gdbusproxy, pass the expected interface definition to g_dbus_proxy_new* instead of using NULL which will use the remote one's.

@@ +631,3 @@
+                                                             &error);
+        if (registration_id == 0) {
+                g_critical ("failed to register screensaver listener: %s",

This could only happen if you try to re-register the object without firstt unregistering it... so I think you can just do g_dbus_connection_unregister_object in your name_lost callback to g_bus_own_name_on_connection and possibly also the session_bus_disconnected callback.

@@ +767,3 @@
+gs_listener_acquire (GSListener  *listener,
+                     GError     **error)
+{

The logic here seems off. You're first checking if the name already exists on the bus, then register your object and try to own the name.

Instead, just register your object, then g_bus_own_name_on_connection, using non-NULL name_acquired/lost handlers. In the first handler, you know you own the name now; while in the second case there was already an owner.

@@ +794,3 @@
+                                     _("not connected to the message bus"));
+                }
+

Unnecessary.

@@ +820,2 @@
                                             listener,
                                             NULL);

Will this really happen, ever?

Anyway, use g_dbus_connection_set_exit_on_close and connect to the "closed" signal instead.

@@ +841,3 @@
+                                                    on_system_bus_disconnected,
+                                                    listener,
+                                                    NULL);

What's the point here? Disconnecting from the system bus isn't really going to happen, ever.

If it did, you could just g_dbus_connection_set_exit_on_close (priv->system_bus, FALSE), and connect to the "closed" signal on it.

@@ +877,1 @@
         }

I think it would be massively easier to use a GDBusProxy for the CK service instead of adding all these signals by hand.

Created attachment 183044 [details] [review]
0003-saver-port-listener-to-GDBus.patch

Yeah, good point about bloating the .rodata section, fixed that.  As well as the additional safety around the g_variant_gets.  Fixed up the registration bit as well (though left the error checking in place).

I agree with you that GDBusProxy would simplify a lot of this logic, but Id rather do that as a follow up change and keep this a more straight-forward equivalent translation.

*** This bug has been marked as a duplicate of bug 622880 ***