On Tue, Jul 18, 2017 at 01:14:49PM +0200, Quentin Glidic wrote:
> From: Quentin Glidic <[email protected]>
> 
> Now we keep track of serial->state association and we discard the states
> that the client ignored.
> 
> Signed-off-by: Quentin Glidic <[email protected]>
> ---
>  libweston-desktop/xdg-shell-v5.c | 45 +++++++++++++++++++++++-----
>  libweston-desktop/xdg-shell-v6.c | 64 
> +++++++++++++++++++++++++++++++++-------
>  2 files changed, 92 insertions(+), 17 deletions(-)
> 
> diff --git a/libweston-desktop/xdg-shell-v5.c 
> b/libweston-desktop/xdg-shell-v5.c
> index b32b7812..f9f7a5d1 100644
> --- a/libweston-desktop/xdg-shell-v5.c
> +++ b/libweston-desktop/xdg-shell-v5.c
> @@ -46,6 +46,13 @@ struct weston_desktop_xdg_surface_state {
>       bool activated;
>  };
>  
> +struct weston_desktop_xdg_surface_configure {
> +     struct wl_list link; /* weston_desktop_xdg_surface::configure_list */
> +     uint32_t serial;
> +     struct weston_desktop_xdg_surface_state state;
> +     struct weston_size size;
> +};
> +
>  struct weston_desktop_xdg_surface {
>       struct wl_resource *resource;
>       struct weston_desktop_surface *surface;
> @@ -53,7 +60,7 @@ struct weston_desktop_xdg_surface {
>       bool added;
>       struct wl_event_source *add_idle;
>       struct wl_event_source *configure_idle;
> -     uint32_t configure_serial;
> +     struct wl_list configure_list; /* 
> weston_desktop_xdg_surface_configure::link */
>       struct {
>               struct weston_desktop_xdg_surface_state state;
>               struct weston_size size;
> @@ -94,13 +101,26 @@ static void
>  weston_desktop_xdg_surface_send_configure(void *data)
>  {
>       struct weston_desktop_xdg_surface *surface = data;
> +     struct weston_desktop_xdg_surface_configure *configure;
>       uint32_t *s;
>       struct wl_array states;
>  
>       surface->configure_idle = NULL;
>  
> -     surface->configure_serial =
> +     configure = zalloc(sizeof(struct weston_desktop_xdg_surface_configure));
> +     if ( configure == NULL ) {

No spacing around expression.

> +             struct weston_desktop_client *client =
> +                     weston_desktop_surface_get_client(surface->surface);
> +             struct wl_client *wl_client =
> +                     weston_desktop_client_get_client(client);
> +             wl_client_post_no_memory(wl_client);
> +             return;
> +     }
> +     wl_list_insert(surface->configure_list.prev, &configure->link);
> +     configure->serial =
>               
> wl_display_next_serial(weston_desktop_get_display(surface->desktop));
> +     configure->state = surface->pending.state;
> +     configure->size = surface->pending.size;
>  
>       wl_array_init(&states);
>       if (surface->pending.state.maximized) {
> @@ -124,7 +144,7 @@ weston_desktop_xdg_surface_send_configure(void *data)
>                                  surface->pending.size.width,
>                                  surface->pending.size.height,
>                                  &states,
> -                                surface->configure_serial);
> +                                configure->serial);
>  
>       wl_array_release(&states);
>  };
> @@ -443,12 +463,23 @@ 
> weston_desktop_xdg_surface_protocol_ack_configure(struct wl_client *wl_client,
>               wl_resource_get_user_data(resource);
>       struct weston_desktop_xdg_surface *surface =
>               weston_desktop_surface_get_implementation_data(dsurface);
> -
> -     if (surface->configure_serial != serial)
> +     struct weston_desktop_xdg_surface_configure *configure, *temp;
> +
> +     wl_list_for_each_safe(configure, temp, &surface->configure_list, link) {
> +             if (configure->serial < serial) {
> +                     wl_list_remove(&configure->link);
> +                     free(configure);
> +             } else if (configure->serial == serial) {
> +                     wl_list_remove(&configure->link);
> +                     break;
> +             }
>               return;
> +     }
> +
> +     surface->next.state = configure->state;
> +     surface->next.size = configure->size;

Must check that we found the acked configure. Bad clients can send a
bogus ack serial.

>  
> -     surface->next.state = surface->pending.state;
> -     surface->next.size = surface->pending.size;
> +     free(configure);
>  }
>  
>  static void
> diff --git a/libweston-desktop/xdg-shell-v6.c 
> b/libweston-desktop/xdg-shell-v6.c
> index f5e46daa..9094260e 100644
> --- a/libweston-desktop/xdg-shell-v6.c
> +++ b/libweston-desktop/xdg-shell-v6.c
> @@ -69,7 +69,7 @@ struct weston_desktop_xdg_surface {
>       struct weston_desktop_surface *desktop_surface;
>       bool configured;
>       struct wl_event_source *configure_idle;
> -     uint32_t configure_serial;
> +     struct wl_list configure_list; /* 
> weston_desktop_xdg_surface_configure::link */
>  
>       bool has_next_geometry;
>       struct weston_geometry next_geometry;
> @@ -77,6 +77,11 @@ struct weston_desktop_xdg_surface {
>       enum weston_desktop_xdg_surface_role role;
>  };
>  
> +struct weston_desktop_xdg_surface_configure {
> +     struct wl_list link; /* weston_desktop_xdg_surface::configure_list */
> +     uint32_t serial;
> +};
> +
>  struct weston_desktop_xdg_toplevel_state {
>       bool maximized;
>       bool fullscreen;
> @@ -84,6 +89,12 @@ struct weston_desktop_xdg_toplevel_state {
>       bool activated;
>  };
>  
> +struct weston_desktop_xdg_toplevel_configure {
> +     struct weston_desktop_xdg_surface_configure base;
> +     struct weston_desktop_xdg_toplevel_state state;
> +     struct weston_size size;
> +};
> +
>  struct weston_desktop_xdg_toplevel {
>       struct weston_desktop_xdg_surface base;
>  
> @@ -115,6 +126,7 @@ struct weston_desktop_xdg_popup {
>  };
>  
>  #define weston_desktop_surface_role_biggest_size (sizeof(struct 
> weston_desktop_xdg_toplevel))
> +#define weston_desktop_surface_configure_biggest_size (sizeof(struct 
> weston_desktop_xdg_toplevel))
>  
>  
>  static struct weston_geometry
> @@ -422,10 +434,11 @@ weston_desktop_xdg_toplevel_protocol_resize(struct 
> wl_client *wl_client,
>  }
>  
>  static void
> -weston_desktop_xdg_toplevel_ack_configure(struct weston_desktop_xdg_toplevel 
> *toplevel)
> +weston_desktop_xdg_toplevel_ack_configure(struct weston_desktop_xdg_toplevel 
> *toplevel,
> +                                       struct 
> weston_desktop_xdg_toplevel_configure *configure)
>  {
> -     toplevel->next.state = toplevel->pending.state;
> -     toplevel->next.size = toplevel->pending.size;
> +     toplevel->next.state = configure->state;
> +     toplevel->next.size = configure->size;
>  }
>  
>  static void
> @@ -529,11 +542,15 @@ 
> weston_desktop_xdg_toplevel_protocol_set_minimized(struct wl_client 
> *wl_client,
>  }
>  
>  static void
> -weston_desktop_xdg_toplevel_send_configure(struct 
> weston_desktop_xdg_toplevel *toplevel)
> +weston_desktop_xdg_toplevel_send_configure(struct 
> weston_desktop_xdg_toplevel *toplevel,
> +                                        struct 
> weston_desktop_xdg_toplevel_configure *configure)
>  {
>       uint32_t *s;
>       struct wl_array states;
>  
> +     configure->state = toplevel->pending.state;
> +     configure->size = toplevel->pending.size;
> +
>       wl_array_init(&states);
>       if (toplevel->pending.state.maximized) {
>               s = wl_array_add(&states, sizeof(uint32_t));
> @@ -848,9 +865,21 @@ static void
>  weston_desktop_xdg_surface_send_configure(void *user_data)
>  {
>       struct weston_desktop_xdg_surface *surface = user_data;
> +     struct weston_desktop_xdg_surface_configure *configure;
>  
>       surface->configure_idle = NULL;
> -     surface->configure_serial =
> +
> +     configure = zalloc(weston_desktop_surface_configure_biggest_size);
> +     if ( configure == NULL ) {
> +             struct weston_desktop_client *client =
> +                     
> weston_desktop_surface_get_client(surface->desktop_surface);
> +             struct wl_client *wl_client =
> +                     weston_desktop_client_get_client(client);
> +             wl_client_post_no_memory(wl_client);
> +             return;
> +     }
> +     wl_list_insert(surface->configure_list.prev, &configure->link);
> +     configure->serial =
>               
> wl_display_next_serial(weston_desktop_get_display(surface->desktop));
>  
>       switch (surface->role) {
> @@ -858,14 +887,15 @@ weston_desktop_xdg_surface_send_configure(void 
> *user_data)
>               assert(0 && "not reached");
>               break;
>       case WESTON_DESKTOP_XDG_SURFACE_ROLE_TOPLEVEL:
> -             weston_desktop_xdg_toplevel_send_configure((struct 
> weston_desktop_xdg_toplevel *) surface);
> +             weston_desktop_xdg_toplevel_send_configure((struct 
> weston_desktop_xdg_toplevel *) surface,
> +                                                        (struct 
> weston_desktop_xdg_toplevel_configure *) configure);
>               break;
>       case WESTON_DESKTOP_XDG_SURFACE_ROLE_POPUP:
>               weston_desktop_xdg_popup_send_configure((struct 
> weston_desktop_xdg_popup *) surface);
>               break;
>       }
>  
> -     zxdg_surface_v6_send_configure(surface->resource, 
> surface->configure_serial);
> +     zxdg_surface_v6_send_configure(surface->resource, configure->serial);
>  }
>  
>  static bool
> @@ -1060,12 +1090,21 @@ 
> weston_desktop_xdg_surface_protocol_ack_configure(struct wl_client *wl_client,
>               wl_resource_get_user_data(resource);
>       struct weston_desktop_xdg_surface *surface =
>               weston_desktop_surface_get_implementation_data(dsurface);
> +     struct weston_desktop_xdg_surface_configure *configure, *temp;
>  
>       if (!weston_desktop_xdg_surface_check_role(surface))
>               return;
>  
> -     if (surface->configure_serial != serial)
> +     wl_list_for_each_safe(configure, temp, &surface->configure_list, link) {
> +             if (configure->serial < serial) {
> +                     wl_list_remove(&configure->link);
> +                     free(configure);
> +             } else if (configure->serial == serial) {
> +                     wl_list_remove(&configure->link);
> +                     break;
> +             }
>               return;
> +     }

Same here as above about checking we found a corresponding configure.

I guess for both those cases it's safe to say raising a client error is
reasonable, as a functioning client should never send bogus
ack_configures.


Jonas


>  
>       surface->configured = true;
>  
> @@ -1074,11 +1113,14 @@ 
> weston_desktop_xdg_surface_protocol_ack_configure(struct wl_client *wl_client,
>               assert(0 && "not reached");
>               break;
>       case WESTON_DESKTOP_XDG_SURFACE_ROLE_TOPLEVEL:
> -             weston_desktop_xdg_toplevel_ack_configure((struct 
> weston_desktop_xdg_toplevel *) surface);
> +             weston_desktop_xdg_toplevel_ack_configure((struct 
> weston_desktop_xdg_toplevel *) surface,
> +                                                       (struct 
> weston_desktop_xdg_toplevel_configure *) configure);
>               break;
>       case WESTON_DESKTOP_XDG_SURFACE_ROLE_POPUP:
>               break;
>       }
> +
> +     free(configure);
>  }
>  
>  static void
> @@ -1290,6 +1332,8 @@ 
> weston_desktop_xdg_shell_protocol_get_xdg_surface(struct wl_client *wl_client,
>                                      "xdg_surface must not have a buffer at 
> creation");
>               return;
>       }
> +
> +     wl_list_init(&surface->configure_list);
>  }
>  
>  static void
> -- 
> 2.13.3
> 
> _______________________________________________
> wayland-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to