[PATCH weston v3] xdg-shell: Send an error when the client uses the not-topmost popup

[Jonas Ådahl]

From: "Jasper St. Pierre" <jstpierre at mecheye.net>

Either in destroy or get_xdg_popup.

[jadahl: Verify that the new popup is the top most when mapping instead
of creating. Some renaming.]
---

Changes since v2:

Say 'topmost' instead of 'toplevel' in description.

Add 'map' as a reason besides 'destroy' for triggering the error.

 desktop-shell/shell.c  | 58 +++++++++++++++++++++++++++++++++++++++++++++-----
 protocol/xdg-shell.xml |  7 ++++++
 2 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 4990c4d..b326d68 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -3370,12 +3370,40 @@ touch_popup_grab_end(struct weston_touch *touch)
 	}
 }
 
-static void
+static struct shell_surface *
+get_top_popup(struct shell_seat *shseat)
+{
+	struct shell_surface *shsurf;
+
+	if (wl_list_empty(&shseat->popup_grab.surfaces_list)) {
+		return NULL;
+	} else {
+		shsurf = container_of(shseat->popup_grab.surfaces_list.next,
+				      struct shell_surface,
+				      popup.grab_link);
+		return shsurf;
+	}
+}
+
+static int
 add_popup_grab(struct shell_surface *shsurf,
 	       struct shell_seat *shseat,
 	       int32_t type)
 {
 	struct weston_seat *seat = shseat->seat;
+	struct shell_surface *parent, *top_surface;
+
+	parent = get_shell_surface(shsurf->parent);
+	top_surface = get_top_popup(shseat);
+	if (shell_surface_is_xdg_popup(shsurf) &&
+	    ((top_surface == NULL && !shell_surface_is_xdg_surface(parent)) ||
+	     (top_surface != NULL && parent != top_surface))) {
+		wl_resource_post_error(shsurf->owner->resource,
+				       XDG_POPUP_ERROR_NOT_THE_TOPMOST_POPUP,
+				       "xdg_popup was not created on the "
+				       "topmost popup");
+		return -1;
+	}
 
 	if (wl_list_empty(&shseat->popup_grab.surfaces_list)) {
 		shseat->popup_grab.type = type;
@@ -3410,6 +3438,8 @@ add_popup_grab(struct shell_surface *shsurf,
 		wl_list_insert(&shseat->popup_grab.surfaces_list,
 			       &shsurf->popup.grab_link);
 	}
+
+	return 0;
 }
 
 static void
@@ -3417,6 +3447,15 @@ remove_popup_grab(struct shell_surface *shsurf)
 {
 	struct shell_seat *shseat = shsurf->popup.shseat;
 
+	if (shell_surface_is_xdg_popup(shsurf) &&
+	    get_top_popup(shseat) != shsurf) {
+		wl_resource_post_error(shsurf->resource,
+				       XDG_POPUP_ERROR_NOT_THE_TOPMOST_POPUP,
+				       "xdg_popup was destroyed while it was "
+				       "not the topmost popup.");
+		return;
+	}
+
 	wl_list_remove(&shsurf->popup.grab_link);
 	wl_list_init(&shsurf->popup.grab_link);
 	if (wl_list_empty(&shseat->popup_grab.surfaces_list)) {
@@ -3430,7 +3469,7 @@ remove_popup_grab(struct shell_surface *shsurf)
 	}
 }
 
-static void
+static int
 shell_map_popup(struct shell_surface *shsurf)
 {
 	struct shell_seat *shseat = shsurf->popup.shseat;
@@ -3445,14 +3484,18 @@ shell_map_popup(struct shell_surface *shsurf)
 
 	if (shseat->seat->pointer &&
 	    shseat->seat->pointer->grab_serial == shsurf->popup.serial) {
-		add_popup_grab(shsurf, shseat, POINTER);
+		if (add_popup_grab(shsurf, shseat, POINTER) != 0)
+			return -1;
 	} else if (shseat->seat->touch &&
 	           shseat->seat->touch->grab_serial == shsurf->popup.serial) {
-		add_popup_grab(shsurf, shseat, TOUCH);
+		if (add_popup_grab(shsurf, shseat, TOUCH) != 0)
+			return -1;
 	} else {
 		shell_surface_send_popup_done(shsurf);
 		shseat->popup_grab.client = NULL;
 	}
+
+	return 0;
 }
 
 static const struct wl_shell_surface_interface shell_surface_implementation = {
@@ -4051,6 +4094,10 @@ create_xdg_popup(struct shell_client *owner, void *shell,
 {
 	struct shell_surface *shsurf;
 
+	/* Verify that we are creating the top most popup when mapping,
+	 * as its not until then we know whether it was mapped as most
+	 * top level or not. */
+
 	shsurf = create_common_surface(owner, shell, surface, client);
 	if (!shsurf)
 		return NULL;
@@ -5451,7 +5498,8 @@ map(struct desktop_shell *shell, struct shell_surface *shsurf,
 		}
 		break;
 	case SHELL_SURFACE_POPUP:
-		shell_map_popup(shsurf);
+		if (shell_map_popup(shsurf) != 0)
+			return;
 		break;
 	case SHELL_SURFACE_NONE:
 		weston_view_set_position(shsurf->view,
diff --git a/protocol/xdg-shell.xml b/protocol/xdg-shell.xml
index 4414d46..82ff917 100644
--- a/protocol/xdg-shell.xml
+++ b/protocol/xdg-shell.xml
@@ -398,6 +398,13 @@
       xdg_popup surfaces are always transient for another surface.
     </description>
 
+    <enum name="error">
+      <description summary="xdg_popup error values">
+	These errors can be emitted in response to xdg_popup requests.
+      </description>
+      <entry name="not_the_topmost_popup" value="0" summary="The client tried to map or destroy a non-topmost popup"/>
+    </enum>
+
     <request name="destroy" type="destructor">
       <description summary="remove xdg_surface interface">
 	The xdg_surface interface is removed from the wl_surface object
-- 
2.1.0