[PATCH 3/3] compositor: properly restore keyboard_focus in notify_keyboard_focus()

[Kristian Hoegsberg]

On Fri, Mar 16, 2012 at 05:25:11PM +0200, Ander Conselvan de Oliveira wrote:
> Commit f992b2fc removed the saved keyboard focus logic to fix a crash
> when the saved surface is destroyed. However, setting keyboard focus to
> the first surface on the list ends up trying to set the focus to the
> cursor surface most of the time. The end result is a NULL keyboard
> focus.
> 
> This patch restores the saved keyboard focus logic and fixes the crash
> mentioned above using a destroy listener.

Thanks, that's a good fix.  This and the two previous fixes applied.
We really need that wl_guard thing that Pekka suggested now...  too
many destroy listeners that just set a pointer to NULL.

We didn't find a really good way to make it both typesafe and
convenient, but I think I prefer the approach where wl_guard manages a
wl_resource pointer internally the best.  We put the wl_resource
pointer and the wl_listener in a wl_guard struct:

  struct wl_resource_ref {
    struct wl_resource *resource;
    struct wl_listener listener;
  };

and then we can just say wl_resource_ref_set(ref, resource).  If we
want a typesafe pointer as well, we have to add it in the containing
struct and set it from resource->data:

  wd->saved_kbd_focus = es;
  wl_guard_set(&wd->saved_kbd_guard, &es->surface.resource);

The wl_resource_ref will not clear the guard->resource pointer, but the
typesafe pointer, so access to the typesafe pointer has to be guarded
by checking ref->resource != NULL:

  es = wd->saved_kbd_focus;
  if (wd->saved_kbd_guard.resource) {
    /* do stuff with es */
  }

Kristian

> ---
>  src/compositor.c |   36 +++++++++++++++++++++++++++++-------
>  src/compositor.h |    2 ++
>  2 files changed, 31 insertions(+), 7 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 09b14ae..ceb36ed 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -1636,6 +1636,18 @@ notify_pointer_focus(struct wl_input_device *device,
>  	}
>  }
>  
> +static void
> +destroy_device_saved_kbd_focus(struct wl_listener *listener,
> +			       struct wl_resource *resource, uint32_t time)
> +{
> +	struct weston_input_device *wd;
> +
> +	wd = container_of(listener, struct weston_input_device,
> +			  saved_kbd_focus_listener);
> +
> +	wd->saved_kbd_focus = NULL;
> +}
> +
>  WL_EXPORT void
>  notify_keyboard_focus(struct wl_input_device *device,
>  		      uint32_t time, struct weston_output *output,
> @@ -1647,12 +1659,6 @@ notify_keyboard_focus(struct wl_input_device *device,
>  	struct weston_surface *es;
>  	uint32_t *k;
>  
> -	if (!wl_list_empty(&compositor->surface_list))
> -		es = container_of(compositor->surface_list.next,
> -				  struct weston_surface, link);
> -	else
> -		es = NULL;
> -
>  	if (output) {
>  		wl_array_copy(&wd->input_device.keys, keys);
>  		wd->modifier_state = 0;
> @@ -1661,14 +1667,30 @@ notify_keyboard_focus(struct wl_input_device *device,
>  			update_modifier_state(wd, *k, 1);
>  		}
>  
> -		if (es && es->surface.resource.client)
> +		es = wd->saved_kbd_focus;
> +
> +		if (es) {
> +			wl_list_remove(&wd->saved_kbd_focus_listener.link);
>  			wl_input_device_set_keyboard_focus(&wd->input_device,
>  							   &es->surface, time);
> +			wd->saved_kbd_focus = NULL;
> +		}
>  	} else {
>  		wl_array_for_each(k, &device->keys)
>  			weston_compositor_idle_release(compositor);
>  
>  		wd->modifier_state = 0;
> +
> +		es = wd->input_device.keyboard_focus;
> +
> +		if (es) {
> +			wd->saved_kbd_focus = es;
> +			wd->saved_kbd_focus_listener.func =
> +				destroy_device_saved_kbd_focus;
> +			wl_list_insert(es->surface.resource.destroy_listener_list.prev,
> +				       &wd->saved_kbd_focus_listener.link);
> +		}
> +
>  		wl_input_device_set_keyboard_focus(&wd->input_device,
>  						   NULL, time);
>  	}
> diff --git a/src/compositor.h b/src/compositor.h
> index 9467cc8..9071c40 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -113,6 +113,8 @@ struct weston_input_device {
>  	struct wl_list link;
>  	uint32_t modifier_state;
>  	int hw_cursor;
> +	struct wl_surface *saved_kbd_focus;
> +	struct wl_listener saved_kbd_focus_listener;
>  
>  	uint32_t num_tp;
>  	struct wl_surface *touch_focus;
> -- 
> 1.7.4.1
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel