Remaining work: printer and users account panels

PKG_CHECK_MODULES(USER_ACCOUNTS_PANEL, $COMMON_MODULES dbus-glib-1
PKG_CHECK_MODULES(PRINTERS_PANEL, $COMMON_MODULES dbus-glib-1

Created attachment 205668 [details] [review]
Port user accounts to using GDBus

I wasn't able to get fprintd to work with my fingerprint scanner so I haven't been able to thoroughly test that part.

Created attachment 205671 [details] [review]
Port printers to using GDBus

For some reason the printers panel is not showing on my build so I can only confirm this compiles - can someone with a working system please test this patch works?

Review of attachment 205668 [details] [review]:

Could you please split the patch into 2, one for the fingerprint bits, one for the rest of the user-accounts panel?

::: panels/user-accounts/um-fingerprint-dialog.c
@@ +71,3 @@
         if (connection == NULL) {
                 g_warning ("Failed to connect to session bus: %s", error->message);
+                g_clear_error (&error);

g_error_free()

@@ +84,3 @@
+                                         NULL);
+        if (manager == NULL) {
+                                         "/net/reactivated/Fprint/Manager",

You don't actually pass error to the call though...

@@ +85,3 @@
+        if (manager == NULL) {
+                g_warning ("Failed to create fingerprint manager proxy: %s", error->message);
+                                         "/net/reactivated/Fprint/Manager",

g_error_free()

@@ +124,3 @@
+                                        NULL);
+        if (device == NULL) {
+                                        device_str,

Again, you're not passing the error.

@@ +125,3 @@
+        if (device == NULL) {
+                g_warning ("Failed to create fingerprint device proxy: %s", error->message);
+                                        "net.reactivated.Fprint.Device",

g_error_free()

@@ +681,1 @@
                 if (g_str_equal (scan_type, "swipe"))

Would prefer g_strcmp0() here, just in case.

::: panels/user-accounts/um-user-manager.c
@@ +345,3 @@
+                                                "org.freedesktop.Accounts",
+                                                NULL,
+                                                NULL);

error checking?

@@ +443,1 @@
                 /* dbus-glib fail:

dbus-glib?

::: panels/user-accounts/um-user.c
@@ +103,1 @@
         error = NULL;

You set it to NULL, and then try to print it. if result == NULL, we'll crash.

@@ +110,1 @@
+        props = g_new0 (UserProperties, 1);

Any chance we could avoid the huge movement of code here?

Robert, could you please move the printers patch into its own bug, with the printers component set? That way I know it gets on Marek's radar.

(In reply to comment #3)
> Created an attachment (id=205671) [details] [review]
> Port printers to using GDBus
> 
> For some reason the printers panel is not showing on my build so I can only
> confirm this compiles - can someone with a working system please test this
> patch works?

That'd be because you're running Unity:
http://git.gnome.org/browse/gnome-control-center/commit/?id=0a8669a5b579d5e4c93a2708a3ebb53efb6da175

(In reply to comment #5)
> Robert, could you please move the printers patch into its own bug, with the
> printers component set? That way I know it gets on Marek's radar.

Just filled bug #669596

Comment on attachment 205671 [details] [review]
Port printers to using GDBus

moved to bug #669596 (thanks Javier!)

Created attachment 207040 [details] [review]
Port to GDBus (excluding fingerprint code)

Split out and with requested changes (sorry for the delay, I wasn't subscribed to the bug for some reason).

Created attachment 207041 [details] [review]
Port fingerprint code to GDBus

Split out as requested.

The was no change to the code movement as requested, as the change is due to not using a callback the g_new needing to be done and it diffs really badly.
The new method is:

static UserProperties *
user_properties_get (GDBusConnection *bus,
                     const gchar     *object_path)
{
        GVariant *result;
        /* ... */
        GError *error = NULL;
                        
        result = g_dbus_connection_call_sync (bus,
                                              "org.freedesktop.Accounts",
                                              object_path,
                                              "org.freedesktop.DBus.Properties",
                                              "GetAll",
                                              g_variant_new ("(s)", "org.freedesktop.Accounts.User"),
                                              G_VARIANT_TYPE ("(a{sv})"),
                                              G_DBUS_CALL_FLAGS_NONE,
                                              -1,
                                              NULL,
                                              &error);
        if (!result) {
                g_debug ("Error calling GetAll() when retrieving properties for %s: %s", object_path, error->message);
                g_error_free (error);
               return NULL;
        }

        props = g_new0 (UserProperties, 1);
        g_variant_get (result, "(a{sv})", &iter);
        while (g_variant_iter_loop (iter, "{&sv}", &key, &value)) {
                if (strcmp (key, "Uid") == 0) {
                        g_variant_get (value, "t", &props->uid);
                }
                /* ... */
                else {
                        g_debug ("unhandled property %s", key);
                }
        }
                
        g_variant_iter_free (iter);
        g_variant_unref (result);
                
        return props;
}

ech, must proof read.  Change that to:
"The was no change to the code movement as requested, as the change is due to gbus not requiring the callback and it diffs really badly."

Review of attachment 207041 [details] [review]:

Could you please also get rid of all the trailing whitespaces (it causes errors when applied with git).

Also:
Could not access 'Digital Persona U.are.U 4000/4000B' device
GDBus.Error:org.freedesktop.DBus.Error.UnknownMethod: Method "Claim" with signature "" on interface "net.reactivated.Fprint.Device" doesn't exist

Indeed, it requires the username being passed:
http://hadess.fedorapeople.org/fprintd/docs/Device.html#Device.Claim

There were a bunch more errors of the same ilk. I've committed the fixes separately, so you can see what the problems were (mostly GVariant type mismatches).

::: panels/user-accounts/um-fingerprint-dialog.c
@@ +85,3 @@
+        if (manager == NULL) {
+                g_warning ("Failed to create fingerprint manager proxy: %s", error->message);
+                                         "/net/reactivated/Fprint/Manager",

g_error_free()

@@ +125,3 @@
+        if (device == NULL) {
+                g_warning ("Failed to create fingerprint device proxy: %s", error->message);
+                                        "net.reactivated.Fprint.Device",

g_error_free()

@@ +515,3 @@
                         msg = g_strdup_printf (_("Could not access '%s' device"), data->name);
+                        d = get_error_dialog (msg, error->message, GTK_WINDOW (data->ass));
+                        g_free_error (error);

g_error_free()