keyboard: put modifier/key listeners in keyboard struct

...so that it can be determined what wlr_keyboard events come from.

This is required to manage virtual keyboards alongside the keyboard_group
of physical keyboards.

include/labwc.h

@@ -76,6 +76,20 @@ struct input {
 	struct wl_list link; /* seat::inputs */
 };
 
+/*
+ * Virtual keyboards should not belong to seat->keyboard_group. As a result we
+ * need to be able to ascertain which wlr_keyboard key/modifer events come from
+ * and we achieve that by using `struct keyboard` which inherits `struct input`
+ * and adds keybord specific listeners and a wlr_keyboard pointer.
+ */
+struct keyboard {
+	struct input base;
+	struct wlr_keyboard *wlr_keyboard;
+	bool is_virtual;
+	struct wl_listener modifier;
+	struct wl_listener key;
+};
+
 struct seat {
 	struct wlr_seat *seat;
 	struct server *server;
@@ -144,9 +158,6 @@ struct seat {
 	struct wl_listener request_set_selection;
 	struct wl_listener request_set_primary_selection;
 
-	struct wl_listener keyboard_key;
-	struct wl_listener keyboard_modifiers;
-
 	struct wl_listener touch_down;
 	struct wl_listener touch_up;
 	struct wl_listener touch_motion;
@@ -516,6 +527,8 @@ struct view *desktop_focused_view(struct server *server);
 void desktop_focus_topmost_mapped_view(struct server *server);
 bool isfocusable(struct view *view);
 
+void keyboard_key_notify(struct wl_listener *listener, void *data);
+void keyboard_modifiers_notify(struct wl_listener *listener, void *data);
 void keyboard_init(struct seat *seat);
 bool keyboard_any_modifiers_pressed(struct wlr_keyboard *keyboard);
 void keyboard_finish(struct seat *seat);

src/keyboard.c

@@ -46,18 +46,18 @@ end_cycling(struct server *server)
 	should_cancel_cycling_on_next_key_release = false;
 }
 
-static void
+void
 keyboard_modifiers_notify(struct wl_listener *listener, void *data)
 {
-	struct seat *seat = wl_container_of(listener, seat, keyboard_modifiers);
+	struct keyboard *keyboard = wl_container_of(listener, keyboard, modifier);
+	struct seat *seat = keyboard->base.seat;
 	struct server *server = seat->server;
+	struct wlr_keyboard_key_event *event = data;
+	struct wlr_keyboard *wlr_keyboard = keyboard->wlr_keyboard;
 
 	if (server->osd_state.cycle_view || seat->workspace_osd_shown_by_modifier) {
-		struct wlr_keyboard_key_event *event = data;
-		struct wlr_keyboard *keyboard = &seat->keyboard_group->keyboard;
-
 		if (event->state == WL_KEYBOARD_KEY_STATE_RELEASED
-				&& !keyboard_any_modifiers_pressed(keyboard))  {
+				&& !keyboard_any_modifiers_pressed(wlr_keyboard))  {
 			if (server->osd_state.cycle_view) {
 				if (key_state_nr_keys()) {
 					should_cancel_cycling_on_next_key_release = true;
@@ -70,9 +70,7 @@ keyboard_modifiers_notify(struct wl_listener *listener, void *data)
 			}
 		}
 	}
-
+	wlr_seat_keyboard_notify_modifiers(seat->seat, &wlr_keyboard->modifiers);
-	wlr_seat_keyboard_notify_modifiers(seat->seat,
-		&seat->keyboard_group->keyboard.modifiers);
 }
 
 static bool
@@ -110,15 +108,16 @@ static bool
 handle_compositor_keybindings(struct wl_listener *listener,
 		struct wlr_keyboard_key_event *event)
 {
-	struct seat *seat = wl_container_of(listener, seat, keyboard_key);
+	struct keyboard *keyboard = wl_container_of(listener, keyboard, key);
+	struct seat *seat = keyboard->base.seat;
 	struct server *server = seat->server;
-	struct wlr_keyboard *keyboard = &seat->keyboard_group->keyboard;
+	struct wlr_keyboard *wlr_keyboard = keyboard->wlr_keyboard;
 
 	/* Translate libinput keycode -> xkbcommon */
 	uint32_t keycode = event->keycode + 8;
 	/* Get a list of keysyms based on the keymap for this keyboard */
 	const xkb_keysym_t *syms;
-	int nsyms = xkb_state_key_get_syms(keyboard->xkb_state, keycode, &syms);
+	int nsyms = xkb_state_key_get_syms(wlr_keyboard->xkb_state, keycode, &syms);
 
 	bool handled = false;
 
@@ -159,7 +158,7 @@ handle_compositor_keybindings(struct wl_listener *listener,
 		return true;
 	}
 
-	uint32_t modifiers = wlr_keyboard_get_modifiers(keyboard);
+	uint32_t modifiers = wlr_keyboard_get_modifiers(wlr_keyboard);
 
 	/* Catch C-A-F1 to C-A-F12 to change tty */
 	if (event->state == WL_KEYBOARD_KEY_STATE_PRESSED) {
@@ -232,21 +231,21 @@ out:
 	return handled;
 }
 
-static void
+void
 keyboard_key_notify(struct wl_listener *listener, void *data)
 {
 	/* This event is raised when a key is pressed or released. */
-	struct seat *seat = wl_container_of(listener, seat, keyboard_key);
+	struct keyboard *keyboard = wl_container_of(listener, keyboard, key);
-	struct server *server = seat->server;
+	struct seat *seat = keyboard->base.seat;
 	struct wlr_keyboard_key_event *event = data;
-	struct wlr_seat *wlr_seat = server->seat.seat;
+	struct wlr_seat *wlr_seat = seat->seat;
-	struct wlr_keyboard *keyboard = &seat->keyboard_group->keyboard;
+	struct wlr_keyboard *wlr_keyboard = keyboard->wlr_keyboard;
 	wlr_idle_notify_activity(seat->wlr_idle, seat->seat);
 
 	bool handled = handle_compositor_keybindings(listener, event);
 
 	if (!handled) {
-		wlr_seat_set_keyboard(wlr_seat, keyboard);
+		wlr_seat_set_keyboard(wlr_seat, wlr_keyboard);
 		wlr_seat_keyboard_notify_key(wlr_seat, event->time_msec,
 			event->keycode, event->state);
 	}
@@ -269,25 +268,16 @@ keyboard_init(struct seat *seat)
 	}
 	xkb_context_unref(context);
 	wlr_keyboard_set_repeat_info(kb, rc.repeat_rate, rc.repeat_delay);
-
-	seat->keyboard_key.notify = keyboard_key_notify;
-	wl_signal_add(&kb->events.key, &seat->keyboard_key);
-	seat->keyboard_modifiers.notify = keyboard_modifiers_notify;
-	wl_signal_add(&kb->events.modifiers, &seat->keyboard_modifiers);
 }
 
 void
 keyboard_finish(struct seat *seat)
 {
+	/*
+	 * All keyboard listeners must be removed before this to avoid use after
+	 * free
+	 */
 	if (seat->keyboard_group) {
-		/*
-		 * Caution - these event listeners are connected to
-		 * seat->keyboard_group->keyboard and must be
-		 * unregistered before wlr_keyboard_group_destroy(),
-		 * otherwise a use-after-free occurs.
-		 */
-		wl_list_remove(&seat->keyboard_key.link);
-		wl_list_remove(&seat->keyboard_modifiers.link);
 		wlr_keyboard_group_destroy(seat->keyboard_group);
 		seat->keyboard_group = NULL;
 	}

src/seat.c

@@ -18,6 +18,13 @@ input_device_destroy(struct wl_listener *listener, void *data)
 	struct input *input = wl_container_of(listener, input, destroy);
 	wl_list_remove(&input->link);
 	wl_list_remove(&input->destroy.link);
+
+	/* `struct keyboard` is derived and has some extra clean up to do */
+	if (input->wlr_input_device->type == WLR_INPUT_DEVICE_KEYBOARD) {
+		struct keyboard *keyboard = (struct keyboard *)input;
+		wl_list_remove(&keyboard->key.link);
+		wl_list_remove(&keyboard->modifier.link);
+	}
 	free(input);
 }
 
@@ -144,10 +151,12 @@ output_by_name(struct server *server, const char *name)
 	return NULL;
 }
 
-void
+static struct input *
-new_pointer(struct seat *seat, struct input *input)
+new_pointer(struct seat *seat, struct wlr_input_device *dev)
 {
-	struct wlr_input_device *dev = input->wlr_input_device;
+	struct input *input = znew(*input);
+	input->wlr_input_device = dev;
+
 	if (wlr_input_device_is_libinput(dev)) {
 		configure_libinput(dev);
 	}
@@ -164,22 +173,41 @@ new_pointer(struct seat *seat, struct input *input)
 		wlr_cursor_map_input_to_output(seat->cursor, dev, output);
 		wlr_cursor_map_input_to_region(seat->cursor, dev, NULL);
 	}
+	return input;
 }
 
