[PATCH weston] libweston-desktop/xdg-shell: Consolidate configure event sending

[Daniel Stone]

Hi Quentin,

On 13 April 2017 at 20:15, Quentin Glidic
<sardemff7+wayland at sardemff7.net> wrote:
> When switching a state twice in a row, we were overwriting the old value
> without setting it back, sending a wrong state to the client.
>
> Now we update our requested state, then check if we need to schedule a
> configure event, if we have one scheduled already or even if we can
> cancel it.

Most of this looks fine, but:

>  static void
> -weston_desktop_xdg_surface_schedule_configure(struct weston_desktop_xdg_surface *surface)
> +weston_desktop_xdg_surface_schedule_configure(struct weston_desktop_xdg_surface *surface,
> +                                             bool force)
>  {
>         struct wl_display *display = weston_desktop_get_display(surface->desktop);
>         struct wl_event_loop *loop = wl_display_get_event_loop(display);
> +       bool requested_same = !force;
>
> -       if (surface->configure_idle != NULL)
> -               return;
> -       surface->configure_idle =
> -               wl_event_loop_add_idle(loop,
> -                                      weston_desktop_xdg_surface_send_configure,
> -                                      surface);
> +       switch (surface->role) {
> +       case WESTON_DESKTOP_XDG_SURFACE_ROLE_NONE:
> +               assert(0 && "not reached");
> +               break;
> +       case WESTON_DESKTOP_XDG_SURFACE_ROLE_TOPLEVEL:
> +               requested_same = requested_same &&
> +                       weston_desktop_xdg_toplevel_state_compare((struct weston_desktop_xdg_toplevel *) surface);
> +               break;
> +       case WESTON_DESKTOP_XDG_SURFACE_ROLE_POPUP:
> +               break;
> +       }
> +
> +       if (surface->configure_idle != NULL) {
> +               if (!requested_same)
> +                       return;
> +
> +               wl_event_source_remove(surface->configure_idle);
> +               surface->configure_idle = NULL;
> +       } else {
> +               if (requested_same)
> +                       return;

So, we'll never send a configure event for popups, unless force is
true. Was this intended / is this OK? If so, then:
Reviewed-by: Daniel Stone <daniels at collabora.com>

Cheers,
Daniel