clearing house: the remainder of bugfixes & readability changes

10 files changed, 441 insertions, 422 deletions

diff --git a/src/Makefile.am b/src/Makefile.am
index 24f2ee8..7bc6306 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -20,7 +20,7 @@ sessionlib_LTLIBRARIES = libsession.la
 libsession_la_SOURCES = \
 	indicator-session.c \
 	gen-session-dbus.xml.h \
-	dbus-shared-names.h \
+	shared-names.h \
 	user-widget.c \
 	user-widget.h
 libsession_la_CFLAGS = \
@@ -130,7 +130,6 @@ indicator_session_service_SOURCES = \
 	session-dbus.c \
 	session-dbus.h \
 	gen-session-dbus.xml.c \
-	settings-helper.c \
 	users-service-dbus.h \
 	users-service-dbus.c \
 	session-menu-mgr.h \
@@ -157,8 +156,6 @@ if BUILD_GTKLOGOUTHELPER
 gtk_logout_helper_SOURCES = \
 	$(dbus_consolekit_manager_sources) \
 	gtk-logout-helper.c \
-	settings-helper.c \
-	settings-helper.h \
 	dialog.c \
 	dialog.h
 
diff --git a/src/gtk-logout-helper.c b/src/gtk-logout-helper.c
index 360dd0f..b311701 100644
--- a/src/gtk-logout-helper.c
+++ b/src/gtk-logout-helper.c
@@ -8,16 +8,16 @@ Authors:
     Ted Gould <ted@canonical.com>
     Christoph Korn <c_korn@gmx.de>
 
-This program is free software: you can redistribute it and/or modify it 
-under the terms of the GNU General Public License version 3, as published 
+This program is free software: you can redistribute it and/or modify it
+under the terms of the GNU General Public License version 3, as published
 by the Free Software Foundation.
 
-This program is distributed in the hope that it will be useful, but 
-WITHOUT ANY WARRANTY; without even the implied warranties of 
-MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR 
+This program is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranties of
+MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
 PURPOSE.  See the GNU General Public License for more details.
 
-You should have received a copy of the GNU General Public License along 
+You should have received a copy of the GNU General Public License along
 with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
@@ -28,7 +28,7 @@ with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include <dbus/dbus-glib.h>
 #include <gtk/gtk.h>
 #include "dialog.h"
-#include "settings-helper.h"
+#include "shared-names.h"
 
 static void
 consolekit_fallback (LogoutDialogType action)
@@ -91,7 +91,7 @@ session_action (LogoutDialogType action)
 	GError * error = NULL;
 	gboolean res = FALSE;
 	