-void
+static struct input *
-new_keyboard(struct seat *seat, struct input *input)
+new_keyboard(struct seat *seat, struct wlr_input_device *device, bool virtual)
 {
-	struct wlr_keyboard *kb =
+	struct wlr_keyboard *kb = wlr_keyboard_from_input_device(device);
-		wlr_keyboard_from_input_device(input->wlr_input_device);
+
+	struct keyboard *keyboard = znew(*keyboard);
+	keyboard->base.wlr_input_device = device;
+	keyboard->wlr_keyboard = kb;
+	keyboard->is_virtual = virtual;
+
 	wlr_keyboard_set_keymap(kb, seat->keyboard_group->keyboard.keymap);
-	wlr_keyboard_group_add_keyboard(seat->keyboard_group, kb);
+
+	if (!virtual) {
+		wlr_keyboard_group_add_keyboard(seat->keyboard_group, kb);
+	}
+
+	keyboard->key.notify = keyboard_key_notify;
+	wl_signal_add(&kb->events.key, &keyboard->key);
+	keyboard->modifier.notify = keyboard_modifiers_notify;
+	wl_signal_add(&kb->events.modifiers, &keyboard->modifier);
+
 	wlr_seat_set_keyboard(seat->seat, kb);
+
+	return (struct input *)keyboard;
 }
 
