From 4772d8cede27c5a521970dfd56bad82102389e3a Mon Sep 17 00:00:00 2001 From: hujianwei Date: Wed, 16 Nov 2022 07:11:03 +0000 Subject: [PATCH] Fix XDG activation in some edge cases * 12to11-test.xml (test_manager) : Add activator_surface parameter. * compositor.h (enum _ClientDataType): New XdgActivationData type. (struct _RoleFuncs, struct _XdgRoleImplementationFuncs): Pass activator surface in `activate'. * seat.c (struct _Seat): New field for the serial of the last entry event. (SendKeyboardEnter, XLSeatCheckActivationSerial): Add workarounds for Firefox. * surface.c (HandleSurfaceDestroy): Allow client data free_functions to be NULL. * test.c (Activate): Accept and send activator surface. * xdg_activation.c (struct _XdgActivationToken): New fields for the surface and destroy callback. (HandleSurfaceDestroyed): New function. (SetSurface): Really record the activator surface. (GetIdForSurface): New function. (Commit): Include activator surface ID. (HandleResourceDestroy): Destroy activator surface. (GetSurfaceForId): New function. (Activate): Pass activator surface whenever specified. * xdg_surface.c (Activate): * xdg_toplevel.c (Activate): Adjust accordingly. * tests/xdg_activation_test.c (check_activation_with_serial): Check activator surface as well. (test_single_step): Fix damage rect for dummy buffer. (handle_test_surface_activated): Record the activator surface. --- 12to11-test.xml | 8 +++ compositor.h | 7 ++- seat.c | 16 ++++- surface.c | 6 +- test.c | 21 +++++-- tests/xdg_activation_test.c | 13 +++- xdg_activation.c | 121 +++++++++++++++++++++++++++++++++--- xdg_surface.c | 5 +- xdg_toplevel.c | 6 +- 9 files changed, 179 insertions(+), 24 deletions(-) diff --git a/12to11-test.xml b/12to11-test.xml index 9f9334d..64180b7 100644 --- a/12to11-test.xml +++ b/12to11-test.xml @@ -175,9 +175,17 @@ the surface associated with the role to be activated. Its parameters constitute the timestamp at which the activation occurred. + + If the surface that created the activation token used to + activate this test surface belongs to the same client that + created the test surface, then it will be sent as the + activator_surface argument to this event. Otherwise, the + argument is left unspecified. + diff --git a/compositor.h b/compositor.h index 54353fd..3894f27 100644 --- a/compositor.h +++ b/compositor.h @@ -985,6 +985,7 @@ enum _ClientDataType ShortcutInhibitData, IdleInhibitData, MaxClientData, + XdgActivationData, }; struct _DestroyCallback @@ -1146,7 +1147,8 @@ struct _RoleFuncs void (*select_extra_events) (Surface *, Role *, unsigned long); void (*note_focus) (Surface *, Role *, FocusMode); void (*outputs_changed) (Surface *, Role *); - void (*activate) (Surface *, Role *, int, Timestamp); + void (*activate) (Surface *, Role *, int, Timestamp, + Surface *); }; struct _Role @@ -1343,7 +1345,8 @@ struct _XdgRoleImplementationFuncs void (*note_focus) (Role *, XdgRoleImplementation *, FocusMode); void (*outputs_changed) (Role *, XdgRoleImplementation *); void (*after_commit) (Role *, Surface *, XdgRoleImplementation *); - void (*activate) (Role *, XdgRoleImplementation *, int, Time); + void (*activate) (Role *, XdgRoleImplementation *, int, Time, + Surface *); void (*rescale) (Role *, XdgRoleImplementation *); }; diff --git a/seat.c b/seat.c index 9efafca..e511596 100644 --- a/seat.c +++ b/seat.c @@ -567,6 +567,9 @@ struct _Seat /* The serial of the last key event sent. */ uint32_t last_keyboard_serial; + /* The serial of the last keyboard enter event sent. */ + uint32_t last_enter_serial; + /* Whether or not a resize is in progress. */ Bool resize_in_progress; @@ -2837,6 +2840,11 @@ SendKeyboardEnter (Seat *seat, Surface *enter) if (!info) return; + /* For some reason Firefox doesn't use the serial of the user event + (key or button press) that triggered the activation, but the + serial of the last keyboard entry event on the seat. */ + seat->last_enter_serial = serial; + keyboard = info->keyboards.next; for (; keyboard != &info->keyboards; keyboard = keyboard->next) @@ -6403,9 +6411,13 @@ XLSeatCheckActivationSerial (Seat *seat, uint32_t serial) return ((seat->last_button_press_serial && serial >= seat->last_button_press_serial) || (seat->last_button_serial - && serial >= seat->last_button_press_serial) + && serial >= seat->last_button_serial) || (seat->last_keyboard_serial - && serial >= seat->last_keyboard_serial)); + && serial >= seat->last_keyboard_serial) + /* Also allow using the serial at which the focus last + changed, if there currently is a keyboard focus. */ + || (serial == seat->last_enter_serial + && seat->focus_surface)); } /* This is a particularly ugly hack, but there is no other way to diff --git a/surface.c b/surface.c index 07dd79e..e46e005 100644 --- a/surface.c +++ b/surface.c @@ -1410,8 +1410,10 @@ HandleSurfaceDestroy (struct wl_resource *resource) while (data) { - /* Free the client data. */ - data->free_function (data->data); + if (data->free_function) + /* Free the client data. */ + data->free_function (data->data); + XLFree (data->data); /* And its record. */ diff --git a/test.c b/test.c index c7b0444..6d67ead 100644 --- a/test.c +++ b/test.c @@ -338,16 +338,29 @@ GetWindow (Surface *surface, Role *role) static void Activate (Surface *surface, Role *role, int deviceid, - Timestamp timestamp) + Timestamp timestamp, Surface *activator_surface) { + struct wl_resource *resource; TestSurface *test; test = TestSurfaceFromRole (role); if (test->role.resource) - test_surface_send_activated (test->role.resource, - timestamp.months, - timestamp.milliseconds); + { + /* If the activator surface belongs to the same client as the + client who created the test surface, set the resource to the + activator surface. */ + if (wl_resource_get_client (activator_surface->resource) + == wl_resource_get_client (test->role.resource)) + resource = activator_surface->resource; + else + resource = NULL; + + test_surface_send_activated (test->role.resource, + timestamp.months, + timestamp.milliseconds, + resource); + } } static const struct test_surface_interface test_surface_impl = diff --git a/tests/xdg_activation_test.c b/tests/xdg_activation_test.c index 1fa5f8d..9204341 100644 --- a/tests/xdg_activation_test.c +++ b/tests/xdg_activation_test.c @@ -58,6 +58,9 @@ static struct wl_surface *wayland_surface; static uint32_t last_activation_months; static uint32_t last_activation_milliseconds; +/* The last activation surface. */ +static struct wl_surface *last_activation_surface; + /* Forward declarations. */ @@ -149,6 +152,10 @@ check_activation_with_serial (uint32_t serial, bool expect_success) || last_activation_milliseconds != 1001) report_test_failure ("activation failed, wrong time or event not" " received"); + + if (last_activation_surface != wayland_surface) + report_test_failure ("activation succeeded, but the activator" + " surface was wrong"); } else if (last_activation_months || last_activation_milliseconds) report_test_failure ("activation succeeded unexpectedly"); @@ -172,7 +179,7 @@ test_single_step (enum test_kind kind) report_test_failure ("failed to load tiny.png"); wl_surface_attach (wayland_surface, buffer, 0, 0); - submit_surface_damage (wayland_surface, 0, 0, 500, 500); + submit_surface_damage (wayland_surface, 0, 0, 4, 4); wl_surface_commit (wayland_surface); wait_for_map (); @@ -290,10 +297,12 @@ handle_test_surface_mapped (void *data, struct test_surface *test_surface, static void handle_test_surface_activated (void *data, struct test_surface *test_surface, - uint32_t months, uint32_t milliseconds) + uint32_t months, uint32_t milliseconds, + struct wl_surface *activator_surface) { last_activation_months = months; last_activation_milliseconds = milliseconds; + last_activation_surface = activator_surface; } static const struct test_surface_listener test_surface_listener = diff --git a/xdg_activation.c b/xdg_activation.c index 8a3669c..48fd384 100644 --- a/xdg_activation.c +++ b/xdg_activation.c @@ -36,6 +36,12 @@ struct _XdgActivationToken /* The seat destroy callback. */ void *seat_destroy_callback; + /* The surface associated with this activation token. */ + Surface *surface; + + /* The destroy callback associated with the surface. */ + DestroyCallback *destroy_callback; + /* The serial associated with this activation token. */ uint32_t serial; }; @@ -45,12 +51,23 @@ static struct wl_global *xdg_activation_global; +static void +HandleSurfaceDestroyed (void *data) +{ + XdgActivationToken *token; + + token = data; + token->destroy_callback = NULL; + token->surface = NULL; +} + static void HandleSeatDestroyed (void *data) { XdgActivationToken *token; token = data; + token->seat_destroy_callback = NULL; token->seat = NULL; token->serial = 0; } @@ -106,7 +123,47 @@ static void SetSurface (struct wl_client *client, struct wl_resource *resource, struct wl_resource *surface_resource) { - /* This information is not useful. */ + XdgActivationToken *token; + Surface *surface; + + token = wl_resource_get_user_data (resource); + surface = wl_resource_get_user_data (surface_resource); + + if (token->surface) + XLSurfaceCancelRunOnFree (token->destroy_callback); + + /* The surface specified here is used by window managers to decide + whether or not to transfer focus. It should be the surface that + the client thinks is currently focused. */ + + token->surface = surface; + token->destroy_callback + = XLSurfaceRunOnFree (surface, HandleSurfaceDestroyed, + token); +} + +static unsigned int +GetIdForSurface (Surface *surface) +{ + unsigned int *data; + static unsigned int id; + + /* Given a surface, return a unique identifier for that surface. */ + data = XLSurfaceGetClientData (surface, XdgActivationData, + sizeof *data, NULL); + + /* If data is 0, then initialize it with a unique id. */ + + if (!*data) + { + id++; + if (!id) + id++; + *data = id; + } + + /* Return the surface's id. */ + return *data; } static void @@ -115,6 +172,7 @@ Commit (struct wl_client *client, struct wl_resource *resource) XdgActivationToken *token; Timestamp last_user_time; char buffer[80]; + unsigned int id; token = wl_resource_get_user_data (resource); @@ -139,12 +197,22 @@ Commit (struct wl_client *client, struct wl_resource *resource) goto finish; } - /* Send the last user time as the activation token. */ + /* Send the last user time as the activation token, along with the + surface id (if set). */ + last_user_time = XLSeatGetLastUserTime (token->seat); - sprintf (buffer, "%"PRIu32".%"PRIu32".%d", + + if (token->surface) + id = GetIdForSurface (token->surface); + else + id = 0; + + sprintf (buffer, "%"PRIu32".%"PRIu32".%d.%u", last_user_time.months, last_user_time.milliseconds, - XLSeatGetPointerDevice (token->seat)); + XLSeatGetPointerDevice (token->seat), + /* If id is 0, then a surface was not specified. */ + id); xdg_activation_token_v1_send_done (token->resource, buffer); /* Free the token. */ @@ -152,6 +220,9 @@ Commit (struct wl_client *client, struct wl_resource *resource) if (token->seat_destroy_callback) XLSeatCancelDestroyListener (token->seat_destroy_callback); + if (token->destroy_callback) + XLSurfaceCancelRunOnFree (token->destroy_callback); + XLFree (token); } @@ -184,6 +255,9 @@ HandleResourceDestroy (struct wl_resource *resource) if (token->seat_destroy_callback) XLSeatCancelDestroyListener (token->seat_destroy_callback); + if (token->destroy_callback) + XLSurfaceCancelRunOnFree (token->destroy_callback); + XLFree (token); } @@ -226,16 +300,37 @@ GetActivationToken (struct wl_client *client, struct wl_resource *resource, token, HandleResourceDestroy); } +static Surface * +GetSurfaceForId (unsigned int id) +{ + Surface *surface; + unsigned int *data; + + surface = all_surfaces.next; + while (surface != &all_surfaces) + { + data = XLSurfaceFindClientData (surface, XdgActivationData); + + if (data && *data == id) + return surface; + + surface = surface->next; + } + + return NULL; +} + static void Activate (struct wl_client *client, struct wl_resource *resource, const char *token, struct wl_resource *surface_resource) { Timestamp timestamp; - Surface *surface; + Surface *surface, *activator_surface; int deviceid; + unsigned int surface_id; - if (sscanf (token, "%"SCNu32".%"SCNu32".%d", ×tamp.months, - ×tamp.milliseconds, &deviceid) != 3) + if (sscanf (token, "%"SCNu32".%"SCNu32".%d.%u", ×tamp.months, + ×tamp.milliseconds, &deviceid, &surface_id) != 4) /* The activation token is invalid. */ return; @@ -243,9 +338,19 @@ Activate (struct wl_client *client, struct wl_resource *resource, surface = wl_resource_get_user_data (surface_resource); + /* If a surface ID was specified, try to find the surface + inside. */ + + if (surface_id) + /* Try to obtain the surface associated with this ID. */ + activator_surface = GetSurfaceForId (surface_id); + else + activator_surface = NULL; + if (surface->role->funcs.activate) surface->role->funcs.activate (surface, surface->role, - deviceid, timestamp); + deviceid, timestamp, + activator_surface); } static const struct xdg_activation_v1_interface xdg_activation_impl = diff --git a/xdg_surface.c b/xdg_surface.c index f0fb3ee..5bca78a 100644 --- a/xdg_surface.c +++ b/xdg_surface.c @@ -1399,7 +1399,7 @@ OutputsChanged (Surface *surface, Role *role) static void Activate (Surface *surface, Role *role, int deviceid, - Timestamp timestamp) + Timestamp timestamp, Surface *activator_surface) { XdgRole *xdg_role; @@ -1408,7 +1408,8 @@ Activate (Surface *surface, Role *role, int deviceid, if (xdg_role->impl && xdg_role->impl->funcs.activate) xdg_role->impl->funcs.activate (role, xdg_role->impl, deviceid, - timestamp.milliseconds); + timestamp.milliseconds, + activator_surface); } void diff --git a/xdg_toplevel.c b/xdg_toplevel.c index 2751f90..ed3a1d6 100644 --- a/xdg_toplevel.c +++ b/xdg_toplevel.c @@ -2292,7 +2292,7 @@ OutputsChanged (Role *role, XdgRoleImplementation *impl) static void Activate (Role *role, XdgRoleImplementation *impl, int deviceid, - Time time) + Time time, Surface *activator_surface) { XEvent message; XdgToplevel *toplevel; @@ -2310,7 +2310,9 @@ Activate (Role *role, XdgRoleImplementation *impl, int deviceid, message.xclient.format = 32; message.xclient.data.l[0] = 1; message.xclient.data.l[1] = time; - message.xclient.data.l[2] = None; + message.xclient.data.l[2] = (activator_surface + ? XLWindowFromSurface (activator_surface) + : None); message.xclient.data.l[3] = 0; message.xclient.data.l[4] = 0;