When logging out, prefer com.canonical.Unity.RequestLogout over org.gnome.SessionManager.Logout. Fixes: 1296814

5 files changed, 191 insertions, 52 deletions

diff --git a/src/backend-dbus/actions.c b/src/backend-dbus/actions.c
index cac7c40..c19d02a 100644
--- a/src/backend-dbus/actions.c
+++ b/src/backend-dbus/actions.c
@@ -471,18 +471,103 @@ my_hibernate (IndicatorSessionActions * self)
 ****  End Session Dialog
 ***/
 
+static gboolean
+is_owned_proxy (gpointer proxy)
+{
+  gboolean owned = FALSE;
+
+  if ((proxy != NULL) && G_IS_DBUS_PROXY (proxy))
+    {
+      char * name_owner = g_dbus_proxy_get_name_owner (proxy);
+
+      if (name_owner != NULL)
+        {
+          owned = TRUE;
+          g_free (name_owner);
+        }
+    }
+
+  return owned;
+}
+
 static void
-logout_now (IndicatorSessionActionsDbus * self)
+on_gnome_logout_response (GObject * o,
+                          GAsyncResult * res,
+                          gpointer unused G_GNUC_UNUSED)
+{
+  GError * err = NULL;
+  gnome_session_manager_call_logout_finish (GNOME_SESSION_MANAGER(o), res, &err);
+  log_and_clear_error (&err, G_STRLOC, G_STRFUNC);
+}
+
+static gboolean
+logout_now_gnome_session_manager (IndicatorSessionActionsDbus * self)
+{
+  gboolean logout_called = FALSE;
+  priv_t * p = self->priv;
+
+  if (is_owned_proxy (p->session_manager))
+    {
+      g_debug ("%s: calling gnome_session_manager_call_logout()", G_STRFUNC);
+      gnome_session_manager_call_logout (p->session_manager,
+                                         1, /* don't prompt */
+                                         p->cancellable,
+                                         on_gnome_logout_response,
+                                         self);
+      logout_called = TRUE;
+    }
+
+  return logout_called;
+}
+
+static void
+on_unity_logout_response (GObject * o,
+                          GAsyncResult * res,
+                          gpointer gself)
+{
+  GError * error;
+
+  error = NULL;
+  unity_session_call_request_logout_finish (UNITY_SESSION(o), res, &error);
+
+  if (error != NULL)
+    {
+      if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+        {
+          g_warning ("%s %s: %s", G_STRLOC, G_STRFUNC, error->message);
+          logout_now_gnome_session_manager(gself);
+        }
+
+      g_clear_error (&error);
+    }
+}
+
+static gboolean
+logout_now_unity (IndicatorSessionActionsDbus * self)
 {
   priv_t * p = self->priv;
+  gboolean called = FALSE;
 
-  g_return_if_fail (p->session_manager != NULL);
+  if (is_owned_proxy (p->unity_session))
+    {
+      called = TRUE;
+      g_debug ("calling unity_session_call_request_logout()");
+      unity_session_call_request_logout (p->unity_session,
+                                         p->cancellable,
+                                         on_unity_logout_response,
+                                         self);
+    }
 
-  gnome_session_manager_call_logout (p->session_manager,
-                                     1, /* don't prompt */
-                                     p->cancellable,
-                                     NULL,
-                                     NULL);
+  return called;
+}
+
+static void
+logout_now (IndicatorSessionActionsDbus * self)
+{
+  if (!logout_now_unity(self) && !logout_now_gnome_session_manager(self))
+    {
+      g_critical("%s can't logout: no Unity nor GNOME session proxy", G_STRFUNC);
+    }
 }
 
 static void
