Attachment 647383 Details for Bug 939571

[patch] gdm-bnc939571-bgo749418-autologin-logout.patch

gdm-bnc939571-bgo749418-autologin-logout.patch (text/plain), 15.41 KB, created by Federico Mena Quintero on 2015-09-15 21:27:15 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Federico Mena Quintero

Created: 2015-09-15 21:27:15 UTC

Size: 15.41 KB

patch

obsolete

>From 54723ab99c25a172ae6c0ca30bf7c04697833f6b Mon Sep 17 00:00:00 2001
>From: Ray Strode <rstrode@redhat.com>
>Date: Thu, 10 Sep 2015 09:36:54 -0400
>Subject: [PATCH 1/8] local-display-factory: spacing clean up
>
>The function declaration for gdm_local_display_factory_sync_seats
>put the return type on the same line as the function name. That
>doesn't follow the GDM codebase's style.
>
>This commit moves the return type to its own line.
>
>https://bugzilla.gnome.org/show_bug.cgi?id=749418
>---
> daemon/gdm-local-display-factory.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/daemon/gdm-local-display-factory.c b/daemon/gdm-local-display-factory.c
>index af15617..e902c3d 100644
>--- a/daemon/gdm-local-display-factory.c
>+++ b/daemon/gdm-local-display-factory.c
>@@ -445,7 +445,8 @@ delete_display (GdmLocalDisplayFactory *factory,
>         gdm_display_store_foreach_remove (store, lookup_by_seat_id, (gpointer) seat_id);
> }
> 
>-static gboolean gdm_local_display_factory_sync_seats (GdmLocalDisplayFactory *factory)
>+static gboolean
>+gdm_local_display_factory_sync_seats (GdmLocalDisplayFactory *factory)
> {
>         GError *error = NULL;
>         GVariant *result;
>-- 
>2.1.4
>
>
>From 78f91789926affd42b159bd7cb037e5dd584e356 Mon Sep 17 00:00:00 2001
>From: Ray Strode <rstrode@redhat.com>
>Date: Thu, 10 Sep 2015 09:38:12 -0400
>Subject: [PATCH 2/8] local-display-factory: call sync function after display
> finishes
>
>Right now if a local display finishes we create a new one of the
>same type to replace it.  But if a user display finishes we don't
>want to create a new user display, we want to create greeter
>display.  Furthermore if the user session is X11, we still want the
>login screen that comes up to be wayland by default.
>
>This commit changes the code to just call sync_seats after a display
>finishes, since sync_seats is where the logic for where the login
>screen display information is stored.
>
>sync_seats is idempotent anyway, and will only create displays for
>seats that need them, so it's safe to call it any time any local
>display finishes.
>
>https://bugzilla.gnome.org/show_bug.cgi?id=749418
>---
> daemon/gdm-local-display-factory.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
>diff --git a/daemon/gdm-local-display-factory.c b/daemon/gdm-local-display-factory.c
>index e902c3d..e670921 100644
>--- a/daemon/gdm-local-display-factory.c
>+++ b/daemon/gdm-local-display-factory.c
>@@ -83,6 +83,7 @@ static void     on_display_status_changed               (GdmDisplay
>                                                          GParamSpec                  *arg1,
>                                                          GdmLocalDisplayFactory      *factory);
> 
>+static gboolean gdm_local_display_factory_sync_seats    (GdmLocalDisplayFactory *factory);
> static gpointer local_display_factory_object = NULL;
> 
> G_DEFINE_TYPE (GdmLocalDisplayFactory, gdm_local_display_factory, GDM_TYPE_DISPLAY_FACTORY)
>@@ -312,12 +313,16 @@ on_display_status_changed (GdmDisplay             *display,
>                 }
>                 gdm_display_store_remove (store, display);
> 
>-                /* Create a new equivalent display if it was static */
>+                /* if this is a local display, do a full resync.  Only
>+                 * seats without displays will get created anyway.  This
>+                 * ensures we get a new login screen when the user logs out,
>+                 * if there isn't one.
>+                 */
>                 if (is_local) {
>                         /* reset num failures */
>                         factory->priv->num_failures = 0;
> 
>-                        create_display (factory, seat_id, session_type, is_initial);
>+                        gdm_local_display_factory_sync_seats (factory);
>                 }
>                 break;
>         case GDM_DISPLAY_FAILED:
>-- 
>2.1.4
>
>
>From 931154ba97e2a37d2580b9aeb59c160e131edfcb Mon Sep 17 00:00:00 2001
>From: Ray Strode <rstrode@redhat.com>
>Date: Thu, 10 Sep 2015 09:41:18 -0400
>Subject: [PATCH 3/8] manager: don't muck with the session type and class of
> stub display (autologin)
>
>Normally at startup a display is created of class "greeter" for the
>login screen to use.  That display isn't used in the case automatic
>login is enabled.  There's some iffy code to try to change the class
>from "greeter" to "user" in the automatic login case.  That code is
>wrong, because in the automatic login case a new display is created
>specifically for the user session.  The greeter session is abandoned,
>instead of used, so mucking with it's class and type is wrong.
>
>This commit removes the code that does that mucking.
>
>https://bugzilla.gnome.org/show_bug.cgi?id=749418
>---
> daemon/gdm-manager.c | 3 ---
> 1 file changed, 3 deletions(-)
>
>diff --git a/daemon/gdm-manager.c b/daemon/gdm-manager.c
>index 5e496f7..bfd5588 100644
>--- a/daemon/gdm-manager.c
>+++ b/daemon/gdm-manager.c
>@@ -1742,9 +1742,6 @@ set_up_automatic_login_session (GdmManager *manager,
>         GdmSession *session;
>         gboolean is_initial;
> 
>-        g_object_set (G_OBJECT (display), "session-class", "user", NULL);
>-        g_object_set (G_OBJECT (display), "session-type", NULL, NULL);
>-
>         /* 0 is root user; since the daemon talks to the session object
>          * directly, itself, for automatic login
>          */
>-- 
>2.1.4
>
>
>From 575ed30021b50599582e699a0b00c5b718962a86 Mon Sep 17 00:00:00 2001
>From: Ray Strode <rstrode@redhat.com>
>Date: Thu, 10 Sep 2015 09:35:52 -0400
>Subject: [PATCH 4/8] manager: fix display leak
>
>If we're doing autologin then we prepare a stub greeter display that
>we don't actually end up using.  This commit makes sure the greeter
>display gets cleaned up so it doesn't stick around in the display
>store forever.
>
>https://bugzilla.gnome.org/show_bug.cgi?id=749418
>---
> daemon/gdm-manager.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/daemon/gdm-manager.c b/daemon/gdm-manager.c
>index bfd5588..ee907c5 100644
>--- a/daemon/gdm-manager.c
>+++ b/daemon/gdm-manager.c
>@@ -2007,6 +2007,10 @@ on_start_user_session (StartUserSessionOperation *operation)
>                 g_object_set_data (G_OBJECT (display), "gdm-embryonic-user-session", NULL);
>                 g_object_set_data (G_OBJECT (operation->session), "gdm-display", NULL);
>                 create_embryonic_user_session_for_display (operation->manager, display, allowed_uid);
>+
>+                /* remove the unused prepared greeter display since we're not going
>+                 * to have a greeter */
>+                gdm_display_store_remove (self->priv->display_store, display);
>                 g_object_unref (display);
> 
>                 /* Give the user session a new display object for bookkeeping purposes */
>-- 
>2.1.4
>
>
>From 0dd2dfcbc8b1987ad72dbb3d898412f5a6b3cfa3 Mon Sep 17 00:00:00 2001
>From: Ray Strode <rstrode@redhat.com>
>Date: Wed, 9 Sep 2015 15:52:31 -0400
>Subject: [PATCH 5/8] manager: make sure user session displays have a seat
> assigned
>
>The local display factory expects all displays it tracks to have
>a seat, and we're going to be tracking automatic login displays
>in the display factory in a subsequent commit.
>
>This commit makes sure the seat-id is properly set on automatic
>login display objects.
>
>https://bugzilla.gnome.org/show_bug.cgi?id=749418
>---
> daemon/gdm-manager.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/daemon/gdm-manager.c b/daemon/gdm-manager.c
>index ee907c5..4c1af48 100644
>--- a/daemon/gdm-manager.c
>+++ b/daemon/gdm-manager.c
>@@ -1927,11 +1927,14 @@ create_display_for_user_session (GdmManager *self,
>                                  const char *session_id)
> {
>         GdmDisplay *display;
>+        /* at the moment we only create GdmLocalDisplay objects on seat0 */
>+        const char *seat_id = "seat0";
> 
>         display = gdm_local_display_new ();
> 
>         g_object_set (G_OBJECT (display),
>                       "session-class", "user",
>+                      "seat-id", seat_id,
>                       "session-id", session_id,
>                       NULL);
>         gdm_display_store_add (self->priv->display_store,
>-- 
>2.1.4
>
>
>From d240d8c661fb88aa46e3d47a8feff4cc1dc7cb62 Mon Sep 17 00:00:00 2001
>From: Ray Strode <rstrode@redhat.com>
>Date: Wed, 9 Sep 2015 15:24:31 -0400
>Subject: [PATCH 6/8] local-display-factory: track autologin displays
>
>At the moment the display-factory only tracks displays it adds itself.
>autologin displays aren't added by the display-factory, but by the
>manager.
>
>This commit makes sure the display-factory is notified about the
>autologin display, so when it finishes a login screen can be put in
>its place.
>
>https://bugzilla.gnome.org/show_bug.cgi?id=749418
>---
> daemon/gdm-local-display-factory.c | 61 ++++++++++++++++++++++++++++++++++----
> 1 file changed, 56 insertions(+), 5 deletions(-)
>
>diff --git a/daemon/gdm-local-display-factory.c b/daemon/gdm-local-display-factory.c
>index e670921..1c773a2 100644
>--- a/daemon/gdm-local-display-factory.c
>+++ b/daemon/gdm-local-display-factory.c
>@@ -186,11 +186,6 @@ store_display (GdmLocalDisplayFactory *factory,
> {
>         GdmDisplayStore *store;
> 
>-        g_signal_connect (display, "notify::status",
>-                          G_CALLBACK (on_display_status_changed), factory);
>-
>-        g_object_weak_ref (G_OBJECT (display), (GWeakNotify)on_display_disposed, factory);
>-
>         store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory));
>         gdm_display_store_add (store, display);
> }
>@@ -577,14 +572,60 @@ gdm_local_display_factory_stop_monitor (GdmLocalDisplayFactory *factory)
> 
> #endif
> 
>+static void
>+on_display_added (GdmDisplayStore        *display_store,
>+                  const char             *id,
>+                  GdmLocalDisplayFactory *factory)
>+{
>+        GdmDisplay *display;
>+
>+        display = gdm_display_store_lookup (display_store, id);
>+
>+        if (display != NULL) {
>+                g_signal_connect (display, "notify::status",
>+                                  G_CALLBACK (on_display_status_changed), factory);
>+
>+                g_object_weak_ref (G_OBJECT (display), (GWeakNotify)on_display_disposed, factory);
>+        }
>+}
>+
>+static void
>+on_display_removed (GdmDisplayStore        *display_store,
>+                    const char             *id,
>+                    GdmLocalDisplayFactory *factory)
>+{
>+        GdmDisplay *display;
>+
>+        display = gdm_display_store_lookup (display_store, id);
>+
>+        if (display != NULL) {
>+                g_signal_handlers_disconnect_by_func (display, G_CALLBACK (on_display_status_changed), factory);
>+                g_object_weak_unref (G_OBJECT (display), (GWeakNotify)on_display_disposed, factory);
>+
>+        }
>+}
>+
> static gboolean
> gdm_local_display_factory_start (GdmDisplayFactory *base_factory)
> {
>         GdmLocalDisplayFactory *factory = GDM_LOCAL_DISPLAY_FACTORY (base_factory);
>         GdmDisplay             *display;
>+        GdmDisplayStore *store;
> 
>         g_return_val_if_fail (GDM_IS_LOCAL_DISPLAY_FACTORY (factory), FALSE);
> 
>+        store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory));
>+
>+        g_signal_connect (G_OBJECT (store),
>+                          "display-added",
>+                          G_CALLBACK (on_display_added),
>+                          factory);
>+
>+        g_signal_connect (G_OBJECT (store),
>+                          "display-removed",
>+                          G_CALLBACK (on_display_removed),
>+                          factory);
>+
> #ifdef WITH_SYSTEMD
>         if (LOGIND_RUNNING()) {
>                 gdm_local_display_factory_start_monitor (factory);
>@@ -602,6 +643,7 @@ static gboolean
> gdm_local_display_factory_stop (GdmDisplayFactory *base_factory)
> {
>         GdmLocalDisplayFactory *factory = GDM_LOCAL_DISPLAY_FACTORY (base_factory);
>+        GdmDisplayStore *store;
> 
>         g_return_val_if_fail (GDM_IS_LOCAL_DISPLAY_FACTORY (factory), FALSE);
> 
>@@ -609,6 +651,15 @@ gdm_local_display_factory_stop (GdmDisplayFactory *base_factory)
>         gdm_local_display_factory_stop_monitor (factory);
> #endif
> 
>+        store = gdm_display_factory_get_display_store (GDM_DISPLAY_FACTORY (factory));
>+
>+        g_signal_handlers_disconnect_by_func (G_OBJECT (store),
>+                                              G_CALLBACK (on_display_added),
>+                                              factory);
>+        g_signal_handlers_disconnect_by_func (G_OBJECT (store),
>+                                              G_CALLBACK (on_display_removed),
>+                                              factory);
>+
>         return TRUE;
> }
> 
>-- 
>2.1.4
>
>
>From 4c02e8e2762af89a047aef5aa2dd7a4ba9d9a02d Mon Sep 17 00:00:00 2001
>From: Ray Strode <rstrode@redhat.com>
>Date: Mon, 14 Sep 2015 10:26:52 -0400
>Subject: [PATCH 7/8] manager: only claen up stub greeter display if it's
> actually a stub
>
>commit 2774c7e43b9fdf5e5e59ef1f53ae7ba29f4aa23c fixed a leak in the
>case we're doing autologin, but it also unconditionally cleans up
>the display in the non-autologin case, too.
>
>At the moment, we don't want to clean up the greeter in that case,
>since it causes unnecessary flicker and slows down fast user switching.
>( though this behavior may get changed again pending the outcome of
>  bug 747339 )
>
>This commit makes sure we only clean up the leaker greeter display, if
>there is a leaked greeter display (namely, the autologin case)
>
>https://bugzilla.gnome.org/show_bug.cgi?id=749418
>---
> daemon/gdm-manager.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
>diff --git a/daemon/gdm-manager.c b/daemon/gdm-manager.c
>index 4c1af48..09efcba 100644
>--- a/daemon/gdm-manager.c
>+++ b/daemon/gdm-manager.c
>@@ -2011,10 +2011,12 @@ on_start_user_session (StartUserSessionOperation *operation)
>                 g_object_set_data (G_OBJECT (operation->session), "gdm-display", NULL);
>                 create_embryonic_user_session_for_display (operation->manager, display, allowed_uid);
> 
>-                /* remove the unused prepared greeter display since we're not going
>-                 * to have a greeter */
>-                gdm_display_store_remove (self->priv->display_store, display);
>-                g_object_unref (display);
>+                if (g_strcmp0 (operation->service_name, "gdm-autologin") == 0) {
>+                        /* remove the unused prepared greeter display since we're not going
>+                         * to have a greeter */
>+                        gdm_display_store_remove (self->priv->display_store, display);
>+                        g_object_unref (display);
>+                }
> 
>                 /* Give the user session a new display object for bookkeeping purposes */
>                 session_id = gdm_session_get_conversation_session_id (operation->session,
>-- 
>2.1.4
>
>
>From 68332491d40380dcfc20ea2eb63669fa2f575117 Mon Sep 17 00:00:00 2001
>From: Ray Strode <rstrode@redhat.com>
>Date: Mon, 14 Sep 2015 10:44:22 -0400
>Subject: [PATCH 8/8] display: don't complain if we dispose PREPARED display
>
>It's expected now that we may dispose a display when it's
>prepared (if it was prepared but then we determined we were
>going to autologin instead).
>
>https://bugzilla.gnome.org/show_bug.cgi?id=749418
>---
> daemon/gdm-display.c | 1 -
> 1 file changed, 1 deletion(-)
>
>diff --git a/daemon/gdm-display.c b/daemon/gdm-display.c
>index 20cda93..ab84ba4 100644
>--- a/daemon/gdm-display.c
>+++ b/daemon/gdm-display.c
>@@ -1138,7 +1138,6 @@ gdm_display_dispose (GObject *object)
>         }
>         g_clear_object (&self->priv->launch_environment);
> 
>-        g_warn_if_fail (self->priv->status != GDM_DISPLAY_PREPARED);
>         g_warn_if_fail (self->priv->status != GDM_DISPLAY_MANAGED);
>         g_warn_if_fail (self->priv->user_access_file == NULL);
>         g_warn_if_fail (self->priv->access_file == NULL);
>-- 
>2.1.4
>

Actions: View | Diff

Attachments on bug 939571: 642197 | 646453 | 646454 | 646575 | 646576 | 646579 | 647383