interface/wlr_keyboard: rework destroy sequence

The destroy member in wlr_keyboard_impl has been removed. The function
`wlr_keyboard_finish` has been introduce to clean up the resources owned by a
wlr_keyboard.

`wlr_input_device_destroy` no longer destroys the wlr_keyboard, attempting to
destroy a wlr_keyboard will result in a no-op.

The field `name` has been added to the wlr_keyboard_impl to be able to identify
a given wlr_keyboard device.

backend/libinput/events.c

@@ -15,7 +15,7 @@
 
 void destroy_libinput_input_device(struct wlr_libinput_input_device *dev) {
 	if (dev->keyboard.impl) {
-		wlr_keyboard_destroy(&dev->keyboard);
+		wlr_keyboard_finish(&dev->keyboard);
 	}
 	if (dev->pointer.impl) {
 		wlr_pointer_destroy(&dev->pointer);

backend/libinput/keyboard.c

@@ -17,12 +17,8 @@ static void keyboard_set_leds(struct wlr_keyboard *wlr_kb, uint32_t leds) {
 	libinput_device_led_update(dev->handle, leds);
 }
 
-static void keyboard_destroy(struct wlr_keyboard *wlr_kb) {
-	/* wlr_keyboard belongs to the wlr_libinput_input_device */
-}
-
 const struct wlr_keyboard_impl libinput_keyboard_impl = {
-	.destroy = keyboard_destroy,
+	.name = "libinput-keyboard",
 	.led_update = keyboard_set_leds
 };
 

backend/wayland/seat.c

@@ -24,7 +24,9 @@
 #include "util/time.h"
 
 static const struct wlr_pointer_impl pointer_impl;
-static const struct wlr_keyboard_impl keyboard_impl;
+static const struct wlr_keyboard_impl keyboard_impl = {
+	.name = "wl-keyboard",
+};
 static const struct wlr_touch_impl touch_impl;
 
 static struct wlr_wl_pointer *output_get_pointer(
@@ -506,7 +508,16 @@ void destroy_wl_input_device(struct wlr_wl_input_device *dev) {
 	 */
 	wlr_input_device_finish(&dev->wlr_input_device);
 	if (dev->wlr_input_device._device) {
-		wlr_input_device_destroy(&dev->wlr_input_device);
+		struct wlr_input_device *wlr_dev = &dev->wlr_input_device;
+		switch (wlr_dev->type) {
+		case WLR_INPUT_DEVICE_KEYBOARD:
+			wlr_keyboard_finish(wlr_dev->keyboard);
+			free(wlr_dev->keyboard);
+			break;
+		default:
+			wlr_input_device_destroy(wlr_dev);
+			break;
+		}
 	}
 	wl_list_remove(&dev->link);
 	free(dev);

backend/x11/backend.c

@@ -185,7 +185,7 @@ static void backend_destroy(struct wlr_backend *backend) {
 		wlr_output_destroy(&output->wlr_output);
 	}
 
-	wlr_keyboard_destroy(&x11->keyboard);
+	wlr_keyboard_finish(&x11->keyboard);
 
 	wlr_backend_finish(backend);
 
@@ -637,7 +637,8 @@ struct wlr_backend *wlr_x11_backend_create(struct wl_display *display,
 	}
 #endif
 
-	wlr_keyboard_init(&x11->keyboard, &x11_keyboard_impl, "x11-keyboard");
+	wlr_keyboard_init(&x11->keyboard, &x11_keyboard_impl,
+		x11_keyboard_impl.name);
 
 	x11->display_destroy.notify = handle_display_destroy;
 	wl_display_add_destroy_listener(display, &x11->display_destroy);

backend/x11/input_device.c

@@ -285,12 +285,8 @@ void handle_x11_xinput_event(struct wlr_x11_backend *x11,
 	}
 }
 
-static void keyboard_destroy(struct wlr_keyboard *wlr_keyboard) {
-	// Don't free the keyboard, it's on the stack
-}
-
 const struct wlr_keyboard_impl x11_keyboard_impl = {
-	.destroy = keyboard_destroy,
+	.name = "x11-keyboard",
 };
 
 static void pointer_destroy(struct wlr_pointer *wlr_pointer) {

include/wlr/interfaces/wlr_keyboard.h

@@ -13,13 +13,18 @@
 #include <wlr/types/wlr_keyboard.h>
 
 struct wlr_keyboard_impl {
-	void (*destroy)(struct wlr_keyboard *keyboard);
+	const char *name;
 	void (*led_update)(struct wlr_keyboard *keyboard, uint32_t leds);
 };
 
 void wlr_keyboard_init(struct wlr_keyboard *keyboard,
 	const struct wlr_keyboard_impl *impl, const char *name);
-void wlr_keyboard_destroy(struct wlr_keyboard *keyboard);
+
+/**
+ * Cleans up all of the resources owned by wlr_keyboard.
+ */
+void wlr_keyboard_finish(struct wlr_keyboard *keyboard);
+
 void wlr_keyboard_notify_key(struct wlr_keyboard *keyboard,
 		struct wlr_event_keyboard_key *event);
 void wlr_keyboard_notify_modifiers(struct wlr_keyboard *keyboard,

types/wlr_input_device.c

@@ -2,7 +2,6 @@
 #include <stdlib.h>
 #include <string.h>
 #include <wayland-server-core.h>
-#include <wlr/interfaces/wlr_keyboard.h>
 #include <wlr/interfaces/wlr_pointer.h>
 #include <wlr/interfaces/wlr_switch.h>
 #include <wlr/interfaces/wlr_tablet_pad.h>
@@ -41,7 +40,7 @@ void wlr_input_device_destroy(struct wlr_input_device *dev) {
 	if (dev->_device) {
 		switch (dev->type) {
 		case WLR_INPUT_DEVICE_KEYBOARD:
-			wlr_keyboard_destroy(dev->keyboard);
+			wlr_log(WLR_ERROR, "wlr_keyboard will not be destroyed");
 			break;
 		case WLR_INPUT_DEVICE_POINTER:
 			wlr_pointer_destroy(dev->pointer);

types/wlr_keyboard.c

@@ -11,6 +11,7 @@
 #include "util/array.h"
 #include "util/shm.h"
 #include "util/signal.h"
+#include "util/time.h"
 
 void keyboard_led_update(struct wlr_keyboard *keyboard) {
 	if (keyboard->xkb_state == NULL) {
@@ -132,25 +133,31 @@ void wlr_keyboard_init(struct wlr_keyboard *kb,
 	kb->repeat_info.delay = 600;
 }
 
-void wlr_keyboard_destroy(struct wlr_keyboard *kb) {
+void wlr_keyboard_finish(struct wlr_keyboard *kb) {
-	if (kb == NULL) {
+	/* Release pressed keys */
-		return;
+	size_t orig_num_keycodes = kb->num_keycodes;
+	for (size_t i = 0; i < orig_num_keycodes; ++i) {
+		assert(kb->num_keycodes == orig_num_keycodes - i);
+		struct wlr_event_keyboard_key event = {
+			.time_msec = get_current_time_msec(),
+			.keycode = kb->keycodes[orig_num_keycodes - i - 1],
+			.update_state = false,
+			.state = WL_KEYBOARD_KEY_STATE_RELEASED,
+		};
+		wlr_keyboard_notify_key(kb, &event);  // updates num_keycodes
 	}
+
 	wlr_signal_emit_safe(&kb->events.destroy, kb);
+
 	wlr_input_device_finish(&kb->base);
 
+	/* Finish xkbcommon resources */
 	xkb_state_unref(kb->xkb_state);
 	xkb_keymap_unref(kb->keymap);
 	free(kb->keymap_string);
 	if (kb->keymap_fd >= 0) {
 		close(kb->keymap_fd);
 	}
-	if (kb->impl && kb->impl->destroy) {
-		kb->impl->destroy(kb);
-	} else {
-		wl_list_remove(&kb->events.key.listener_list);
-		free(kb);
-	}
 }
 
 void wlr_keyboard_led_update(struct wlr_keyboard *kb, uint32_t leds) {

types/wlr_keyboard_group.c

@@ -37,18 +37,8 @@ static void keyboard_set_leds(struct wlr_keyboard *kb, uint32_t leds) {
 	}
 }
 
-static void keyboard_destroy(struct wlr_keyboard *kb) {
-	// Just remove the event listeners. The keyboard will be freed as part of
-	// the wlr_keyboard_group in wlr_keyboard_group_destroy.
-	wl_list_remove(&kb->events.key.listener_list);
-	wl_list_remove(&kb->events.modifiers.listener_list);
-	wl_list_remove(&kb->events.keymap.listener_list);
-	wl_list_remove(&kb->events.repeat_info.listener_list);
-	wl_list_remove(&kb->events.destroy.listener_list);
-}
-
 static const struct wlr_keyboard_impl impl = {
-	.destroy = keyboard_destroy,
+	.name = "keyboard-group",
 	.led_update = keyboard_set_leds
 };
 
@@ -60,7 +50,7 @@ struct wlr_keyboard_group *wlr_keyboard_group_create(void) {
 		return NULL;
 	}
 
-	wlr_keyboard_init(&group->keyboard, &impl, "keyboard-group");
+	wlr_keyboard_init(&group->keyboard, &impl, "wlr_keyboard_group");
 	wl_list_init(&group->devices);
 	wl_list_init(&group->keys);
 
@@ -325,7 +315,7 @@ void wlr_keyboard_group_destroy(struct wlr_keyboard_group *group) {
 	wl_list_for_each_safe(device, tmp, &group->devices, link) {
 		wlr_keyboard_group_remove_keyboard(group, device->keyboard);
 	}
-	wlr_keyboard_destroy(&group->keyboard);
+	wlr_keyboard_finish(&group->keyboard);
 	wl_list_remove(&group->events.enter.listener_list);
 	wl_list_remove(&group->events.leave.listener_list);
 	free(group);

types/wlr_virtual_keyboard_v1.c

@@ -9,36 +9,10 @@
 #include <wlr/util/log.h>
 #include <xkbcommon/xkbcommon.h>
 #include "util/signal.h"
-#include "util/time.h"
 #include "virtual-keyboard-unstable-v1-protocol.h"
 
-/**
- * Send release event for each pressed key to bring the keyboard back to
- * neutral state.
- *
- * This may be needed for virtual keyboards. For physical devices, kernel
- * or libinput will deal with the removal of devices.
- */
-static void keyboard_release_pressed_keys(struct wlr_keyboard *keyboard) {
-	size_t orig_num_keycodes = keyboard->num_keycodes;
-	for (size_t i = 0; i < orig_num_keycodes; ++i) {
-		assert(keyboard->num_keycodes == orig_num_keycodes - i);
-		struct wlr_event_keyboard_key event = {
-			.time_msec = get_current_time_msec(),
-			.keycode = keyboard->keycodes[orig_num_keycodes - i - 1],
-			.update_state = false,
-			.state = WL_KEYBOARD_KEY_STATE_RELEASED,
-		};
-		wlr_keyboard_notify_key(keyboard, &event);  // updates num_keycodes
-	}
-}
-
-static void keyboard_destroy(struct wlr_keyboard *wlr_kb) {
-	/* no-op, keyboard belongs to the wlr_virtual_keyboard_v1 */
-}
-
 static const struct wlr_keyboard_impl keyboard_impl = {
-	.destroy = keyboard_destroy,
+	.name = "virtual-keyboard",
 };
 
 static const struct zwp_virtual_keyboard_v1_interface virtual_keyboard_impl;
@@ -135,10 +109,10 @@ static void virtual_keyboard_destroy_resource(struct wl_resource *resource) {
 		return;
 	}
 
-	/* TODO: rework wlr_keyboard device destruction */
-	keyboard_release_pressed_keys(&keyboard->keyboard);
 	wlr_signal_emit_safe(&keyboard->events.destroy, keyboard);
-	wlr_keyboard_destroy(&keyboard->keyboard);
+
+	wlr_keyboard_finish(&keyboard->keyboard);
+
 	wl_resource_set_user_data(keyboard->resource, NULL);
 	wl_list_remove(&keyboard->link);
 	free(keyboard);
@@ -179,7 +153,7 @@ static void virtual_keyboard_manager_create_virtual_keyboard(
 	}
 
 	wlr_keyboard_init(&virtual_keyboard->keyboard, &keyboard_impl,
-		"virtual-keyboard");
+		"wlr_virtual_keyboard_v1");
 
 	struct wl_resource *keyboard_resource = wl_resource_create(client,
 		&zwp_virtual_keyboard_v1_interface, wl_resource_get_version(resource),