@@ -550,7 +635,17 @@ on_open_end_session_dialog_ready (GObject      * o,
 {
   GError * err = NULL;
   end_session_dialog_call_open_finish (END_SESSION_DIALOG(o), res, &err);
-  log_and_clear_error (&err, G_STRLOC, G_STRFUNC);
+  if (err != NULL)
+    {
+      if (!g_error_matches (err, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+        g_warning ("%s %s: %s", G_STRFUNC, G_STRLOC, err->message);
+
+      /* Treat errors as user confirmation.
+         Otherwise how will the user ever log out? */
+      logout_now(INDICATOR_SESSION_ACTIONS_DBUS(gself));
+
+      g_clear_error(&err);
+    }
 }
 
 static void
@@ -584,6 +679,7 @@ zenity_question (IndicatorSessionActionsDbus * self,
 {
   char * command_line;
   int exit_status;
+  GError * error;
   gboolean confirmed;
 
   command_line = g_strdup_printf ("%s"
@@ -601,11 +697,16 @@ zenity_question (IndicatorSessionActionsDbus * self,
                                   ok_label,
                                   cancel_label);
 
+  /* Treat errors as user confirmation.
+     Otherwise how will the user ever log out? */
   exit_status = -1;
-  if (!g_spawn_command_line_sync (command_line, NULL, NULL, &exit_status, NULL))
+  error = NULL;
+  if (!g_spawn_command_line_sync (command_line, NULL, NULL, &exit_status, &error))
+    {
+      confirmed = TRUE;
+    }
+  else if (!g_spawn_check_exit_status (exit_status, &error))
     {
-      /* Treat failure-to-prompt as user confirmation.
-         Otherwise how will the user ever log out? */
       confirmed = TRUE;
     }
   else
@@ -613,6 +714,7 @@ zenity_question (IndicatorSessionActionsDbus * self,
       confirmed = exit_status == 0;
     }
 
+  log_and_clear_error (&error, G_STRLOC, G_STRFUNC);
   g_free (command_line);
   return confirmed;
 }
@@ -776,32 +878,12 @@ my_about (IndicatorSessionActions * self G_GNUC_UNUSED)
 ****
 ***/
 
-static gboolean
-have_unity_session (IndicatorSessionActions * self)
-{
-  priv_t * p = INDICATOR_SESSION_ACTIONS_DBUS(self)->priv;
-  gchar * name_owner;
-
-  if (G_IS_DBUS_PROXY (p->unity_session))
-    {
-      name_owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (p->unity_session));
-
-      if (name_owner)
-        {
-          g_free (name_owner);
-          return TRUE;
-        }
-    }
-
-  return FALSE;
-}
-
 static void
 lock_current_session (IndicatorSessionActions * self, gboolean immediate)
 {
   priv_t * p = INDICATOR_SESSION_ACTIONS_DBUS(self)->priv;
 
-  if (have_unity_session (self))
+  if (is_owned_proxy (p->unity_session))
     {
       if (immediate)
         {
diff --git a/tests/backend-dbus/gtest-mock-dbus-fixture.h b/tests/backend-dbus/gtest-mock-dbus-fixture.h
index d4f598c..16b4648 100644
--- a/tests/backend-dbus/gtest-mock-dbus-fixture.h
+++ b/tests/backend-dbus/gtest-mock-dbus-fixture.h
@@ -43,8 +43,6 @@ class GTestMockDBusFixture: public GTestDBusFixture
   protected:
 
     MockScreenSaver * screen_saver;
-    MockUnitySession * unity_session;
-    MockSessionManager * session_manager;
     MockDisplayManagerSeat * dm_seat;
     MockAccounts * accounts;
     MockLogin1Manager * login1_manager;
@@ -60,9 +58,7 @@ class GTestMockDBusFixture: public GTestDBusFixture
 
       webcredentials = new MockWebcredentials (loop, conn);
       end_session_dialog = new MockEndSessionDialog (loop, conn);
-      session_manager = new MockSessionManager (loop, conn);
       screen_saver = new MockScreenSaver (loop, conn);
-      unity_session = new MockUnitySession (loop, conn);
       dm_seat = new MockDisplayManagerSeat (loop, conn);
       g_setenv ("XDG_SEAT_PATH", dm_seat->path(), TRUE);
       dm_seat->set_guest_allowed (false);
@@ -86,8 +82,6 @@ class GTestMockDBusFixture: public GTestDBusFixture
       delete login1_manager;
       delete dm_seat;
       delete screen_saver;
-      delete unity_session;
-      delete session_manager;
       delete end_session_dialog;
       delete webcredentials;
 
diff --git a/tests/backend-dbus/mock-unity-session.cc b/tests/backend-dbus/mock-unity-session.cc
index c996310..d32e339 100644
--- a/tests/backend-dbus/mock-unity-session.cc
+++ b/tests/backend-dbus/mock-unity-session.cc
@@ -21,7 +21,7 @@
 
 
 gboolean
-MockUnitySession :: handle_lock (UnitySession      * us,
+MockUnitySession :: handle_lock (UnitySession          * us,
                                  GDBusMethodInvocation * inv,
                                  gpointer                gself)
 {
@@ -31,7 +31,7 @@ MockUnitySession :: handle_lock (UnitySession      * us,
 }
 
 gboolean
-MockUnitySession :: handle_prompt_lock (UnitySession      * us,
+MockUnitySession :: handle_prompt_lock (UnitySession          * us,
                                         GDBusMethodInvocation * inv,
                                         gpointer                gself)
 {
@@ -40,6 +40,16 @@ MockUnitySession :: handle_prompt_lock (UnitySession      * us,
   return true;
 }
 
+gboolean
+MockUnitySession :: handle_request_logout (UnitySession          * us,
+                                           GDBusMethodInvocation * inv,
+                                           gpointer                gself)
+{
+  static_cast<MockUnitySession*>(gself)->my_last_action = RequestLogout;
+  unity_session_complete_request_logout (us, inv);
+  return true;
+}
+
 /***
 ****
 ***/
@@ -61,6 +71,8 @@ MockUnitySession :: MockUnitySession (GMainLoop       * loop,
                     G_CALLBACK(handle_lock), this);
   g_signal_connect (my_skeleton, "handle-prompt-lock",
                     G_CALLBACK(handle_prompt_lock), this);
+  g_signal_connect (my_skeleton, "handle-request-logout",
+                    G_CALLBACK(handle_request_logout), this);
 
   set_skeleton (G_DBUS_INTERFACE_SKELETON(my_skeleton));
 }
diff --git a/tests/backend-dbus/mock-unity-session.h b/tests/backend-dbus/mock-unity-session.h
index ded246a..9ca5fe9 100644
--- a/tests/backend-dbus/mock-unity-session.h
+++ b/tests/backend-dbus/mock-unity-session.h
@@ -35,6 +35,7 @@ class MockUnitySession: public MockObject
 
     enum Action { None, Lock, PromptLock, RequestLogout, RequestShutdown, RequestReboot };
     Action last_action () { return my_last_action; }
+    void clear_last_action () { my_last_action = None; }
 
   private:
 
@@ -47,6 +48,9 @@ class MockUnitySession: public MockObject
     static gboolean handle_prompt_lock (UnitySession *,
                                         GDBusMethodInvocation *,
                                         gpointer);
+    static gboolean handle_request_logout (UnitySession *,
+                                           GDBusMethodInvocation *,
+                                           gpointer);
 
 };
 
diff --git a/tests/backend-dbus/test-actions.cc b/tests/backend-dbus/test-actions.cc
index 717509d..ff5a24d 100644
--- a/tests/backend-dbus/test-actions.cc
+++ b/tests/backend-dbus/test-actions.cc
@@ -263,9 +263,11 @@ TEST_F (Actions, PowerOff)
   g_settings_reset (indicator_settings, SUPPRESS_KEY);
 }
 
-TEST_F (Actions, Logout)
+TEST_F (Actions, LogoutUnity)
 {
-  ASSERT_EQ (MockSessionManager::None, session_manager->last_action ());
+  MockUnitySession unity_session(loop, conn);
+  ASSERT_EQ (MockUnitySession::None, unity_session.last_action());
+  wait_msec();
 
   // confirm that user is prompted
   // and that no action is taken when the user cancels the dialog
@@ -274,26 +276,64 @@ TEST_F (Actions, Logout)
   ASSERT_TRUE (end_session_dialog->is_open());
   end_session_dialog->cancel();
   wait_msec (50);
-  ASSERT_EQ (MockSessionManager::None, session_manager->last_action ());
+  ASSERT_EQ (MockUnitySession::None, unity_session.last_action());
 
   // confirm that user is prompted
-  // and that no action is taken when the user cancels the dialog
+  // and that logout is called when user confirms the logout dialog
   indicator_session_actions_logout (actions);
   wait_msec (50);
   ASSERT_TRUE (end_session_dialog->is_open ());
   end_session_dialog->confirm_logout ();
   wait_msec (100);
-  ASSERT_EQ (MockSessionManager::LogoutQuiet, session_manager->last_action ());
+  ASSERT_EQ (MockUnitySession::RequestLogout, unity_session.last_action());
 
   // confirm that we try to call SessionManager::LogoutQuet
   // when prompts are disabled
   login1_manager->clear_last_action ();
+  unity_session.clear_last_action ();
   ASSERT_EQ ("", login1_manager->last_action());
+  ASSERT_EQ (MockUnitySession::None, unity_session.last_action ());
   g_settings_set_boolean (indicator_settings, SUPPRESS_KEY, TRUE);
   wait_msec (50);
   indicator_session_actions_logout (actions);
   wait_msec (50);
-  ASSERT_EQ (MockSessionManager::LogoutQuiet, session_manager->last_action ());
+  ASSERT_EQ (MockUnitySession::RequestLogout, unity_session.last_action ());
+  g_settings_reset (indicator_settings, SUPPRESS_KEY);
+}
+
+TEST_F (Actions, LogoutGnome)
+{
+  MockSessionManager session_manager (loop, conn);
+  ASSERT_EQ (MockSessionManager::None, session_manager.last_action ());
+  wait_msec(50);
+
+  // confirm that user is prompted
+  // and that no action is taken when the user cancels the dialog
+  indicator_session_actions_logout (actions);
+  wait_msec (50);
+  ASSERT_TRUE (end_session_dialog->is_open());
+  end_session_dialog->cancel();
+  wait_msec (50);
+  ASSERT_EQ (MockSessionManager::None, session_manager.last_action ());
+
+  // confirm that user is prompted
+  // and that logout is called when user confirms in the dialog
+  indicator_session_actions_logout (actions);
+  wait_msec (50);
+  ASSERT_TRUE (end_session_dialog->is_open ());
+  end_session_dialog->confirm_logout ();
+  wait_msec (100);
+  ASSERT_EQ (MockSessionManager::LogoutQuiet, session_manager.last_action ());
+ 
+  // confirm that we try to call SessionManager::LogoutQuet
+  // when prompts are disabled
+  login1_manager->clear_last_action ();
+  ASSERT_EQ ("", login1_manager->last_action());
+  g_settings_set_boolean (indicator_settings, SUPPRESS_KEY, TRUE);
+  wait_msec (50);
+  indicator_session_actions_logout (actions);
+   wait_msec (50);
+  ASSERT_EQ (MockSessionManager::LogoutQuiet, session_manager.last_action ());
 
   g_settings_reset (indicator_settings, SUPPRESS_KEY);
 }
@@ -316,24 +356,30 @@ TEST_F (Actions, Hibernate)
 
 TEST_F (Actions, SwitchToScreensaver)
 {
-  ASSERT_EQ (MockUnitySession::None, unity_session->last_action());
+  MockUnitySession unity_session(loop, conn);
+
+  ASSERT_EQ (MockUnitySession::None, unity_session.last_action());
   indicator_session_actions_switch_to_screensaver (actions);
   wait_msec (50);
-  ASSERT_EQ (MockUnitySession::Lock, unity_session->last_action());
+  ASSERT_EQ (MockUnitySession::Lock, unity_session.last_action());
 }
 
 TEST_F (Actions, SwitchToGreeter)
 {
+  MockUnitySession unity_session(loop, conn);
+
   ASSERT_NE (MockDisplayManagerSeat::GREETER, dm_seat->last_action());
-  ASSERT_EQ (MockUnitySession::None, unity_session->last_action());
+  ASSERT_EQ (MockUnitySession::None, unity_session.last_action());
   indicator_session_actions_switch_to_greeter (actions);
   wait_msec (50);
-  ASSERT_EQ (MockUnitySession::PromptLock, unity_session->last_action());
+  ASSERT_EQ (MockUnitySession::PromptLock, unity_session.last_action());
   ASSERT_EQ (MockDisplayManagerSeat::GREETER, dm_seat->last_action());
 }
 
 TEST_F (Actions, SwitchToGuest)
 {
+  MockUnitySession unity_session(loop, conn);
+
   // allow guests
   dm_seat->set_guest_allowed (true);
 
@@ -348,11 +394,12 @@ TEST_F (Actions, SwitchToGuest)
   wait_for_signal (login1_seat->skeleton(), "notify::active-session");
   ASSERT_EQ (guest_session_tag, login1_seat->active_session());
   wait_msec (50);
-  ASSERT_EQ (MockUnitySession::PromptLock, unity_session->last_action());
+  ASSERT_EQ (MockUnitySession::PromptLock, unity_session.last_action());
 }
 
 TEST_F (Actions, SwitchToUsername)
 {
+  MockUnitySession unity_session(loop, conn);
   const char * const dr1_username = "whartnell";
   const char * const dr2_username = "ptroughton";
   MockUser * dr1_user;
@@ -370,7 +417,7 @@ TEST_F (Actions, SwitchToUsername)
   wait_for_signal (login1_seat->skeleton(), "notify::active-session");
   ASSERT_EQ (dr1_session, login1_seat->active_session());
   wait_msec (50);
-  ASSERT_EQ (MockUnitySession::PromptLock, unity_session->last_action());
+  ASSERT_EQ (MockUnitySession::PromptLock, unity_session.last_action());
 
   indicator_session_actions_switch_to_username (actions, dr2_username);
   wait_for_signal (login1_seat->skeleton(), "notify::active-session");