-	sbus = dbus_g_bus_get(DBUS_BUS_SESSION, NULL); 
+	sbus = dbus_g_bus_get(DBUS_BUS_SESSION, NULL);
 	if (sbus == NULL) {
 		g_warning("Unable to get DBus session bus.");
 		return;
@@ -113,15 +113,15 @@ session_action (LogoutDialogType action)
 	
 	if (action == LOGOUT_DIALOG_TYPE_LOG_OUT) {
 		g_debug("Asking Session manager to 'Logout'");
-		res = dbus_g_proxy_call_with_timeout (sm_proxy, "Logout", INT_MAX, &error, 
+		res = dbus_g_proxy_call_with_timeout (sm_proxy, "Logout", INT_MAX, &error,
 											  G_TYPE_UINT, 1, G_TYPE_INVALID, G_TYPE_INVALID);
 	} else if (action == LOGOUT_DIALOG_TYPE_SHUTDOWN) {
 		g_debug("Asking Session manager to 'RequestShutdown'");
-		res = dbus_g_proxy_call_with_timeout (sm_proxy, "RequestShutdown", INT_MAX, &error, 
+		res = dbus_g_proxy_call_with_timeout (sm_proxy, "RequestShutdown", INT_MAX, &error,
 											  G_TYPE_INVALID, G_TYPE_INVALID);
 	} else if (action == LOGOUT_DIALOG_TYPE_RESTART) {
 		g_debug("Asking Session manager to 'RequestReboot'");
-		res = dbus_g_proxy_call_with_timeout (sm_proxy, "RequestReboot", INT_MAX, &error, 
+		res = dbus_g_proxy_call_with_timeout (sm_proxy, "RequestReboot", INT_MAX, &error,
 											  G_TYPE_INVALID, G_TYPE_INVALID);
 	} else {
 		g_warning ("Unknown session action");
@@ -177,6 +177,17 @@ static GOptionEntry options[] = {
 	{NULL}
 };
 
+static gboolean
+suppress_confirmations (void)
+{
+  GSettings * s = g_settings_new (SESSION_SCHEMA);
+  const gboolean suppress = g_settings_get_boolean (s, SUPPRESS_KEY);
+  g_clear_object (&s);
+  return suppress;
+}
+
+
+
 int
 main (int argc, char * argv[])
 {
@@ -205,7 +216,7 @@ main (int argc, char * argv[])
 	                                  INDICATOR_ICONS_DIR);
 
 	GtkWidget * dialog = NULL;
-	if (!supress_confirmations()) {
+	if (!suppress_confirmations()) {
 		g_debug("Showing dialog to ask for user confirmation");
 		dialog = GTK_WIDGET(logout_dialog_new(type));
 	}
diff --git a/src/indicator-session.c b/src/indicator-session.c
index d6c2599..53ff87e 100644
--- a/src/indicator-session.c
+++ b/src/indicator-session.c
@@ -1,23 +1,23 @@
 /*
 A small wrapper utility to load indicators and put them as menu items
-into the gnome-panel using it's applet interface.
+into the gnome-panel using its applet interface.
 
 Copyright 2009 Canonical Ltd.
 
 Authors:
     Ted Gould <ted@canonical.com>
     Conor Curran <conor.curran@canonical.com>
-    
-This program is free software: you can redistribute it and/or modify it 
-under the terms of the GNU General Public License version 3, as published 
+
+This program is free software: you can redistribute it and/or modify it
+under the terms of the GNU General Public License version 3, as published
 by the Free Software Foundation.
 
-This program is distributed in the hope that it will be useful, but 
-WITHOUT ANY WARRANTY; without even the implied warranties of 
-MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR 
+This program is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranties of
+MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
 PURPOSE.  See the GNU General Public License for more details.
 
-You should have received a copy of the GNU General Public License along 
+You should have received a copy of the GNU General Public License along
 with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
@@ -38,7 +38,7 @@ with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include <libindicator/indicator-service-manager.h>
 #include <libindicator/indicator-image-helper.h>
 
-#include "dbus-shared-names.h"
+#include "shared-names.h"
 #include "user-widget.h"
 
 #define INDICATOR_SESSION_TYPE            (indicator_session_get_type ())
@@ -97,10 +97,10 @@ static void indicator_session_finalize   (GObject *object);
 static GList* indicator_session_get_entries (IndicatorObject* obj);
 static guint indicator_session_get_location (IndicatorObject * io,
                                              IndicatorObjectEntry * entry);
-                                             
+
 G_DEFINE_TYPE (IndicatorSession, indicator_session, INDICATOR_OBJECT_TYPE);
 
-static void 
+static void
 indicator_session_class_init (IndicatorSessionClass *klass)
 {
 	GObjectClass *object_class = G_OBJECT_CLASS (klass);
@@ -139,12 +139,12 @@ indicator_session_init (IndicatorSession *self)
   g_settings_bind (self->settings, "show-real-name-on-panel",
                    self->entry.label, "visible",
                    G_SETTINGS_BIND_GET);
-  
+
   gtk_widget_show (GTK_WIDGET(self->entry.menu));
   gtk_widget_show (GTK_WIDGET(self->entry.image));
   g_object_ref_sink (self->entry.menu);
   g_object_ref_sink (self->entry.image);
-  
+
   // set up the handlers
   DbusmenuClient * menu_client = DBUSMENU_CLIENT(dbusmenu_gtkmenu_get_client(DBUSMENU_GTKMENU(self->entry.menu)));
   dbusmenu_client_add_type_handler (menu_client,
@@ -198,7 +198,7 @@ static guint
 indicator_session_get_location (IndicatorObject * io,
                                 IndicatorObjectEntry * entry)
 {
-  return 0;  
+  return 0;
 }
 
 /* callback for the service manager state of being */
@@ -218,10 +218,10 @@ service_connection_cb (IndicatorServiceManager * sm, gboolean connected, gpointe
                          -1,
                          NULL,
                          user_real_name_get_cb,
-                         user_data);      
+                         user_data);
       return;
     }
-    
+
 	  self->service_proxy_cancel = g_cancellable_new();
 	  g_dbus_proxy_new_for_bus (G_BUS_TYPE_SESSION,
                               G_DBUS_PROXY_FLAGS_NONE,
@@ -232,7 +232,7 @@ service_connection_cb (IndicatorServiceManager * sm, gboolean connected, gpointe
                               self->service_proxy_cancel,
                               service_proxy_cb,
                               self);
-  }    
+  }
 	return;
 }
 
@@ -260,7 +260,7 @@ service_proxy_cb (GObject * object, GAsyncResult * res, gpointer user_data)
 	self->service_proxy = proxy;
 
 	g_signal_connect(proxy, "g-signal", G_CALLBACK(receive_signal), self);
-  
+
   // Fetch the user's real name for the user entry label
   g_dbus_proxy_call (self->service_proxy,
                      "GetUserRealName",
@@ -277,17 +277,13 @@ service_proxy_cb (GObject * object, GAsyncResult * res, gpointer user_data)
 static gboolean
 new_user_item (DbusmenuMenuitem * newitem,
                DbusmenuMenuitem * parent,
-               DbusmenuClient * client,
-               gpointer user_data)
+               DbusmenuClient   * client,
+               gpointer           user_data)
 {
-  
-
-  GtkWidget* user_item = NULL;
-
-  g_return_val_if_fail(DBUSMENU_IS_MENUITEM(newitem), FALSE);
-  g_return_val_if_fail(DBUSMENU_IS_GTKCLIENT(client), FALSE);
+  g_return_val_if_fail (DBUSMENU_IS_MENUITEM(newitem), FALSE);
+  g_return_val_if_fail (DBUSMENU_IS_GTKCLIENT(client), FALSE);
 
-  user_item = user_widget_new(newitem);
+  GtkWidget * user_item = user_widget_new (newitem);
 
   GtkMenuItem *user_widget = GTK_MENU_ITEM(user_item);
 
@@ -308,22 +304,23 @@ new_user_item (DbusmenuMenuitem * newitem,
 static void
 user_real_name_get_cb (GObject * obj, GAsyncResult * res, gpointer user_data)
 {
-	IndicatorSession * self = INDICATOR_SESSION(user_data);
-	GError * error = NULL;
-	GVariant * result;
+  IndicatorSession * self = INDICATOR_SESSION(user_data);
 
-	result = g_dbus_proxy_call_finish(self->service_proxy, res, &error);
+  GError * error = NULL;
+  GVariant * result = g_dbus_proxy_call_finish(self->service_proxy, res, &error);
 
-	if (error != NULL) {
-    g_warning ("unable to complete real name dbus query");
-    g_error_free (error);
-		return;
-	}
-  
-  const gchar* username = NULL;
-  g_variant_get (result, "(&s)", &username);
-  indicator_session_update_users_label (self, username);
-	return;
+  if (error != NULL)
+    {
+      g_warning ("Unable to complete real name dbus query: %s", error->message);
+      g_clear_error (&error);
+    }
+  else
+    {
+      const gchar * username = NULL;
+      g_variant_get (result, "(&s)", &username);
+      indicator_session_update_users_label (self, username);
+      g_variant_unref (result);
+    }
 }
 
 /* Receives all signals from the service, routed to the appropriate functions */
@@ -409,8 +406,8 @@ build_restart_item (DbusmenuMenuitem * newitem,
 }
 
 static void
-indicator_session_update_users_label (IndicatorSession * self, 
+indicator_session_update_users_label (IndicatorSession * self,
                                       const gchar      * name)
-{  
+{
   gtk_label_set_text (self->entry.label, name ? name : "");
 }
diff --git a/src/session-dbus.c b/src/session-dbus.c
index 4b9d45f..4ece444 100644
--- a/src/session-dbus.c
+++ b/src/session-dbus.c
@@ -235,11 +235,8 @@ session_dbus_set_users_real_name (SessionDbus * session, const gchar * name)
 {
 	SessionDbusPrivate * priv = SESSION_DBUS_GET_PRIVATE(session);
 	GError * error = NULL;
-	if (priv->name != NULL) {
-		g_free(priv->name);
-		priv->name = NULL;
-	}
 
+	g_free (priv->name);
 	priv->name = g_strdup(name);
 
 	if (priv->bus != NULL) {
diff --git a/src/session-menu-mgr.c b/src/session-menu-mgr.c
index c6de0b1..752a6a6 100644
--- a/src/session-menu-mgr.c
+++ b/src/session-menu-mgr.c
@@ -2,26 +2,26 @@
 Copyright 2011 Canonical Ltd.
 
 Authors:
-    Conor Curran <conor.curran@canonical.com>
     Charles Kerr <charles.kerr@canonical.com>
+    Conor Curran <conor.curran@canonical.com>
 
-This program is free software: you can redistribute it and/or modify it 
-under the terms of the GNU General Public License version 3, as published 
+This program is free software: you can redistribute it and/or modify it
+under the terms of the GNU General Public License version 3, as published
 by the Free Software Foundation.
 
-This program is distributed in the hope that it will be useful, but 
-WITHOUT ANY WARRANTY; without even the implied warranties of 
-MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR 
+This program is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranties of
+MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
 PURPOSE.  See the GNU General Public License for more details.
 
-You should have received a copy of the GNU General Public License along 
+You should have received a copy of the GNU General Public License along
 with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
 #include <config.h>
 
 #include <sys/types.h>
-#include <pwd.h>
+#include <pwd.h> /* geteuid(), getpwuid() */
 
 #include <glib.h>
 #include <glib/gi18n.h>
@@ -29,11 +29,13 @@ with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include <libdbusmenu-glib/client.h>
 #include <libdbusmenu-gtk/menuitem.h>
 
-#include "dbus-shared-names.h"
 #include "dbus-upower.h"
 #include "session-menu-mgr.h"
+#include "shared-names.h"
 #include "users-service-dbus.h"
 
+#define DEBUG_SHOW_ALL FALSE
+
 #define UPOWER_ADDRESS    "org.freedesktop.UPower"
 #define UPOWER_PATH       "/org/freedesktop/UPower"
 
@@ -41,13 +43,15 @@ with this program.  If not, see <http://www.gnu.org/licenses/>.
 #define CMD_INFO            "gnome-control-center info"
 #define CMD_SYSTEM_SETTINGS "gnome-control-center"
 #ifdef HAVE_GTKLOGOUTHELPER
+ #define HAVE_RESTART_CMD TRUE
+ #define CMD_RESTART  LIBEXECDIR"/gtk-logout-helper --restart"
  #define CMD_LOGOUT   LIBEXECDIR"/gtk-logout-helper --logout"
  #define CMD_SHUTDOWN LIBEXECDIR"/gtk-logout-helper --shutdown"
- #define CMD_RESTART  LIBEXECDIR"/gtk-logout-helper --restart"
 #else
+ #define HAVE_RESTART_CMD FALSE /* hmm, no gnome-session-quit --restart? */
+ #define CMD_RESTART  ""
  #define CMD_LOGOUT   "gnome-session-quit --logout"
  #define CMD_SHUTDOWN "gnome-session-quit --power-off"
- #define CMD_RESTART  CMD_SHUTDOWN /* hmm, no gnome-session-quit --restart? */
 #endif
 
 /**
@@ -64,18 +68,23 @@ typedef enum
 }
 SwitcherMode;
 
-typedef struct
-{
-  SessionMenuMgr * mgr;
-  AccountsUser * user;
-}
-ActivateUserSessionData;
-
+/**
+ * Creates and manages the menumodel and associated actions for the
+ * session menu described at <https://wiki.ubuntu.com/SystemMenu>.
+ *
+ * This is a pretty straightforward class: it creates the menumodel
+ * and listens for events that can affect the model's properties.
+ *
+ * Simple event sources, such as GSettings and a UPower DBus proxy,
+ * are handled here. More involved event sources are delegated to
+ * UsersServiceDBus facade class.
+ */
 struct _SessionMenuMgr
 {
   GObject parent_instance;
 
   DbusmenuMenuitem * parent_mi;
+  DbusmenuMenuitem * screensaver_mi;
   DbusmenuMenuitem * lock_mi;
   DbusmenuMenuitem * lock_switch_mi;
   DbusmenuMenuitem * guest_mi;
@@ -92,19 +101,22 @@ struct _SessionMenuMgr
   GSettings * indicator_settings;
   GSettings * keybinding_settings;
 
+  /* cached settings taken from the upower proxy */
   gboolean can_hibernate;
   gboolean can_suspend;
   gboolean allow_hibernate;
   gboolean allow_suspend;
+
   gboolean greeter_mode;
 
   GCancellable * cancellable;
   DBusUPower * upower_proxy;
-  SessionDbus * session_dbus;  
+  SessionDbus * session_dbus;
   UsersServiceDbus * users_dbus_facade;
 };
 
 static SwitcherMode get_switcher_mode         (SessionMenuMgr *);
+
 static void init_upower_proxy                 (SessionMenuMgr *);
 
 static void update_screensaver_shortcut       (SessionMenuMgr *);
@@ -118,21 +130,21 @@ static void action_func_hibernate             (SessionMenuMgr *);
 static void action_func_switch_to_lockscreen  (SessionMenuMgr *);
 static void action_func_switch_to_greeter     (SessionMenuMgr *);
 static void action_func_switch_to_guest       (SessionMenuMgr *);
-static void action_func_switch_to_user        (ActivateUserSessionData *);
+static void action_func_switch_to_user        (AccountsUser   *);
 static void action_func_spawn_async           (const char * fmt, ...);
 
-static gboolean is_this_guest_session (void);
-static gboolean is_this_live_session (void);
+static gboolean is_this_guest_session         (void);
+static gboolean is_this_live_session          (void);
 
-static void on_guest_logged_in_changed (UsersServiceDbus *,
-                                        SessionMenuMgr   *);
+static void on_guest_logged_in_changed        (UsersServiceDbus *,
+                                               SessionMenuMgr   *);
 
-static void on_user_logged_in_changed (UsersServiceDbus *,
-                                       AccountsUser     *,
-                                       SessionMenuMgr  *);
+static void on_user_logged_in_changed         (UsersServiceDbus *,
+                                               AccountsUser     *,
+                                               SessionMenuMgr   *);
 
 /**
-***  GObject init / dispose / finalize
+***  GObject init / dispose
 **/
 
 G_DEFINE_TYPE (SessionMenuMgr, session_menu_mgr, G_TYPE_OBJECT);
@@ -140,14 +152,9 @@ G_DEFINE_TYPE (SessionMenuMgr, session_menu_mgr, G_TYPE_OBJECT);
 static void
 session_menu_mgr_init (SessionMenuMgr *mgr)
 {
-  mgr->can_hibernate = TRUE;
-  mgr->can_suspend = TRUE;
-  mgr->allow_hibernate = TRUE;
-  mgr->allow_suspend = TRUE;
-
   /* Lockdown settings */
   GSettings * s = g_settings_new ("org.gnome.desktop.lockdown");
-  g_signal_connect_swapped (s, "changed",
+  g_signal_connect_swapped (s, "changed::disable-log-out",
                             G_CALLBACK(update_session_menuitems), mgr);
   g_signal_connect_swapped (s, "changed::disable-lock-screen",
                             G_CALLBACK(update_user_menuitems), mgr);
@@ -159,7 +166,12 @@ session_menu_mgr_init (SessionMenuMgr *mgr)
   s = g_settings_new ("com.canonical.indicator.session");
   g_signal_connect_swapped (s, "changed::suppress-logout-restart-shutdown",
                             G_CALLBACK(update_confirmation_labels), mgr);
-  g_signal_connect (s, "changed", G_CALLBACK(update_session_menuitems), mgr);
+  g_signal_connect_swapped (s, "changed::suppress-logout-menuitem",
+                            G_CALLBACK(update_session_menuitems), mgr);
+  g_signal_connect_swapped (s, "changed::suppress-restart-menuitem",
+                            G_CALLBACK(update_session_menuitems), mgr);
+  g_signal_connect_swapped (s, "changed::suppress-shutdown-menuitem",
+                            G_CALLBACK(update_session_menuitems), mgr);
   mgr->indicator_settings = s;
 
   /* Keybinding settings */
@@ -168,7 +180,7 @@ session_menu_mgr_init (SessionMenuMgr *mgr)
                             G_CALLBACK(update_screensaver_shortcut), mgr);
   mgr->keybinding_settings = s;
 
-  /* listen for users who appear or log in or log out */
+  /* listen for user events */
   mgr->users_dbus_facade = g_object_new (USERS_SERVICE_DBUS_TYPE, NULL);
   g_signal_connect_swapped (mgr->users_dbus_facade, "user-list-changed",
                             G_CALLBACK (update_user_menuitems), mgr);
@@ -177,7 +189,7 @@ session_menu_mgr_init (SessionMenuMgr *mgr)
   g_signal_connect (mgr->users_dbus_facade, "guest-logged-in-changed",
                     G_CALLBACK(on_guest_logged_in_changed), mgr);
 
-  init_upower_proxy (mgr);  
+  init_upower_proxy (mgr);
 }
 
 static void
@@ -197,13 +209,7 @@ session_menu_mgr_dispose (GObject *object)
   g_clear_object (&mgr->upower_proxy);
   g_clear_object (&mgr->users_dbus_facade);
 
-  G_OBJECT_CLASS (session_menu_mgr_parent_class)->finalize (object);
-}
-
-static void
-session_menu_mgr_finalize (GObject *object)
-{
-  G_OBJECT_CLASS (session_menu_mgr_parent_class)->finalize (object);
+  G_OBJECT_CLASS (session_menu_mgr_parent_class)->dispose (object);
 }
 
 static void
@@ -211,61 +217,16 @@ session_menu_mgr_class_init (SessionMenuMgrClass *klass)
 {
   GObjectClass* object_class = G_OBJECT_CLASS (klass);
   object_class->dispose = session_menu_mgr_dispose;
-  object_class->finalize = session_menu_mgr_finalize;
-}
-
-/***
-****  Menuitem Helpers
-***/
-
-static inline void
-mi_set_label (DbusmenuMenuitem * mi, const char * str)
-{
-  dbusmenu_menuitem_property_set (mi, DBUSMENU_MENUITEM_PROP_LABEL, str);
-}
-
-static inline void
-mi_set_type (DbusmenuMenuitem * mi, const char * str)
-{
-  dbusmenu_menuitem_property_set (mi, DBUSMENU_MENUITEM_PROP_TYPE, str);
-}
-
-static inline void
-mi_set_visible (DbusmenuMenuitem * mi, gboolean b)
-{
-  dbusmenu_menuitem_property_set_bool (mi, DBUSMENU_MENUITEM_PROP_VISIBLE, b);
-}
-
-static inline void
-mi_set_logged_in (DbusmenuMenuitem * mi, gboolean b)
-{
-  dbusmenu_menuitem_property_set_bool (mi, USER_ITEM_PROP_LOGGED_IN, b);
-}
-
-static DbusmenuMenuitem*
-mi_new_separator (void)
-{
-  DbusmenuMenuitem * mi = dbusmenu_menuitem_new ();
-  mi_set_type (mi, DBUSMENU_CLIENT_TYPES_SEPARATOR);
-  return mi;
-}
-
-static DbusmenuMenuitem*
-mi_new (const char * label)
-{
-  DbusmenuMenuitem * mi = dbusmenu_menuitem_new ();
-  mi_set_label (mi, label);
-  return mi;
 }
 
 /***
 ****  UPower Proxy:
 ****
 ****  1. While bootstrapping, we invoke the AllowSuspend and AllowHibernate
-****     methods to find out whether or not those functions are allowed.
+****     methods to find out whether or not those features are allowed.
 ****  2. While bootstrapping, we get the CanSuspend and CanHibernate properties
 ****     and also listen for property changes.
-****  3. These four values are used to set suspend and hibernate's visibility
+****  3. These four values are used to set suspend and hibernate's visibility.
 ****
 ***/
 
@@ -273,14 +234,14 @@ static void
 on_upower_properties_changed (SessionMenuMgr * mgr)
 {
   gboolean b;
-  gboolean refresh = FALSE;
+  gboolean need_refresh = FALSE;
 
   /* suspend */
   b = dbus_upower_get_can_suspend (mgr->upower_proxy);
   if (mgr->can_suspend != b)
     {
       mgr->can_suspend = b;
-      refresh = TRUE;
+      need_refresh = TRUE;
     }
 
   /* hibernate */
@@ -288,10 +249,10 @@ on_upower_properties_changed (SessionMenuMgr * mgr)
   if (mgr->can_hibernate != b)
     {
       mgr->can_hibernate = b;
-      refresh = TRUE;
+      need_refresh = TRUE;
     }
 
-  if (refresh)
+  if (need_refresh)
     {
       update_session_menuitems (mgr);
     }
@@ -300,6 +261,12 @@ on_upower_properties_changed (SessionMenuMgr * mgr)
 static void
 init_upower_proxy (SessionMenuMgr * mgr)
 {
+  /* default values */
+  mgr->can_suspend = TRUE;
+  mgr->can_hibernate = TRUE;
+  mgr->allow_suspend = TRUE;
+  mgr->allow_hibernate = TRUE;
+
   mgr->cancellable = g_cancellable_new ();
 
   GError * error = NULL;
@@ -344,44 +311,92 @@ init_upower_proxy (SessionMenuMgr * mgr)
 }
 
 /***
-****  Admin Menuitems
+****  Menuitem Helpers
 ***/
 
-#define DEBUG_SHOW_ALL 1
+static inline void
+mi_set_label (DbusmenuMenuitem * mi, const char * str)
+{
+  dbusmenu_menuitem_property_set (mi, DBUSMENU_MENUITEM_PROP_LABEL, str);
+}
 
-static void
-build_admin_menuitems (SessionMenuMgr * mgr)
+static inline void
+mi_set_type (DbusmenuMenuitem * mi, const char * str)
 {
-  DbusmenuMenuitem * mi;
-  const gboolean show_settings = DEBUG_SHOW_ALL || !mgr->greeter_mode;
+  dbusmenu_menuitem_property_set (mi, DBUSMENU_MENUITEM_PROP_TYPE, str);
+}
 
-  mi = mi_new (_("About This Computer"));
-  dbusmenu_menuitem_child_append (mgr->parent_mi, mi);
-  g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
-                            G_CALLBACK(action_func_spawn_async), CMD_INFO);
+static inline void
+mi_set_visible (DbusmenuMenuitem * mi, gboolean b)
+{
+  dbusmenu_menuitem_property_set_bool (mi, DBUSMENU_MENUITEM_PROP_VISIBLE,
+                                       b || DEBUG_SHOW_ALL);
+}
 
-  mi = mi_new (_("Ubuntu Help"));
-  dbusmenu_menuitem_child_append (mgr->parent_mi, mi);
-  g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
-                            G_CALLBACK(action_func_spawn_async), CMD_HELP);
+static inline void
+mi_set_logged_in (DbusmenuMenuitem * mi, gboolean b)
+{
+  dbusmenu_menuitem_property_set_bool (mi, USER_ITEM_PROP_LOGGED_IN, b);
+}
 
-  mi = mi_new_separator ();
-  mi_set_visible (mi, show_settings);
-  dbusmenu_menuitem_child_append (mgr->parent_mi, mi);
+static DbusmenuMenuitem*
+mi_new_separator (void)
+{
+  DbusmenuMenuitem * mi = dbusmenu_menuitem_new ();
+  mi_set_type (mi, DBUSMENU_CLIENT_TYPES_SEPARATOR);
+  return mi;
+}
 
-  mi = mi_new (_("System Settings\342\200\246"));
-  mi_set_visible (mi, show_settings);
-  dbusmenu_menuitem_child_append (mgr->parent_mi, mi);
-  g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
-                            G_CALLBACK(action_func_spawn_async),
-                            CMD_SYSTEM_SETTINGS);
+static DbusmenuMenuitem*
+mi_new (const char * label)
+{
+  DbusmenuMenuitem * mi = dbusmenu_menuitem_new ();
+  mi_set_label (mi, label);
+  return mi;
+}
 
-  mi = mi_new_separator ();
-  dbusmenu_menuitem_child_append (mgr->parent_mi, mi);
+/***
+****  Admin Menuitems
+****  <https://wiki.ubuntu.com/SystemMenu#Admin_items>
+***/
+
+static void
+build_admin_menuitems (SessionMenuMgr * mgr)
+{
+  if (!mgr->greeter_mode)
+  {
+    DbusmenuMenuitem * mi;
+    const gboolean show_settings = !mgr->greeter_mode;
+
+    mi = mi_new (_("About This Computer"));
+    dbusmenu_menuitem_child_append (mgr->parent_mi, mi);
+    g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
+                              G_CALLBACK(action_func_spawn_async), CMD_INFO);
+
+    mi = mi_new (_("Ubuntu Help"));
+    dbusmenu_menuitem_child_append (mgr->parent_mi, mi);
+    g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
+                              G_CALLBACK(action_func_spawn_async), CMD_HELP);
+
+    mi = mi_new_separator ();
+    mi_set_visible (mi, show_settings);
+    dbusmenu_menuitem_child_append (mgr->parent_mi, mi);
+
+    mi = mi_new (_("System Settings"));
+    mi_set_visible (mi, show_settings);
+    dbusmenu_menuitem_child_append (mgr->parent_mi, mi);
+    g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
+                              G_CALLBACK(action_func_spawn_async),
+                              CMD_SYSTEM_SETTINGS);
+
+    mi = mi_new_separator ();
+    dbusmenu_menuitem_child_append (mgr->parent_mi, mi);
+  }
 }
 
 /***
 ****  Session Menuitems
+****  <https://wiki.ubuntu.com/SystemMenu#Session_items>
 ***/
 
 static void
@@ -392,7 +407,7 @@ update_session_menuitems (SessionMenuMgr * mgr)
 
   v = !mgr->greeter_mode
    && !is_this_live_session()
-   && !g_settings_get_boolean (mgr->lockdown_settings, "disable-logout")
+   && !g_settings_get_boolean (mgr->lockdown_settings, "disable-log-out")
    && !g_settings_get_boolean (s, "suppress-logout-menuitem");
   mi_set_visible (mgr->logout_mi, v);
 
@@ -404,15 +419,20 @@ update_session_menuitems (SessionMenuMgr * mgr)
    && mgr->allow_hibernate;
   mi_set_visible (mgr->hibernate_mi, v);
 
-  v = !g_settings_get_boolean (s, "suppress-restart-menuitem");
+  v = HAVE_RESTART_CMD
+   && !g_settings_get_boolean (s, "suppress-restart-menuitem");
   mi_set_visible (mgr->restart_mi, v);
 
   v = !g_settings_get_boolean (s, "suppress-shutdown-menuitem");
   mi_set_visible (mgr->shutdown_mi, v);
 }
 
-/* if confirmation is enabled,
-   add ellipsis to the labels of items whose actions need confirmation */
+/* Update the ellipses when the confirmation setting changes.
+ *
+ * <http://developer.gnome.org/hig-book/3.0/menus-design.html.en>:
+ * "Label the menu item with a trailing ellipsis ("...") only if the
+ * command requires further input from the user before it can be performed."
+ */
 static void
 update_confirmation_labels (SessionMenuMgr * mgr)
 {
@@ -436,9 +456,6 @@ build_session_menuitems (SessionMenuMgr* mgr)
 {
   DbusmenuMenuitem * mi;
 
-  mi = mi_new_separator ();
-  dbusmenu_menuitem_child_append (mgr->parent_mi, mi);
-
   mi = mgr->logout_mi = mi_new (_("Log Out\342\200\246"));
   dbusmenu_menuitem_child_append (mgr->parent_mi, mi);
   g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
@@ -454,24 +471,28 @@ build_session_menuitems (SessionMenuMgr* mgr)
   g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
                             G_CALLBACK(action_func_hibernate), mgr);
 
-  mi = mgr->restart_mi = mi_new (_("Restart\342\200\246"));
+  mi = mgr->restart_mi = dbusmenu_menuitem_new ();
+  mi_set_type (mi, RESTART_ITEM_TYPE);
+  dbusmenu_menuitem_property_set (mi, RESTART_ITEM_LABEL, _("Restart\342\200\246"));
   dbusmenu_menuitem_child_append (mgr->parent_mi, mi);
   g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
                             G_CALLBACK(action_func_spawn_async), CMD_RESTART);
- 
+
   mi = mgr->shutdown_mi = mi_new (_("Switch Off\342\200\246"));
   dbusmenu_menuitem_child_append (mgr->parent_mi, mi);
   g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
                             G_CALLBACK(action_func_spawn_async), CMD_SHUTDOWN);
 
   update_confirmation_labels (mgr);
