Created attachment 240647 [details] [review]
simple-slave: Be smarter about launching initial-setup

Review of attachment 240647 [details] [review]:

::: daemon/gdm-simple-slave.c
@@ +1170,3 @@
+        ActUserManager *user_manager = act_user_manager_get_default ();
+        gboolean enabled = FALSE;
+        GSList *users;

need to initialize this to NULL

@@ +1180,3 @@
         }
 
+        users = act_user_manager_list_users (user_manager);

can't call list_users until the manager is loaded, do you know that's true at this point?

@@ +1181,3 @@
 
+        users = act_user_manager_list_users (user_manager);
+        if (g_slist_length (users) > 0) {

users != NULL is going to be cheaper if there are a list of users.

@@ +1220,3 @@
+        } else {
+                g_signal_connect_swapped (user_manager, "notify::is-loaded",
+                                          G_CALLBACK (setup_session), slave);

there may be a weird race here where the slave goes down before the manager object is loaded, leading to crash on exit.

@@ +1234,3 @@
         if (res) {
                 setup_server (slave);
+                wait_until_user_manager_loaded (slave);

ah okay you're good.

Review of attachment 240647 [details] [review]:

This looks right to me, but I'd like if Ray reviewed too.

Review of attachment 240647 [details] [review]:

::: daemon/gdm-simple-slave.c
@@ +1180,2 @@
         }
 

Might be nice to avoid getting the user manager if we're going out anyway?
Ie defer the act_ser_manager_get_default call until here

@@ +1207,3 @@
+
+static void
+wait_until_user_manager_loaded (GdmSimpleSlave *slave)

Confusing name - this function does not wait

Created attachment 240793 [details] [review]
simple-slave: Be smarter about launching initial-setup

Instead of using a trigger file, detect when the system has
0 users.

Review of attachment 240793 [details] [review]:

::: daemon/gdm-simple-slave.c
@@ +1212,3 @@
+        g_signal_handlers_disconnect_by_func (user_manager,
+                                              user_manager_loaded,
+                                              slave);

Should probably check that it's actually loaded and not just getting called with a spurious is-loaded == FALSE notification.

@@ +1501,3 @@
         }
 
+        g_signal_handlers_disconnect_by_func (user_manager,

You're missing a

ActUserManager *user_manager = act_user_manager_get_default ();

somewhere i think.

Created attachment 240973 [details] [review]
simple-slave: Be smarter about launching initial-setup

Instead of using a trigger file, detect when the system has
0 users.

Created attachment 240978 [details] [review]
simple-slave: Be smarter about launching initial-setup

Instead of using a trigger file, detect when the system has
0 users.

Review of attachment 240978 [details] [review]:

seems to work okay in brief testing.

Attachment 240978 [details] pushed as fdc017e - simple-slave: Be smarter about launching initial-setup

Hmm, I'm getting initial-setup when I shouldn't be.

See https://bugzilla.gnome.org/show_bug.cgi?id=697594

so, this should be re-closed then, I assume ?