-void
+static struct input *
-new_touch(struct seat *seat, struct input *input)
+new_touch(struct seat *seat, struct wlr_input_device *dev)
 {
-	struct wlr_input_device *dev = input->wlr_input_device;
+	struct input *input = znew(*input);
+	input->wlr_input_device = dev;
+
 	if (wlr_input_device_is_libinput(dev)) {
 		configure_libinput(dev);
 	}
@@ -196,6 +224,7 @@ new_touch(struct seat *seat, struct input *input)
 		wlr_cursor_map_input_to_output(seat->cursor, dev, output);
 		wlr_cursor_map_input_to_region(seat->cursor, dev, NULL);
 	}
+	return input;
 }
 
 static void
@@ -203,7 +232,7 @@ seat_update_capabilities(struct seat *seat)
 {
 	struct input *input = NULL;
 	uint32_t caps = 0;
-	
+
 	wl_list_for_each(input, &seat->inputs, link) {
 		switch (input->wlr_input_device->type) {
 		case WLR_INPUT_DEVICE_KEYBOARD:
@@ -238,22 +267,21 @@ new_input_notify(struct wl_listener *listener, void *data)
 {
 	struct seat *seat = wl_container_of(listener, seat, new_input);
 	struct wlr_input_device *device = data;
-	struct input *input = znew(*input);
+	struct input *input = NULL;
-	input->wlr_input_device = device;
 
 	switch (device->type) {
 	case WLR_INPUT_DEVICE_KEYBOARD:
-		new_keyboard(seat, input);
+		input = new_keyboard(seat, device, false);
 		break;
 	case WLR_INPUT_DEVICE_POINTER:
-		new_pointer(seat, input);
+		input = new_pointer(seat, device);
 		break;
 	case WLR_INPUT_DEVICE_TOUCH:
-		new_touch(seat, input);
+		input = new_touch(seat, device);
 		break;
 	default:
 		wlr_log(WLR_INFO, "unsupported input device");
-		break;
+		return;
 	}
 
 	seat_add_device(seat, input);
@@ -293,14 +321,10 @@ new_virtual_pointer(struct wl_listener *listener, void *data)
 	struct wlr_virtual_pointer_v1_new_pointer_event *event = data;
 	struct wlr_virtual_pointer_v1 *pointer = event->new_pointer;
 	struct wlr_input_device *device = &pointer->pointer.base;
-	struct input *input = calloc(1, sizeof(struct input));
 
+	struct input *input = new_pointer(seat, device);
 	device->data = input;
-	input->wlr_input_device = device;
-
 	seat_add_device(seat, input);
-	new_pointer(seat, input);
-
 	if (event->suggested_output) {
 		wlr_cursor_map_input_to_output(seat->cursor, device,
 			event->suggested_output);
@@ -311,15 +335,12 @@ static void
 new_virtual_keyboard(struct wl_listener *listener, void *data)
 {
 	struct seat *seat = wl_container_of(listener, seat, virtual_keyboard_new);
-	struct wlr_virtual_keyboard_v1 *keyboard = data;
+	struct wlr_virtual_keyboard_v1 *virtual_keyboard = data;
-	struct wlr_input_device *device = &keyboard->keyboard.base;
+	struct wlr_input_device *device = &virtual_keyboard->keyboard.base;
-	struct input *input = calloc(1, sizeof(struct input));
 
+	struct input *input = new_keyboard(seat, device, true);
 	device->data = input;
-	input->wlr_input_device = device;
-
 	seat_add_device(seat, input);
-	new_keyboard(seat, input);
 }
 
 void
@@ -375,6 +396,12 @@ seat_finish(struct server *server)
 {
 	struct seat *seat = &server->seat;
 	wl_list_remove(&seat->new_input.link);
+
+	struct input *input, *next;
+	wl_list_for_each_safe(input, next, &seat->inputs, link) {
+		input_device_destroy(&input->destroy, NULL);
+	}
+
 	keyboard_finish(seat);
 	/*
 	 * Caution - touch_finish() unregisters event listeners from