+  update_session_menuitems (mgr);
 }
 
 /****
 *****  User Menuitems
+*****  https://wiki.ubuntu.com/SystemMenu#Account-switching_items
 ****/
 
-/* Local Extensions to AccountsUser */
+/* Local extensions to AccountsUser */
 
 static GQuark
 get_menuitem_quark (void)
@@ -493,14 +514,48 @@ user_get_menuitem (AccountsUser * user)
 }
 
 static void
+user_clear_menuitem (AccountsUser * user)
+{
+  g_object_steal_qdata (G_OBJECT(user), get_menuitem_quark());
+}
+
+static void
 user_set_menuitem (AccountsUser * user, DbusmenuMenuitem * mi)
 {
-  g_message ("%s %s() associating user %s with mi %p",
-             G_STRLOC, G_STRFUNC, accounts_user_get_user_name(user), mi);
-  g_object_set_qdata_full (G_OBJECT(user), get_menuitem_quark(),
-                           g_object_ref(G_OBJECT(mi)), g_object_unref);
+  g_object_set_qdata (G_OBJECT(user), get_menuitem_quark(), mi);
+
+  g_object_weak_ref (G_OBJECT(mi), (GWeakNotify)user_clear_menuitem, user);
 }
 
+/***/
+
+static GQuark
+get_mgr_quark (void)
+{
+  static GQuark q = 0;
+
+  if (G_UNLIKELY(!q))
+    {
+      q = g_quark_from_static_string ("session-menu-mgr");
+    }
+
+  return q;
+}
+
+static SessionMenuMgr*
+user_get_mgr (AccountsUser * user)
+{
+  return g_object_get_qdata (G_OBJECT(user), get_mgr_quark());
+}
+
+static void
+user_set_mgr (AccountsUser * user, SessionMenuMgr * mgr)
+{
+  g_object_set_qdata (G_OBJECT(user), get_mgr_quark(), mgr);
+}
+
+/***/
+
 static GQuark
 get_collision_quark (void)
 {
@@ -534,8 +589,11 @@ static void
 on_guest_logged_in_changed (UsersServiceDbus * usd,
                             SessionMenuMgr   * mgr)
 {
-  mi_set_logged_in (mgr->guest_mi,
-                    users_service_dbus_is_guest_logged_in (usd));
+  if (mgr->guest_mi != NULL)
+    {
+      mi_set_logged_in (mgr->guest_mi,
+                        users_service_dbus_is_guest_logged_in (usd));
+    }
 }
 
 /* When a user's login state changes,
@@ -557,19 +615,31 @@ static void
 update_screensaver_shortcut (SessionMenuMgr * mgr)
 {
   gchar * s = g_settings_get_string (mgr->keybinding_settings, "screensaver");
-  g_debug ("Keybinding changed to: %s", s);
-  dbusmenu_menuitem_property_set_shortcut_string (mgr->lock_mi, s);
-  dbusmenu_menuitem_property_set_shortcut_string (mgr->lock_switch_mi, s);
+  g_debug ("%s Screensaver shortcut changed to: '%s'", G_STRLOC, s);
+
+  if (mgr->lock_mi != NULL)
+    {
+      dbusmenu_menuitem_property_set_shortcut_string (mgr->lock_mi, s);
+    }
+
+  if (mgr->lock_switch_mi != NULL)
+    {
+      dbusmenu_menuitem_property_set_shortcut_string (mgr->lock_switch_mi, s);
+    }
+
+  if (mgr->screensaver_mi != NULL)
+    {
+      dbusmenu_menuitem_property_set_shortcut_string (mgr->screensaver_mi, s);
+    }
+
   g_free (s);
 }
 
 static void
-on_user_icon_file_changed (AccountsUser      * user,
-                           GParamSpec        * pspec G_GNUC_UNUSED,
-                           DbusmenuMenuitem  * mi)
+update_user_menuitem_icon (DbusmenuMenuitem * mi, AccountsUser * user)
 {
   const gchar * str = accounts_user_get_icon_file (user);
-  
+
   if (!str || !*str)
     {
       str = USER_ITEM_ICON_DEFAULT;
@@ -578,19 +648,62 @@ on_user_icon_file_changed (AccountsUser      * user,
   dbusmenu_menuitem_property_set (mi, USER_ITEM_PROP_ICON, str);
 }
 
+static void
+update_user_menuitem_name (DbusmenuMenuitem * mi, AccountsUser * user)
+{
+  GString * gstr = g_string_new (accounts_user_get_real_name (user));
+
+  if (user_has_name_collision (user))
+    {
+      g_string_append_printf (gstr, " (%s)", accounts_user_get_user_name(user));
+    }
+
+  dbusmenu_menuitem_property_set (mi, USER_ITEM_PROP_NAME, gstr->str);
+  g_string_free (gstr, TRUE);
+}
+
+static void
+on_user_property_changed (AccountsUser      * user,
+                          GParamSpec        * pspec,
+                          DbusmenuMenuitem  * mi)
+{
+  static const char * interned_icon_file = NULL;
+  static const char * interned_real_name = NULL;
+  static const char * interned_user_name = NULL;
+
+  if (G_UNLIKELY (interned_icon_file == NULL))
+    {
+      interned_icon_file = g_intern_static_string ("icon-file");
+      interned_user_name = g_intern_static_string ("user-name");
+      interned_real_name = g_intern_static_string ("real-name");
+    }
+
+  if (pspec->name == interned_icon_file)
+    {
+      update_user_menuitem_icon (mi, user);
+    }
+  else if ((pspec->name == interned_real_name)
+        || (pspec->name == interned_user_name))
+    {
+      /* name changing can affect other menuitems too by invalidating
+         the sort order or name collision flags... so let's rebuild */
+      update_user_menuitems (user_get_mgr (user));
+    }
+}
+
 typedef struct
 {
-  AccountsUser * user;
+  gpointer instance;
   gulong handler_id;
 }
