From f750c7445d37e58749e6d4ce7b4ce587f2231cc6 Mon Sep 17 00:00:00 2001
From: Kirill Primak <vyivel@eclair.cafe>
Date: Sat, 14 Oct 2023 19:58:01 +0300
Subject: [PATCH] layer-shell: don't use wlr_surface_role.unmap hook

A layer-shell surface can be unmapped if wlr_layer_shell_v1 is
destroyed or the client has committed a NULL buffer. Let's use the
previously introduced wlr_surface.unmap_commit to handle the latter
case instead; this is more consistent with the xdg_surface
implementation logic, where using the hook is more trouble than it's
worth.

Additionally, this commit adds an unconditional surface reset on
destroy, so popups are properly cleaned up even if originally created
with an unmapped layer-shell surface as a parent. Doing so with the
role unmap hook would either result in possibly resetting the surface
twice, which is suboptimal, or having an awkward
`if (mapped) { unmap() } else { reset() }` check.
---
 types/wlr_layer_shell_v1.c | 58 ++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 30 deletions(-)

diff --git a/types/wlr_layer_shell_v1.c b/types/wlr_layer_shell_v1.c
index 927b8762..39feb2cc 100644
--- a/types/wlr_layer_shell_v1.c
+++ b/types/wlr_layer_shell_v1.c
@@ -22,8 +22,32 @@ static void resource_handle_destroy(struct wl_client *client,
 static const struct zwlr_layer_shell_v1_interface layer_shell_implementation;
 static const struct zwlr_layer_surface_v1_interface layer_surface_implementation;
 
+static void layer_surface_configure_destroy(
+		struct wlr_layer_surface_v1_configure *configure) {
+	if (configure == NULL) {
+		return;
+	}
+	wl_list_remove(&configure->link);
+	free(configure);
+}
+
+static void layer_surface_reset(struct wlr_layer_surface_v1 *surface) {
+	surface->configured = false;
+
+	struct wlr_xdg_popup *popup, *popup_tmp;
+	wl_list_for_each_safe(popup, popup_tmp, &surface->popups, link) {
+		wlr_xdg_popup_destroy(popup);
+	}
+
+	struct wlr_layer_surface_v1_configure *configure, *tmp;
+	wl_list_for_each_safe(configure, tmp, &surface->configure_list, link) {
+		layer_surface_configure_destroy(configure);
+	}
+}
+
 static void layer_surface_destroy(struct wlr_layer_surface_v1 *surface) {
 	wlr_surface_unmap(surface->surface);
+	layer_surface_reset(surface);
 
 	wl_signal_emit_mutable(&surface->events.destroy, surface);
 	wl_resource_set_user_data(surface->resource, NULL);
@@ -55,15 +79,6 @@ struct wlr_layer_surface_v1 *wlr_layer_surface_v1_try_from_wlr_surface(
 	return wlr_layer_surface_v1_from_resource(surface->role_resource);
 }
 
-static void layer_surface_configure_destroy(
-		struct wlr_layer_surface_v1_configure *configure) {
-	if (configure == NULL) {
-		return;
-	}
-	wl_list_remove(&configure->link);
-	free(configure);
-}
-
 static void layer_surface_handle_ack_configure(struct wl_client *client,
 		struct wl_resource *resource, uint32_t serial) {
 	struct wlr_layer_surface_v1 *surface =
@@ -334,6 +349,10 @@ static void layer_surface_role_commit(struct wlr_surface *wlr_surface) {
 		return;
 	}
 
+	if (surface->surface->unmap_commit) {
+		layer_surface_reset(surface);
+	}
+
 	surface->current = surface->pending;
 	surface->pending.committed = 0;
 
@@ -350,26 +369,6 @@ static void layer_surface_role_commit(struct wlr_surface *wlr_surface) {
 	}
 }
 
-static void layer_surface_role_unmap(struct wlr_surface *wlr_surface) {
-	struct wlr_layer_surface_v1 *surface =
-		wlr_layer_surface_v1_try_from_wlr_surface(wlr_surface);
-	if (surface == NULL) {
-		return;
-	}
-
-	surface->configured = false;
-
-	struct wlr_xdg_popup *popup, *popup_tmp;
-	wl_list_for_each_safe(popup, popup_tmp, &surface->popups, link) {
-		wlr_xdg_popup_destroy(popup);
-	}
-
-	struct wlr_layer_surface_v1_configure *configure, *tmp;
-	wl_list_for_each_safe(configure, tmp, &surface->configure_list, link) {
-		layer_surface_configure_destroy(configure);
-	}
-}
-
 static void layer_surface_role_destroy(struct wlr_surface *wlr_surface) {
 	struct wlr_layer_surface_v1 *surface =
 		wlr_layer_surface_v1_try_from_wlr_surface(wlr_surface);
@@ -383,7 +382,6 @@ static void layer_surface_role_destroy(struct wlr_surface *wlr_surface) {
 static const struct wlr_surface_role layer_surface_role = {
 	.name = "zwlr_layer_surface_v1",
 	.commit = layer_surface_role_commit,
-	.unmap = layer_surface_role_unmap,
 	.destroy = layer_surface_role_destroy,
 };