xdg-shell: Gate all configure events behind first commit

The protocol requires clients to perform an initial commit before they
receive configure events:
> After creating a role-specific object and setting it up (e.g. by sending
> the title, app ID, size constraints, parent, etc), the client must
> perform an initial commit without any buffer attached. The compositor
> will reply with initial wl_surface state such as
> wl_surface.preferred_buffer_scale followed by an xdg_surface.configure
> event. The client must acknowledge it and is then allowed to attach a
> buffer to map the surface.

Previous to this patch various calls such as set_fullscreen() or
set_maximized() would schedule configure events, resulting in clients
being able skip the initial commit. This again made it possible to write
clients that only work on Weston, in violation of the protocol.

For xdg-popups we already tracked the initial commit status. Move it
to xdg-surface, guard schedule_configure() on it and ensure to run the
later on the initial commit.

Incorporate tests that checks if we get configure events when calling
set_fullscreen/set_maximized and tests that uses the main xdg_surface as
a parent to a sub-surface (which initially triggered this issue).

Signed-off-by: Robert Mader <robert.mader@collabora.com>

libweston/desktop/xdg-shell.c

@@ -75,6 +75,7 @@ struct weston_desktop_xdg_surface {
 	struct weston_desktop *desktop;
 	struct weston_surface *surface;
 	struct weston_desktop_surface *desktop_surface;
+	bool committed;
 	bool configured;
 	struct wl_event_source *configure_idle;
 	struct wl_list configure_list; /* weston_desktop_xdg_surface_configure::link */
@@ -128,7 +129,6 @@ struct weston_desktop_xdg_popup {
 	struct weston_desktop_xdg_surface base;
 
 	struct wl_resource *resource;
-	bool committed;
 	struct weston_desktop_xdg_surface *parent;
 	struct weston_desktop_seat *seat;
 	struct weston_geometry geometry;
@@ -1285,7 +1285,7 @@ weston_desktop_xdg_popup_protocol_grab(struct wl_client *wl_client,
 		return;
 	}
 
-	if (popup->committed) {
+	if (popup->base.committed) {
 		wl_resource_post_error(popup->resource,
 				       XDG_POPUP_ERROR_INVALID_GRAB,
 				       "xdg_popup already is mapped");
@@ -1353,8 +1353,7 @@ weston_desktop_xdg_popup_protocol_reposition(struct wl_client *wl_client,
 							   parent_dsurface);
 	popup->pending_reposition = true;
 	popup->pending_reposition_token = token;
-	if (popup->committed)
-		weston_desktop_xdg_surface_schedule_configure(&popup->base);
+	weston_desktop_xdg_surface_schedule_configure(&popup->base);
 }
 
 static void
@@ -1386,9 +1385,6 @@ weston_desktop_xdg_popup_committed(struct weston_desktop_xdg_popup *popup)
 	wl_list_for_each(view, &wsurface->views, surface_link)
 		weston_view_update_transform(view);
 
-	if (!popup->committed)
-		weston_desktop_xdg_surface_schedule_configure(&popup->base);
-	popup->committed = true;
 	weston_desktop_xdg_popup_update_position(popup->base.desktop_surface,
 						 popup);
 
@@ -1580,6 +1576,9 @@ weston_desktop_xdg_surface_schedule_configure(struct weston_desktop_xdg_surface
 	struct wl_event_loop *loop = wl_display_get_event_loop(display);
 	bool pending_same = false;
 
+	if (!surface->committed)
+		return;
+
 	switch (surface->role) {
 	case WESTON_DESKTOP_XDG_SURFACE_ROLE_NONE:
 		assert(0 && "not reached");
@@ -1890,6 +1889,11 @@ weston_desktop_xdg_surface_committed(struct weston_desktop_surface *dsurface,
 						    surface->next_geometry);
 	}
 
+	if (!surface->committed) {
+		surface->committed = true;
+		weston_desktop_xdg_surface_schedule_configure(surface);
+	}
+
 	switch (surface->role) {
 	case WESTON_DESKTOP_XDG_SURFACE_ROLE_NONE:
 		wl_resource_post_error(surface->resource,

tests/meson.build

@@ -371,6 +371,14 @@ if get_option('shell-desktop')
 				text_input_unstable_v1_protocol_c,
 			],
 		},
+                {
+                        'name': 'xdg-shell-initial-commit',
+                        'sources': [
+                                'xdg-shell-intial-commit.c',
+                                xdg_shell_client_protocol_h,
+                                xdg_shell_protocol_c,
+                        ],
+                },
 	]
 endif
 

tests/xdg-client-helper.c

@@ -153,6 +153,20 @@ xdg_surface_make_toplevel(struct xdg_surface_data *xdg_surface,
 	xdg_toplevel_set_title(xdg_surface->xdg_toplevel, title);
 }
 
+void
+xdg_surface_set_fullscreen(struct xdg_surface_data *xdg_surface)
+{
+	test_assert_ptr_not_null(xdg_surface->xdg_toplevel);
+	xdg_toplevel_set_fullscreen(xdg_surface->xdg_toplevel, NULL);
+}
+
+void
+xdg_surface_set_maximized(struct xdg_surface_data *xdg_surface)
+{
+	test_assert_ptr_not_null(xdg_surface->xdg_toplevel);
+	xdg_toplevel_set_maximized(xdg_surface->xdg_toplevel);
+}
+
 void
 xdg_surface_wait_configure(struct xdg_surface_data *xdg_surface)
 {

tests/xdg-client-helper.h

@@ -69,6 +69,11 @@ xdg_client_destroy(struct xdg_client *xdg_client);
 void
 xdg_surface_make_toplevel(struct xdg_surface_data *xdg_surface,
 			  const char *app_id, const char *title);
+void
+xdg_surface_set_fullscreen(struct xdg_surface_data *xdg_surface);
+
+void
+xdg_surface_set_maximized(struct xdg_surface_data *xdg_surface);
 
 void
 xdg_surface_wait_configure(struct xdg_surface_data *xdg_surface);

tests/xdg-shell-intial-commit.c

@@ -0,0 +1,184 @@
+/*
+ * Copyright © 2025 Collabora, Ltd.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "config.h"
+
+#include <stdio.h>
+#include <string.h>
+#include <sys/mman.h>
+
+#include "libweston-internal.h"
+#include "libweston/desktop.h"
+#include "shared/xalloc.h"
+#include "weston-test-client-helper.h"
+#include "weston-test-fixture-compositor.h"
+#include "weston-test-assert.h"
+#include "tests/test-config.h"
+#include "xdg-client-helper.h"
+
+
+static enum test_result_code
+fixture_setup(struct weston_test_harness *harness)
+{
+	struct compositor_setup setup;
+
+	compositor_setup_defaults(&setup);
+	setup.renderer = WESTON_RENDERER_PIXMAN;
+	setup.width = 320;
+	setup.height = 240;
+	setup.shell = SHELL_DESKTOP;
+	setup.logging_scopes = "proto,log,test-harness-plugin";
+	setup.refresh = HIGHEST_OUTPUT_REFRESH;
+
+	return weston_test_harness_execute_as_client(harness, &setup);
+}
+DECLARE_FIXTURE_SETUP(fixture_setup);
+
+TEST(initial_commit_without_a_buffer)
+{
+	struct xdg_client *xdg_client = create_xdg_client();
+	struct xdg_surface_data *xdg_surface = create_xdg_surface(xdg_client);
+
+	test_assert_ptr_not_null(xdg_client);
+
+	xdg_surface_make_toplevel(xdg_surface, "weston.test", "one");
+	xdg_surface_wait_configure(xdg_surface);
+
+	destroy_xdg_surface(xdg_surface);
+	xdg_client_destroy(xdg_client);
+
+	return RESULT_OK;
+}
+
+TEST(initial_commit_with_a_buffer)
+{
+	struct xdg_client *xdg_client = create_xdg_client();
+	struct xdg_surface_data *xdg_surface = create_xdg_surface(xdg_client);
+
+	test_assert_ptr_not_null(xdg_client);
+
+	xdg_surface_make_toplevel(xdg_surface, "weston.test", "one");
+	xdg_surface_commit_solid(xdg_surface, 255, 0, 0);
+
+	/* we should be expecting a protocol error */
+	expect_protocol_error(xdg_client->client, &xdg_surface_interface,
+			      XDG_SURFACE_ERROR_UNCONFIGURED_BUFFER);
+
+	destroy_xdg_surface(xdg_surface);
+	xdg_client_destroy(xdg_client);
+
+	return RESULT_OK;
+}
+
+TEST(initial_commit_with_fullscreen_state)
+{
+	struct xdg_client *xdg_client = create_xdg_client();
+	struct xdg_surface_data *xdg_surface = create_xdg_surface(xdg_client);
+
+	test_assert_ptr_not_null(xdg_client);
+
+	xdg_surface_make_toplevel(xdg_surface, "weston.test", "one");
+	xdg_surface_set_fullscreen(xdg_surface);
+	client_roundtrip(xdg_client->client);
+
+	/* we shouldn't be getting a configure event */
+	test_assert_u32_eq(xdg_surface->configure.serial, 0);
+
+	destroy_xdg_surface(xdg_surface);
+	xdg_client_destroy(xdg_client);
+
+	return RESULT_OK;
+}
+
+TEST(initial_commit_with_max_state)
+{
+	struct xdg_client *xdg_client = create_xdg_client();
+	struct xdg_surface_data *xdg_surface = create_xdg_surface(xdg_client);
+
+	test_assert_ptr_not_null(xdg_client);
+
+	xdg_surface_make_toplevel(xdg_surface, "weston.test", "one");
+	xdg_surface_set_maximized(xdg_surface);
+	client_roundtrip(xdg_client->client);
+
+	/* we shouldn't be getting a configure event */
+	test_assert_u32_eq(xdg_surface->configure.serial, 0);
+
+	destroy_xdg_surface(xdg_surface);
+	xdg_client_destroy(xdg_client);
+
+	return RESULT_OK;
+}
+
+TEST(initial_commit_without_a_buffer_subsurface)
+{
+	struct xdg_client *xdg_client = create_xdg_client();
+	struct xdg_surface_data *xdg_surface = create_xdg_surface(xdg_client);
+	struct wl_subcompositor *subco;
+	struct wl_subsurface *sub;
+	struct wl_surface *parent;
+	struct wl_surface *new_surf;
+	pixman_color_t color;
+	struct buffer *buf;
+	int width, height;
+
+	test_assert_ptr_not_null(xdg_client);
+	xdg_surface_make_toplevel(xdg_surface, "weston.test", "one");
+	xdg_surface_set_fullscreen(xdg_surface);
+
+	subco = client_get_subcompositor(xdg_client->client);
+	/* create a new surface and use the client as the parent when creating
+	 * a subsurface */
+	new_surf = wl_compositor_create_surface(xdg_client->client->wl_compositor);
+	parent = xdg_surface->surface->wl_surface;
+	sub = wl_subcompositor_get_subsurface(subco, new_surf, parent);
+
+	width = DEFAULT_WINDOW_SIZE;
+	height = DEFAULT_WINDOW_SIZE;
+
+	color_rgb888(&color, 255, 0, 0);
+	buf = create_shm_buffer_solid(xdg_surface->surface->client,
+				      width, height, &color);
+	test_assert_ptr_not_null(buf);
+
+	wl_surface_attach(new_surf, buf->proxy, 0, 0);
+	wl_surface_damage_buffer(xdg_surface->surface->wl_surface,
+				 0, 0, width, height);
+
+	wl_surface_commit(new_surf);
+
+	client_roundtrip(xdg_client->client);
+	/* this used to incorrectly trigger/schedule an configure event */
+	test_assert_u32_eq(xdg_surface->configure.serial, 0);
+
+	buffer_destroy(buf);
+	wl_subsurface_destroy(sub);
+	wl_surface_destroy(new_surf);
+	wl_subcompositor_destroy(subco);
+	destroy_xdg_surface(xdg_surface);
+	xdg_client_destroy(xdg_client);
+
+	return RESULT_OK;
+}