-UserChangeListenerData;
+SignalHandlerData;
 
-/* when the menuitem is destroyed,
-   it should stop listening for changes to the UserAccount properties :) */
+/* when a user menuitem is destroyed,
+   it should stop listening for its UserAccount's property changes */
 static void
-on_user_menuitem_destroyed (UserChangeListenerData * data)
+on_user_menuitem_destroyed (SignalHandlerData * data)
 {
-  g_signal_handler_disconnect (data->user, data->handler_id);
+  g_signal_handler_disconnect (data->instance, data->handler_id);
   g_free (data);
 }
 
@@ -600,27 +713,20 @@ user_menuitem_new (AccountsUser * user, SessionMenuMgr * mgr)
   DbusmenuMenuitem * mi = dbusmenu_menuitem_new ();
   mi_set_type (mi, USER_ITEM_TYPE);
 
-  /* set the name property */
-  GString * gstr = g_string_new (accounts_user_get_real_name (user));
-  if (user_has_name_collision (user))
-    {
-      g_string_append_printf (gstr, " (%s)", accounts_user_get_user_name(user));
-    }
-  dbusmenu_menuitem_property_set (mi, USER_ITEM_PROP_NAME, gstr->str);
-  g_string_free (gstr, TRUE);
+  /* set the name & icon and listen for property changes */
+  update_user_menuitem_name (mi, user);
+  update_user_menuitem_icon (mi, user);
+  SignalHandlerData * hd = g_new0 (SignalHandlerData, 1);
+  hd->instance = user;
+  hd->handler_id = g_signal_connect (user, "notify",
+                                     G_CALLBACK(on_user_property_changed), mi);
+  g_object_weak_ref (G_OBJECT(mi), (GWeakNotify)on_user_menuitem_destroyed, hd);
+
 
   /* set the logged-in property */
   mi_set_logged_in (mi,
          users_service_dbus_is_user_logged_in (mgr->users_dbus_facade, user));
 
