xdg-{shell,decoration}: improve events

This commit changes the behavior of `new_*` signals to better match
their names (see https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3608).
wlr_xdg_shell.events.new_surface is now emitted when an xdg_surface is
created, and wlr_xdg_shell.events.new_{toplevel,popup} events are
introduced to get notified when an xdg_{toplevel,popup} is created.
Same applies to
`wlr_xdg_decoration_manager_v1.events.new_toplevel_decoration`. As a
result, wlr_xdg_surface.added and wlr_xdg_toplevel_decoration_v1.added
are removed, as we no longer need to track whether the corresponding
event was emitted.

Additionally, this commit changes the behavior of
wlr_xdg_surface.events.destroy: it is now emitted when the xdg_surface
is destroyed, as the name suggests.
wlr_xdg_{toplevel,popup}.events.destroy events are added to get
notified when an xdg_{toplevel,popup} is destroyed.

12 files changed, 95 insertions, 122 deletions

diff --git a/include/wlr/types/wlr_xdg_decoration_v1.h b/include/wlr/types/wlr_xdg_decoration_v1.h
index 61ce4bc1..d8ec038d 100644
--- a/include/wlr/types/wlr_xdg_decoration_v1.h
+++ b/include/wlr/types/wlr_xdg_decoration_v1.h
@@ -45,8 +45,6 @@ struct wlr_xdg_toplevel_decoration_v1 {
 	enum wlr_xdg_toplevel_decoration_v1_mode scheduled_mode;
 	enum wlr_xdg_toplevel_decoration_v1_mode requested_mode;
 
-	bool added;
-
 	struct wl_list configure_list; // wlr_xdg_toplevel_decoration_v1_configure.link
 
 	struct {
@@ -54,7 +52,7 @@ struct wlr_xdg_toplevel_decoration_v1 {
 		struct wl_signal request_mode;
 	} events;
 
-	struct wl_listener surface_destroy;
+	struct wl_listener toplevel_destroy;
 	struct wl_listener surface_configure;
 	struct wl_listener surface_ack_configure;
 	struct wl_listener surface_commit;
diff --git a/include/wlr/types/wlr_xdg_foreign_v1.h b/include/wlr/types/wlr_xdg_foreign_v1.h
index 45f060df..bb26fcfc 100644
--- a/include/wlr/types/wlr_xdg_foreign_v1.h
+++ b/include/wlr/types/wlr_xdg_foreign_v1.h
@@ -34,7 +34,7 @@ struct wlr_xdg_exported_v1 {
 	struct wlr_xdg_foreign_exported base;
 
 	struct wl_resource *resource;
-	struct wl_listener xdg_surface_destroy;
+	struct wl_listener xdg_toplevel_destroy;
 
 	struct wl_list link; // wlr_xdg_foreign_v1.exporter.objects
 };
@@ -54,7 +54,7 @@ struct wlr_xdg_imported_child_v1 {
 
 	struct wl_list link; // wlr_xdg_imported_v1.children
 
-	struct wl_listener xdg_surface_destroy;
+	struct wl_listener xdg_toplevel_destroy;
 	struct wl_listener xdg_toplevel_set_parent;
 };
 
diff --git a/include/wlr/types/wlr_xdg_foreign_v2.h b/include/wlr/types/wlr_xdg_foreign_v2.h
index b091b459..d82854b4 100644
--- a/include/wlr/types/wlr_xdg_foreign_v2.h
+++ b/include/wlr/types/wlr_xdg_foreign_v2.h
@@ -34,7 +34,7 @@ struct wlr_xdg_exported_v2 {
 	struct wlr_xdg_foreign_exported base;
 
 	struct wl_resource *resource;
-	struct wl_listener xdg_surface_destroy;
+	struct wl_listener xdg_toplevel_destroy;
 
 	struct wl_list link; // wlr_xdg_foreign_v2.exporter.objects
 };
@@ -54,7 +54,7 @@ struct wlr_xdg_imported_child_v2 {
 
 	struct wl_list link; // wlr_xdg_imported_v2.children
 
-	struct wl_listener xdg_surface_destroy;
+	struct wl_listener xdg_toplevel_destroy;
 	struct wl_listener xdg_toplevel_set_parent;
 };
 
diff --git a/include/wlr/types/wlr_xdg_shell.h b/include/wlr/types/wlr_xdg_shell.h
index 29ced596..9df690f9 100644
--- a/include/wlr/types/wlr_xdg_shell.h
+++ b/include/wlr/types/wlr_xdg_shell.h
@@ -26,6 +26,8 @@ struct wlr_xdg_shell {
 
 	struct {
 		struct wl_signal new_surface; // struct wlr_xdg_surface
+		struct wl_signal new_toplevel; // struct wlr_xdg_toplevel
+		struct wl_signal new_popup; // struct wlr_xdg_popup
 		struct wl_signal destroy;
 	} events;
 
@@ -102,6 +104,8 @@ struct wlr_xdg_popup {
 	struct wlr_xdg_popup_state current, pending;
 
 	struct {
+		struct wl_signal destroy;
+
 		struct wl_signal reposition;
 	} events;
 
@@ -184,6 +188,8 @@ struct wlr_xdg_toplevel {
 	char *app_id;
 
 	struct {
+		struct wl_signal destroy;
+
 		// Note: as per xdg-shell protocol, the compositor has to
 		// handle state requests by sending a configure event,
 		// even if it didn't actually change the state. Therefore,
@@ -249,7 +255,7 @@ struct wlr_xdg_surface {
 
 	struct wl_list popups; // wlr_xdg_popup.link
 
-	bool added, configured;
+	bool configured;
 	struct wl_event_source *configure_idle;
 	uint32_t scheduled_serial;
 	struct wl_list configure_list;
diff --git a/tinywl/tinywl.c b/tinywl/tinywl.c
index 5a57addd..1dfca170 100644
--- a/tinywl/tinywl.c
+++ b/tinywl/tinywl.c
@@ -42,7 +42,8 @@ struct tinywl_server {
 	struct wlr_scene_output_layout *scene_layout;
 
 	struct wlr_xdg_shell *xdg_shell;
-	struct wl_listener new_xdg_surface;
+	struct wl_listener new_xdg_toplevel;
+	struct wl_listener new_xdg_popup;
 	struct wl_list toplevels;
 
 	struct wlr_cursor *cursor;
@@ -767,58 +768,53 @@ static void xdg_toplevel_request_fullscreen(
 	wlr_xdg_surface_schedule_configure(toplevel->xdg_toplevel->base);
 }
 
-static void server_new_xdg_surface(struct wl_listener *listener, void *data) {
-	/* This event is raised when wlr_xdg_shell receives a new xdg surface from a
-	 * client, either a toplevel (application window) or popup. */
-	struct tinywl_server *server =
-		wl_container_of(listener, server, new_xdg_surface);
-	struct wlr_xdg_surface *xdg_surface = data;
-
-	/* We must add xdg popups to the scene graph so they get rendered. The
-	 * wlroots scene graph provides a helper for this, but to use it we must
-	 * provide the proper parent scene node of the xdg popup. To enable this,
-	 * we always set the user data field of xdg_surfaces to the corresponding
-	 * scene node. */
-	if (xdg_surface->role == WLR_XDG_SURFACE_ROLE_POPUP) {
-		struct wlr_xdg_surface *parent =
-			wlr_xdg_surface_try_from_wlr_surface(xdg_surface->popup->parent);
-		assert(parent != NULL);
-		struct wlr_scene_tree *parent_tree = parent->data;
-		xdg_surface->data = wlr_scene_xdg_surface_create(
-			parent_tree, xdg_surface);
-		return;
-	}
-	assert(xdg_surface->role == WLR_XDG_SURFACE_ROLE_TOPLEVEL);
+static void server_new_xdg_toplevel(struct wl_listener *listener, void *data) {
+	/* This event is raised when a client creates a new toplevel (application window). */
+	struct tinywl_server *server = wl_container_of(listener, server, new_xdg_toplevel);
+	struct wlr_xdg_toplevel *xdg_toplevel = data;
 
 	/* Allocate a tinywl_toplevel for this surface */
 	struct tinywl_toplevel *toplevel = calloc(1, sizeof(*toplevel));
 	toplevel->server = server;
-	toplevel->xdg_toplevel = xdg_surface->toplevel;
-	toplevel->scene_tree = wlr_scene_xdg_surface_create(
-			&toplevel->server->scene->tree, toplevel->xdg_toplevel->base);
+	toplevel->xdg_toplevel = xdg_toplevel;
+	toplevel->scene_tree =
+		wlr_scene_xdg_surface_create(&toplevel->server->scene->tree, xdg_toplevel->base);
 	toplevel->scene_tree->node.data = toplevel;
-	xdg_surface->data = toplevel->scene_tree;
+	xdg_toplevel->base->data = toplevel->scene_tree;
 
 	/* Listen to the various events it can emit */
 	toplevel->map.notify = xdg_toplevel_map;
-	wl_signal_add(&xdg_surface->surface->events.map, &toplevel->map);
+	wl_signal_add(&xdg_toplevel->base->surface->events.map, &toplevel->map);
 	toplevel->unmap.notify = xdg_toplevel_unmap;
-	wl_signal_add(&xdg_surface->surface->events.unmap, &toplevel->unmap);
+	wl_signal_add(&xdg_toplevel->base->surface->events.unmap, &toplevel->unmap);
+
 	toplevel->destroy.notify = xdg_toplevel_destroy;
-	wl_signal_add(&xdg_surface->events.destroy, &toplevel->destroy);
+	wl_signal_add(&xdg_toplevel->events.destroy, &toplevel->destroy);
 
 	/* cotd */
-	struct wlr_xdg_toplevel *xdg_toplevel = xdg_surface->toplevel;
 	toplevel->request_move.notify = xdg_toplevel_request_move;
 	wl_signal_add(&xdg_toplevel->events.request_move, &toplevel->request_move);
 	toplevel->request_resize.notify = xdg_toplevel_request_resize;
 	wl_signal_add(&xdg_toplevel->events.request_resize, &toplevel->request_resize);
 	toplevel->request_maximize.notify = xdg_toplevel_request_maximize;
-	wl_signal_add(&xdg_toplevel->events.request_maximize,
-		&toplevel->request_maximize);
+	wl_signal_add(&xdg_toplevel->events.request_maximize, &toplevel->request_maximize);
 	toplevel->request_fullscreen.notify = xdg_toplevel_request_fullscreen;
-	wl_signal_add(&xdg_toplevel->events.request_fullscreen,
-		&toplevel->request_fullscreen);
+	wl_signal_add(&xdg_toplevel->events.request_fullscreen, &toplevel->request_fullscreen);
+}
+
+static void server_new_xdg_popup(struct wl_listener *listener, void *data) {
+	/* This event is raised when a client creates a new popup. */
+	struct wlr_xdg_popup *xdg_popup = data;
+
+	/* We must add xdg popups to the scene graph so they get rendered. The
+	 * wlroots scene graph provides a helper for this, but to use it we must
+	 * provide the proper parent scene node of the xdg popup. To enable this,
+	 * we always set the user data field of xdg_surfaces to the corresponding
+	 * scene node. */
+	struct wlr_xdg_surface *parent = wlr_xdg_surface_try_from_wlr_surface(xdg_popup->parent);
+	assert(parent != NULL);
+	struct wlr_scene_tree *parent_tree = parent->data;
+	xdg_popup->base->data = wlr_scene_xdg_surface_create(parent_tree, xdg_popup->base);
 }
 
 int main(int argc, char *argv[]) {
@@ -914,9 +910,10 @@ int main(int argc, char *argv[]) {
 	 */
 	wl_list_init(&server.toplevels);
 	server.xdg_shell = wlr_xdg_shell_create(server.wl_display, 3);
-	server.new_xdg_surface.notify = server_new_xdg_surface;
-	wl_signal_add(&server.xdg_shell->events.new_surface,
-			&server.new_xdg_surface);
+	server.new_xdg_toplevel.notify = server_new_xdg_toplevel;
+	wl_signal_add(&server.xdg_shell->events.new_toplevel, &server.new_xdg_toplevel);
+	server.new_xdg_popup.notify = server_new_xdg_popup;
+	wl_signal_add(&server.xdg_shell->events.new_popup, &server.new_xdg_popup);
 
 	/*
 	 * Creates a cursor, which is a wlroots utility for tracking the cursor
diff --git a/types/wlr_xdg_decoration_v1.c b/types/wlr_xdg_decoration_v1.c
index 0cc8f6d9..9d5cf747 100644
--- a/types/wlr_xdg_decoration_v1.c
+++ b/types/wlr_xdg_decoration_v1.c
@@ -63,7 +63,7 @@ static void toplevel_decoration_handle_resource_destroy(
 		toplevel_decoration_from_resource(resource);
 	wl_signal_emit_mutable(&decoration->events.destroy, decoration);
 	wl_list_remove(&decoration->surface_commit.link);
-	wl_list_remove(&decoration->surface_destroy.link);
+	wl_list_remove(&decoration->toplevel_destroy.link);
 	wl_list_remove(&decoration->surface_configure.link);
 	wl_list_remove(&decoration->surface_ack_configure.link);
 	struct wlr_xdg_toplevel_decoration_v1_configure *configure, *tmp;
@@ -74,10 +74,10 @@ static void toplevel_decoration_handle_resource_destroy(
 	free(decoration);
 }
 
-static void toplevel_decoration_handle_surface_destroy(
+static void toplevel_decoration_handle_toplevel_destroy(
 		struct wl_listener *listener, void *data) {
 	struct wlr_xdg_toplevel_decoration_v1 *decoration =
-		wl_container_of(listener, decoration, surface_destroy);
+		wl_container_of(listener, decoration, toplevel_destroy);
 
 	wl_resource_post_error(decoration->resource,
 		ZXDG_TOPLEVEL_DECORATION_V1_ERROR_ORPHANED,
@@ -145,15 +145,8 @@ static void toplevel_decoration_handle_surface_commit(
 		struct wl_listener *listener, void *data) {
 	struct wlr_xdg_toplevel_decoration_v1 *decoration =
 		wl_container_of(listener, decoration, surface_commit);
-	struct wlr_xdg_decoration_manager_v1 *manager = decoration->manager;
 
 	decoration->current = decoration->pending;
-
-	if (decoration->toplevel->base->added && !decoration->added) {
-		decoration->added = true;
-		wl_signal_emit_mutable(&manager->events.new_toplevel_decoration,
-			decoration);
-	}
 }
 
 static const struct zxdg_decoration_manager_v1_interface decoration_manager_impl;
@@ -223,30 +216,18 @@ static void decoration_manager_handle_get_toplevel_decoration(
 	wl_signal_init(&decoration->events.destroy);
 	wl_signal_init(&decoration->events.request_mode);
 
-	wl_signal_add(&toplevel->base->events.destroy,
-		&decoration->surface_destroy);
-	decoration->surface_destroy.notify =
-		toplevel_decoration_handle_surface_destroy;
-	wl_signal_add(&toplevel->base->events.configure,
-		&decoration->surface_configure);
-	decoration->surface_configure.notify =
-		toplevel_decoration_handle_surface_configure;
-	wl_signal_add(&toplevel->base->events.ack_configure,
-		&decoration->surface_ack_configure);
-	decoration->surface_ack_configure.notify =
-		toplevel_decoration_handle_surface_ack_configure;
-	wl_signal_add(&toplevel->base->surface->events.commit,
-		&decoration->surface_commit);
-	decoration->surface_commit.notify =
-		toplevel_decoration_handle_surface_commit;
+	wl_signal_add(&toplevel->events.destroy, &decoration->toplevel_destroy);
+	decoration->toplevel_destroy.notify = toplevel_decoration_handle_toplevel_destroy;
+	wl_signal_add(&toplevel->base->events.configure, &decoration->surface_configure);
+	decoration->surface_configure.notify = toplevel_decoration_handle_surface_configure;
+	wl_signal_add(&toplevel->base->events.ack_configure, &decoration->surface_ack_configure);
+	decoration->surface_ack_configure.notify = toplevel_decoration_handle_surface_ack_configure;
+	wl_signal_add(&toplevel->base->surface->events.commit, &decoration->surface_commit);
+	decoration->surface_commit.notify = toplevel_decoration_handle_surface_commit;
 
 	wl_list_insert(&manager->decorations, &decoration->link);
 
-	if (toplevel->base->added) {
-		decoration->added = true;
-		wl_signal_emit_mutable(&manager->events.new_toplevel_decoration,
-			decoration);
-	}
+	wl_signal_emit_mutable(&manager->events.new_toplevel_decoration, decoration);
 }
 
 static const struct zxdg_decoration_manager_v1_interface
diff --git a/types/wlr_xdg_foreign_v1.c b/types/wlr_xdg_foreign_v1.c
index a6cb01f3..d4bcc6a7 100644
--- a/types/wlr_xdg_foreign_v1.c
+++ b/types/wlr_xdg_foreign_v1.c
@@ -40,15 +40,15 @@ static struct wlr_xdg_toplevel *verify_is_toplevel(struct wl_resource *resource,
 
 static void destroy_imported_child(struct wlr_xdg_imported_child_v1 *child) {
 	wl_list_remove(&child->xdg_toplevel_set_parent.link);
-	wl_list_remove(&child->xdg_surface_destroy.link);
+	wl_list_remove(&child->xdg_toplevel_destroy.link);
 	wl_list_remove(&child->link);
 	free(child);
 }
 
-static void handle_child_xdg_surface_destroy(
+static void handle_child_xdg_toplevel_destroy(
 		struct wl_listener *listener, void *data) {
 	struct wlr_xdg_imported_child_v1 *child =
-		wl_container_of(listener, child, xdg_surface_destroy);
+		wl_container_of(listener, child, xdg_toplevel_destroy);
 	destroy_imported_child(child);
 }
 
@@ -98,7 +98,7 @@ static void xdg_imported_handle_set_parent_of(struct wl_client *client,
 		return;
 	}
 	child->surface = wlr_surface_child;
-	child->xdg_surface_destroy.notify = handle_child_xdg_surface_destroy;
+	child->xdg_toplevel_destroy.notify = handle_child_xdg_toplevel_destroy;
 	child->xdg_toplevel_set_parent.notify = handle_xdg_toplevel_set_parent;
 
 	if (!wlr_xdg_toplevel_set_parent(child_toplevel, surface->toplevel)) {
@@ -110,10 +110,8 @@ static void xdg_imported_handle_set_parent_of(struct wl_client *client,
 	}
 
 	wlr_xdg_toplevel_set_parent(child_toplevel, surface->toplevel);
-	wl_signal_add(&child_toplevel->base->events.destroy,
-			&child->xdg_surface_destroy);
-	wl_signal_add(&child_toplevel->events.set_parent,
-			&child->xdg_toplevel_set_parent);
+	wl_signal_add(&child_toplevel->events.destroy, &child->xdg_toplevel_destroy);
+	wl_signal_add(&child_toplevel->events.set_parent, &child->xdg_toplevel_set_parent);
 
 	wl_list_insert(&imported->children, &child->link);
 }
@@ -173,7 +171,7 @@ static void destroy_imported(struct wlr_xdg_imported_v1 *imported) {
 static void destroy_exported(struct wlr_xdg_exported_v1 *exported) {
 	wlr_xdg_foreign_exported_finish(&exported->base);
 
-	wl_list_remove(&exported->xdg_surface_destroy.link);
+	wl_list_remove(&exported->xdg_toplevel_destroy.link);
 	wl_list_remove(&exported->link);
 	wl_resource_set_user_data(exported->resource, NULL);
 	free(exported);
@@ -189,10 +187,9 @@ static void xdg_exported_handle_resource_destroy(
 	}
 }
 
-static void handle_xdg_surface_destroy(
-		struct wl_listener *listener, void *data) {
+static void handle_xdg_toplevel_destroy(struct wl_listener *listener, void *data) {
 	struct wlr_xdg_exported_v1 *exported =
-		wl_container_of(listener, exported, xdg_surface_destroy);
+		wl_container_of(listener, exported, xdg_toplevel_destroy);
 
 	destroy_exported(exported);
 }
@@ -240,8 +237,8 @@ static void xdg_exporter_handle_export(struct wl_client *wl_client,
 
 	zxdg_exported_v1_send_handle(exported->resource, exported->base.handle);
 
-	exported->xdg_surface_destroy.notify = handle_xdg_surface_destroy;
-	wl_signal_add(&xdg_toplevel->base->events.destroy, &exported->xdg_surface_destroy);
+	exported->xdg_toplevel_destroy.notify = handle_xdg_toplevel_destroy;
+	wl_signal_add(&xdg_toplevel->base->events.destroy, &exported->xdg_toplevel_destroy);
 }
 
 static const struct zxdg_exporter_v1_interface xdg_exporter_impl = {
diff --git a/types/wlr_xdg_foreign_v2.c b/types/wlr_xdg_foreign_v2.c
index eb0368af..f600d533 100644
--- a/types/wlr_xdg_foreign_v2.c
+++ b/types/wlr_xdg_foreign_v2.c
@@ -45,15 +45,15 @@ static struct wlr_xdg_toplevel *verify_is_toplevel(struct wl_resource *resource,
 
 static void destroy_imported_child(struct wlr_xdg_imported_child_v2 *child) {
 	wl_list_remove(&child->xdg_toplevel_set_parent.link);
-	wl_list_remove(&child->xdg_surface_destroy.link);
+	wl_list_remove(&child->xdg_toplevel_destroy.link);
 	wl_list_remove(&child->link);
 	free(child);
 }
 
-static void handle_child_xdg_surface_destroy(
+static void handle_child_xdg_toplevel_destroy(
 		struct wl_listener *listener, void *data) {
 	struct wlr_xdg_imported_child_v2 *child =
-		wl_container_of(listener, child, xdg_surface_destroy);
+		wl_container_of(listener, child, xdg_toplevel_destroy);
 	destroy_imported_child(child);
 }
 
@@ -101,7 +101,7 @@ static void xdg_imported_handle_set_parent_of(struct wl_client *client,
 		return;
 	}
 	child->surface = wlr_surface_child;
-	child->xdg_surface_destroy.notify = handle_child_xdg_surface_destroy;
+	child->xdg_toplevel_destroy.notify = handle_child_xdg_toplevel_destroy;
 	child->xdg_toplevel_set_parent.notify = handle_xdg_toplevel_set_parent;
 
 	if (!wlr_xdg_toplevel_set_parent(child_toplevel, surface->toplevel)) {
@@ -113,10 +113,8 @@ static void xdg_imported_handle_set_parent_of(struct wl_client *client,
 	}
 
 	wlr_xdg_toplevel_set_parent(child_toplevel, surface->toplevel);
-	wl_signal_add(&child_toplevel->base->events.destroy,
-			&child->xdg_surface_destroy);
-	wl_signal_add(&child_toplevel->events.set_parent,
-			&child->xdg_toplevel_set_parent);
+	wl_signal_add(&child_toplevel->events.destroy, &child->xdg_toplevel_destroy);
+	wl_signal_add(&child_toplevel->events.set_parent, &child->xdg_toplevel_set_parent);
 
 	wl_list_insert(&imported->children, &child->link);
 }
@@ -176,7 +174,7 @@ static void destroy_imported(struct wlr_xdg_imported_v2 *imported) {
 static void destroy_exported(struct wlr_xdg_exported_v2 *exported) {
 	wlr_xdg_foreign_exported_finish(&exported->base);
 
-	wl_list_remove(&exported->xdg_surface_destroy.link);
+	wl_list_remove(&exported->xdg_toplevel_destroy.link);
 	wl_list_remove(&exported->link);
 	wl_resource_set_user_data(exported->resource, NULL);
 	free(exported);
@@ -192,10 +190,9 @@ static void xdg_exported_handle_resource_destroy(
 	}
 }
 
-static void handle_xdg_surface_destroy(
-		struct wl_listener *listener, void *data) {
+static void handle_xdg_toplevel_destroy(struct wl_listener *listener, void *data) {
 	struct wlr_xdg_exported_v2 *exported =
-		wl_container_of(listener, exported, xdg_surface_destroy);
+		wl_container_of(listener, exported, xdg_toplevel_destroy);
 
 	destroy_exported(exported);
 }
@@ -243,8 +240,8 @@ static void xdg_exporter_handle_export(struct wl_client *wl_client,
 
 	zxdg_exported_v2_send_handle(exported->resource, exported->base.handle);
 
-	exported->xdg_surface_destroy.notify = handle_xdg_surface_destroy;
-	wl_signal_add(&xdg_toplevel->base->events.destroy, &exported->xdg_surface_destroy);
+	exported->xdg_toplevel_destroy.notify = handle_xdg_toplevel_destroy;
+	wl_signal_add(&xdg_toplevel->base->events.destroy, &exported->xdg_toplevel_destroy);
 }
 
 static const struct zxdg_exporter_v2_interface xdg_exporter_impl = {
diff --git a/types/xdg_shell/wlr_xdg_popup.c b/types/xdg_shell/wlr_xdg_popup.c
index e8035878..6a6a6e43 100644
--- a/types/xdg_shell/wlr_xdg_popup.c
+++ b/types/xdg_shell/wlr_xdg_popup.c
@@ -415,6 +415,7 @@ void create_xdg_popup(struct wlr_xdg_surface *surface,
 		&positioner->rules, &surface->popup->scheduled.geometry);
 	surface->popup->scheduled.rules = positioner->rules;
 
+	wl_signal_init(&surface->popup->events.destroy);
 	wl_signal_init(&surface->popup->events.reposition);
 
 	if (parent) {
@@ -426,6 +427,8 @@ void create_xdg_popup(struct wlr_xdg_surface *surface,
 	}
 
 	set_xdg_surface_role_object(surface, surface->popup->resource);
+
+	wl_signal_emit_mutable(&surface->client->shell->events.new_popup, surface->popup);
 }
 
 void reset_xdg_popup(struct wlr_xdg_popup *popup) {
@@ -460,14 +463,9 @@ void destroy_xdg_popup(struct wlr_xdg_popup *popup) {
 	wlr_surface_unmap(popup->base->surface);
 	reset_xdg_popup(popup);
 
-	// TODO: improve events
-	if (popup->base->added) {
-		wl_signal_emit_mutable(&popup->base->events.destroy, NULL);
-		popup->base->added = false;
-	}
+	wl_signal_emit_mutable(&popup->events.destroy, NULL);
 
 	popup->base->popup = NULL;
-
 	wl_list_remove(&popup->link);
 	wl_resource_set_user_data(popup->resource, NULL);
 	free(popup);
diff --git a/types/xdg_shell/wlr_xdg_shell.c b/types/xdg_shell/wlr_xdg_shell.c
index 33e99e33..3baea04e 100644
--- a/types/xdg_shell/wlr_xdg_shell.c
+++ b/types/xdg_shell/wlr_xdg_shell.c
@@ -158,6 +158,8 @@ struct wlr_xdg_shell *wlr_xdg_shell_create(struct wl_display *display,
 	xdg_shell->global = global;
 
 	wl_signal_init(&xdg_shell->events.new_surface);
+	wl_signal_init(&xdg_shell->events.new_toplevel);
+	wl_signal_init(&xdg_shell->events.new_popup);
 	wl_signal_init(&xdg_shell->events.destroy);
 
 	xdg_shell->display_destroy.notify = handle_display_destroy;
diff --git a/types/xdg_shell/wlr_xdg_surface.c b/types/xdg_shell/wlr_xdg_surface.c
index 28d5dbad..035d345e 100644
--- a/types/xdg_shell/wlr_xdg_surface.c
+++ b/types/xdg_shell/wlr_xdg_surface.c
@@ -307,12 +307,6 @@ static void xdg_surface_role_commit(struct wlr_surface *wlr_surface) {
 		break;
 	}
 
-	if (!surface->added) {
-		surface->added = true;
-		wl_signal_emit_mutable(&surface->client->shell->events.new_surface,
-			surface);
-	}
-
 	if (wlr_surface_has_buffer(wlr_surface)) {
 		wlr_surface_map(wlr_surface);
 	}
@@ -392,6 +386,8 @@ void create_xdg_surface(struct wlr_xdg_client *client, struct wlr_surface *wlr_s
 	wl_list_insert(&client->surfaces, &surface->link);
 
 	wlr_surface_set_role_object(wlr_surface, surface->resource);
+
+	wl_signal_emit_mutable(&surface->client->shell->events.new_surface, surface);
 }
 
 bool set_xdg_surface_role(struct wlr_xdg_surface *surface, enum wlr_xdg_surface_role role) {
@@ -466,6 +462,8 @@ void destroy_xdg_surface(struct wlr_xdg_surface *surface) {
 	destroy_xdg_surface_role_object(surface);
 	reset_xdg_surface(surface);
 
+	wl_signal_emit_mutable(&surface->events.destroy, NULL);
+
 	wl_list_remove(&surface->link);
 
 	wl_resource_set_user_data(surface->resource, NULL);
diff --git a/types/xdg_shell/wlr_xdg_toplevel.c b/types/xdg_shell/wlr_xdg_toplevel.c
index 677c2821..3fe49c5a 100644
--- a/types/xdg_shell/wlr_xdg_toplevel.c
+++ b/types/xdg_shell/wlr_xdg_toplevel.c
@@ -476,6 +476,7 @@ void create_xdg_toplevel(struct wlr_xdg_surface *surface,
 	}
 	surface->toplevel->base = surface;
 
+	wl_signal_init(&surface->toplevel->events.destroy);
 	wl_signal_init(&surface->toplevel->events.request_maximize);
 	wl_signal_init(&surface->toplevel->events.request_fullscreen);
 	wl_signal_init(&surface->toplevel->events.request_minimize);
@@ -499,6 +500,8 @@ void create_xdg_toplevel(struct wlr_xdg_surface *surface,
 		&xdg_toplevel_implementation, surface->toplevel, NULL);
 
 	set_xdg_surface_role_object(surface, surface->toplevel->resource);
+
+	wl_signal_emit_mutable(&surface->client->shell->events.new_toplevel, surface->toplevel);
 }
 
 void reset_xdg_toplevel(struct wlr_xdg_toplevel *toplevel) {
@@ -524,11 +527,7 @@ void destroy_xdg_toplevel(struct wlr_xdg_toplevel *toplevel) {
 	wlr_surface_unmap(toplevel->base->surface);
 	reset_xdg_toplevel(toplevel);
 
-	// TODO: improve events
-	if (toplevel->base->added) {
-		wl_signal_emit_mutable(&toplevel->base->events.destroy, NULL);
-		toplevel->base->added = false;
-	}
+	wl_signal_emit_mutable(&toplevel->events.destroy, NULL);
 
 	toplevel->base->toplevel = NULL;
 	wl_resource_set_user_data(toplevel->resource, NULL);