Merge pull request #1262 from nyorain/session_fix

Improve session handling

backend/backend.c

@@ -15,6 +15,7 @@
 #include <wlr/backend/wayland.h>
 #include <wlr/config.h>
 #include <wlr/util/log.h>
+#include "backend/multi.h"
 
 /* WLR_HAS_X11_BACKEND needs to be after wlr/config.h */
 #ifdef WLR_HAS_X11_BACKEND
@@ -56,6 +57,13 @@ struct wlr_renderer *wlr_backend_get_renderer(struct wlr_backend *backend) {
 	return NULL;
 }
 
+struct wlr_session *wlr_backend_get_session(struct wlr_backend *backend) {
+	if (backend->impl->get_session) {
+		return backend->impl->get_session(backend);
+	}
+	return NULL;
+}
+
 static size_t parse_outputs_env(const char *name) {
 	const char *outputs_str = getenv(name);
 	if (outputs_str == NULL) {
@@ -158,10 +166,12 @@ static struct wlr_backend *attempt_backend_by_name(struct wl_display *display,
 		return attempt_headless_backend(display, create_renderer_func);
 	} else if (strcmp(name, "drm") == 0 || strcmp(name, "libinput") == 0) {
 		// DRM and libinput need a session
-		*session = wlr_session_create(display);
 		if (!*session) {
-			wlr_log(WLR_ERROR, "failed to start a session");
-			return NULL;
+			*session = wlr_session_create(display);
+			if (!*session) {
+				wlr_log(WLR_ERROR, "failed to start a session");
+				return NULL;
+			}
 		}
 
 		if (strcmp(name, "libinput") == 0) {
@@ -178,13 +188,12 @@ static struct wlr_backend *attempt_backend_by_name(struct wl_display *display,
 struct wlr_backend *wlr_backend_autocreate(struct wl_display *display,
 		wlr_renderer_create_func_t create_renderer_func) {
 	struct wlr_backend *backend = wlr_multi_backend_create(display);
+	struct wlr_multi_backend *multi = (struct wlr_multi_backend *)backend;
 	if (!backend) {
 		wlr_log(WLR_ERROR, "could not allocate multibackend");
 		return NULL;
 	}
 
-	struct wlr_session *session = NULL;
-
 	char *names = getenv("WLR_BACKENDS");
 	if (names) {
 		names = strdup(names);
@@ -197,12 +206,12 @@ struct wlr_backend *wlr_backend_autocreate(struct wl_display *display,
 		char *saveptr;
 		char *name = strtok_r(names, ",", &saveptr);
 		while (name != NULL) {
-			struct wlr_backend *subbackend =
-				attempt_backend_by_name(display, backend, &session, name, create_renderer_func);
+			struct wlr_backend *subbackend = attempt_backend_by_name(display,
+				backend, &multi->session, name, create_renderer_func);
 			if (subbackend == NULL) {
 				wlr_log(WLR_ERROR, "failed to start backend '%s'", name);
 				wlr_backend_destroy(backend);
-				wlr_session_destroy(session);
+				wlr_session_destroy(multi->session);
 				free(names);
 				return NULL;
 			}
@@ -210,7 +219,7 @@ struct wlr_backend *wlr_backend_autocreate(struct wl_display *display,
 			if (!wlr_multi_backend_add(backend, subbackend)) {
 				wlr_log(WLR_ERROR, "failed to add backend '%s'", name);
 				wlr_backend_destroy(backend);
-				wlr_session_destroy(session);
+				wlr_session_destroy(multi->session);
 				free(names);
 				return NULL;
 			}
@@ -245,29 +254,30 @@ struct wlr_backend *wlr_backend_autocreate(struct wl_display *display,
 #endif
 
 	// Attempt DRM+libinput
-	session = wlr_session_create(display);
-	if (!session) {
+	multi->session = wlr_session_create(display);
+	if (!multi->session) {
 		wlr_log(WLR_ERROR, "Failed to start a DRM session");
 		wlr_backend_destroy(backend);
 		return NULL;
 	}
 
-	struct wlr_backend *libinput = wlr_libinput_backend_create(display, session);
+	struct wlr_backend *libinput = wlr_libinput_backend_create(display,
+		multi->session);
 	if (!libinput) {
 		wlr_log(WLR_ERROR, "Failed to start libinput backend");
 		wlr_backend_destroy(backend);
-		wlr_session_destroy(session);
+		wlr_session_destroy(multi->session);
 		return NULL;
 	}
 	wlr_multi_backend_add(backend, libinput);
 
-	struct wlr_backend *primary_drm =
-		attempt_drm_backend(display, backend, session, create_renderer_func);
+	struct wlr_backend *primary_drm = attempt_drm_backend(display, backend,
+		multi->session, create_renderer_func);
 	if (!primary_drm) {
 		wlr_log(WLR_ERROR, "Failed to open any DRM device");
 		wlr_backend_destroy(libinput);
 		wlr_backend_destroy(backend);
-		wlr_session_destroy(session);
+		wlr_session_destroy(multi->session);
 		return NULL;
 	}
 

backend/drm/backend.c

@@ -203,8 +203,3 @@ error_fd:
 	free(drm);
 	return NULL;
 }
-
-struct wlr_session *wlr_drm_backend_get_session(struct wlr_backend *backend) {
-	struct wlr_drm_backend *drm = get_drm_backend_from_backend(backend);
-	return drm->session;
-}

backend/multi/backend.c

@@ -1,7 +1,6 @@
 #include <assert.h>
 #include <stdbool.h>
 #include <stdlib.h>
-#include <wlr/backend/drm.h>
 #include <wlr/backend/interface.h>
 #include <wlr/backend/session.h>
 #include <wlr/util/log.h>
@@ -72,10 +71,17 @@ static struct wlr_renderer *multi_backend_get_renderer(
 	return NULL;
 }
 
+static struct wlr_session *multi_backend_get_session(
+		struct wlr_backend *_backend) {
+	struct wlr_multi_backend *backend = multi_backend_from_backend(_backend);
+	return backend->session;
+}
+
 struct wlr_backend_impl backend_impl = {
 	.start = multi_backend_start,
 	.destroy = multi_backend_destroy,
 	.get_renderer = multi_backend_get_renderer,
+	.get_session = multi_backend_get_session,
 };
 
 static void handle_display_destroy(struct wl_listener *listener, void *data) {
@@ -191,17 +197,6 @@ void wlr_multi_backend_remove(struct wlr_backend *_multi,
 	}
 }
 
-struct wlr_session *wlr_multi_get_session(struct wlr_backend *_backend) {
-	struct wlr_multi_backend *backend = multi_backend_from_backend(_backend);
-	struct subbackend_state *sub;
-	wl_list_for_each(sub, &backend->backends, link) {
-		if (wlr_backend_is_drm(sub->backend)) {
-			return wlr_drm_backend_get_session(sub->backend);
-		}
-	}
-	return NULL;
-}
-
 bool wlr_multi_is_empty(struct wlr_backend *_backend) {
 	assert(wlr_backend_is_multi(_backend));
 	struct wlr_multi_backend *backend = (struct wlr_multi_backend *)_backend;

backend/session/logind.c

@@ -35,6 +35,11 @@ struct logind_session {
 
 	char *id;
 	char *path;
+
+	// specifies whether a drm device was taken
+	// if so, the session will be (de)activated with the drm fd,
+	// otherwise with the dbus PropertiesChanged on "active" signal
+	bool has_drm;
 };
 
 static struct logind_session *logind_session_from_session(
@@ -57,6 +62,10 @@ static int logind_take_device(struct wlr_session *base, const char *path) {
 		return -1;
 	}
 
+	if (major(st.st_rdev) == DRM_MAJOR) {
+		session->has_drm = true;
+	}
+
 	ret = sd_bus_call_method(session->bus, "org.freedesktop.login1",
 		session->path, "org.freedesktop.login1.Session", "TakeDevice",
 		&error, &msg, "uu", major(st.st_rdev), minor(st.st_rdev));
@@ -262,6 +271,7 @@ static int pause_device(sd_bus_message *msg, void *userdata, sd_bus_error *ret_e
 	}
 
 	if (major == DRM_MAJOR) {
+		assert(session->has_drm);
 		session->base.active = false;
 		wlr_signal_emit_safe(&session->base.session_signal, session);
 	}
@@ -307,6 +317,115 @@ error:
 	return 0;
 }
 
+static int properties_changed(sd_bus_message *msg, void *userdata,
+		sd_bus_error *ret_error) {
+	struct logind_session *session = userdata;
+	int ret = 0;
+
+	// if we have a drm fd we don't depend on this
+	if (session->has_drm) {
+		return 0;
+	}
+
+	// PropertiesChanged arg 1: interface
+	const char *interface;
+	ret = sd_bus_message_read_basic(msg, 's', &interface); // skip path
+	if (ret < 0) {
+		goto error;
+	}
+
+	if (strcmp(interface, "org.freedesktop.login1.Session") != 0) {
+		// not interesting for us; ignore
+		wlr_log(WLR_DEBUG, "ignoring PropertiesChanged from %s", interface);
+		return 0;
+	}
+
+	// PropertiesChanged arg 2: changed properties with values
+	ret = sd_bus_message_enter_container(msg, 'a', "{sv}");
+	if (ret < 0) {
+		goto error;
+	}
+
+	const char *s;
+	while ((ret = sd_bus_message_enter_container(msg, 'e', "sv")) > 0) {
+		ret = sd_bus_message_read_basic(msg, 's', &s);
+		if (ret < 0) {
+			goto error;
+		}
+
+		if (strcmp(s, "Active") == 0) {
+			int ret;
+			ret = sd_bus_message_enter_container(msg, 'v', "b");
+			if (ret < 0) {
+				goto error;
+			}
+
+			bool active;
+			ret = sd_bus_message_read_basic(msg, 'b', &active);
+			if (ret < 0) {
+				goto error;
+			}
+
+			if (session->base.active != active) {
+				session->base.active = active;
+				wlr_signal_emit_safe(&session->base.session_signal, session);
+			}
+			return 0;
+		} else {
+			sd_bus_message_skip(msg, "{sv}");
+		}
+
+		ret = sd_bus_message_exit_container(msg);
+		if (ret < 0) {
+			goto error;
+		}
+	}
+
+	if (ret < 0) {
+		goto error;
+	}
+
+	ret = sd_bus_message_exit_container(msg);
+	if (ret < 0) {
+		goto error;
+	}
+
+	// PropertiesChanged arg 3: changed properties without values
+	sd_bus_message_enter_container(msg, 'a', "s");
+	while ((ret = sd_bus_message_read_basic(msg, 's', &s)) > 0) {
+		if (strcmp(s, "Active") == 0) {
+			sd_bus_error error = SD_BUS_ERROR_NULL;
+			bool active;
+			ret = sd_bus_get_property_trivial(session->bus,
+				"org.freedesktop.login1", session->path,
+				"org.freedesktop.login1.Session", "Active", &error,
+				'b', &active);
+			if (ret < 0) {
+				wlr_log(WLR_ERROR, "Failed to get 'Active' property: '%s' (%s)",
+					error.message, strerror(ret));
+				return 0;
+			}
+
+			if (session->base.active != active) {
+				session->base.active = active;
+				wlr_signal_emit_safe(&session->base.session_signal, session);
+			}
+			return 0;
+		}
+	}
+
+	if (ret < 0) {
+		goto error;
+	}
+
+	return 0;
+
+error:
+	wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged %s",
+		strerror(-ret));
+	return 0;
+}
+
 static bool add_signal_matches(struct logind_session *session) {
 	int ret;
 
@@ -338,6 +457,14 @@ static bool add_signal_matches(struct logind_session *session) {
 		return false;
 	}
 
+	ret = sd_bus_match_signal(session->bus, NULL, "org.freedesktop.login1",
+		session->path, "org.freedesktop.DBus.Properties", "PropertiesChanged",
+		properties_changed, session);
+	if (ret < 0) {
+		wlr_log(WLR_ERROR, "Failed to add D-Bus match: %s", strerror(-ret));
+		return false;
+	}
+
 	return true;
 }
 

include/backend/multi.h

@@ -8,6 +8,7 @@
 
 struct wlr_multi_backend {
 	struct wlr_backend backend;
+	struct wlr_session *session;
 
 	struct wl_list backends;
 

include/wlr/backend.h

@@ -57,5 +57,10 @@ void wlr_backend_destroy(struct wlr_backend *backend);
  * Obtains the wlr_renderer reference this backend is using.
  */
 struct wlr_renderer *wlr_backend_get_renderer(struct wlr_backend *backend);
+/**
+ * Obtains the wlr_session reference from this backend if there is any.
+ * Might return NULL for backends that don't use a session.
+ */
+struct wlr_session *wlr_backend_get_session(struct wlr_backend *backend);
 
 #endif

include/wlr/backend/drm.h

@@ -34,6 +34,4 @@ bool wlr_output_is_drm(struct wlr_output *output);
 typedef struct _drmModeModeInfo drmModeModeInfo;
 bool wlr_drm_connector_add_mode(struct wlr_output *output, const drmModeModeInfo *mode);
 
-struct wlr_session *wlr_drm_backend_get_session(struct wlr_backend *backend);
-
 #endif

include/wlr/backend/interface.h

@@ -17,6 +17,7 @@ struct wlr_backend_impl {
 	bool (*start)(struct wlr_backend *backend);
 	void (*destroy)(struct wlr_backend *backend);
 	struct wlr_renderer *(*get_renderer)(struct wlr_backend *backend);
+	struct wlr_session *(*get_session)(struct wlr_backend *backend);
 };
 
 /**

include/wlr/backend/multi.h

@@ -28,7 +28,6 @@ void wlr_multi_backend_remove(struct wlr_backend *multi,
 	struct wlr_backend *backend);
 
 bool wlr_backend_is_multi(struct wlr_backend *backend);
-struct wlr_session *wlr_multi_get_session(struct wlr_backend *base);
 bool wlr_multi_is_empty(struct wlr_backend *backend);
 
 void wlr_multi_for_each_backend(struct wlr_backend *backend,

rootston/keyboard.c

@@ -5,7 +5,6 @@
 #include <sys/wait.h>
 #include <unistd.h>
 #include <wayland-server.h>
-#include <wlr/backend/multi.h>
 #include <wlr/backend/session.h>
 #include <wlr/types/wlr_input_device.h>
 #include <wlr/types/wlr_pointer_constraints_v1.h>
@@ -201,14 +200,13 @@ static bool keyboard_execute_compositor_binding(struct roots_keyboard *keyboard,
 	if (keysym >= XKB_KEY_XF86Switch_VT_1 &&
 			keysym <= XKB_KEY_XF86Switch_VT_12) {
 		struct roots_server *server = keyboard->input->server;
-		if (wlr_backend_is_multi(server->backend)) {
-			struct wlr_session *session =
-				wlr_multi_get_session(server->backend);
-			if (session) {
-				unsigned vt = keysym - XKB_KEY_XF86Switch_VT_1 + 1;
-				wlr_session_change_vt(session, vt);
-			}
+
+		struct wlr_session *session = wlr_backend_get_session(server->backend);
+		if (session) {
+			unsigned vt = keysym - XKB_KEY_XF86Switch_VT_1 + 1;
+			wlr_session_change_vt(session, vt);
 		}
+
 		return true;
 	}