-  /* set the icon property & listen for changes */
-  UserChangeListenerData * cd = g_new0 (UserChangeListenerData, 1);
-  cd->user = user;
-  cd->handler_id = g_signal_connect (user, "notify::icon-file",
-                                     G_CALLBACK(on_user_icon_file_changed), mi);
-  g_object_weak_ref (G_OBJECT(mi), (GWeakNotify)on_user_menuitem_destroyed, cd);
-  on_user_icon_file_changed (user, NULL, mi);
-
   /* set the is-current-user property */
   const gboolean is_current_user =
               !g_strcmp0 (g_get_user_name(), accounts_user_get_user_name(user));
@@ -628,17 +734,13 @@ user_menuitem_new (AccountsUser * user, SessionMenuMgr * mgr)
                                        USER_ITEM_PROP_IS_CURRENT_USER,
                                        is_current_user);
 
-  /* set the activate callback */
-  ActivateUserSessionData * data = g_new (ActivateUserSessionData, 1);
-  data->user = user;
-  data->mgr = mgr;
-  g_signal_connect_data (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
-                         G_CALLBACK (action_func_switch_to_user),
-                         data, (GClosureNotify)g_free,
-                         G_CONNECT_SWAPPED);
+  /* set the switch-to-user action */
+  g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
+                            G_CALLBACK (action_func_switch_to_user), user);
 
   /* give this AccountsUser a hook back to this menuitem */
   user_set_menuitem (user, mi);
