Update xdg-shell implementation

* compositor.h: Update prototypes.
* positioner.c (SetSize, SetAnchorRect, SetAnchor, SetGravity):
Signal specific errors upon invalid input.
(XLPositionerIsReactive): New function.
* xdg-shell.xml: Update from wayland-protocols 1.28.
* xdg_popup.c (struct _XdgPopup): Remove unused next, last
fields.
(DestroyBacking): Adjust accordingly.
(Reposition, XLGetXdgPopup): Check positioner validity.
(XLInitPopups): Remove unused popup list.
* xdg_toplevel.c (SetMaxSize, SetMinSize): Raise
XDG_TOPLEVEL_ERROR_INVALID_SIZE.
* xdg_wm.c (Destroy): Don't allow destroying wm_base with
surfaces.

compositor.h

@@ -1386,6 +1386,7 @@ extern void XLPositionerCalculateGeometry (Positioner *, Role *, int *, int *,
 extern void XLRetainPositioner (Positioner *);
 extern void XLReleasePositioner (Positioner *);
 extern Bool XLPositionerIsReactive (Positioner *);
+extern void XLCheckPositionerComplete (Positioner *);
 
 /* Defined in xdg_toplevel.c.  */
 

positioner.c

@@ -99,7 +99,7 @@ SetSize (struct wl_client *client, struct wl_resource *resource,
 
   if (width < 1 || height < 1)
     {
-      wl_resource_post_error (resource, XDG_POSITIONER_ERROR_INVALID_INPUT,
+      wl_resource_post_error (resource, XDG_SURFACE_ERROR_INVALID_SIZE,
 			      "invalid size %d %d", width, height);
       return;
     }
@@ -118,7 +118,7 @@ SetAnchorRect (struct wl_client *client, struct wl_resource *resource,
   if (width < 1 || height < 1)
     {
       wl_resource_post_error (resource, XDG_POSITIONER_ERROR_INVALID_INPUT,
-			      "invalid size %d %d", width, height);
+			      "invalid size specified (%d %d)", width, height);
       return;
     }
 
@@ -138,7 +138,7 @@ SetAnchor (struct wl_client *client, struct wl_resource *resource,
   if (anchor > XDG_POSITIONER_ANCHOR_BOTTOM_RIGHT)
     {
       wl_resource_post_error (resource, XDG_POSITIONER_ERROR_INVALID_INPUT,
-			      "not an anchor");
+			      "invalid anchor specified (%"PRIu32")", anchor);
       return;
     }
 
@@ -154,8 +154,8 @@ SetGravity (struct wl_client *client, struct wl_resource *resource,
 
   if (gravity > XDG_POSITIONER_GRAVITY_BOTTOM_RIGHT)
     {
-      wl_resource_post_error (resource, XDG_POSITIONER_GRAVITY_BOTTOM_RIGHT,
+      wl_resource_post_error (resource, XDG_POSITIONER_ERROR_INVALID_INPUT,
-			      "not a gravity");
+			      "invalid gravity specified (%"PRIu32")", gravity);
       return;
     }
 
@@ -940,3 +940,14 @@ XLPositionerIsReactive (Positioner *positioner)
 {
   return positioner->reactive;
 }
+
+void
+XLCheckPositionerComplete (Positioner *positioner)
+{
+  if (positioner->anchor_width && positioner->width)
+    return;
+
+  wl_resource_post_error (positioner->resource,
+			  XDG_WM_BASE_ERROR_INVALID_POSITIONER,
+			  "the specified positioner is incomplete");
+}

xdg-shell.xml

@@ -50,6 +50,8 @@
 	     summary="the client provided an invalid surface state"/>
       <entry name="invalid_positioner" value="5"
 	     summary="the client provided an invalid positioner"/>
+      <entry name="unresponsive" value="6"
+	     summary="the client didn’t respond to a ping event in time"/>
     </enum>
 
     <request name="destroy" type="destructor">
@@ -58,7 +60,7 @@
 
 	Destroying a bound xdg_wm_base object while there are surfaces
 	still alive created by this xdg_wm_base object instance is illegal
-	and will result in a protocol error.
+	and will result in a defunct_surfaces error.
       </description>
     </request>
 
@@ -77,7 +79,7 @@
 	itself is not a role, the corresponding surface may only be assigned
 	a role extending xdg_surface, such as xdg_toplevel or xdg_popup. It is
 	illegal to create an xdg_surface for a wl_surface which already has an
-	assigned role and this will result in a protocol error.
+	assigned role and this will result in a role error.
 
 	This creates an xdg_surface for the given surface. An xdg_surface is
 	used as basis to define a role to a given surface, such as xdg_toplevel
@@ -94,7 +96,8 @@
     <request name="pong">
       <description summary="respond to a ping event">
 	A client must respond to a ping event with a pong request or
-	the client may be deemed unresponsive. See xdg_wm_base.ping.
+	the client may be deemed unresponsive. See xdg_wm_base.ping
+	and xdg_wm_base.error.unresponsive.
       </description>
       <arg name="serial" type="uint" summary="serial of the ping event"/>
     </request>
@@ -108,7 +111,9 @@
 	Compositors can use this to determine if the client is still
 	alive. It's unspecified what will happen if the client doesn't
 	respond to the ping request, or in what timeframe. Clients should
-	try to respond in a reasonable amount of time.
+	try to respond in a reasonable amount of time. The “unresponsive”
+	error is provided for compositors that wish to disconnect unresponsive
+	clients.
 
 	A compositor is free to ping in any way it wants, but a client must
 	always respond to any xdg_wm_base object it created.
@@ -137,7 +142,7 @@
       For an xdg_positioner object to be considered complete, it must have a
       non-zero size set by set_size, and a non-zero anchor rectangle set by
       set_anchor_rect. Passing an incomplete xdg_positioner object when
-      positioning a surface raises an error.
+      positioning a surface raises an invalid_positioner error.
     </description>
 
     <enum name="error">
@@ -225,7 +230,8 @@
 	specified (e.g. 'bottom_right' or 'top_left'), then the child surface
 	will be placed towards the specified gravity; otherwise, the child
 	surface will be centered over the anchor point on any axis that had no
-	gravity specified.
+	gravity specified. If the gravity is not in the ‘gravity’ enum, an
+	invalid_input error is raised.
       </description>
       <arg name="gravity" type="uint" enum="gravity"
 	   summary="gravity direction"/>
@@ -451,10 +457,16 @@
     </description>
 
     <enum name="error">
-      <entry name="not_constructed" value="1"/>
+      <entry name="not_constructed" value="1"
-      <entry name="already_constructed" value="2"/>
+	     summary="Surface was not fully constructed"/>
-      <entry name="unconfigured_buffer" value="3"/>
+      <entry name="already_constructed" value="2"
-      <entry name="invalid_serial" value="4"/>
+	     summary="Surface was already constructed"/>
+      <entry name="unconfigured_buffer" value="3"
+	     summary="Attaching a buffer to an unconfigured surface"/>
+      <entry name="invalid_serial" value="4"
+	     summary="Invalid serial number when acking a configure event"/>
+      <entry name="invalid_size" value="5"
+	     summary="Width or height was zero or negative"/>
     </enum>
 
     <request name="destroy" type="destructor">
@@ -518,10 +530,10 @@
 	the wl_surface associated with this xdg_surface.
 
 	The width and height must be greater than zero. Setting an invalid size
-	will raise an error. When applied, the effective window geometry will be
+	will raise an invalid_size error. When applied, the effective window
-	the set window geometry clamped to the bounding rectangle of the
+	geometry will be the set window geometry clamped to the bounding
-	combined geometry of the surface of the xdg_surface and the associated
+	rectangle of the combined geometry of the surface of the xdg_surface and
-	subsurfaces.
+	the associated subsurfaces.
       </description>
       <arg name="x" type="int"/>
       <arg name="y" type="int"/>
@@ -542,6 +554,8 @@
 
 	If the client receives multiple configure events before it
 	can respond to one, it only has to ack the last configure event.
+	Acking a configure event that was never sent raises an invalid_serial
+	error.
 
 	A client is not required to commit immediately after sending
 	an ack_configure request - it may even ack_configure several times
@@ -622,6 +636,8 @@
         not a valid variant of the resize_edge enum"/>
       <entry name="invalid_parent" value="1"
         summary="invalid parent toplevel"/>
+      <entry name="invalid_size" value="2"
+	summary="client provided an invalid min or max size"/>
     </enum>
 
     <request name="set_parent">
@@ -687,7 +703,7 @@
 	application identifiers and how they relate to well-known D-Bus
 	names and .desktop files.
 
-	[0] http://standards.freedesktop.org/desktop-entry-spec/
+	[0] https://standards.freedesktop.org/desktop-entry-spec/
       </description>
       <arg name="app_id" type="string"/>
     </request>
@@ -701,7 +717,8 @@
 	This request asks the compositor to pop up such a window menu at
 	the given position, relative to the local surface coordinates of
 	the parent surface. There are no guarantees as to what menu items
-	the window menu contains.
+	the window menu contains, or even if a window menu will be drawn
+	at all.
 
 	This request must be used in response to some sort of user action
 	like a button press, key press, or touch down event.
@@ -890,11 +907,11 @@
 	request.
 
 	Requesting a maximum size to be smaller than the minimum size of
-	a surface is illegal and will result in a protocol error.
+	a surface is illegal and will result in an invalid_size error.
 
 	The width and height must be greater than or equal to zero. Using
-	strictly negative values for width and height will result in a
+	strictly negative values for width or height will result in a
-	protocol error.
+	invalid_size error.
       </description>
       <arg name="width" type="int"/>
       <arg name="height" type="int"/>
@@ -931,11 +948,11 @@
 	request.
 
 	Requesting a minimum size to be larger than the maximum size of
-	a surface is illegal and will result in a protocol error.
+	a surface is illegal and will result in an invalid_size error.
 
 	The width and height must be greater than or equal to zero. Using
 	strictly negative values for width and height will result in a
-	protocol error.
+	invalid_size error.
       </description>
       <arg name="width" type="int"/>
       <arg name="height" type="int"/>

xdg_popup.c

@@ -105,16 +105,10 @@ struct _XdgPopup
 
   /* Reconstrain callback associated with the parent.  */
   void *reconstrain_callback_key;
-
-  /* The next and last popups in this list.  */
-  XdgPopup *next, *last;
 };
 
 
 
-/* List of all current popups.  */
-XdgPopup live_popups;
-
 /* Forward declarations.  */
 
 static void DoGrab (XdgPopup *, Seat *, uint32_t);
@@ -144,10 +138,6 @@ DestroyBacking (XdgPopup *popup)
   if (popup->pending_callback_key)
     XLSeatCancelDestroyListener (popup->pending_callback_key);
 
-  /* Unlink the popup from the list.  */
-  popup->last->next = popup->next;
-  popup->next->last = popup->last;
-
   /* Release the positioner and free the popup.  */
   XLReleasePositioner (popup->positioner);
   XLFree (popup);
@@ -682,6 +672,9 @@ Reposition (struct wl_client *client, struct wl_resource *resource,
     = wl_resource_get_user_data (positioner_resource);
   XLRetainPositioner (popup->positioner);
 
+  /* Make sure that the positioner is complete.  */
+  XLCheckPositionerComplete (popup->positioner);
+
   xdg_popup_send_repositioned (resource, token);
   InternalReposition (popup);
 }
@@ -841,18 +834,15 @@ XLGetXdgPopup (struct wl_client *client, struct wl_resource *resource,
   popup->positioner = wl_resource_get_user_data (positioner);
   XLRetainPositioner (popup->positioner);
 
+  /* Make sure that the positioner is complete.  */
+  XLCheckPositionerComplete (popup->positioner);
+
   wl_resource_set_implementation (popup->resource, &xdg_popup_impl,
 				  popup, HandleResourceDestroy);
   popup->refcount++;
 
   XLXdgRoleAttachImplementation (role, &popup->impl);
 
-  /* Link the popup onto the list of all popups.  */
-  popup->last = &live_popups;
-  popup->next = live_popups.next;
-  live_popups.next->last = popup;
-  live_popups.next = popup;
-
   /* Send the initial configure event.  */
   InternalReposition (popup);
 }
@@ -869,6 +859,5 @@ XLHandleXEventForXdgPopups (XEvent *event)
 void
 XLInitPopups (void)
 {
-  live_popups.next = &live_popups;
+  /* Nothing to do here.  */
-  live_popups.last = &live_popups;
 }

xdg_toplevel.c

@@ -2044,7 +2044,7 @@ SetMaxSize (struct wl_client *client, struct wl_resource *resource,
 
   if (width < 0 || height < 0)
     {
-      wl_resource_post_error (resource, WL_SURFACE_ERROR_INVALID_SIZE,
+      wl_resource_post_error (resource, XDG_TOPLEVEL_ERROR_INVALID_SIZE,
 			      "invalid max size %d %d", width, height);
       return;
     }
@@ -2067,7 +2067,7 @@ SetMinSize (struct wl_client *client, struct wl_resource *resource,
 
   if (width < 0 || height < 0)
     {
-      wl_resource_post_error (resource, WL_SURFACE_ERROR_INVALID_SIZE,
+      wl_resource_post_error (resource, XDG_TOPLEVEL_ERROR_INVALID_SIZE,
 			      "invalid min size %d %d", width, height);
       return;
     }

xdg_wm.c

@@ -70,6 +70,17 @@ Pong (struct wl_client *client, struct wl_resource *resource,
 static void
 Destroy (struct wl_client *client, struct wl_resource *resource)
 {
+  XdgWmBase *wm_base;
+
+  /* If there are still xdg_surfaces created by this xdg_wm_base
+     resource, post an error.  */
+  wm_base = wl_resource_get_user_data (resource);
+
+  if (wm_base->list.next != &wm_base->list)
+    wl_resource_post_error (resource, XDG_WM_BASE_ERROR_DEFUNCT_SURFACES,
+			    "surfaces created by this xdg_wm_base still"
+			    " exist, yet it is being destroyed");
+
   wl_resource_destroy (resource);
 }