From b5f50028f268827f9c3465fb23eb03d0b6e4422b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Wed, 4 Sep 2019 18:35:08 +0200 Subject: [PATCH] wayland: Untie MetaWindowXwayland lifetime from the wl_surface For the most part, a MetaWindow is expected to live roughly as long as the associated wl_surface, give or take asynchronous API discrepancies. The exception to this rule is handling of reparenting when decorating or undecorating a window, when a MetaWindow on X11 is made to survive the unmap/map cycle. The fact that this didn't hold on Wayland caused various issues, such as a feedback loop where the X11 window kept being remapped. By making the MetaWindow lifetime for Xwayland windows being the same as they are on plain X11, we remove the different semantics here, which seem to lower the risk of hitting the race condition causing the feedback loop mentioned above. What this commit do is separate MetaWindow lifetime handling between native Wayland windows and Xwayland windows. Wayland windows are handled just as they were, i.e. unmanaged together as part of the wl_surface destruction; while during the Xwayland wl_surface destruction, the MetaWindow <-> MetaWaylandSurface association is simply broken. Related: https://gitlab.freedesktop.org/xorg/xserver/issues/740 Fixes: https://gitlab.gnome.org/GNOME/mutter/issues/762 https://gitlab.gnome.org/GNOME/mutter/merge_requests/774 --- src/wayland/meta-wayland-legacy-xdg-shell.c | 19 ++++++------ src/wayland/meta-wayland-shell-surface.c | 34 +++++++++++++++++++++ src/wayland/meta-wayland-shell-surface.h | 2 ++ src/wayland/meta-wayland-surface.c | 20 ------------ src/wayland/meta-wayland-surface.h | 2 -- src/wayland/meta-wayland-wl-shell.c | 10 +++--- src/wayland/meta-wayland-xdg-shell.c | 25 +++++++-------- src/wayland/meta-xwayland.c | 24 +++++++++++++++ 8 files changed, 85 insertions(+), 51 deletions(-) diff --git a/src/wayland/meta-wayland-legacy-xdg-shell.c b/src/wayland/meta-wayland-legacy-xdg-shell.c index 823064177..0311a264a 100644 --- a/src/wayland/meta-wayland-legacy-xdg-shell.c +++ b/src/wayland/meta-wayland-legacy-xdg-shell.c @@ -165,9 +165,10 @@ zxdg_toplevel_v6_destructor (struct wl_resource *resource) { MetaWaylandZxdgToplevelV6 *xdg_toplevel = wl_resource_get_user_data (resource); - MetaWaylandSurface *surface = surface_from_xdg_toplevel_resource (resource); + MetaWaylandShellSurface *shell_surface = + META_WAYLAND_SHELL_SURFACE (xdg_toplevel); - meta_wayland_surface_destroy_window (surface); + meta_wayland_shell_surface_destroy_window (shell_surface); xdg_toplevel->resource = NULL; } @@ -549,17 +550,15 @@ handle_popup_parent_destroyed (struct wl_listener *listener, META_WAYLAND_ZXDG_SURFACE_V6 (xdg_popup); struct wl_resource *xdg_shell_resource = meta_wayland_zxdg_surface_v6_get_shell_resource (xdg_surface); - MetaWaylandSurfaceRole *surface_role = - META_WAYLAND_SURFACE_ROLE (xdg_popup); - MetaWaylandSurface *surface = - meta_wayland_surface_role_get_surface (surface_role); + MetaWaylandShellSurface *shell_surface = + META_WAYLAND_SHELL_SURFACE (xdg_popup); wl_resource_post_error (xdg_shell_resource, ZXDG_SHELL_V6_ERROR_NOT_THE_TOPMOST_POPUP, "destroyed popup not top most popup"); xdg_popup->parent_surface = NULL; - meta_wayland_surface_destroy_window (surface); + meta_wayland_shell_surface_destroy_window (shell_surface); } static void @@ -944,7 +943,7 @@ finish_popup_setup (MetaWaylandZxdgPopupV6 *xdg_popup) if (popup == NULL) { zxdg_popup_v6_send_popup_done (xdg_popup->resource); - meta_wayland_surface_destroy_window (surface); + meta_wayland_shell_surface_destroy_window (shell_surface); return; } @@ -1099,6 +1098,8 @@ meta_wayland_zxdg_popup_v6_dismiss (MetaWaylandPopupSurface *popup_surface) META_WAYLAND_ZXDG_SURFACE_V6 (xdg_popup); struct wl_resource *xdg_shell_resource = meta_wayland_zxdg_surface_v6_get_shell_resource (xdg_surface); + MetaWaylandShellSurface *shell_surface = + META_WAYLAND_SHELL_SURFACE (xdg_popup); MetaWaylandSurfaceRole *surface_role = META_WAYLAND_SURFACE_ROLE (xdg_popup); MetaWaylandSurface *surface = meta_wayland_surface_role_get_surface (surface_role); @@ -1114,7 +1115,7 @@ meta_wayland_zxdg_popup_v6_dismiss (MetaWaylandPopupSurface *popup_surface) xdg_popup->popup = NULL; - meta_wayland_surface_destroy_window (surface); + meta_wayland_shell_surface_destroy_window (shell_surface); } static MetaWaylandSurface * diff --git a/src/wayland/meta-wayland-shell-surface.c b/src/wayland/meta-wayland-shell-surface.c index e628460ba..7a9a804b6 100644 --- a/src/wayland/meta-wayland-shell-surface.c +++ b/src/wayland/meta-wayland-shell-surface.c @@ -212,6 +212,37 @@ meta_wayland_shell_surface_sync_actor_state (MetaWaylandActorSurface *actor_surf actor_surface_class->sync_actor_state (actor_surface); } +void +meta_wayland_shell_surface_destroy_window (MetaWaylandShellSurface *shell_surface) +{ + MetaWaylandSurfaceRole *surface_role = + META_WAYLAND_SURFACE_ROLE (shell_surface); + MetaWaylandSurface *surface = + meta_wayland_surface_role_get_surface (surface_role); + MetaWindow *window; + MetaDisplay *display; + uint32_t timestamp; + + window = surface->window; + if (!window) + return; + + display = meta_window_get_display (window); + timestamp = meta_display_get_current_time_roundtrip (display); + meta_window_unmanage (surface->window, timestamp); + g_assert (!surface->window); +} + +static void +meta_wayland_shell_surface_finalize (GObject *object) +{ + MetaWaylandShellSurface *shell_surface = META_WAYLAND_SHELL_SURFACE (object); + + meta_wayland_shell_surface_destroy_window (shell_surface); + + G_OBJECT_CLASS (meta_wayland_shell_surface_parent_class)->finalize (object); +} + static void meta_wayland_shell_surface_init (MetaWaylandShellSurface *role) { @@ -220,11 +251,14 @@ meta_wayland_shell_surface_init (MetaWaylandShellSurface *role) static void meta_wayland_shell_surface_class_init (MetaWaylandShellSurfaceClass *klass) { + GObjectClass *object_class = G_OBJECT_CLASS (klass); MetaWaylandSurfaceRoleClass *surface_role_class = META_WAYLAND_SURFACE_ROLE_CLASS (klass); MetaWaylandActorSurfaceClass *actor_surface_class = META_WAYLAND_ACTOR_SURFACE_CLASS (klass); + object_class->finalize = meta_wayland_shell_surface_finalize; + surface_role_class->commit = meta_wayland_shell_surface_surface_commit; actor_surface_class->get_geometry_scale = diff --git a/src/wayland/meta-wayland-shell-surface.h b/src/wayland/meta-wayland-shell-surface.h index 39a5e4a50..bfb77d0b4 100644 --- a/src/wayland/meta-wayland-shell-surface.h +++ b/src/wayland/meta-wayland-shell-surface.h @@ -71,4 +71,6 @@ void meta_wayland_shell_surface_determine_geometry (MetaWaylandShellSurface *she void meta_wayland_shell_surface_set_window (MetaWaylandShellSurface *shell_surface, MetaWindow *window); +void meta_wayland_shell_surface_destroy_window (MetaWaylandShellSurface *shell_surface); + #endif /* META_WAYLAND_SHELL_SURFACE_H */ diff --git a/src/wayland/meta-wayland-surface.c b/src/wayland/meta-wayland-surface.c index 2612e53f9..5def3c5d6 100644 --- a/src/wayland/meta-wayland-surface.c +++ b/src/wayland/meta-wayland-surface.c @@ -367,20 +367,6 @@ meta_wayland_surface_queue_pending_state_frame_callbacks (MetaWaylandSurface wl_list_init (&pending->frame_callback_list); } -void -meta_wayland_surface_destroy_window (MetaWaylandSurface *surface) -{ - if (surface->window) - { - MetaDisplay *display = meta_get_display (); - guint32 timestamp = meta_display_get_current_time_roundtrip (display); - - meta_window_unmanage (surface->window, timestamp); - } - - g_assert (surface->window == NULL); -} - MetaWaylandBuffer * meta_wayland_surface_get_buffer (MetaWaylandSurface *surface) { @@ -1364,12 +1350,6 @@ wl_surface_destructor (struct wl_resource *resource) g_clear_object (&surface->role); - /* If we still have a window at the time of destruction, that means that - * the client is disconnecting, as the resources are destroyed in a random - * order. Simply destroy the window in this case. */ - if (surface->window) - meta_wayland_surface_destroy_window (surface); - if (surface->unassigned.buffer) { meta_wayland_surface_unref_buffer_use_count (surface); diff --git a/src/wayland/meta-wayland-surface.h b/src/wayland/meta-wayland-surface.h index 9b225fc6f..5f867be9d 100644 --- a/src/wayland/meta-wayland-surface.h +++ b/src/wayland/meta-wayland-surface.h @@ -300,8 +300,6 @@ MetaWaylandSurface * meta_wayland_surface_role_get_surface (MetaWaylandSurfaceRo cairo_region_t * meta_wayland_surface_calculate_input_region (MetaWaylandSurface *surface); -void meta_wayland_surface_destroy_window (MetaWaylandSurface *surface); - gboolean meta_wayland_surface_begin_grab_op (MetaWaylandSurface *surface, MetaWaylandSeat *seat, MetaGrabOp grab_op, diff --git a/src/wayland/meta-wayland-wl-shell.c b/src/wayland/meta-wayland-wl-shell.c index 539fb9858..466106232 100644 --- a/src/wayland/meta-wayland-wl-shell.c +++ b/src/wayland/meta-wayland-wl-shell.c @@ -591,7 +591,7 @@ wl_shell_surface_role_commit (MetaWaylandSurfaceRole *surface_role, if (wl_shell_surface->popup) meta_wayland_popup_dismiss (wl_shell_surface->popup); else - meta_wayland_surface_destroy_window (surface); + meta_wayland_shell_surface_destroy_window (shell_surface); return; } @@ -680,14 +680,12 @@ meta_wayland_wl_shell_surface_popup_dismiss (MetaWaylandPopupSurface *popup_surf { MetaWaylandWlShellSurface *wl_shell_surface = META_WAYLAND_WL_SHELL_SURFACE (popup_surface); - MetaWaylandSurfaceRole *surface_role = - META_WAYLAND_SURFACE_ROLE (popup_surface); - MetaWaylandSurface *surface = - meta_wayland_surface_role_get_surface (surface_role); + MetaWaylandShellSurface *shell_surface = + META_WAYLAND_SHELL_SURFACE (wl_shell_surface); wl_shell_surface->popup = NULL; - meta_wayland_surface_destroy_window (surface); + meta_wayland_shell_surface_destroy_window (shell_surface); } static MetaWaylandSurface * diff --git a/src/wayland/meta-wayland-xdg-shell.c b/src/wayland/meta-wayland-xdg-shell.c index fa0207a03..7b1bff579 100644 --- a/src/wayland/meta-wayland-xdg-shell.c +++ b/src/wayland/meta-wayland-xdg-shell.c @@ -171,9 +171,10 @@ static void xdg_toplevel_destructor (struct wl_resource *resource) { MetaWaylandXdgToplevel *xdg_toplevel = wl_resource_get_user_data (resource); - MetaWaylandSurface *surface = surface_from_xdg_toplevel_resource (resource); + MetaWaylandShellSurface *shell_surface = + META_WAYLAND_SHELL_SURFACE (xdg_toplevel); - meta_wayland_surface_destroy_window (surface); + meta_wayland_shell_surface_destroy_window (shell_surface); xdg_toplevel->resource = NULL; } @@ -488,10 +489,8 @@ static const struct xdg_toplevel_interface meta_wayland_xdg_toplevel_interface = static void meta_wayland_xdg_popup_unmap (MetaWaylandXdgPopup *xdg_popup) { - MetaWaylandSurfaceRole *surface_role = - META_WAYLAND_SURFACE_ROLE (xdg_popup); - MetaWaylandSurface *surface = - meta_wayland_surface_role_get_surface (surface_role); + MetaWaylandShellSurface *shell_surface = + META_WAYLAND_SHELL_SURFACE (xdg_popup); g_assert (!xdg_popup->popup); @@ -502,7 +501,7 @@ meta_wayland_xdg_popup_unmap (MetaWaylandXdgPopup *xdg_popup) xdg_popup->parent_surface = NULL; } - meta_wayland_surface_destroy_window (surface); + meta_wayland_shell_surface_destroy_window (shell_surface); } static void @@ -562,17 +561,15 @@ on_parent_surface_unmapped (MetaWaylandSurface *parent_surface, MetaWaylandXdgSurface *xdg_surface = META_WAYLAND_XDG_SURFACE (xdg_popup); struct wl_resource *xdg_wm_base_resource = meta_wayland_xdg_surface_get_wm_base_resource (xdg_surface); - MetaWaylandSurfaceRole *surface_role = - META_WAYLAND_SURFACE_ROLE (xdg_popup); - MetaWaylandSurface *surface = - meta_wayland_surface_role_get_surface (surface_role); + MetaWaylandShellSurface *shell_surface = + META_WAYLAND_SHELL_SURFACE (xdg_popup); wl_resource_post_error (xdg_wm_base_resource, XDG_WM_BASE_ERROR_NOT_THE_TOPMOST_POPUP, "destroyed popup not top most popup"); xdg_popup->parent_surface = NULL; - meta_wayland_surface_destroy_window (surface); + meta_wayland_shell_surface_destroy_window (shell_surface); } static void @@ -790,7 +787,7 @@ meta_wayland_xdg_toplevel_reset (MetaWaylandXdgSurface *xdg_surface) surface = meta_wayland_surface_role_get_surface (surface_role); - meta_wayland_surface_destroy_window (surface); + meta_wayland_shell_surface_destroy_window (shell_surface); meta_wayland_actor_surface_reset_actor (META_WAYLAND_ACTOR_SURFACE (surface_role)); window = meta_window_wayland_new (meta_get_display (), surface); @@ -1010,7 +1007,7 @@ finish_popup_setup (MetaWaylandXdgPopup *xdg_popup) if (popup == NULL) { xdg_popup_send_popup_done (xdg_popup->resource); - meta_wayland_surface_destroy_window (surface); + meta_wayland_shell_surface_destroy_window (shell_surface); return; } diff --git a/src/wayland/meta-xwayland.c b/src/wayland/meta-xwayland.c index 927a7a332..f1ad12aa3 100644 --- a/src/wayland/meta-xwayland.c +++ b/src/wayland/meta-xwayland.c @@ -927,6 +927,27 @@ xwayland_surface_sync_actor_state (MetaWaylandActorSurface *actor_surface) actor_surface_class->sync_actor_state (actor_surface); } +static void +xwayland_surface_finalize (GObject *object) +{ + MetaWaylandSurfaceRole *surface_role = + META_WAYLAND_SURFACE_ROLE (object); + MetaWaylandSurface *surface = + meta_wayland_surface_role_get_surface (surface_role); + GObjectClass *parent_object_class = + G_OBJECT_CLASS (meta_wayland_surface_role_xwayland_parent_class); + MetaWindow *window; + + window = surface->window; + if (window) + { + meta_wayland_surface_set_window (surface, NULL); + window->surface = NULL; + } + + parent_object_class->finalize (object); +} + static void meta_wayland_surface_role_xwayland_init (MetaWaylandSurfaceRoleXWayland *role) { @@ -935,11 +956,14 @@ meta_wayland_surface_role_xwayland_init (MetaWaylandSurfaceRoleXWayland *role) static void meta_wayland_surface_role_xwayland_class_init (MetaWaylandSurfaceRoleXWaylandClass *klass) { + GObjectClass *object_class = G_OBJECT_CLASS (klass); MetaWaylandSurfaceRoleClass *surface_role_class = META_WAYLAND_SURFACE_ROLE_CLASS (klass); MetaWaylandActorSurfaceClass *actor_surface_class = META_WAYLAND_ACTOR_SURFACE_CLASS (klass); + object_class->finalize = xwayland_surface_finalize; + surface_role_class->assigned = xwayland_surface_assigned; surface_role_class->commit = xwayland_surface_commit; surface_role_class->get_toplevel = xwayland_surface_get_toplevel;