+  user_set_mgr (user, mgr);
 
   return mi;
 }
@@ -654,7 +756,7 @@ compare_users_by_login_frequency (gconstpointer a, gconstpointer b)
   return 0;
 }
 
-/* for sorting AccountsUsers by name */
+/* for sorting AccountsUsers alphabetically */
 static gint
 compare_users_by_username (gconstpointer ga, gconstpointer gb)
 {
@@ -690,6 +792,8 @@ is_user_switching_allowed (SessionMenuMgr * mgr)
 static void
 build_user_menuitems (SessionMenuMgr * mgr)
 {
+  g_return_if_fail (!mgr->greeter_mode);
+
   DbusmenuMenuitem * mi;
   GSList * items = NULL;
   gint pos = mgr->user_menuitem_index;
@@ -702,63 +806,78 @@ build_user_menuitems (SessionMenuMgr * mgr)
   ***  Lock / Switch Account...
   **/
 
-  const gboolean show_all = DEBUG_SHOW_ALL;
   const SwitcherMode mode = get_switcher_mode (mgr);
 
-  mi = mi_new (_("Start Screen Saver"));
-  mi_set_visible (mi, show_all || (mode == SWITCHER_MODE_SCREENSAVER));
+  mi = mgr->screensaver_mi = mi_new (_("Start Screen Saver"));
+  mi_set_visible (mi, mode == SWITCHER_MODE_SCREENSAVER);
   dbusmenu_menuitem_child_add_position (mgr->parent_mi, mi, pos++);
   items = g_slist_prepend (items, mi);
   g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
                             G_CALLBACK (action_func_lock), mgr);
 
   mi = mi_new (_("Switch User Account\342\200\246"));
-  mi_set_visible (mi, show_all || (mode == SWITCHER_MODE_SWITCH));
+  mi_set_visible (mi, mode == SWITCHER_MODE_SWITCH);
   dbusmenu_menuitem_child_add_position (mgr->parent_mi, mi, pos++);
   items = g_slist_prepend (items, mi);
   g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
                             G_CALLBACK (action_func_switch_to_greeter), mgr);
 
   mi = mgr->lock_mi = mi_new (_("Lock"));
-  mi_set_visible (mi, show_all || (mode == SWITCHER_MODE_LOCK));
+  mi_set_visible (mi, mode == SWITCHER_MODE_LOCK);
   dbusmenu_menuitem_child_add_position (mgr->parent_mi, mi, pos++);
   items = g_slist_prepend (items, mi);
   g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
                             G_CALLBACK (action_func_switch_to_lockscreen), mgr);
 
   mi = mgr->lock_switch_mi = mi_new (_("Lock/Switch Account\342\200\246"));
-  mi_set_visible (mi, show_all || (mode == SWITCHER_MODE_SWITCH_OR_LOCK));
+  mi_set_visible (mi, mode == SWITCHER_MODE_SWITCH_OR_LOCK);
   dbusmenu_menuitem_child_add_position (mgr->parent_mi, mi, pos++);
   items = g_slist_prepend (items, mi);
   g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
                             G_CALLBACK (action_func_switch_to_lockscreen), mgr);
 
-  const gboolean guest_allowed =
-           users_service_dbus_guest_session_enabled (mgr->users_dbus_facade);
-  const gboolean is_guest = is_this_guest_session();
-  mi = mi_new (_("Guest Session"));
+  const gboolean is_guest = is_this_guest_session ();
+  const gboolean guest_allowed = users_service_dbus_guest_session_enabled (mgr->users_dbus_facade);
+  mi = mgr->guest_mi = dbusmenu_menuitem_new ();
   mi_set_type (mi, USER_ITEM_TYPE);
-  mi_set_visible (mi, guest_allowed && !is_guest);
+  mi_set_visible (mi, !is_guest && guest_allowed);
+  dbusmenu_menuitem_property_set (mi, USER_ITEM_PROP_NAME, _("Guest Session"));
   dbusmenu_menuitem_child_add_position (mgr->parent_mi, mi, pos++);
   on_guest_logged_in_changed (mgr->users_dbus_facade, mgr);
   items = g_slist_prepend (items, mi);
   g_signal_connect_swapped (mi, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
                             G_CALLBACK (action_func_switch_to_guest), mgr);
-  mgr->guest_mi = mi;
+
   if (guest_allowed && is_guest)
     {
       current_real_name = _("Guest");
     }
-  
+
   /***
   ****  Users
   ***/
 
   /* if we can switch to another user account, show them here */
-  if (is_user_switching_allowed (mgr))
+  const char * const username = g_get_user_name();
+  GList * users = users_service_dbus_get_user_list (mgr->users_dbus_facade);
+
+  /* since we're building (or rebuilding) from scratch,
+     clear the name collision flags */
+  GList * u;
+  for (u=users; u!=NULL; u=u->next)
     {
-      GList * users = users_service_dbus_get_user_list (mgr->users_dbus_facade);
+      AccountsUser * user = ACCOUNTS_USER(u->data);
+
+      user_set_name_collision (user, FALSE);
+
+      if (!g_strcmp0 (username, accounts_user_get_user_name(user)))
+        {
+          current_real_name = accounts_user_get_real_name (user);
+        }
+    }
 
+  if (is_user_switching_allowed (mgr))
+    {
       /* pick the most frequently used accounts */
       const int MAX_USERS = 12; /* this limit comes from the spec */
       if (g_list_length(users) > MAX_USERS)
@@ -776,21 +895,14 @@ build_user_menuitems (SessionMenuMgr * mgr)
 
       /* Create menuitems for them */
       int i;
-      GList * u;
-      const char * const username = g_get_user_name();
       for (i=0, u=users; i<MAX_USERS && u!=NULL; u=u->next, i++)
         {
           AccountsUser * user = u->data;
           DbusmenuMenuitem * mi = user_menuitem_new (user, mgr);
           dbusmenu_menuitem_child_add_position (mgr->parent_mi, mi, pos++);
           items = g_slist_prepend (items, mi);
-
-          if (!g_strcmp0 (username, accounts_user_get_user_name(user)))
-            {
-              current_real_name = accounts_user_get_real_name (user);
-            }
         }
-      g_list_free(users);
+      g_list_free (users);
     }
 
   /* separator */
