wl: Check wl_surface before use it

[psaavedra]

... on pointer_on_enter, touch_on_down and keyboard_on_enter.

It can be possible a situation during the destruction of a CogView where the UI-process were receiving and trying to process an input event for a non existant wl_surface.

Under this scenario, the wl_surface associated could be not longer accesible so the input handler will receive a null-pointer (0x0).

Backtrace:

0  0xb9999999 in wl_proxy_get_user_data (proxy=proxy@entry=0x0)
    at ../wayland/src/wayland-client.c:2135

    at /usr/include/wayland-client-protocol.h:3393
    fixed_x=0, fixed_y=5, surface=0x0 <<<< , data=0xa9999999)

[aperezdc]

This fix is not 100% ideal: compositors are not allowed to pass NULL surfaces for these events, because the protocol definition does not have the allow-null="true" attribute for the surface parameter.

In theory we should read pending events before destroying surfaces, to ensure we don't have any left for them, but in practice that fix would be more complicated. Also, I have seen similar fixes in other projects (like glfw/glfw#1191) so I think we can go with the approach suggested in this PR for now.

Please remove the g_warning() calls before merging, though.

[psaavedra]

This fix is not 100% ideal: compositors are not allowed to pass NULL surfaces for these events, because the protocol definition does not have the allow-null="true" attribute for the surface parameter.

In theory we should read pending events before destroying surfaces, to ensure we don't have any left for them,

Yes, for this reason I am handling this with a warning message because, to me,it is like a buggy implementation is the Wayland compositor more like an issue in the Cog side.

but in practice that fix would be more complicated. Also, I have seen similar fixes in other projects (like glfw/glfw#1191) so I think we can go with the approach suggested in this PR for now.

Please remove the g_warning() calls before merging, though.

One idea it could handle this with a listener for the wl_surface destruction

struct wl_surface_listener my_surface_listener = {
    // Callback on wl_surface destroy
    .destroy = wl_surface_on_destroy,
};

but the key here is that the pointer|touch|... event handlers are actually associated to the Wayland Seat and we are using the same handlers for all the wl_surfaces from different Views. Actually we rely on the wl_surface object passed as argument to get the CogView attached to it.

[aperezdc]

@psaavedra I think it's not worth it to bother with the destroy listener, let's merge this as-is.

Maybe changing warnings into a comment—but I won't insist much on it. IMO there is nothing wrong with deciding that our code handles the NULLs instead of doing something more complicated, so I think there is no point in warning the user.

[psaavedra]

@psaavedra I think it's not worth it to bother with the destroy listener, let's merge this as-is.

Maybe changing warnings into a comment—but I won't insist much on it. IMO there is nothing wrong with deciding that our code handles the NULLs instead of doing something more complicated, so I think there is no point in warning the user.

OK. Let's do it then.