@@ -821,11 +933,14 @@ update_user_menuitems (SessionMenuMgr * mgr)
   mgr->user_menuitems = NULL;
 
   /* add fresh user menuitems */
-  build_user_menuitems (mgr);
+  if (!mgr->greeter_mode)
+    {
+      build_user_menuitems (mgr);
+    }
 }
 
 /***
-****
+****  Actions!
 ***/
 
 static void
@@ -835,14 +950,15 @@ action_func_spawn_async (const char * fmt, ...)
   va_start (marker, fmt);
   gchar * cmd = g_strdup_vprintf (fmt, marker);
   va_end (marker);
-  
-  GError * error = NULL; 
-  if (!g_spawn_command_line_async (cmd, &error))
+
+  GError * error = NULL;
+  g_spawn_command_line_async (cmd, &error);
+  if (error != NULL)
     {
-      g_warning ("Unable to show \"%s\": %s", cmd, error->message);
+      g_warning ("Unable to execute \"%s\": %s", cmd, error->message);
+      g_clear_error (&error);
     }
 
-  g_clear_error (&error);
   g_free (cmd);
 }
 
@@ -908,11 +1024,12 @@ action_func_switch_to_greeter (SessionMenuMgr * mgr)
 }
 
 static void
-action_func_switch_to_user (ActivateUserSessionData * data)
+action_func_switch_to_user (AccountsUser * user)
 {
-  action_func_lock (data->mgr);
-  users_service_dbus_activate_user_session (data->mgr->users_dbus_facade,
-                                            data->user);
+  SessionMenuMgr * mgr = user_get_mgr (user);
+  g_return_if_fail (mgr != NULL);
+  action_func_lock (mgr);
+  users_service_dbus_activate_user_session (mgr->users_dbus_facade, user);
 }
 
 static void
@@ -961,6 +1078,8 @@ action_func_hibernate (SessionMenuMgr * mgr)
 static gboolean
 is_this_guest_session (void)
 {
+  /* FIXME: this test has been here awhile and seems to work,
+     but seems brittle to me */
   return geteuid() < 500;
 }
 
@@ -996,15 +1115,16 @@ get_switcher_mode (SessionMenuMgr * mgr)
     {
       mode = SWITCHER_MODE_SWITCH;
     }
-  else
+  else /* both locking & switching are allowed */
     {
       GList * l = users_service_dbus_get_user_list (mgr->users_dbus_facade);
       const size_t user_count = g_list_length (l);
       g_list_free (l);
 
-      mode = user_count < 2
-           ? SWITCHER_MODE_LOCK /* you can't switch if no other users */
-           : SWITCHER_MODE_SWITCH_OR_LOCK;
+      /* only show switch mode if we have users to switch to */
+      mode = user_count > (is_this_guest_session() ? 0 : 1)
+           ? SWITCHER_MODE_SWITCH_OR_LOCK
+           : SWITCHER_MODE_LOCK;
     }
 
   return mode;
@@ -1026,7 +1146,7 @@ SessionMenuMgr* session_menu_mgr_new (DbusmenuMenuitem  * parent_mi,
   build_admin_menuitems (mgr);
   const guint n = g_list_length (dbusmenu_menuitem_get_children (parent_mi));
   mgr->user_menuitem_index = n;
-  build_user_menuitems (mgr);
+  update_user_menuitems (mgr);
   build_session_menuitems (mgr);
   return mgr;
 }
diff --git a/src/session-service.c b/src/session-service.c
index 598fcce..2349684 100644
--- a/src/session-service.c
+++ b/src/session-service.c
@@ -11,16 +11,16 @@ Authors:
     Conor Curran <conor.curran@canonical.com>
     Charles Kerr <charles.kerr@canonical.com>
 
-This program is free software: you can redistribute it and/or modify it 
-under the terms of the GNU General Public License version 3, as published 
+This program is free software: you can redistribute it and/or modify it
+under the terms of the GNU General Public License version 3, as published
 by the Free Software Foundation.
 
-This program is distributed in the hope that it will be useful, but 
-WITHOUT ANY WARRANTY; without even the implied warranties of 
-MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR 
+This program is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranties of
+MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
 PURPOSE.  See the GNU General Public License for more details.
 
-You should have received a copy of the GNU General Public License along 
+You should have received a copy of the GNU General Public License along
 with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
@@ -42,34 +42,31 @@ with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 #include <libindicator/indicator-service.h>
 
-#include "dbus-shared-names.h"
-#include "users-service-dbus.h"
 #include "session-dbus.h"
 #include "session-menu-mgr.h"
+#include "shared-names.h"
+#include "users-service-dbus.h"
 
-//static UsersServiceDbus  *dbus_interface = NULL;
-static SessionDbus       *session_dbus = NULL;
+static SessionDbus * session_dbus = NULL;
 static GMainLoop * mainloop = NULL;
 
 
-/* When the service interface starts to shutdown, we
-   should follow it. */
+/* When the service interface starts to shutdown,
+   we should follow it. */
 void
 service_shutdown (IndicatorService * service, gpointer user_data)
 {
-	if (mainloop != NULL) {
-		g_debug("Service shutdown");
-		g_main_loop_quit(mainloop);
-	}
-	return;
+  if (mainloop != NULL)
+    {
+      g_debug ("Service shutdown");
+      g_main_loop_quit (mainloop);
+    }
 }
 
-static gboolean
-get_greeter_mode (void)
+static inline gboolean
+is_greeter_mode (void)
 {
-  const gchar *var;
-  var = g_getenv("INDICATOR_GREETER_MODE");
-  return (g_strcmp0(var, "1") == 0);
+  return !g_strcmp0 (g_getenv ("INDICATOR_GREETER_MODE"), "1");
 }
 
 /* Main, is well, main.  It brings everything up and throws
@@ -77,34 +74,29 @@ get_greeter_mode (void)
 int
 main (int argc, char ** argv)
 {
-  gboolean greeter_mode;
-
   g_type_init();
 
-	/* Setting up i18n and gettext.  Apparently, we need
-	   all of these. */
-	setlocale (LC_ALL, "");
-	bindtextdomain (GETTEXT_PACKAGE, GNOMELOCALEDIR);
-	textdomain (GETTEXT_PACKAGE);
+  /* Setting up i18n and gettext.
+     Apparently we need all of these. */
+  setlocale (LC_ALL, "");
+  bindtextdomain (GETTEXT_PACKAGE, GNOMELOCALEDIR);
+  textdomain (GETTEXT_PACKAGE);
 
-	IndicatorService * service = indicator_service_new_version (INDICATOR_SESSION_DBUS_NAME,
-      		                                                    INDICATOR_SESSION_DBUS_VERSION);
-	g_signal_connect (G_OBJECT(service),
-                    INDICATOR_SERVICE_SIGNAL_SHUTDOWN,
+  IndicatorService * service = indicator_service_new_version (INDICATOR_SESSION_DBUS_NAME,
+      		                                              INDICATOR_SESSION_DBUS_VERSION);
+  g_signal_connect (G_OBJECT(service), INDICATOR_SERVICE_SIGNAL_SHUTDOWN,
                     G_CALLBACK(service_shutdown), NULL);
 
-	session_dbus = session_dbus_new();
-
-  greeter_mode = get_greeter_mode();
+  session_dbus = session_dbus_new();
 
   DbusmenuMenuitem * root_item = dbusmenu_menuitem_new ();
-  session_menu_mgr_new (root_item, session_dbus, greeter_mode);
+  session_menu_mgr_new (root_item, session_dbus, is_greeter_mode());
   DbusmenuServer* server = dbusmenu_server_new (INDICATOR_SESSION_DBUS_OBJECT);
   dbusmenu_server_set_root (server, root_item);
 
   mainloop = g_main_loop_new(NULL, FALSE);
   g_main_loop_run(mainloop);
-  
+
   return 0;
 }
 
diff --git a/src/settings-helper.c b/src/settings-helper.c
deleted file mode 100644
index 0ab188a..0000000
--- a/src/settings-helper.c
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
-A small wrapper utility for connecting to GSettings.
-
-Copyright 2009 Canonical Ltd.
-
-Authors:
-    Christoph Korn <c_korn@gmx.de>
-
-This program is free software: you can redistribute it and/or modify it 
-under the terms of the GNU General Public License version 3, as published 
-by the Free Software Foundation.
-
-This program is distributed in the hope that it will be useful, but 
-WITHOUT ANY WARRANTY; without even the implied warranties of 
-MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR 
-PURPOSE.  See the GNU General Public License for more details.
-
-You should have received a copy of the GNU General Public License along 
-with this program.  If not, see <http://www.gnu.org/licenses/>.
-*/
-
-#include <gio/gio.h>
-
-#include "dbus-shared-names.h"
-#include "settings-helper.h"
-
-static GSettings* settings = NULL;
-
-static gboolean
-build_settings (void)
-{
-  if (G_UNLIKELY(settings == NULL))
-    {
-      settings = g_settings_new (SESSION_SCHEMA);
-    }
-
-  return settings != NULL;
-}
-
-gboolean
-supress_confirmations (void)
-{
-  g_return_val_if_fail (build_settings(), FALSE);
-
-  return g_settings_get_boolean (settings, SUPPRESS_KEY) ;
-}
diff --git a/src/settings-helper.h b/src/settings-helper.h
deleted file mode 100644
index 8c4b2e8..0000000
--- a/src/settings-helper.h
+++ /dev/null
@@ -1,35 +0,0 @@
-/*
-A small wrapper utility for connecting to GSettings.
-
-Copyright 2009 Canonical Ltd.
-
-Authors:
-    Christoph Korn <c_korn@gmx.de>
-
-This program is free software: you can redistribute it and/or modify it 
-under the terms of the GNU General Public License version 3, as published 
-by the Free Software Foundation.
-
-This program is distributed in the hope that it will be useful, but 
-WITHOUT ANY WARRANTY; without even the implied warranties of 
-MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR 
-PURPOSE.  See the GNU General Public License for more details.
-
-You should have received a copy of the GNU General Public License along 
-with this program.  If not, see <http://www.gnu.org/licenses/>.
-*/
-
-
-#ifndef __GCONF_HELPER_H__
-#define __GCONF_HELPER_H__ 
-
-#define SESSION_SCHEMA                "com.canonical.indicator.session"
-#define SUPPRESS_KEY                  "suppress-logout-restart-shutdown"
-#define LOGOUT_KEY                    "suppress-logout-menuitem"
-#define RESTART_KEY                   "suppress-restart-menuitem"
-#define SHUTDOWN_KEY                  "suppress-shutdown-menuitem"
-#define SHOW_USER_MENU                "user-show-menu"
-
-gboolean supress_confirmations (void);
-
-#endif /* __GCONF_HELPER__ */
diff --git a/src/dbus-shared-names.h b/src/shared-names.h
index 876735a..dcda182 100644
--- a/src/dbus-shared-names.h
+++ b/src/shared-names.h
@@ -23,33 +23,12 @@ with this program.  If not, see <http://www.gnu.org/licenses/>.
 #ifndef __DBUS_SHARED_NAMES_H__
 #define __DBUS_SHARED_NAMES_H__ 
 
-typedef enum {
-  UP_TO_DATE,
-  CHECKING_FOR_UPDATES,
-  UPDATES_AVAILABLE,
-  UPGRADE_IN_PROGRESS, 
-  FINISHED, 
-  RESTART_NEEDED,
-  DONT_KNOW
-}AptState;
-
-typedef enum {
-  SIMULATION,
-  REAL
-}TransactionType;
-
-//#define INDICATOR_USERS_DBUS_NAME  INDICATOR_SESSION_DBUS_NAME
-//#define INDICATOR_USERS_DBUS_OBJECT "/com/canonical/indicator/users/menu"
-//#define INDICATOR_USERS_SERVICE_DBUS_OBJECT "/org/gnome/DisplayManager/UserManager"
-//#define INDICATOR_USERS_SERVICE_DBUS_INTERFACE "org.gnome.DisplayManager.UserManager"
-
 #define INDICATOR_SESSION_DBUS_NAME  "com.canonical.indicator.session"
 #define INDICATOR_SESSION_DBUS_OBJECT "/com/canonical/indicator/session/menu"
 #define INDICATOR_SESSION_DBUS_VERSION  0
 
 #define INDICATOR_SESSION_SERVICE_DBUS_OBJECT "/com/canonical/indicator/session/service"
 #define INDICATOR_SESSION_SERVICE_DBUS_IFACE  "com.canonical.indicator.session.service"
-
 #define USER_ITEM_TYPE            "x-canonical-user-item"
 #define USER_ITEM_PROP_NAME       "user-item-name"
 #define USER_ITEM_PROP_LOGGED_IN  "user-item-logged-in"
@@ -66,4 +45,13 @@ typedef enum {
 #define GREETER_ICON_DEFAULT      "system-shutdown-panel"
 #define GREETER_ICON_RESTART      "system-shutdown-panel-restart"
 
+/* the session indicator's settings */
+#define SESSION_SCHEMA                "com.canonical.indicator.session"
+#define SUPPRESS_KEY                  "suppress-logout-restart-shutdown"
+#define LOGOUT_KEY                    "suppress-logout-menuitem"
+#define RESTART_KEY                   "suppress-restart-menuitem"
+#define SHUTDOWN_KEY                  "suppress-shutdown-menuitem"
+#define SHOW_USER_MENU                "user-show-menu"
+
+
 #endif /* __DBUS_SHARED_NAMES_H__ */
diff --git a/src/users-service-dbus.c b/src/users-service-dbus.c
index 57cc3a6..8a050fc 100644
--- a/src/users-service-dbus.c
+++ b/src/users-service-dbus.c
@@ -30,13 +30,13 @@
 
 #include <pwd.h> /* getpwuid() */
 
-#include "dbus-shared-names.h"
 #include "dbus-accounts.h"
 #include "dbus-consolekit-manager.h"
 #include "dbus-consolekit-seat.h"
 #include "dbus-consolekit-session.h"
 #include "dbus-display-manager.h"
 #include "dbus-user.h"
+#include "shared-names.h"
 #include "users-service-dbus.h"
 
 #define CK_ADDR             "org.freedesktop.ConsoleKit"
@@ -326,7 +326,6 @@ create_consolekit_session_proxy (const char * ssid)
                             ssid,
                             NULL,
                             &error);
-
   if (error != NULL)
     {
       g_warning ("%s: %s", G_STRLOC, error->message);
@@ -644,7 +643,6 @@ update_user_list (UsersServiceDbus *self)
   GError * error = NULL;
   char ** object_paths = NULL;
   UsersServiceDbusPrivate * priv = self->priv;
-  g_debug ("%s updating the user list", G_STRLOC);
 
   accounts_call_list_cached_users_sync (priv->accounts_proxy,
                                         &object_paths,
@@ -680,7 +678,7 @@ on_user_added (Accounts          * o          G_GNUC_UNUSED,
 {
   /* We see a new user but we might not want to list it --
      for example, lightdm shows up when we switch to the greeter.
-     So instead of adding the user directly here, let's ask 
+     So instead of adding the user directly here, let's ask
      org.freedesktop.Accounts for a fresh list of users
      because it filters out special cases. */
   update_user_list (service);
@@ -790,7 +788,7 @@ get_unix_username_from_ssid (UsersServiceDbus * self,
               g_warning ("Failed to lookup user id %d: %s", (int)uid, g_strerror(errno));
             }
           else
-            { 
+            {
               username = g_strdup (pwent->pw_name);
             }
         }
@@ -807,7 +805,7 @@ is_guest_username (const char * username)
   if (!g_strcmp0 (username, "guest"))
     return TRUE;
 
-  if ((strlen(username)==12) && !memcmp(username,"guest-",6))
+  if (username && (strlen(username)==12) && !memcmp(username,"guest-",6))
     return TRUE;
 
   return FALSE;